sherlock-audit / 2024-05-aleo-judging

0 stars 0 forks source link

haxatron - Coinbase reward calculation is incorrect due to incorrect `ANCHOR_HEIGHT` #22

Closed sherlock-admin3 closed 1 week ago

sherlock-admin3 commented 2 weeks ago

haxatron

General Health

Coinbase reward calculation is incorrect due to incorrect ANCHOR_HEIGHT

Summary

Note: This issue was taken from https://github.com/AleoNet/snarkVM/issues/2426 and submitted here as a "just-in-case" finding, since it wasn't marked as known issues and qualified as general health issue.

Coinbase reward calculation is incorrect due to incorrect ANCHOR_HEIGHT

Vulnerability Detail

The coinbase reward calculation will be incorrect due to incorrect ANCHOR_HEIGHT. First, let's take a look at the calculation of ANCHOR_HEIGHT.

https://github.com/sherlock-audit/2024-05-aleo/blob/main/snarkVM/console/network/src/lib.rs#L132-L137

    /// The anchor height, defined as the expected number of blocks to reach the coinbase target.
    const ANCHOR_HEIGHT: u32 = Self::ANCHOR_TIME as u32 / Self::BLOCK_TIME as u32;
    /// The anchor time in seconds.
    const ANCHOR_TIME: u16 = 25;
    /// The expected time per block in seconds.
    const BLOCK_TIME: u16 = 10;

Here, we note that the ANCHOR_HEIGHT is supposed to be ANCHOR_TIME / BLOCK_TIME and the intended value is 2.5, however because of integer division and rounding down, the actual value calculated will be 2.

This affects the coinbase reward calculation, in the sense that the scaling will be incorrect and the coinbase rewards will only be 80% of the expected value. The formula for coinbase rewards is determined by the following function

https://github.com/sherlock-audit/2024-05-aleo/blob/main/snarkVM/ledger/block/src/helpers/target.rs#L51-L80

pub fn coinbase_reward(
    block_height: u32,
    starting_supply: u64,
    anchor_height: u32,
    block_time: u16,
    combined_proof_target: u128,
    cumulative_proof_target: u64,
    coinbase_target: u64,
) -> Result<u64> {
    // Compute the remaining coinbase target.
    let remaining_coinbase_target = coinbase_target.saturating_sub(cumulative_proof_target);
    // Compute the remaining proof target.
    let remaining_proof_target = combined_proof_target.min(remaining_coinbase_target as u128);

    /* Until the anchor block height at year 10, the coinbase reward is determined by this equation: */
    /*   anchor_block_reward * remaining_proof_target / coinbase_target */

    // Compute the anchor block reward.
    let anchor_block_reward = anchor_block_reward_at_height(block_height, starting_supply, anchor_height, block_time);

    // Calculate the coinbase reward.
    let reward = anchor_block_reward.saturating_mul(remaining_proof_target).saturating_div(coinbase_target as u128);

    // Ensure the coinbase reward is less than the maximum coinbase reward.
    ensure!(reward <= MAX_COINBASE_REWARD as u128, "Coinbase reward ({reward}) exceeds maximum {MAX_COINBASE_REWARD}");

    // Return the coinbase reward.
    // Note: This '.expect' is guaranteed to be safe, as we ensure the reward is within a safe bound.
    Ok(u64::try_from(reward).expect("Coinbase reward exceeds u64::MAX"))
}

In the above calculation, the incorrect ANCHOR_HEIGHT value will be used in anchor_block_reward_at_height function

https://github.com/sherlock-audit/2024-05-aleo/blob/main/snarkVM/ledger/block/src/helpers/target.rs#L88-L104

const fn anchor_block_reward_at_height(
    block_height: u32,
    starting_supply: u64,
    anchor_height: u32,
    block_time: u16,
) -> u128 {
    // Calculate the block height at year 10.
    let block_height_at_year_10 = block_height_at_year(block_time, 10) as u128;
    // Compute the remaining blocks until year 10, as a u64.
    let num_remaining_blocks_to_year_10 = block_height_at_year_10.saturating_sub(block_height as u128);
    // Compute the numerator.
    let numerator = 2 * starting_supply as u128 * anchor_height as u128 * num_remaining_blocks_to_year_10;
    // Compute the denominator.
    let denominator = block_height_at_year_10 * (block_height_at_year_10 + 1);
    // Return the anchor block reward.
    numerator / denominator
}

And thereby the subsequent coinbase_reward computation will be 80% of the expected value.

Impact

coinbase_reward computation will be 80% of the expected value.

Code Snippet

https://github.com/sherlock-audit/2024-05-aleo/blob/main/snarkVM/console/network/src/lib.rs#L132-L137

Tool used

Manual Review

Recommendation

Instead of multiplying by ANCHOR_HEIGHT, first multiply by the ANCHOR_TIME and then divide the entire numerator by BLOCK_TIME in anchor_block_reward_at_height computation to prevent this large rounding error.

const fn anchor_block_reward_at_height(
    block_height: u32,
    starting_supply: u64,
-   anchor_height: u32,
+   anchor_time: u16,
    block_time: u16,
) -> u128 {
    // Calculate the block height at year 10.
    let block_height_at_year_10 = block_height_at_year(block_time, 10) as u128;
    // Compute the remaining blocks until year 10, as a u64.
    let num_remaining_blocks_to_year_10 = block_height_at_year_10.saturating_sub(block_height as u128);
    // Compute the numerator.
-   let numerator = 2 * starting_supply as u128 * anchor_height as u128 * num_remaining_blocks_to_year_10;
+   let numerator = 2 * starting_supply as u128 * anchor_time as u128 * num_remaining_blocks_to_year_10 / block_time as u128;
    // Compute the denominator.
    let denominator = block_height_at_year_10 * (block_height_at_year_10 + 1);
    // Return the anchor block reward.
    numerator / denominator
}