feat!(seismic-tx): add auth_list to TxSeismic + bump deps#332
feat!(seismic-tx): add auth_list to TxSeismic + bump deps#332
Conversation
PR Review SummaryReviewed commit: ChangesAdds an Blocking Issues1. When building a authorization_list: vec![],If the Seismic node serializes a non-empty auth list in its RPC response (which the upstream seismic-alloy PR #82 presumably adds), this conversion will silently drop it. The downstream effect is a different sighash, causing incorrect sender recovery and breaking any code path that re-signs or re-verifies the deserialized transaction. The 2. Missing compact codec test for non-empty The only compact roundtrip test constructs a
Suggestions1. TODO comments in
2. Positive Notes
|
7c4163f to
7409c7d
Compare
|
@claude fixed your 2 blocking issues:
|
Re-Review SummaryReviewed commit: Previously Flagged — Now Resolved
Previously Flagged — Still Unresolved
New Issues in Fix CommitsNone found. New Suggestions
|
61a0176 to
7c27124
Compare
|
rebased on top of #336 |
We just bump the dependencies of other repos to get the new tx type, and then chain it everywhere needed.
7c27124 to
50e7b6e
Compare
|
rebased on top of #340 |
) Group of related PRs: - SeismicSystems/seismic-alloy#82 - #209 - SeismicSystems/seismic-evm#40 - SeismicSystems/seismic-reth#332 Before the function was gating and only allowing 7702 txs to have an auth list. Made it more generic such that any tx with an auth_list will now run the function. In particular, our TxSeismic changes to add an auth_list will now work. See SeismicSystems/seismic-alloy#82 Interestingly, we don't even need to update our seismic-alloy commit given that this change is generic.
) Group of related PRs: - SeismicSystems/seismic-alloy#82 - #209 - SeismicSystems/seismic-evm#40 - SeismicSystems/seismic-reth#332 Before the function was gating and only allowing 7702 txs to have an auth list. Made it more generic such that any tx with an auth_list will now run the function. In particular, our TxSeismic changes to add an auth_list will now work. See SeismicSystems/seismic-alloy#82 Interestingly, we don't even need to update our seismic-alloy commit given that this change is generic.
) Group of related PRs: - SeismicSystems/seismic-alloy#82 - SeismicSystems/seismic-evm#40 - SeismicSystems/seismic-reth#332 Before the function was gating and only allowing 7702 txs to have an auth list. Made it more generic such that any tx with an auth_list will now run the function. In particular, our TxSeismic changes to add an auth_list will now work. See SeismicSystems/seismic-alloy#82 Interestingly, we don't even need to update our seismic-alloy commit given that this change is generic.
Group of related PRs: - #82 - SeismicSystems/seismic-revm#218 - SeismicSystems/seismic-evm#40 - SeismicSystems/seismic-reth#332 As we plan to support gas sponsorship, it is nice if the 7702 delegation can be done in the same tx as the userOp. Issue is that the normal 7702 txs don't support encrypted calldata. So we add authorization_list to the TxSeismic so that a user delegation to a smart account followed by a shielded call to that contract can be composed in the same tx. ## Note provider tests that are failing in CI are because sanvil hasn't been updated yet, which is obvious since sanvil needs this PR to get merged. This is a circular dependency that we need to break by merging here first. --------- Co-authored-by: Henry Baldwin <[email protected]>
Group of related PRs: - SeismicSystems/seismic-alloy#82 - SeismicSystems/seismic-revm#209 - #40 - SeismicSystems/seismic-reth#332 Needed to fix one test, nothing else.
|
Updates seismic-revm, seismic-evm, and seismic-alloy dependencies and removes RngMode from transaction abstraction while adding EIP-7702 authorization list support. Phase 1
Phase 2
LGTM - The dependency updates clean up the RngMode abstraction properly across the codebase. The new code override rejection provides good security for Seismic's privacy model with comprehensive test coverage. The authorization_list addition follows EIP-7702 standards and includes proper roundtrip testing. |
HenryMBaldwin
left a comment
There was a problem hiding this comment.
This overlaps with #367
I think this is fine, as the way we merged seismic-evm sort of makes this necessary. We can close that pr, but we need to just make sure we get everything from it. I think the only thing we're missing is the e2e tests here
If you add those and we can merge this pr and close 367.
There was a problem hiding this comment.
I went ahead and added the tests in 5dd8b19. Assuming they pass in ci I think we're gtg.
|
CI is stuck. closing and reopening PR. |
Update seismic dependencies (alloy, revm, evm) and fix anything that needs updates in reth:
authorization_list: Vec<SignedAuthorization>(EIP-7702) to TxSeismic, including Compact codec encoding/decoding and testsCodeOverrideNotPermittedvariant toEthApiErrorto handle the newStateOverrideErrorvariant from seismic-evm (Seismic disallows code overrides for privacy)RngModefrom all call sites — seismic-revm no longer exposes it onSeismicTransaction(rng mode is now determined internally)SeismicTypedTransaction::SeismictoTxEnv::from_recovered_txnow that seismic-evm provides the implRelated PRs:
Note that this is a breaking change, since the tx's rlp encoding is changed, which breaks all of: wire format, db format (whats stored in tx trie), and signatures (signing over different rlp encoding).