fix: enforce blacklist on burns; add forceBurn for seizure#198
fix: enforce blacklist on burns; add forceBurn for seizure#198dmytro-horbatenko wants to merge 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces an adminBurn function to the mToken contract, allowing authorized burners to remove tokens from blacklisted addresses. This is implemented using a state variable flag, _adminBurnInProgress, to conditionally bypass blacklist checks within the token transfer hooks. The storage layout was preserved by adjusting the contract's storage gap, and comprehensive tests were added to verify the new functionality and its security constraints. Review feedback highlights that the state-flag approach could be fragile if the contract is extended with additional hooks, potentially leading to unintended bypasses of blacklist checks in subclasses.
contracts/mToken.sol
Outdated
| _adminBurnInProgress = true; | ||
| _burn(from, amount); | ||
| _adminBurnInProgress = false; |
There was a problem hiding this comment.
The use of a state variable flag _adminBurnInProgress to bypass blacklist checks in _beforeTokenTransfer is a common pattern but can be fragile in an inheritance context. If this contract is extended by subclasses that implement hooks (such as _afterTokenTransfer) which trigger further token operations, those operations will also bypass the blacklist check on the from address because the flag remains true until the adminBurn execution completes. While the risk is currently mitigated by the onlyRole restriction on adminBurn, consider if a more localized context or a counter-based approach would be more robust for future-proofing the contract against unexpected side effects in extended hooks.
fa7cf29 to
427a736
Compare
No description provided.