-
Notifications
You must be signed in to change notification settings - Fork 41
test: add balances delay flow #65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
noa-starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 3 files reviewed, 5 unresolved discussions (waiting on @arad-starkware)
src/flow_test/flows.cairo line 7147 at r1 (raw file):
/// Flow: /// Epoch 0: /// Staker stake
why not delegators enter here and increase later?
src/flow_test/flows.cairo line 7155 at r1 (raw file):
/// Delegators increase delegation STRK and BTC /// Test total staking power - according to Epoch 0 /// Test staker balance with attestation rewards - according to Epoch 0
maybe use actual numbers or x,y,... to make it clearer?
Code quote:
according to Epoch 0src/flow_test/flows.cairo line 7170 at r1 (raw file):
pub(crate) staker: Option<Staker>, pub(crate) stake_amount: Option<Amount>, pub(crate) commission: Option<Commission>,
You can get that from staker_info after upgrade instead of saving it
Code quote:
pub(crate) commission: Option<Commission>,src/flow_test/flows.cairo line 7343 at r1 (raw file):
let staker_stake = stake_amount * 3; let strk_pool_balance = strk_delegated_amount * 2; let btc_pool_balance = btc_delegated_amount * 2;
Suggestion:
let staker_stake = staker_stake + stake_amount;
let strk_pool_balance = strk_pool_balance * 2;
let btc_pool_balance = btc_pool_balance * 2;src/flow_test/flows.cairo line 7375 at r1 (raw file):
); assert!(expected_btc_commission_rewards.is_non_zero()); assert!(expected_btc_pool_rewards.is_non_zero());
assert rewards are not equal to prev rewards?
Code quote:
assert!(expected_staker_rewards.is_non_zero());
assert!(expected_strk_pool_rewards.is_non_zero());
let (expected_btc_commission_rewards, expected_btc_pool_rewards) =
calculate_staker_btc_pool_rewards_v2(
pool_balance: btc_pool_balance,
:commission,
:staking_contract,
:minting_curve_contract,
token_address: system.btc_token.contract_address(),
);
assert!(expected_btc_commission_rewards.is_non_zero());
assert!(expected_btc_pool_rewards.is_non_zero());e4e94f2 to
33b6d35
Compare
33b6d35 to
bfc06f8
Compare
e8b4e7d to
631614f
Compare
8a264ba to
4c26f6d
Compare
arad-starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @arad-starkware and @noa-starkware)
src/flow_test/flows.cairo line 7147 at r1 (raw file):
Previously, noa-starkware wrote…
why not delegators enter here and increase later?
Why yes?
The staker does it because we can't attest unless an epoch has passed since the initial stake, but there's no such limitation on the delegators.
src/flow_test/flows.cairo line 7155 at r1 (raw file):
Previously, noa-starkware wrote…
maybe use actual numbers or x,y,... to make it clearer?
wdym?
src/flow_test/flows.cairo line 7170 at r1 (raw file):
Previously, noa-starkware wrote…
You can get that from staker_info after upgrade instead of saving it
Technically I can infer everything here from the staker address and the view functions, why draw the line at commission?
src/flow_test/flows.cairo line 7375 at r1 (raw file):
Previously, noa-starkware wrote…
assert rewards are not equal to prev rewards?
Done
src/flow_test/flows.cairo line 7343 at r1 (raw file):
let staker_stake = stake_amount * 3; let strk_pool_balance = strk_delegated_amount * 2; let btc_pool_balance = btc_delegated_amount * 2;
Done
noa-starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @arad-starkware)
src/flow_test/flows.cairo line 7375 at r1 (raw file):
Previously, arad-starkware wrote…
Done
Why bigger?
4c26f6d to
85b9981
Compare
631614f to
a1ef5f2
Compare
a1ef5f2 to
b027a32
Compare
85b9981 to
9ce74b6
Compare
9ce74b6 to
1204107
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #65 +/- ##
=======================================
Coverage 95.60% 95.60%
=======================================
Files 42 42
Lines 9754 9754
=======================================
Hits 9325 9325
Misses 429 429 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
arad-starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @noa-starkware)
src/flow_test/flows.cairo line 7375 at r1 (raw file):
Previously, noa-starkware wrote…
Why bigger?
total supply goes up -> yearly mint goes up -> rewards go up
There are several places where rewards might have gone down though:
- Second stake for the staker, since his relative share of the staking power goes down (strk delegator enters).
- Subsequent delegations by the strk delegator, since the staker's share grows more than the delegator's share (
stake_amount > strk_delegation_amount).
It seems as though in both cases the increase in total supply causes an increase in rewards that makes up for this though, which is why the test passes.
Maybe we should only assert different rewards, instead of larger ones, in those cases. WDYT?
c909703 to
d3ed532
Compare
noa-starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @arad-starkware)
src/flow_test/flows.cairo line 7375 at r1 (raw file):
Previously, arad-starkware wrote…
total supply goes up -> yearly mint goes up -> rewards go up
There are several places where rewards might have gone down though:
- Second stake for the staker, since his relative share of the staking power goes down (strk delegator enters).
- Subsequent delegations by the strk delegator, since the staker's share grows more than the delegator's share (
stake_amount > strk_delegation_amount).
It seems as though in both cases the increase in total supply causes an increase in rewards that makes up for this though, which is why the test passes.
Maybe we should only assert different rewards, instead of larger ones, in those cases. WDYT?
I think only assert different rewards is better
arad-starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @noa-starkware)
src/flow_test/flows.cairo line 7375 at r1 (raw file):
Previously, noa-starkware wrote…
I think only assert different rewards is better
Done
071a55d to
6ad074b
Compare
noa-starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @arad-starkware)
src/flow_test/flows.cairo line 8676 at r6 (raw file):
// Test total staking power - according to Epoch 0. let staker_stake = stake_amount;
mut and += later
src/flow_test/flows.cairo line 8714 at r6 (raw file):
let staker_stake = staker_stake + stake_amount; let strk_pool_balance = strk_delegated_amount; let btc_pool_balance = btc_delegated_amount;
mut and += later
Code quote:
let strk_pool_balance = strk_delegated_amount;
let btc_pool_balance = btc_delegated_amount;6ad074b to
5f4a056
Compare
Merge activity
|

This change is
Note
Adds a new BalancesDelayFlow and mainnet fork test to verify staking power and STRK/BTC rewards timing across epochs pre/post-upgrade.
src/flow_test/flows.cairo):BalancesDelayFlow: New flow exercising delayed balance effects across epochs and an upgrade.src/flow_test/fork_test.cairo):balances_delay_flow_test(MAINNET_LATEST) to run the new flow.Written by Cursor Bugbot for commit 5f4a056. This will update automatically on new commits. Configure here.