Skip to content

Integrate with Tenderly and perform hash matching#783

Closed
alcueca wants to merge 38 commits intomainfrom
alcueca/verify-hashes
Closed

Integrate with Tenderly and perform hash matching#783
alcueca wants to merge 38 commits intomainfrom
alcueca/verify-hashes

Conversation

@alcueca
Copy link
Copy Markdown
Contributor

@alcueca alcueca commented Mar 22, 2025

Description

Please export the tenderly access token in the 1password EVM Safety vault as TENDERLY_ACCESS_TOKEN for the below to work.

  • get-tenderly-hashes.sh now takes a simulation link as input
  • MultisigTask.sol no longer generates a tenderly json payload
  • just simulate-tenderly <path-to-task> <optional-safe-name> in src/improvements simulates a task both locally and on tenderly, both single and nested. Returns the usual output, the tenderly simulation link (already simulated, to save some time) and the hashes.
  • just verify-non-terminal-tasks simulates locally and on tenderly all tasks that are not SIGNED or EXECUTED, and compares the domain and message hashes in VALIDATIONS.md against the ones in the simulations.

@alcueca alcueca requested review from a team as code owners March 22, 2025 08:48
@alcueca alcueca requested a review from maurelian March 22, 2025 08:48
@alcueca
Copy link
Copy Markdown
Contributor Author

alcueca commented Mar 22, 2025

Note that just verify-non-terminal-tasks fails with sep/001-opcm-upgrade-v200 because MultisigTask doesn't print out its individual domain and message hash. I think that's a bug in MultisigTask.

@blmalone
Copy link
Copy Markdown
Contributor

blmalone commented Mar 25, 2025

This approach is great. Few comments:

Running:

cd src/improvements
just simulate-tenderly sep/001-opcm-upgrade-v200 nested

I get an error:

Error: script failed: vm.readFile: failed to read from "/Users/blaine/code/superchain-ops/tasks/sep/001-opcm-upgrade-v200/config.toml": No such file or directory (os error 2)
error: Recipe `simulate-tenderly` failed with exit code 1

Running

cd src/improvements
just simulate-tenderly src/improvements/tasks/sep/001-opcm-upgrade-v200 nested

I get an error:

/Users/blaine/code/superchain-ops/src/improvements/script/simulate-task.sh: line 24: pushd: src/improvements/tasks/sep/001-opcm-upgrade-v200: No such file or directory
error: Recipe `simulate-tenderly` failed with exit code 1

It would be good to have some tests for this functionality. Also, is this a step that we're going to get signers to execute separately? How does it slot into the current flow? Or is that a future task?

Copy link
Copy Markdown
Contributor

@blmalone blmalone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a comment on the PR. I ran into some errors.

Comment thread src/improvements/script/get-tenderly-hashes.sh Outdated
Comment thread src/improvements/script/get-tenderly-hashes.sh Outdated
@alcueca
Copy link
Copy Markdown
Contributor Author

alcueca commented Mar 25, 2025

Fixed, please try:

cd src/improvements
just simulate-tenderly `pwd`/tasks/sep/000-opcm-upgrade-v200 council
just simulate-tenderly `pwd`/tasks/sep/003-unichain-superchain-config-fix

alcueca and others added 10 commits March 25, 2025 15:59
* refactor: addresses.toml not longer holding safe addresses.

* fix: regression template tests fixed.

* fix: nit

* fix: add more docs to readme of base task.

* fix: note to update base state diffs.

* fix: fixed base sepolia validations.
* Minor fixes to sep/001

* mark ready
* update forge to use nightly version

* generate op verify input as json files

* fixes

* fixes

* add docs for it, use libstring and format json file

* fixxx

* update readme

* Update src/improvements/justfile

Co-authored-by: blaine <blainemalone01@gmail.com>

* fix readme

* fix readme

* fixes

* fixes

* simplify, it's no more a breaking change

* reduce diff

* add natspec for outputOPVerifyInputToFile

* use snake case for op verify inputs json folder, fix readme

* fixx

* improve log

* add support for op-verify qr

* Update src/improvements/tasks/MultisigTask.sol

Co-authored-by: smartcontracts <kelvinfichter@gmail.com>

* do not use json

* ...

* ...

* ...

* ...

* fix failing test

* fixes

* Update src/improvements/tasks/MultisigTask.sol

Co-authored-by: smartcontracts <kelvinfichter@gmail.com>

* Apply suggestions from code review

Co-authored-by: smartcontracts <kelvinfichter@gmail.com>

---------

Co-authored-by: blaine <blainemalone01@gmail.com>
Co-authored-by: smartcontracts <kelvinfichter@gmail.com>
@alcueca
Copy link
Copy Markdown
Contributor Author

alcueca commented Mar 26, 2025

Scripts working again, although most tasks are broken themselves.

just simulate-non-terminal-tasks works, but it exits with an error because 000-opcm-upgrade-v200 reverts on execution.
just simulate-tenderly tasks/sep/003-unichain-superchain-config-fix is the only one that works. It will extract the hashes but won't verify against VALIDATIONS.md because only just simulate-non-terminal-tasks does that.

Copy link
Copy Markdown
Contributor

@blmalone blmalone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left some comments on this. For some reason simulations aren't working for our U13 tasks anymore with these changes. I think it's because of the base contracts submodule change. Why is this included? It's not adding the TENDERLY_GAS env var to the Tenderly simulation link anymore.

I also ran just simulate-tenderly tasks/sep/001-opcm-upgrade-v200 child-safe-1 and I get the following error:

View the simulation in Tenderly dashboard:
https://dashboard.tenderly.co/oplabs/task-simulation/simulator/52633456-f754-41d8-8a16-f992f3c278a2
Error: Transaction failed in Tenderly
error: Recipe `simulate-tenderly` failed with exit code 1

When is this meant to get executed? I was expecting to see some CI job that runs this on every PR (or something like that - could be every merge to main).

Also, given how much churn has been included in this PR, can you add some tests? This could be a CI job that runs these checks.

Comment thread src/improvements/justfile Outdated
Comment thread src/improvements/justfile Outdated
Comment thread src/improvements/justfile
Comment thread src/improvements/justfile Outdated
@alcueca
Copy link
Copy Markdown
Contributor Author

alcueca commented Mar 28, 2025

I left some comments on slack, but the short of it is that the tasks in main are all broken in some way.

We can ignore exclude 000-opcm-upgrade-v200 because is already executed and 003-unichain-superchain-config-fix because it's a draft, but 001-opcm-upgrade-v200 needs to be fixed before it's signed.

I've added a CI step that will pass once that task is fixed. I suggest that the scripts in this branch are used to facilitate that:
Use just simulate-tenderly tasks/sep/001-opcm-upgrade-v200 child-safe-1 to quickly check if a task fails on Tenderly.
Once that passes, just verify-ready-tasks will tell you if that task (and any other ready ones) has matching hashes in VALIDATION.md, local simulation and tenderly simulation.

@alcueca
Copy link
Copy Markdown
Contributor Author

alcueca commented Apr 4, 2025

I'll try again to get this merged when I'm back from leave. The conflicts shouldn't build up much, just on config.toml files that can be easily fixed.

@blmalone
Copy link
Copy Markdown
Contributor

Going to close this PR. Superchain-ops has changed significantly since this PR was opened. If we want this feature, we will need to reprioritize it and then go back to the design stage. Let me know if I'm missing something here and you'd like to keep this PR open for another reason.

@blmalone blmalone closed this Aug 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants