Description
Affected Asset: steets.move
The Protocol documentation states that a Rapper NFT can be staked to:
Remove vices
Earn CRED per full day staked.
The vices to remove and how much CRED to mint is calculated when the player unstakes their NFT.
While the logic for removing vices is implemented correctly, the logic for earning CRED is incomplete.
The Issue
The logic for earning CRED ends after only 4 days - when all vices have been removed on a Rapper. This means past 4 days, any additional staking duration will not acure CRED. This breaks the protocols invariant of earning CRED per full day staked as the maximum CRED a player can earn is 4 regardless of how many days they stake past 4.
The Problematic Code
public entry fun unstake(staker: &signer, module_owner: &signer, rapper_token: Object<Token>) acquires StakeInfo {
let staker_addr = signer::address_of(staker);
let token_id = object::object_address(&rapper_token);
assert!(exists<StakeInfo>(staker_addr), E_TOKEN_NOT_STAKED);
let stake_info = borrow_global<StakeInfo>(staker_addr);
assert!(stake_info.owner == staker_addr, E_NOT_OWNER);
let staked_duration = timestamp::now_seconds() - stake_info.start_time_seconds;
let days_staked = staked_duration / 86400;
if (days_staked > 0) {
let (wk, ha, ss, cr, wins) = one_shot::read_stats(token_id);
let final_wk = if (days_staked >= 1) { false } else { wk };
let final_ha = if (days_staked >= 2) { false } else { ha };
let final_ss = if (days_staked >= 3) { false } else { ss };
let final_cr = if (days_staked >= 4) { true } else { cr };
if (days_staked >= 1) { cred_token::mint(module_owner, staker_addr, 1); };
if (days_staked >= 2) { cred_token::mint(module_owner, staker_addr, 1); };
if (days_staked >= 3) { cred_token::mint(module_owner, staker_addr, 1); };
if (days_staked >= 4) { cred_token::mint(module_owner, staker_addr, 1); };
one_shot::write_stats(token_id, final_wk, final_ha, final_ss, final_cr, wins);
};
let StakeInfo { start_time_seconds: _, owner: _ } = move_from<StakeInfo>(staker_addr);
one_shot::transfer_record_only(token_id, @battle_addr, staker_addr);
object::transfer(module_owner, rapper_token, staker_addr);
event::emit(UnstakedEvent {
owner: staker_addr,
token_id,
staked_duration,
});
}
Risk
Likelihood
The Likelihood of this issue being triggered is High when a player stakes their NFT for more than 4 days. Any additional days staked has no effect on the amount of CRED they will earn.
Impact
The impact of this issue is:
-
A partially broken protocol invariant: the protocol states that players will "earn CRED per full day staked." However, this is not entirely true as stakes past 4 days will not earn CRED.
-
A loss of value for players: Players will stake their NFT for a long duration believing they are earning CRED while only earning a maximum of 4 CRED. E.g staking for 10 days will not result in 10 CRED.
Proof of Concept
The below runnable Move test demonstrates the issue by having a player stake their Rapper NFT for 10 days. They should have received 10 CRED for each day staked but instead have only 4.
The below steps need to be carried out for the test to run successfully.
Setup
In order to run the test, the following test functions and variables needs to be added into each file.
Move.toml
[addresses]
battle_addr = "_"
minter_addr = "_"
aptos_token_v2 = "0x4"
player_1 = "_"
player_2 = "_"
[dev-addresses]
battle_addr = "0x42"
minter_addr = "0x100"
player_1 = "0x65"
player_2 = "0x14"
cred_token.move
#[test_only]
public fun init_contract(owner: &signer){
init_module(owner);
}
public fun return_balance(account: address):u64{
let balance = coin::balance<CRED>(account);
balance
}
one_shot.move
#[test_only]
public fun extract_events(): vector<MintRapperEvent>{
let extract_events = event::emitted_events<MintRapperEvent>();
extract_events
}
The Test
#[test_only]
module battle_addr::poc_flawed_staking_logic{
use std::signer;
use aptos_framework::account;
use aptos_token_v2::token::{Self as token};
use std::debug;
use std::vector;
use std::object;
use std::string;
use aptos_framework::timestamp;
use aptos_framework::genesis;
use aptos_framework::coin;
use battle_addr::one_shot;
use battle_addr::cred_token;
use battle_addr::streets;
const E_NO_RAPPER_NFT: u64 = 100;
#[test(aptos_framework = @0x1, module_owner = @0x561)]
public fun poc_cred_not_earned_properly_in_unstaking(aptos_framework: &signer, module_owner: &signer) {
genesis::setup();
coin::create_coin_conversion_map(aptos_framework);
timestamp::set_time_has_started_for_testing(aptos_framework);
let module_owner = account::create_account_for_test(@battle_addr);
let player_1 = account::create_account_for_test(@player_1);
let player1_address = signer::address_of(&player_1);
cred_token::init_contract(&module_owner);
cred_token::register(&player_1);
one_shot::mint_rapper(&module_owner, player1_address);
let p1_balance = one_shot::balance_of(player1_address);
assert!(p1_balance == 1, E_NO_RAPPER_NFT);
let extracted_events = one_shot::extract_events();
let event_message = string::utf8(b"PRINTING EXTRACTED `MintRapperEvent` TO OBTAIN `token_id`");
debug::print(&event_message);
debug::print(&extracted_events);
let player1_tokenid = @0xe46a3c36283330c97668b5d4693766b8626420a5701c18eb64026075c3ec8a0a;
let token_object = object::address_to_object<token::Token>(player1_tokenid);
streets::stake(&player_1, token_object );
let seconds_in_one_day = 86400;
let days_to_stake = 10;
timestamp::update_global_time_for_test_secs((days_to_stake * seconds_in_one_day));
let current_time = timestamp::now_seconds();
let time_message = string::utf8(b"THE CURRENT TIME (IN SECONDS)");
debug::print(&time_message);
debug::print(¤t_time);
streets::unstake(&player_1, &module_owner, token_object);
let p1_cred_balance = cred_token::return_balance(player1_address);
let cred_balance_message = string::utf8(b"PLAYER 1 CRED BALANCE AFTER STAKING FOR 10 DAYS");
debug::print(&cred_balance_message);
debug::print(&p1_cred_balance);
assert!(p1_cred_balance != days_to_stake);
}
}
Recommended mitigation
The recommended mitigation is to mint players CRED equal to the amount of full days that the NFT has been staked for. The conditional minting logic should be removed.
public entry fun unstake(staker: &signer, module_owner: &signer, rapper_token: Object<Token>) acquires StakeInfo {
let staker_addr = signer::address_of(staker);
let token_id = object::object_address(&rapper_token);
assert!(exists<StakeInfo>(staker_addr), E_TOKEN_NOT_STAKED);
let stake_info = borrow_global<StakeInfo>(staker_addr);
assert!(stake_info.owner == staker_addr, E_NOT_OWNER);
let staked_duration = timestamp::now_seconds() - stake_info.start_time_seconds;
let days_staked = staked_duration / 86400;
if (days_staked > 0) {
let (wk, ha, ss, cr, wins) = one_shot::read_stats(token_id); //weak-knees, heavy arms, spaghetti_sweater, calm&ready, wins
let final_wk = if (days_staked >= 1) { false } else { wk };
let final_ha = if (days_staked >= 2) { false } else { ha };
let final_ss = if (days_staked >= 3) { false } else { ss };
let final_cr = if (days_staked >= 4) { true } else { cr };
//@audit-issue: Broken logic. If staked for > 4 days, the maximum CRED is only 4 and not per full day staked.
- if (days_staked >= 1) { cred_token::mint(module_owner, staker_addr, 1); };
- if (days_staked >= 2) { cred_token::mint(module_owner, staker_addr, 1); };
- if (days_staked >= 3) { cred_token::mint(module_owner, staker_addr, 1); };
- if (days_staked >= 4) { cred_token::mint(module_owner, staker_addr, 1); };
//@audit-fix: mints CRED for each full day staked.
+ let tokens_to_mint = days_staked;
+ cred_token::mint(module_owner, staker_addr, tokens_to_mint);
one_shot::write_stats(token_id, final_wk, final_ha, final_ss, final_cr, wins);
};
let StakeInfo { start_time_seconds: _, owner: _ } = move_from<StakeInfo>(staker_addr);
one_shot::transfer_record_only(token_id, @battle_addr, staker_addr);
object::transfer(module_owner, rapper_token, staker_addr);
event::emit(UnstakedEvent {
owner: staker_addr,
token_id,
staked_duration,
});
}
