smartcontractkit / smartcontractkit/chainlink-aptos
`burn(...)` function would enable burners to burn users assets without their consent
Nobody has claimed this yet.
- Dominant language
- Move
- Stars
- 10
- Forks
- 14
- Avg merge
- 1d 7h
- Merged PRs (30d)
- 29
Description
Hi team
I'm studying your implementation to learn Aptos Move development and noticed what appears to be a significant difference from standard token burning patterns. Your burn function allows privileged addresses to burn tokens from any user's account without their explicit consent or approval.
current implementation :
public entry fun burn(
burner: &signer, from: address, amount: u64
) acquires TokenMetadataRefs, TokenState {
let burner_addr = signer::address_of(burner);
assert_is_allowed_burner(burner_addr);
if (amount == 0) { return };
primary_fungible_store::burn(
&borrow_token_metadata_refs().burn_ref, from, amount
);
event::emit(Burn { burner: burner_addr, from, amount });
}
The test even clearly has it
#[test(owner = @0x999, recipient = @0xcafe, managed_token = @managed_token)]
public fun test_burn_managed_token(
owner: &signer, recipient: &signer, managed_token: &signer
) {
// Setup env and mint managed_token first, mint 100 to recipient
test_mint_managed_token(owner, recipient, managed_token);
let recipient_addr = signer::address_of(recipient);
let burn_amount: u64 = 50;
managed_token::burn(owner, recipient_addr, burn_amount);
let metadata_obj =
object::address_to_object<Metadata>(managed_token::token_metadata());
// 100 is the mint amount, 50 is the burn amount
let mint_amount: u64 = 100;
assert!(
primary_fungible_store::balance(recipient_addr, metadata_obj)
== mint_amount - burn_amount
);
// Assert tokens are burned from existing supply
assert!(
fungible_asset::supply(metadata_obj)
== option::some((mint_amount - burn_amount) as u128)
);
}
This implementation enables any address with "burner" privileges to burn tokens from any user's account (as shown in the test where owner burns tokens from recipient_addr). This is a significant deviation from standard token burning patterns.
In OpenZeppelin's ERC20 implementation:
- burn() only burns caller's own tokens
- burnFrom() requires explicit allowance from the token owner
// OpenZeppelin Contracts (last updated v5.0.0) (token/ERC20/extensions/ERC20Burnable.sol)
/**
* @dev Destroys a `value` amount of tokens from the caller.
*
* See {ERC20-_burn}.
*/
function burn(uint256 value) public virtual {
_burn(_msgSender(), value);
}
/**
* @dev Destroys a `value` amount of tokens from `account`, deducting from
* the caller's allowance.
*
* See {ERC20-_burn} and {ERC20-allowance}.
*
* Requirements:
*
* - the caller must have allowance for ``accounts``'s tokens of at least
* `value`.
*/
function burnFrom(address account, uint256 value) public virtual {
_spendAllowance(account, _msgSender(), value);
_burn(account, value);
}
I'm wondering if the move implementation is a design choice so I would really appreciate any clarification
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reviewing the public burn entry function and the test_burn_managed_token test shown in the issue, then compare their authorization behavior with the project's intended token model. Done means maintainers have decided whether privileged burning is intended and the behavior, tests, or documentation clearly reflect that decision.
Written by the indexing model from the issue text.
Assessment
- Domain
- blockchain, security
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100