test(evm): pin chain-id enforcement for typed transactions - #3895
test(evm): pin chain-id enforcement for typed transactions#3895SatisLoeb wants to merge 1 commit into
Conversation
PR SummaryLow Risk Overview The test documents why this matters: Sei’s sender recovery is chain-id agnostic, so the mismatch is only caught in ante—not by go-ethereum-style Reviewed by Cursor Bugbot for commit 43c9e0c. Bugbot is set up for automated code reviews on this repo. Configure here. |
Sender recovery is chain-id agnostic. helpers.RecoverAddressesFromTx builds the sig hash from the signer's chain id and recovers the key, which is the last step of go-ethereum's modernSigner.Sender; the tx.ChainId() != s.chainID comparison that Sender also performs is done in EvmStatelessChecks instead. Two consequences are worth pinning. The ChainId field of a typed tx is not covered by the signature, so it can be edited on a signed tx without invalidating it. And because recovery does not compare chain ids, EvmStatelessChecks is the only step that rejects a mismatch on a typed tx, where a legacy tx would still be caught by AdjustV. The test asserts both halves on the synchronous giga driver, the giga OCC driver and v2: recovery still returns the true sender for an edited ChainId, and the transaction is nevertheless rejected with ErrInvalidChainID, moving no value and consuming no nonce. A correctly signed tx in the same setup still executes, so the guard is not over-broad.
aba4f40 to
43c9e0c
Compare
Describe your changes and provide context
Adds a regression test pinning chain-id enforcement for typed (non-legacy) transactions across all three executor paths: the synchronous giga driver, the giga OCC driver, and v2.
To be clear up front: this is not a vulnerability report. Current behaviour is correct on every path. The reason for the test is that the enforcement is load-bearing while living in only one place, and nothing currently covers it.
Sender recovery is chain-id agnostic.
helpers.RecoverAddressesFromTxbuilds the sig hash from the signer's chain id (modernSigner.Hashistx.inner.sigHash(s.chainID)) and recovers the key, which is the last step of go-ethereum'smodernSigner.Sender.Senderadditionally rejectstx.ChainId() != s.chainID; Sei performs that comparison inEvmStatelessChecks(app/ante/evm_checktx.go) instead, which both giga drivers call before execution (app/app.gosynchronous driver andmakeGigaDeliverTx).Two consequences seemed worth pinning:
ChainIdfield of a typed tx is not covered by the signature — it can be edited on an already-signed transaction without invalidating it.EvmStatelessChecksis the only step that rejects a mismatch on a typed tx. A legacy tx would still be caught independently byAdjustV(V - 2*chainID - 8leaves V outside {0,1} on a mismatch), but a typed tx has no such second line.So the test asserts both halves rather than just the outcome: recovery still returns the true sender for an edited
ChainId, and the transaction is nevertheless rejected withErrInvalidChainID, moving no value and consuming no nonce. A correctly signed transaction in the same setup still executes, so the guard is shown not to be over-broad.Happy to move this under
app/ante/or reshape it if you'd rather the coverage live closer to the check itself — it sits ingiga/testsbecause that harness can exercise all three executors through a real block.Testing performed to validate your change
go test ./giga/tests/ -run TestChainIDEnforced_TypedTx -count=1 -v— passes onGigaSequential,GigaOCCandV2withOCC.I also verified the test actually detects the regression it claims to, rather than passing regardless. Temporarily disabling the non-legacy branch in
EvmStatelessCheckslocally makes the test fail on all three executors, and the mismatched-chain-id transaction then executes successfully (code 0) on both giga drivers:The guard was restored before committing; the diff contains only the new test file.
gofmt -s -lis clean on it.