[token] tokendata maximum should not be changed from or to 0. #5382
Conversation
|
good catch - thanks for putting out the PR!! |
| @@ -773,6 +773,8 @@ module aptos_token::token { | |||
| assert_tokendata_exists(creator, token_data_id); | |||
| let all_token_data = &mut borrow_global_mut<Collections>(token_data_id.creator).token_data; | |||
| let token_data = table::borrow_mut(all_token_data, token_data_id); | |||
| // cannot change maximum from 0 and cannot change maximum to 0 | |||
| assert!(token_data.maximum != 0 && maximum != 0, error::invalid_argument(EINVALID_MAXIMUM)); | |||
There was a problem hiding this comment.
IIUC, please also add an assertion before line 1072, otherwise the invariant can not be guaranteed at initialization.
There was a problem hiding this comment.
Are you referring to this line?
IIUC, it is valid to have
token_data.maximum = 0. The case here is to prevent mutating maximum from or to zero because when the maximum is zero, the supply is untracked. For example,There was a problem hiding this comment.
So it is allowed to have maximum = 0 at initialization and immutable since then. It seems we did the same for Collections and maximum = 0 means no supply tracking...
n00b Q: Do we have any use case for that? cc @areshand
There was a problem hiding this comment.
So it is allowed to have maximum = 0 at initialization and immutable since then. Nope, that's what this PR hopes to fix. The only usecase that should be allowed is : having maximum > 0 at initialization, and then mutate it to a valid (>supply) non-zero value since then.
There was a problem hiding this comment.
talked to @areshand offline. = 0 at initialization is a hack for some special case. I'll follow up offline.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ Forge suite
|
✅ Forge suite
|
Description
Tokendata maximum should not be able to be mutated to or from 0.
Test Plan
1 unit test was added.
This change is