Skip to content

test: add disable_rewards intermittently flow#91

Open
arad-starkware wants to merge 10 commits into
arad/test_add_update_rewards_epoch_length_flowfrom
arad/test_add_disable_rewards_intermittently_flow
Open

test: add disable_rewards intermittently flow#91
arad-starkware wants to merge 10 commits into
arad/test_add_update_rewards_epoch_length_flowfrom
arad/test_add_disable_rewards_intermittently_flow

Conversation

@arad-starkware

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

Copy link
Copy Markdown
Contributor

This change is Reviewable


Note

Adds RewardSupplier EIC and block duration configuration (with clamped avg block time) and caches per-epoch block rewards in Staking; updates tests and docs accordingly.

  • Reward Supplier:
    • EIC & Storage: Introduces RewardSupplierEIC initializing avg_block_duration and BlockDurationConfig with validations.
    • Config API: Adds IRewardSupplierConfig (set_block_duration_config) and view get_block_duration_config; new errors (INVALID_*, etc.).
    • Computation: Clamps computed avg block duration between min_block_duration and max_block_duration; sets sensible defaults.
  • Staking:
    • Per-epoch block rewards: Adds last_calculated_epoch and cached block_rewards; calculate_block_rewards now updates/reads per-epoch via update_current_epoch_block_rewards.
    • Consensus init: set_consensus_rewards_first_epoch initializes block rewards and tracking.
  • Tests/Utils:
    • Adds flows (multiple BTC pools, empty pools, staker intent, same-epoch idempotence, intermittent disable_rewards).
    • Adds RewardSupplier EIC/config tests; introduces advance_blocks, block-reward calculators, and reward supplier upgrade helpers; replaces direct block advances.
  • Docs:
    • Documents get_block_duration_config, set_block_duration_config, BlockDurationConfig, and INVALID_MIN_MAX_BLOCK_DURATION.
  • Deps:
    • Bumps starkware-starknet-utils revision.

Written by Cursor Bugbot for commit 7670413. 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
staking_contract: system.staking.address,
minting_curve_contract: system.minting_curve.address,
);
assert!(rewards == expected_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: Missing non-zero assertion could mask broken rewards

The test assertion assert!(rewards == expected_rewards) lacks the sanity check assert!(expected_rewards.is_non_zero()) that exists in the similar test update_rewards_disable_rewards_consensus_rewards_flow_test (lines 2880-2881). The test's purpose is to verify that exactly one block's worth of rewards is given when disable_rewards: false is called. Without the non-zero check, if both values were unexpectedly zero due to a bug in the rewards calculation, the equality assertion would still pass, masking a failure in the rewards mechanism.

Fix in Cursor Fix in Web

@arad-starkware arad-starkware force-pushed the arad/test_add_disable_rewards_intermittently_flow branch from 5b321b5 to fec770a Compare November 27, 2025 15:28
@arad-starkware arad-starkware force-pushed the arad/test_add_update_rewards_epoch_length_flow branch from 2cd8a0c to 486ac4d Compare November 27, 2025 15:28
@arad-starkware arad-starkware force-pushed the arad/test_add_disable_rewards_intermittently_flow branch from fec770a to eb46cf8 Compare November 27, 2025 17:11
@arad-starkware arad-starkware force-pushed the arad/test_add_update_rewards_epoch_length_flow branch from 486ac4d to bdb1849 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 3841 at r2 (raw file):

/// update rewards with disable_rewards = false.
/// Advance block.
/// update rewards with disable_rewards = true.

Can you add one with false again here?

@arad-starkware arad-starkware force-pushed the arad/test_add_disable_rewards_intermittently_flow branch from eb46cf8 to feac4a7 Compare November 28, 2025 09:49
@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
Comment thread src/flow_test/test.cairo Outdated
@arad-starkware arad-starkware force-pushed the arad/test_add_disable_rewards_intermittently_flow branch from feac4a7 to 7670413 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