-
Notifications
You must be signed in to change notification settings - Fork 375
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
feat: add onlyAggregator to respondToTask #883
feat: add onlyAggregator to respondToTask #883
Conversation
…-in-payment-from-batcher-to-aggregator-make-the-respond-to-ask-only-callable-by-one-agg
Changes to gas cost
🧾 Summary (10% most significant diffs)
Full diff report 👇
|
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.
I have done the following steps indicated in the PR description:
- Checkout to main.
- Deploy Aligned contracts.
- Use that anvil state in this branch.
- Upgrade.
- Call the contract.
But I'm getting transaction reverted
same comment as @NicolasRampoldi on my side too |
I have fixed the steps to test this PR , the contract had a refactor renaming the |
…r-to-aggregator-make-the-respond-to-ask-only-callable-by-one-agg
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.
LGTM. Ready for test in stage
…max-limit-eth-in-payment-from-batcher-to-aggregator-make-the-respond-to-ask-only-callable-by-one-agg
…max-limit-eth-in-payment-from-batcher-to-aggregator-make-the-respond-to-ask-only-callable-by-one-agg
…r-to-aggregator-make-the-respond-to-ask-only-callable-by-one-agg
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.
lgtm. The contract is deployed on stage
Note
Merge after #875This branch already has 875 contents, and is running on STAGEThis PR
onlyAggregator
modifier was added torespondToTask
.For this,
address aggregator
was added on storage (and GAP decreased)Implementation
To implement this contract upgrade, the same structure of implement pausable PR was followed;
alignedAggregator
.Deploying
Devnet:
Testnet:
contracts/scripts/.env
filecontracts/script/deploy/config/holesky/aligned.holesky.config.json
Testing this PR
The following call should revert:
cast call 0x1613beB3B2C4f22Ee086B2b38C1476A3cE7f78E8 "alignedAggregator()(address)" --rpc-url 127.0.0.1:8545
But the following call should not revert:
(this is just to verify we have the correct ServiceManager address)
cast call 0x1613beB3B2C4f22Ee086B2b38C1476A3cE7f78E8 "balanceOf(address)(uint256)" 0x1613beB3B2C4f22Ee086B2b38C1476A3cE7f78E8 --rpc-url 127.0.0.1:8545
Then close anvil
You must preserve this anvil state before you switch to the new branch, for this you can:
The following upgrade should run and work without problems:
Then you must start the devnet
And the following call should return the aggregator's address:
cast call 0x1613beB3B2C4f22Ee086B2b38C1476A3cE7f78E8 "alignedAggregator()(address)" --rpc-url 127.0.0.1:8545
Testing the modifier
To make sure the
onlyAggregator
modifier actually works, you must:anvil.aggregator.ecdsa.key.json
operator-3.ecdsa.key.json
with the nameanvil.aggregator.ecdsa.key.json
, where this was beforeAggregator should revert when trying to submit: