-
Notifications
You must be signed in to change notification settings - Fork 152
dev 2.1 > dev 3.0 #521
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
dev 2.1 > dev 3.0 #521
Conversation
* Initial optimization * Tests fixed * comments removed * Make some functions external
for (uint256 i = 0; i < tiers.length; i++) { | ||
tokensMinted = tokensMinted.add(tiers[i].mintedTotal); | ||
} | ||
return tokensMinted; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optimization Suggestion?
I could be wrong here but is there a logic to using tokensMinted
instead of totalTokensSold
? Are they not essentially the same thing? totalTokensSold
only seems to be update after the STO is finaliszed. Instead there currently is an additional function that regularly calculates tokensMinted
. If totalTokensSold
was update as tokens were minted instead it would save the need for getTokensMinted()
, which is called multiple times and recalculates each time. It could simply be replaced with totalTokensSold
. This would then allow you to remove getTokensMinted()
and also remove the getTokensSold()
function as totalTokensSold
already has a public getter.
So by adding totalTokensSold = totalTokensSold.add(tierPurchasedTokens)
after tierData.mintedTotal = tierData.mintedTotal.add(tierPurchasedTokens)
in the _calculateTier
function I think it allows this optimization.
The finalize()
function could then also be tidied up a bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @F-OBrien - I've marked this as an issue to see where it can be cleaned up. The difference is meant to be the amount of tokens which were actually sold through the STO, vs. the number of tokens which have been minted (including minting back to the reserve wallet). As you say we could probably clean this up though. #535
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything is good.
No description provided.