feat: reward supplier eic#70
Conversation
8641ce1 to
5fe4899
Compare
a9ccb51 to
2793f13
Compare
5fe4899 to
af42a28
Compare
f4ee42e to
4465436
Compare
9b08472 to
5036954
Compare
4465436 to
24e1ab6
Compare
5036954 to
ce8d6f9
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
remollemo
left a comment
There was a problem hiding this comment.
@remollemo reviewed 2 files and made 2 comments.
Reviewable status: 2 of 7 files reviewed, 3 unresolved discussions (waiting on NirLevi-starkware and noa-starkware).
src/reward_supplier/eic.cairo line 40 at r4 (raw file):
"{}", Error::INVALID_MIN_MAX_BLOCK_DURATION, );
remove (checked two lines up)
Code quote:
assert!(
min_block_duration <= max_block_duration,
"{}",
Error::INVALID_MIN_MAX_BLOCK_DURATION,
);src/reward_supplier/errors.cairo line 24 at r4 (raw file):
Error::INVALID_MIN_MAX_BLOCK_DURATION => "Invalid min/max block duration", Error::INVALID_AVG_BLOCK_DURATION => "Invalid avg block duration", }
why adding stuff for the EIC? it's ok (and recommended) for the EIC to use existing code/types etc. but why dirting the code with its sepcific error messages?
Code quote:
INVALID_AVG_BLOCK_DURATION,
}
impl DescribableError of Describable<Error> {
fn describe(self: @Error) -> ByteArray {
match self {
Error::ON_RECEIVE_NOT_FROM_STARKGATE => "Only StarkGate can call on_receive",
Error::UNEXPECTED_TOKEN => "Unexpected token",
Error::BLOCK_DURATION_OVERFLOW => "Block duration calculation overflow",
Error::INVALID_BLOCK_NUMBER => "Invalid block number",
Error::INVALID_BLOCK_TIMESTAMP => "Invalid block timestamp",
Error::INVALID_MIN_MAX_BLOCK_DURATION => "Invalid min/max block duration",
Error::INVALID_AVG_BLOCK_DURATION => "Invalid avg block duration",
}24e1ab6 to
e017847
Compare
ce8d6f9 to
0682062
Compare
noa-starkware
left a comment
There was a problem hiding this comment.
@noa-starkware made 2 comments and resolved 2 discussions.
Reviewable status: 1 of 7 files reviewed, 2 unresolved discussions (waiting on NirLevi-starkware and remollemo).
src/reward_supplier/errors.cairo line 24 at r4 (raw file):
Previously, remollemo wrote…
why adding stuff for the EIC? it's ok (and recommended) for the EIC to use existing code/types etc. but why dirting the code with its sepcific error messages?
Done.
src/reward_supplier/eic.cairo line 19 at r3 (raw file):
} /// Expected data : [avg_block_duration, min_block_duration, max_block_duration]
Consider remove from EIC and initialize after upgrade with the setter
Code quote:
min_block_duration, max_block_duration
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #70 +/- ##
==========================================
- Coverage 95.82% 95.80% -0.02%
==========================================
Files 42 43 +1
Lines 10434 10519 +85
==========================================
+ Hits 9998 10078 +80
- Misses 436 441 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
remollemo
left a comment
There was a problem hiding this comment.
@remollemo made 2 comments and resolved 1 discussion.
Reviewable status: 1 of 7 files reviewed, 1 unresolved discussion (waiting on NirLevi-starkware and noa-starkware).
src/reward_supplier/eic.cairo line 19 at r3 (raw file):
Previously, noa-starkware wrote…
Consider remove from EIC and initialize after upgrade with the setter
- EIC is a viable option for upgrade and instantaneous migration on upgrade.
- having a setter, or not, is a different duscussion, where the
Elsepath is usually "ok we can do this with an EIC should we need to". - The "guideline" would be - do we think we need to tune it every now and then (then we better have setters) or we really don't think so, and then we don't need a setter, knowing we can always resort to an EIC, or upgrade to the code to have a setter, if we see our previous understanding had been incorrect.

This change is
Note
Medium Risk
Introduces a new External Initializer Contract used during
RewardSupplierupgrades and updates upgrade/test harnesses to invoke it; incorrect init data could break upgrade flows or set invalid reward calculation parameters.Overview
Adds a new
RewardSupplierEICexternal initializer contract to setavg_block_durationandblock_duration_configduring reward-supplier upgrades, with validation of the provided init calldata.Updates the V3 upgrade path in
flow_test/utils.cairoand sharedtest_utils.cairoto declare the new EIC, pass default init data viaEICData, and ensure the reward-supplier has anupgrade_governorrole configured.Extends
reward_suppliertests with coverage for successful EIC-based upgrades and multiple failure cases (wrong calldata length and invalid min/max/avg combinations).Written by Cursor Bugbot for commit 0682062. This will update automatically on new commits. Configure here.