Skip to content

adjustCheckpoints after mint or burn #517

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

Closed
AlexejShevchenko opened this issue Jan 12, 2019 · 4 comments
Closed

adjustCheckpoints after mint or burn #517

AlexejShevchenko opened this issue Jan 12, 2019 · 4 comments

Comments

@AlexejShevchenko
Copy link

AlexejShevchenko commented Jan 12, 2019

In SecurityToken.sol line 504 (adjusts totalSupply at checkpoint AFTER minting or burning tokens) but it is called on lines 683 and 726 BEFORE change totalSupply_

For example,
checkpoint = 0
checkpoint value = 0
total supply = 0

Somebody buy 100 tokens. checkpoint value = 0, total supply = 100
Somebody buy 200 tokens. checkpoint value = 100, total supply = 300
We create a new checkpoin. Remember, total supply is 300 now!!!
checkpoint = 1
checkpoint value = 100
total supply = 300

Somebody buy 300 tokens. checkpoint value = 300, total supply = 600
We want to see total supply at checkpoint 0. It returns 100, but should return 300.

@maxsam4
Copy link
Contributor

maxsam4 commented Jan 14, 2019

@AlexejShevchenko Thanks for trying out our Security token!
I just tried playing with checkpoints and they seemed to be functioning fine. There are two things that might have went wrong with your test:

  1. Your create checkpoint transaction got mined before the other person's buy token transaction.
  2. You are using incorrect checkpoint ID. Checkpoints start with Checkpoint ID 1 and not 0.

If you think none of the above caused this Issue, please share the address of your security token and I'll look into what went wrong. If you used a private network then maybe share the script or exact steps you took? Thanks!

@AlexejShevchenko
Copy link
Author

AlexejShevchenko commented Jan 14, 2019

I'm not testing token yet, I'm just read a code of contracts.
Line 502

@notice Internal - adjusts totalSupply at checkpoint after minting or burning tokens

As I understand, this function should be called AFTER totalSupply changes
But it is called on lines 683 and 726 BEFORE change totalSupply_

adjustTotalSupplyCheckpoints();
totalSupply
= totalSupply_.add(_value);

@maxsam4
Copy link
Contributor

maxsam4 commented Jan 14, 2019

@AlexejShevchenko adjustTotalSupplyCheckpoints() is used to update last created checkpoint (if it hasn't been updated yet). Hence, It is called before updating totalSupply.

When a checkpoint is created, total supply and balances are not snapshotted instantly. They are stored in the next transaction and hence they need to be stored before the total supply is updated by that transaction. We'll update the code comment to reflect this. Thanks for pointing this out!

@adamdossa
Copy link
Contributor

Thanks @AlexejShevchenko for reviewing this. We have fixed the comments in 2.1 version of the code!
#519

@adamdossa adamdossa mentioned this issue Jan 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants