Description
Users who haven't registered for the CRED coin type cannot unstake their rapper NFTs to claim rewards. The transaction fails with a misleading error message "Cannot destroy non-zero coins" instead of properly handling the registration requirement or providing a clear error message.
Root Cause
The issue occurs in two places:
-
In cred_token::mint() at lines 45-66, the function incorrectly attempts to call destroy_zero() on coins with non-zero value when the recipient isn't registered. This is impossible and will always fail.
-
In streets::unstake(), there's no check or auto-registration for users before attempting to mint CRED rewards to them.
public(friend) fun mint(
module_owner: &signer,
to: address,
amount: u64
) acquires CredCapabilities {
let caps = borrow_global<CredCapabilities>(signer::address_of(module_owner));
let coins = coin::mint<CRED>(amount, &caps.mint_cap);
if (coin::is_account_registered<CRED>(to)) {
coin::deposit(to, coins);
} else {
coin::destroy_zero(coins);
};
}
Risk
Likelihood: Medium - Any user who hasn't registered for CRED will encounter this error when unstaking
Impact: Low - Only affects user experience; users can register and retry the transaction
Impact
Low severity because:
-
Breaks core functionality: Users cannot unstake and claim rewards without manual registration
-
Wasted gas fees: Users lose transaction fees on failed unstake attempts
-
Confusing error messages: Technical "destroy_zero" error doesn't explain the actual problem
-
No permanent fund loss - rewards remain claimable after registration
-
Workaround exists via cred_token::register() followed by unstaking
-
Does not compromise protocol security or smart contract integrity
Proof of Concept
The test demonstrates the issue when feature flag 90 (NEW_ACCOUNTS_DEFAULT_TO_FA_STORE) is disabled. Note that the test requires adding a helper function to cred_token.move:
#[test_only]
public fun init_module_test(sender: &signer) {
let (burn_cap, freeze_cap, mint_cap) = coin::initialize<CRED>(
sender,
string::utf8(b"Credibility"),
string::utf8(b"CRED"),
8,
false,
);
move_to(sender, CredCapabilities { mint_cap, burn_cap });
coin::destroy_freeze_cap(freeze_cap);
}
Test demonstrating the issue:
#[test]
#[expected_failure(abort_code = 65543, location = aptos_framework::coin)]
public fun test_unable_to_unstake_if_not_registered() {
let one_day = 86_400_000_000;
let aptos_framework = account::create_account_for_test(@0x1);
timestamp::set_time_has_started_for_testing(&aptos_framework);
timestamp::update_global_time_for_test(one_day);
let std = account::create_account_for_test(@std);
features::change_feature_flags_for_testing(&std, vector[], vector[90]);
coin::create_coin_conversion_map(&aptos_framework);
let module_owner = account::create_account_for_test(@battle_addr);
let minter = account::create_account_for_test(@minter_addr);
cred_token::init_module_test(&module_owner);
one_shot::mint_rapper(&module_owner, signer::address_of(&minter));
let mint_events = event::emitted_events<MintRapperEvent>();
let first_event = vector::borrow(&mint_events, 0);
let token_id = one_shot::get_mint_event_token_id(first_event);
let rapper_token = object::address_to_object<Token>(
token_id
);
streets::stake(&minter, rapper_token);
timestamp::update_global_time_for_test(5 * one_day);
streets::unstake(&minter, &module_owner, rapper_token);
}
Recommended Mitigation
There are two proper solutions that avoid the current broken behavior:
Option 1 (Recommended): Auto-register users in the unstake function
// In streets.move:unstake()
public entry fun unstake(
staker: &signer,
module_owner: &signer,
rapper_token: Object<Token>
) acquires StakedRapper {
// ... existing staking checks ...
+ // Auto-register user for CRED if not already registered
+ if (!coin::is_account_registered<CRED>(staker_address)) {
+ coin::register<CRED>(staker);
+ }
// Now mint will always succeed
cred_token::mint(module_owner, staker_address, rewards);
// ... rest of function ...
}
Option 2: Add clear assertion in mint function
public(friend) fun mint(
module_owner: &signer,
to: address,
amount: u64
) acquires CredCapabilities {
+ // Require registration upfront with clear error message
+ assert!(
+ coin::is_account_registered<CRED>(to),
+ error::not_found(EUSER_NOT_REGISTERED_FOR_CRED)
+ );
+
let caps = borrow_global<CredCapabilities>(signer::address_of(module_owner));
let coins = coin::mint<CRED>(amount, &caps.mint_cap);
- if (coin::is_account_registered<CRED>(to)) {
- coin::deposit(to, coins);
- } else {
- coin::destroy_zero(coins);
- };
+ coin::deposit(to, coins);
}
Option 1 provides the best user experience by seamlessly handling registration. Option 2 at least provides a clear error message.
