Skip to content

test: add update_rewards epoch length flow#90

Closed
arad-starkware wants to merge 10 commits into
arad/test_add_update_rewards_same_epoch_flowfrom
arad/test_add_update_rewards_epoch_length_flow
Closed

test: add update_rewards epoch length flow#90
arad-starkware wants to merge 10 commits into
arad/test_add_update_rewards_same_epoch_flowfrom
arad/test_add_update_rewards_epoch_length_flow

Conversation

@arad-starkware

@arad-starkware arad-starkware commented Nov 27, 2025

Copy link
Copy Markdown
Contributor

This change is Reviewable


Note

Introduce per-block consensus rewards driven by average block duration, add RewardSupplier EIC and config, and wire staking to cache/use epoch block rewards with extensive tests and spec updates.

  • Consensus rewards (block-duration based):
    • Add SECONDS_IN_YEAR constant and compute block rewards using average block duration.
  • Reward Supplier:
    • New update_current_epoch_block_rewards to compute per-block (STRK/BTC) rewards.
    • Track avg_block_duration, block_snapshot, and BlockDurationConfig with min/max clamping.
    • Add get_block_duration_config and set_block_duration_config (app governor-only).
    • Add new errors for block/avg duration and overflows.
    • Introduce RewardSupplierEIC to initialize new storage (avg/min/max) during V2→V3 upgrade.
  • Staking:
    • Cache epoch block_rewards and last_calculated_epoch; compute via RewardSupplier once per epoch.
    • On set_consensus_rewards_first_epoch, pre-initialize next-epoch block rewards.
  • Docs/Spec:
    • Document new Reward Supplier APIs, config struct BlockDurationConfig, and error INVALID_MIN_MAX_BLOCK_DURATION.
  • Tests/Flows/Utils:
    • Extensive new tests for block-reward path, EIC, config validation, epoch-length effects, and migration flows.
    • Rename/test helpers to use AVG_BLOCK_DURATION; add advance_blocks and custom epoch-time helpers.

Written by Cursor Bugbot for commit 4dcf4e2. This will update automatically on new commits. Configure here.

arad-starkware commented Nov 27, 2025

Copy link
Copy Markdown
Contributor Author

Comment thread src/flow_test/test.cairo
system.start_consensus_rewards();

system.update_rewards(:staker, disable_rewards: false);
let default_length_rewards = system.staker_claim_rewards(:staker);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Missing epoch advancement before baseline rewards measurement

The test claims default_length_rewards immediately after start_consensus_rewards() without advancing through any epochs where rewards are active. The start_consensus_rewards() function sets rewards to start at current_epoch + K and advances exactly K epochs, arriving at the start of the first rewards epoch with zero accumulated rewards. Therefore default_length_rewards would be 0 or nearly 0. When the test later claims increased_length_rewards after advancing an epoch, this value would be positive. The assertion increased_length_rewards < default_length_rewards would evaluate to positive < 0, which is false and would cause the test to fail.

Fix in Cursor Fix in Web

@arad-starkware arad-starkware force-pushed the arad/test_add_update_rewards_same_epoch_flow branch from ba4450d to 77ea9d3 Compare November 27, 2025 15:28
@arad-starkware arad-starkware force-pushed the arad/test_add_update_rewards_epoch_length_flow branch 2 times, most recently from 486ac4d to bdb1849 Compare November 27, 2025 17:11
@arad-starkware arad-starkware force-pushed the arad/test_add_update_rewards_same_epoch_flow branch from 77ea9d3 to 555b38d Compare November 27, 2025 17:11

@noa-starkware noa-starkware left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @arad-starkware)


src/flow_test/test.cairo line 3830 at r2 (raw file):

    let decreased_length_rewards = system.staker_claim_rewards(:staker);

    assert!(increased_length_rewards < default_length_rewards);

assert all not zero

@arad-starkware arad-starkware force-pushed the arad/test_add_update_rewards_epoch_length_flow branch from bdb1849 to 4dcf4e2 Compare November 28, 2025 09:50
@arad-starkware arad-starkware force-pushed the arad/test_add_update_rewards_same_epoch_flow branch from 555b38d to 453b967 Compare November 28, 2025 09:51
Comment thread src/flow_test/test.cairo
let decreased_length_rewards = system.staker_claim_rewards(:staker);

assert!(increased_length_rewards < default_length_rewards);
assert!(default_length_rewards < decreased_length_rewards);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Test assertions contradict V3 block rewards calculation logic

The test update_rewards_epoch_length_flow_test asserts that changing epoch_length changes per-block rewards (increased_length_rewards < default_length_rewards < decreased_length_rewards). However, in V3 consensus rewards, block rewards are calculated as yearly_mint * avg_block_duration / (BLOCK_DURATION_SCALE * SECONDS_IN_YEAR), where avg_block_duration = time_delta / num_blocks. Since advance_epoch() uses constant AVG_BLOCK_DURATION per block, the ratio time_delta/num_blocks remains constant regardless of epoch_length. The block rewards should be identical for all epoch lengths, making these assertions incorrect for V3 behavior.

Fix in Cursor Fix in Web

@arad-starkware arad-starkware force-pushed the arad/test_add_update_rewards_same_epoch_flow branch from 453b967 to e4e5171 Compare December 1, 2025 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants