Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #49 +/- ##
==========================================
+ Coverage 95.43% 95.58% +0.14%
==========================================
Files 45 45
Lines 9446 9730 +284
==========================================
+ Hits 9015 9300 +285
+ Misses 431 430 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
a9665cf to
a8d6962
Compare
noa-starkware
left a comment
There was a problem hiding this comment.
@noa-starkware reviewed 1 of 3 files at r1.
Reviewable status: 1 of 3 files reviewed, 33 unresolved discussions (waiting on @NirLevi-starkware)
src/flow_test/utils.cairo line 1506 at r1 (raw file):
} fn set_consensus_rewards_first_epoch(self: SystemState, epoch_id: Epoch) {
Is there a more fitting trait than staker trait?
src/flow_test/test.cairo line 2630 at r1 (raw file):
/// Flow: /// Staker stake /// Disable rewards is true with v3 off
Suggestion:
consensussrc/flow_test/test.cairo line 2632 at r1 (raw file):
/// Disable rewards is true with v3 off /// Test rewards are not distributed /// Attempt again same epoch - panic
Suggestion:
blocksrc/flow_test/test.cairo line 2633 at r1 (raw file):
/// Test rewards are not distributed /// Attempt again same epoch - panic /// Disable rewards is false with v3 off
Suggestion:
consensussrc/flow_test/test.cairo line 2635 at r1 (raw file):
/// Disable rewards is false with v3 off /// Test rewards are not distributed /// Attempt again same epoch - panic
Suggestion:
blocksrc/flow_test/test.cairo line 2637 at r1 (raw file):
/// Attempt again same epoch - panic /// Enable consensus rewards (in 2 epochs) /// Disable rewards is true with v3 before consensus epoch
What does it mean?
Code quote:
with v3 src/flow_test/test.cairo line 2639 at r1 (raw file):
/// Disable rewards is true with v3 before consensus epoch /// Test rewards are not distributed /// Attempt again same epoch - panic
Suggestion:
blocksrc/flow_test/test.cairo line 2640 at r1 (raw file):
/// Test rewards are not distributed /// Attempt again same epoch - panic /// Disable rewards is false with v3 before consensus epoch
same
Code quote:
with v3src/flow_test/test.cairo line 2642 at r1 (raw file):
/// Disable rewards is false with v3 before consensus epoch /// Test rewards are not distributed /// Attempt again same epoch - panic
Suggestion:
blocksrc/flow_test/test.cairo line 2643 at r1 (raw file):
/// Test rewards are not distributed /// Attempt again same epoch - panic /// Disable rewards is true with v3 on
Suggestion:
consensussrc/flow_test/test.cairo line 2645 at r1 (raw file):
/// Disable rewards is true with v3 on /// Test rewards are not distributed /// Attempt again same epoch - panic
Suggestion:
blocksrc/flow_test/test.cairo line 2646 at r1 (raw file):
/// Test rewards are not distributed /// Attempt again same epoch - panic /// Disable rewards is false with v3 on
Suggestion:
consensussrc/flow_test/test.cairo line 2648 at r1 (raw file):
/// Disable rewards is false with v3 on /// Test rewards are distributed /// Attempt again same epoch - panic
Suggestion:
blocksrc/flow_test/test.cairo line 2651 at r1 (raw file):
#[test] #[feature("safe_dispatcher")] fn update_rewards_disable_rewards_flow_test() {
Suggestion:
update_rewards_disable_rewards_consensus_rewardssrc/flow_test/test.cairo line 2659 at r1 (raw file):
system.advance_k_epochs(); // Disable rewards is true with v3 off
Suggestion:
Disable rewards = true with consensus off - no rewards.src/flow_test/test.cairo line 2662 at r1 (raw file):
system.update_rewards(:staker, disable_rewards: true); // Test rewards are not distributed let rewards = system.staker_claim_rewards(:staker);
Is it better than staker_info.unclaimed_rewards?
Code quote:
claim_rewards(:staker)src/flow_test/test.cairo line 2663 at r1 (raw file):
// Test rewards are not distributed let rewards = system.staker_claim_rewards(:staker); assert!(rewards == Zero::zero());
Suggestion:
.is_zero()src/flow_test/test.cairo line 2664 at r1 (raw file):
let rewards = system.staker_claim_rewards(:staker); assert!(rewards == Zero::zero()); // Attempt again same epoch - panic
Suggestion:
blocksrc/flow_test/test.cairo line 2672 at r1 (raw file):
:result, expected_error: StakingError::REWARDS_ALREADY_UPDATED.describe(), ); system.advance_epoch();
Suggestion:
advance_blocksrc/flow_test/test.cairo line 2674 at r1 (raw file):
system.advance_epoch(); // Disable rewards is false with v3 off
Suggestion:
Disable rewards = false with consensus off - no rewards.src/flow_test/test.cairo line 2679 at r1 (raw file):
let rewards = system.staker_claim_rewards(:staker); assert!(rewards == Zero::zero()); // Attempt again same epoch - panic
Suggestion:
blocksrc/flow_test/test.cairo line 2687 at r1 (raw file):
:result, expected_error: StakingError::REWARDS_ALREADY_UPDATED.describe(), ); system.advance_epoch();
Suggestion:
.advance_blocksrc/flow_test/test.cairo line 2693 at r1 (raw file):
.set_consensus_rewards_first_epoch(epoch_id: system.staking.get_current_epoch() + K.into()); // Disable rewards is true with v3 before consensus epoch
Suggestion:
Disable rewards = true before consensus epoch - no rewards.src/flow_test/test.cairo line 2698 at r1 (raw file):
let rewards = system.staker_claim_rewards(:staker); assert!(rewards == Zero::zero()); // Attempt again same epoch - panic
Suggestion:
blocksrc/flow_test/test.cairo line 2706 at r1 (raw file):
:result, expected_error: StakingError::REWARDS_ALREADY_UPDATED.describe(), ); system.advance_epoch();
Suggestion:
advance_blocksrc/flow_test/test.cairo line 2708 at r1 (raw file):
system.advance_epoch(); // Disable rewards is false with v3 before consensus epoch
Suggestion:
Disable rewards = false before consensus epoch - no rewards.src/flow_test/test.cairo line 2713 at r1 (raw file):
let rewards = system.staker_claim_rewards(:staker); assert!(rewards == Zero::zero()); // Attempt again same epoch - panic
Suggestion:
blocksrc/flow_test/test.cairo line 2723 at r1 (raw file):
system.advance_epoch(); // Disable rewards is true with v3 on
Suggestion:
Disable rewards = true with consesus on - no rewards.src/flow_test/test.cairo line 2728 at r1 (raw file):
let rewards = system.staker_claim_rewards(:staker); assert!(rewards == Zero::zero()); // Attempt again same epoch - panic
Suggestion:
blocksrc/flow_test/test.cairo line 2736 at r1 (raw file):
:result, expected_error: StakingError::REWARDS_ALREADY_UPDATED.describe(), ); system.advance_epoch();
Suggestion:
advance_blocksrc/flow_test/test.cairo line 2738 at r1 (raw file):
system.advance_epoch(); // Disable rewards is false with v3 on
Suggestion:
Disable rewards = false with consensus on - distribute rewards.src/flow_test/test.cairo line 2740 at r1 (raw file):
// Disable rewards is false with v3 on system.update_rewards(:staker, disable_rewards: false); // Test rewards are not distributed
Suggestion:
Test rewards are distributed.src/flow_test/test.cairo line 2750 at r1 (raw file):
); assert!(rewards == expected_rewards); // Attempt again same epoch - panic
Suggestion:
blocka8d6962 to
1ca1905
Compare
arad-starkware
left a comment
There was a problem hiding this comment.
Reviewable status: 0 of 3 files reviewed, 30 unresolved discussions (waiting on @NirLevi-starkware and @noa-starkware)
src/flow_test/test.cairo line 2637 at r1 (raw file):
Previously, noa-starkware wrote…
What does it mean?
means it's a typo
src/flow_test/test.cairo line 2640 at r1 (raw file):
Previously, noa-starkware wrote…
same
same :(
src/flow_test/test.cairo line 2662 at r1 (raw file):
Previously, noa-starkware wrote…
Is it better than staker_info.unclaimed_rewards?
I'm used to using that since it doesn't aggregate unclaimed rewards between checks.
In this test it doesn't matter since all rewards are zero except for the last one, but I like this methodology.
src/flow_test/utils.cairo line 1506 at r1 (raw file):
Previously, noa-starkware wrote…
Is there a more fitting trait than staker trait?
yes
src/flow_test/test.cairo line 2630 at r1 (raw file):
/// Flow: /// Staker stake /// Disable rewards is true with v3 off
Done.
src/flow_test/test.cairo line 2632 at r1 (raw file):
/// Disable rewards is true with v3 off /// Test rewards are not distributed /// Attempt again same epoch - panic
Done.
src/flow_test/test.cairo line 2633 at r1 (raw file):
/// Test rewards are not distributed /// Attempt again same epoch - panic /// Disable rewards is false with v3 off
Done.
src/flow_test/test.cairo line 2635 at r1 (raw file):
/// Disable rewards is false with v3 off /// Test rewards are not distributed /// Attempt again same epoch - panic
Done.
src/flow_test/test.cairo line 2639 at r1 (raw file):
/// Disable rewards is true with v3 before consensus epoch /// Test rewards are not distributed /// Attempt again same epoch - panic
Done.
src/flow_test/test.cairo line 2642 at r1 (raw file):
/// Disable rewards is false with v3 before consensus epoch /// Test rewards are not distributed /// Attempt again same epoch - panic
Done.
src/flow_test/test.cairo line 2643 at r1 (raw file):
/// Test rewards are not distributed /// Attempt again same epoch - panic /// Disable rewards is true with v3 on
Done.
src/flow_test/test.cairo line 2645 at r1 (raw file):
/// Disable rewards is true with v3 on /// Test rewards are not distributed /// Attempt again same epoch - panic
Done.
src/flow_test/test.cairo line 2646 at r1 (raw file):
/// Test rewards are not distributed /// Attempt again same epoch - panic /// Disable rewards is false with v3 on
Done.
src/flow_test/test.cairo line 2648 at r1 (raw file):
/// Disable rewards is false with v3 on /// Test rewards are distributed /// Attempt again same epoch - panic
Done.
src/flow_test/test.cairo line 2651 at r1 (raw file):
#[test] #[feature("safe_dispatcher")] fn update_rewards_disable_rewards_flow_test() {
Done
src/flow_test/test.cairo line 2659 at r1 (raw file):
system.advance_k_epochs(); // Disable rewards is true with v3 off
Done
src/flow_test/test.cairo line 2663 at r1 (raw file):
// Test rewards are not distributed let rewards = system.staker_claim_rewards(:staker); assert!(rewards == Zero::zero());
Done
src/flow_test/test.cairo line 2664 at r1 (raw file):
let rewards = system.staker_claim_rewards(:staker); assert!(rewards == Zero::zero()); // Attempt again same epoch - panic
Done.
src/flow_test/test.cairo line 2672 at r1 (raw file):
:result, expected_error: StakingError::REWARDS_ALREADY_UPDATED.describe(), ); system.advance_epoch();
Done.
src/flow_test/test.cairo line 2674 at r1 (raw file):
system.advance_epoch(); // Disable rewards is false with v3 off
Done.
src/flow_test/test.cairo line 2679 at r1 (raw file):
let rewards = system.staker_claim_rewards(:staker); assert!(rewards == Zero::zero()); // Attempt again same epoch - panic
Done.
src/flow_test/test.cairo line 2687 at r1 (raw file):
:result, expected_error: StakingError::REWARDS_ALREADY_UPDATED.describe(), ); system.advance_epoch();
Done.
src/flow_test/test.cairo line 2693 at r1 (raw file):
.set_consensus_rewards_first_epoch(epoch_id: system.staking.get_current_epoch() + K.into()); // Disable rewards is true with v3 before consensus epoch
Done.
src/flow_test/test.cairo line 2698 at r1 (raw file):
let rewards = system.staker_claim_rewards(:staker); assert!(rewards == Zero::zero()); // Attempt again same epoch - panic
Done.
src/flow_test/test.cairo line 2706 at r1 (raw file):
:result, expected_error: StakingError::REWARDS_ALREADY_UPDATED.describe(), ); system.advance_epoch();
Done.
src/flow_test/test.cairo line 2708 at r1 (raw file):
system.advance_epoch(); // Disable rewards is false with v3 before consensus epoch
Done.
src/flow_test/test.cairo line 2713 at r1 (raw file):
let rewards = system.staker_claim_rewards(:staker); assert!(rewards == Zero::zero()); // Attempt again same epoch - panic
Done.
src/flow_test/test.cairo line 2723 at r1 (raw file):
system.advance_epoch(); // Disable rewards is true with v3 on
Done.
src/flow_test/test.cairo line 2728 at r1 (raw file):
let rewards = system.staker_claim_rewards(:staker); assert!(rewards == Zero::zero()); // Attempt again same epoch - panic
Done.
src/flow_test/test.cairo line 2736 at r1 (raw file):
:result, expected_error: StakingError::REWARDS_ALREADY_UPDATED.describe(), ); system.advance_epoch();
Done.
src/flow_test/test.cairo line 2738 at r1 (raw file):
system.advance_epoch(); // Disable rewards is false with v3 on
Done.
src/flow_test/test.cairo line 2740 at r1 (raw file):
// Disable rewards is false with v3 on system.update_rewards(:staker, disable_rewards: false); // Test rewards are not distributed
Done.
src/flow_test/test.cairo line 2750 at r1 (raw file):
); assert!(rewards == expected_rewards); // Attempt again same epoch - panic
Done.
noa-starkware
left a comment
There was a problem hiding this comment.
@noa-starkware reviewed 1 of 3 files at r2.
Reviewable status: 1 of 3 files reviewed, 7 unresolved discussions (waiting on @NirLevi-starkware)
src/flow_test/test.cairo line 2633 at r2 (raw file):
/// Staker stake /// Disable rewards is true with consensus off /// Test rewards are not distributed
Suggestion:
/// Disable rewards is true with consensus off - test no rewardssrc/flow_test/test.cairo line 2636 at r2 (raw file):
/// Attempt again same block - panic /// Disable rewards is false with consensus off /// Test rewards are not distributed
same
Code quote:
/// Disable rewards is false with consensus off
/// Test rewards are not distributedsrc/flow_test/test.cairo line 2638 at r2 (raw file):
/// Test rewards are not distributed /// Attempt again same block - panic /// Enable consensus rewards (in 2 epochs)
Suggestion:
Ksrc/flow_test/test.cairo line 2640 at r2 (raw file):
/// Enable consensus rewards (in 2 epochs) /// Disable rewards is true before consensus epoch /// Test rewards are not distributed
same
Code quote:
/// Disable rewards is true before consensus epoch
/// Test rewards are not distributedsrc/flow_test/test.cairo line 2643 at r2 (raw file):
/// Attempt again same block - panic /// Disable rewards is false before consensus epoch /// Test rewards are not distributed
same
Code quote:
/// Disable rewards is false before consensus epoch
/// Test rewards are not distributedsrc/flow_test/test.cairo line 2646 at r2 (raw file):
/// Attempt again same block - panic /// Disable rewards is true with consensus on /// Test rewards are not distributed
same
Code quote:
/// Disable rewards is true with consensus on
/// Test rewards are not distributedsrc/flow_test/test.cairo line 2649 at r2 (raw file):
/// Attempt again same block - panic /// Disable rewards is false with consensus on /// Test rewards are distributed
same
Code quote:
/// Disable rewards is false with consensus on
/// Test rewards are distributed1ca1905 to
7de0c7c
Compare
arad-starkware
left a comment
There was a problem hiding this comment.
Reviewable status: 1 of 3 files reviewed, 7 unresolved discussions (waiting on @NirLevi-starkware and @noa-starkware)
src/flow_test/test.cairo line 2636 at r2 (raw file):
Previously, noa-starkware wrote…
same
Done.
src/flow_test/test.cairo line 2640 at r2 (raw file):
Previously, noa-starkware wrote…
same
Done.
src/flow_test/test.cairo line 2643 at r2 (raw file):
Previously, noa-starkware wrote…
same
Done.
src/flow_test/test.cairo line 2646 at r2 (raw file):
Previously, noa-starkware wrote…
same
Done.
src/flow_test/test.cairo line 2649 at r2 (raw file):
Previously, noa-starkware wrote…
same
Done.
src/flow_test/test.cairo line 2633 at r2 (raw file):
/// Staker stake /// Disable rewards is true with consensus off /// Test rewards are not distributed
Done.
src/flow_test/test.cairo line 2638 at r2 (raw file):
/// Test rewards are not distributed /// Attempt again same block - panic /// Enable consensus rewards (in 2 epochs)
Done.
noa-starkware
left a comment
There was a problem hiding this comment.
Reviewable status: 1 of 3 files reviewed, 1 unresolved discussion (waiting on @NirLevi-starkware)
src/flow_test/test.cairo line 2744 at r3 (raw file):
staking_contract: system.staking.address, minting_curve_contract: system.minting_curve.address, );
assert expected rewards is non zero
7de0c7c to
670c521
Compare
arad-starkware
left a comment
There was a problem hiding this comment.
Reviewable status: 1 of 3 files reviewed, 1 unresolved discussion (waiting on @NirLevi-starkware and @noa-starkware)
src/flow_test/test.cairo line 2744 at r3 (raw file):
Previously, noa-starkware wrote…
assert expected rewards is non zero
Done.
noa-starkware
left a comment
There was a problem hiding this comment.
Reviewable status: 1 of 3 files reviewed, all discussions resolved (waiting on @NirLevi-starkware)
670c521 to
ff2da76
Compare
Merge activity
|

This change is
Note
Adds a flow test for
update_rewardswithdisable_rewardsacross consensus-rewards states, and exposes RewardsManager dispatchers and config helper to support it.src/flow_test/test.cairo):update_rewards_disable_rewards_consensus_rewards_flow_testverifying:disable_rewardsis true (consensus off/on) and panic on same-block re-call.disable_rewardsis false before consensus epoch; rewards after consensus epoch.StakingError::REWARDS_ALREADY_UPDATED,advance_block_number_global, and expected V3 rewards viacalculate_staker_strk_rewards_with_balances_v3.src/flow_test/utils.cairo):rewards_manager_dispatcher,rewards_manager_safe_dispatcher(interfaces wired instaking::staking::interface).set_consensus_rewards_first_epoch(epoch_id)helper viaIStakingConfigDispatcher.SystemState::update_rewards(staker, disable_rewards)convenience wrapper.src/flow_test/flow_ideas.md):disable_rewardsscenarios.Written by Cursor Bugbot for commit ff2da76. This will update automatically on new commits. Configure here.