Moved adjust total supply checkpoints inside of create checkpoint #520

[dev1644]

Please check if the PR fulfills these requirements

• The commit message follows our Submission guidelines
• Tests for the changes have been added (for bug fixes/features)
• Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce?

(Bug fix, feature, docs update, ...)

What is the current behavior?

(You can also link to an open issue here)
#520

What is the new behavior?

(Define and describe any new functionality. Clarify if this is a feature change)

Does this PR introduce a breaking change?

(What changes might users need to make in their application due to this PR?)
No

Any Other information:

[maxsam4]

@dev1644 Thanks for the PR!
The changes in SecurityToken look good.
These changes allow us to remove a couple of edge case checks from TokenLib. To avoid spoilers in the learning process, I am not gonna mention which :P.

If you need help finding them, you know where to find me :)

[F-OBrien]

The description for the adjustTotalSupplyCheckpoints() function should now be changed to remove reference to minting or burning also. I guess it's also now questionable if this even needs to be a separate function as it's only called in one place when the checkpoint is created?

/** * @notice Internal - adjusts totalSupply at checkpoint after minting or burning tokens */ function _adjustTotalSupplyCheckpoints() internal { TokenLib.adjustCheckpoints(checkpointTotalSupply, totalSupply(), currentCheckpointId); }

[maxsam4]

@dev1644 I guess we need the function in the tokenLib for adjusting checkpoints of user balance. Updating total supply with a single statement is fine, just don't remove the function from tokenLib.

The other optimization I was talking about was getValueAt for total supply. You can directly return totalSupply from checkpoint here if checkpointID is within the limit.