feat: add adminBurn function to allow owner to destroy illegitimate t… - #302
Conversation
There was a problem hiding this comment.
Hey - I've reviewed your changes and they look great!
Sourcery assessment
Needs a human reviewer. The owner can now irreversibly destroy any holder's token balance, including potentially all circulating supply, without the holder's consent or an allowance. Reverting the contract would not restore tokens that were already burned, and an incorrect owner decision or compromised owner key would make the loss immediate and potentially unbounded.
There was a problem hiding this comment.
🟡 Changes recommended
adminBurn currently lacks a zero-address guard (since SuperfluidToken._burn doesn’t check it), which can lead to misleading burn/transfer events and inconsistent revert behavior versus standard burn paths.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adds an owner-only adminBurn(address,uint256) to SuperGoodDollar to enable DAO-controlled clawback of illegitimate balances (including while paused) by burning via SuperfluidToken._burn (bypassing ERC777 hooks), and removes recover(IERC20) to help stay under the EIP-170 contract size limit.
Changes:
- Add
adminBurntoSuperGoodDollarand expose it viaIGoodDollarCustom. - Remove
recover(IERC20)fromSuperGoodDollar. - Add test coverage for
adminBurnbehavior (supply reduction, auth, paused operation, insufficient available balance).
File summaries
| File | Description |
|---|---|
| test/token/SuperGoodDollar.test.ts | Adds unit tests validating adminBurn authorization and burn behavior (including while paused). |
| contracts/token/superfluid/SuperGoodDollar.sol | Implements adminBurn (owner-only, bypasses ERC777 hooks/pausable/fees) and removes recover. |
| contracts/token/superfluid/ISuperGoodDollar.sol | Extends IGoodDollarCustom interface to include adminBurn. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Description
Adds adminBurn(address account, uint256 amount) to SuperGoodDollar so the DAO can destroy illegitimate G$ (e.g. tokens obtained through an exploit).
Removes recover(IERC20) to stay under the EIP-170 contract size limit.
Why not reuse
_burn?burn/burnFromgo throughSuperGoodDollar._burn, which is unusable for a clawback:_burn(override)SuperToken._burnSuperfluidToken._burn(used)tokensToSendhookSuperGoodDollar._burncalls_onlyNotPaused(). Pausing is the first move in an incident and burning the illegitimate supply is the next, so the function would revert exactly when it is needed.SuperToken._burncalls_callTokensToSend, which invokes a contract the holder registered in the ERC1820 registry. A holder can register an implementer that always reverts and make themselves permanently un-burnable. The hook also fires before balances are updated, so it is a reentrancy surface — andadminBurnruns while paused, when other guards are off.adminBurntherefore drops toSuperfluidToken._burnand emitsBurned/Transferitself. The available-balance check inSuperfluidToken._burnstill applies, so a burn can never eat into the deposit backing an active Superfluid stream.Access is
_onlyOwner()(DEFAULT_ADMIN_ROLE). No new role was introduced — clawback is the most dangerous power on the token and should not be delegable by default.About # (link your issue here)
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist:
Summary by Sourcery
Enable the token owner to safely claw back illegitimate G$ while preserving Superfluid balance constraints.
New Features:
Enhancements:
Tests: