-
Notifications
You must be signed in to change notification settings - Fork 9
fix upgrade agglayer with specified version using kurtosis service up… #138
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
fix upgrade agglayer with specified version using kurtosis service up… #138
Conversation
jhkimqd
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.
a few comments + the bridging seems to fail after the run - seems like the agglayer port is misconfigured after update
[2025-08-14 12:24:57] [cdk-node-001--23617411676b405995494c6879e537f6] �2025-08-14T12:24:57.430Z ERROR aggregator/aggregator.go:540 failed to send tx to the agglayer: Post "http://agglayer:4444": dial tcp 172.16.0.18:4444: connect: connection refused {"pid": 9, "version": "v0.5.4-rc1", "module": "aggregator"}
scenarios/agglayer-upgrade-with-supplied-version/assets/cdk-erigon-pp.yml
Show resolved
Hide resolved
scenarios/agglayer-upgrade-with-supplied-version/assets/cdk-erigon-rollup.yml
Show resolved
Hide resolved
obynonwane
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.
Update to setup misconfiguration of e2e scenario for upgrade and downgrade of agglayer
jhkimqd
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.
Even with the Spin up cdk-erigon pp commented out and running the script, it doesn't seem to be working
[2025-08-18 09:26:14] [cdk-node-001--7d35cc4db0dd478eb0d6643048f2c668] �2025-08-18T09:24:50.755Z ERROR aggregator/aggregator.go:540 failed to send tx to the agglayer: Post "http://agglayer:4444": dial tcp 172.16.0.18:4444: connect: connection refused {"pid": 9, "version": "v0.5.4-rc1", "module": "aggregator"}
[2025-08-18 09:26:14] [cdk-node-001--7d35cc4db0dd478eb0d6643048f2c668] �2025-08-18T09:24:50.922Z ERROR aggregator/aggregator.go:540 failed to send tx to the agglayer: Post "http://agglayer:4444": dial tcp 172.16.0.18:4444: connect: connection refused {"pid": 9, "version": "v0.5.4-rc1", "module": "aggregator"}
[2025-08-18 09:26:14] [cdk-node-001--7d35cc4db0dd478eb0d6643048f2c668] �2025-08-18T09:25:20.928Z ERROR aggregator/aggregator.go:540 failed to send tx to the agglayer: Post "http://agglayer:4444": dial tcp 172.16.0.18:4444: connect: connection refused {"pid": 9, "version": "v0.5.4-rc1", "module": "aggregator"}
[2025-08-18 09:26:14] [cdk-node-001--7d35cc4db0dd478eb0d6643048f2c668] �2025-08-18T09:25:50.934Z ERROR aggregator/aggregator.go:540 failed to send tx to the agglayer: Post "http://agglayer:4444": dial tcp 172.16.0.18:4444: connect: connection refused {"pid": 9, "version": "v0.5.4-rc1", "module": "aggregator"}
| echo '║ A T T A C H I N G C D K E R I G O N P E R S I M I S T I C P R O O F (P P) ║' | ||
| echo '╚═══════════════════════════════════════════════════════════════════════════════════════════╝' | ||
|
|
||
| Spin up cdk-erigon pp |
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.
this should be commented out - the script won't run
obynonwane
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.
Run the upgrade with close version - fixes
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.
@obynonwane could you resolve previous comments which you think are resolved?
jhkimqd
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.
I've tested with the current main commit of kurtosis be8cab2e9ed799ab91319fee0f488fab04a393ac in the env file with something like
ENCLAVE_NAME=cdk
KURTOSIS_PACKAGE_HASH=be8cab2e9ed799ab91319fee0f488fab04a393ac
SP1_NETWORK_KEY=<some_dummy_key> # Replace with valid key
FROM_TAG=0.3.4
TO_TAG=0.3.5
and ran with ./run.sh 0.3.4 0.3.5
I seem to get an error.
My understanding was that this would ideally be used to tested upgrade/downgrades of new agglayer releases, so I think the above scenario should be working.
obynonwane
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.
Updated the args file to use the <TO_IMAGE> when creating the initial config file
jhkimqd
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.
scenarios/agglayer-upgrade-with-supplied-version/assets/cdk-erigon-rollup.yml
Outdated
Show resolved
Hide resolved
obynonwane
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.
fixed the additional_services that requires tx_spammer
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.
obynonwane
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.
The script now can run with the latest commit, it is important to note that the configuration for the input argument file is diferent depening on the kurtosis commit hash been ran.
obynonwane
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.
fixed issue with roolup registration on agglayer
… branch so as to enable the docker build workflow to pass since the branch is built from a fork
obynonwane
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.
Removed docker secret from the agglayer-upgrade-with-supplied-version branch so as to enable the docker build workflow to pass since the branch is built from a fork and do not required for this job
praetoriansentry
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.
Some minor changes. The most important thing I think would be to get a success criteria at the end.
In the longer term (we can do it with a separate PR) it's going to be important to have different types of rollups. Ideally:
- cdk-erigon-validium (you already have this)
- cdk-opgeth PP
- cdk-opgeth FEP
Each one of those uses a different code path so it would be great if we could have each network type running in the same enclave and then at the end we make sure there is a settlement in each network.
| sed -i "s#^FROM_IMAGE=.*#FROM_IMAGE=$FROM_IMAGE#" .env | ||
| sed -i "s#^TO_IMAGE=.*#TO_IMAGE=$TO_IMAGE#" .env | ||
| sed -i "s#^FROM_TAG=.*#FROM_TAG=$FROM_TAG#" .env | ||
| sed -i "s#^TO_TAG=.*#TO_TAG=$TO_TAG#" .env |
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 would say this type of manipulation is a little odd. I think usually I would assume the .env file should be configured by the user. I wouldn't expect running a script would modify the configuration file. It might be worth considering a better way to do this.
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.
For this I think, in my opinion it is more convenient to update the .env from the script, especially since we are supplying the FROM_TAG and TO_TAG when executing the script e.g "./run FROM_TAG TO_TAG"
Hence no need for user to go and manually input those environments in there .env. However if the user would have to manually supply those in the .env then we would just run the script without the FROM_TAG and TO_TAG e.g just ./run.sh
However FROM_IMAGE and TO_IMAGE have been removed not needed.
|
|
||
|
|
||
| fi No newline at end of file |
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 would say at the end of this script, need to at a minimum wait for a new settlement through the agglayer. You can do something with cast logs... e.g.
- Get the current block number on L1
- Run
cast logson the rollup manager looking for a verification event - If you get one, then finish successfully. Optionally, you could wait for one for each network.
- If 10 minutes (or some timeout) goes by without a verification, fail.
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.
This was added checked for specific event logs VerifyBatchesTrustedAggregator, waited for 3 minutes
obynonwane
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.
added script to check for event logs to determine deployment success
obynonwane
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.
removed unused shell variable
obynonwane
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.
update remove tmp file after execution
obynonwane
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.
shellchecked to clean up
scenarios/agglayer-upgrade-with-supplied-version/check_verification.sh
Outdated
Show resolved
Hide resolved
obynonwane
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.
updated script format to be consistent across board
|
Closing in favor of #171 |


fix upgrade agglayer with specified version using kurtosis service up…