-
Notifications
You must be signed in to change notification settings - Fork 41
test: add token state after upgrade flow #59
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
base: arad/test_add_update_rewards_toggle_tokens_flow_test
Are you sure you want to change the base?
test: add token state after upgrade flow #59
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
cde9c29 to
87dd152
Compare
a3d9239 to
1765bb9
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, 5 unresolved discussions (waiting on @arad-starkware)
src/flow_test/flows.cairo line 7147 at r1 (raw file):
/// Flow: /// Add tokens A and B
/// Delegate for both tokens
src/flow_test/flows.cairo line 7149 at r1 (raw file):
/// Add tokens A and B /// Enable token B /// Advance epoch
not relevant to the test, remove from doc
Code quote:
/// Enable token B
/// Advance epochsrc/flow_test/flows.cairo line 7237 at r1 (raw file):
); staking_attestation .update_rewards_from_attestation_contract(staker_address: staker.staker.address);
why not system.attest(staker)?
src/flow_test/flows.cairo line 7237 at r1 (raw file):
); staking_attestation .update_rewards_from_attestation_contract(staker_address: staker.staker.address);
test also staker rewards (own+commission from pool a)
src/flow_test/flows.cairo line 7250 at r1 (raw file):
assert!(expected_rewards.is_non_zero()); assert!(rewards_a == expected_rewards); assert!(rewards_b.is_zero());
test also with V3 rewards? we have such test somewhere?
87dd152 to
da91327
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, 5 unresolved discussions (waiting on @arad-starkware and @noa-starkware)
src/flow_test/flows.cairo line 7147 at r1 (raw file):
Previously, noa-starkware wrote…
/// Delegate for both tokens
Done.
src/flow_test/flows.cairo line 7149 at r1 (raw file):
Previously, noa-starkware wrote…
not relevant to the test, remove from doc
Done.
src/flow_test/flows.cairo line 7237 at r1 (raw file):
Previously, noa-starkware wrote…
why not system.attest(staker)?
Done.
src/flow_test/flows.cairo line 7237 at r1 (raw file):
Previously, noa-starkware wrote…
test also staker rewards (own+commission from pool a)
Done.
src/flow_test/flows.cairo line 7250 at r1 (raw file):
Previously, noa-starkware wrote…
test also with V3 rewards? we have such test somewhere?
I'm not sure what's the scenario you're describing, testing V3 rewards with disabled tokens after upgrade?
1765bb9 to
3573ad2
Compare
da91327 to
ae0c797
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, 4 unresolved discussions (waiting on @arad-starkware)
src/flow_test/flows.cairo line 7250 at r1 (raw file):
Previously, arad-starkware wrote…
I'm not sure what's the scenario you're describing, testing V3 rewards with disabled tokens after upgrade?
Yes, just continue the test with start consensus epoch, then call update_rewards and test rewards again (only a token enabled)
src/flow_test/flows.cairo line 7191 at r3 (raw file):
system.advance_epoch(); system.staking.disable_token(token_address: token_b.contract_address()); system.staking.enable_token(token_address: token_a.contract_address());
I think these lines might be redundant
Code quote:
system.staking.disable_token(token_address: token_b.contract_address());
system.staking.enable_token(token_address: token_a.contract_address());src/flow_test/flows.cairo line 7240 at r3 (raw file):
calculate_staker_btc_pool_rewards_v2( pool_balance: delegation_amount, commission: 200,
Suggestion:
system.staker_info_v1(:staker).pool_info.unwrap().commissionae0c797 to
b46a1ff
Compare
3573ad2 to
ffdb8ec
Compare
b46a1ff to
4b0ff1c
Compare
484a333 to
215d37b
Compare
2c8afd0 to
e48b02c
Compare
215d37b to
5deab3b
Compare
e48b02c to
845927c
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, 2 unresolved discussions (waiting on @arad-starkware and @noa-starkware)
src/flow_test/flows.cairo line 7250 at r1 (raw file):
Previously, noa-starkware wrote…
Yes, just continue the test with start consensus epoch, then call update_rewards and test rewards again (only a token enabled)
Done.
src/flow_test/flows.cairo line 7191 at r3 (raw file):
Previously, noa-starkware wrote…
I think these lines might be redundant
I wanted to explicitly disable a token instead of just leaving it disabled by default. Do you think it's unnecessary?
src/flow_test/flows.cairo line 7240 at r3 (raw file):
calculate_staker_btc_pool_rewards_v2( pool_balance: delegation_amount, commission: 200,
Done
845927c to
1fc2a9f
Compare
550520d to
a9c72c0
Compare
96cb2c9 to
625acb6
Compare
ed9fb2d to
878cc79
Compare
625acb6 to
d3512f2
Compare
2451b5c to
731bc76
Compare
8a0dadc to
bbf04ed
Compare
731bc76 to
bdca6a8
Compare
bbf04ed to
eb7dc58
Compare
bdca6a8 to
2edc4c4
Compare
2edc4c4 to
a123f39
Compare
41be9ca to
ddcbf57
Compare
6fa473d to
9734280
Compare
ddcbf57 to
0b18f21
Compare
0b18f21 to
129a1c6
Compare
9734280 to
0b9ed88
Compare
129a1c6 to
ca85233
Compare
0b9ed88 to
8a44fec
Compare

This change is
Note
Adds a new flow and test to verify token enable/disable behavior across upgrade and during consensus rewards, and imports v3 BTC pool reward calc.
EnableDisableTokenBeforeAfterUpgradeFlowinsrc/flow_test/flows.cairo:get_tokensview and rewards distribution pre/post-upgrade and under consensus rewards.enable_disable_token_before_after_upgrade_flow_testinsrc/flow_test/fork_test.cairo.calculate_staker_btc_pool_rewards_v3for v3 reward calculations.src/flow_test/flow_ideas.mdby removing obsolete token test idea.Written by Cursor Bugbot for commit 8a44fec. This will update automatically on new commits. Configure here.