fix: route the impersonated fork signer through the nonce queue - #2976
Open
apexearth wants to merge 1 commit into
Open
fix: route the impersonated fork signer through the nonce queue#2976apexearth wants to merge 1 commit into
apexearth wants to merge 1 commit into
Conversation
getSigner() wraps the AWS KMS and private-key branches in maybeWrap(), but branch 3 (FORK=true + IMPERSONATE) returns provider.getSigner(address) directly. sendTransaction then bypasses wrapSignerWithNonceQueueV5 and Talos never records the transaction — a run against a fork completes with an empty transaction list. DATABASE_URL is present in the runner container, so maybeWrap() does attach the queue; nothing else was missing. getSigner() is synchronous on ethers v5 (5.7.2 here), so no await is added. Surfaced by talos making an anvil fork with an impersonated relayer the default dev environment.
apexearth
requested review from
clement-ux,
naddison36,
shahthepro and
sparrowDom
as code owners
August 18, 2026 19:53
Contributor
Author
|
merge if you approve |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
getSigner()wraps the KMS and private-key branches inmaybeWrap(), but branch 3 does not:return maybeWrap(new DirectKmsTransactionSigner(...))✅DEPLOYER_PK/GOVERNOR_PKreturn maybeWrap(new ethers.Wallet(pk, provider))✅FORK=true+IMPERSONATEreturn provider.getSigner(address)❌Without the wrapper,
sendTransactionnever routes throughwrapSignerWithNonceQueueV5, so Talos does not record the transaction. The visible symptom is a run that completes successfully with an empty transaction list.No
await—getSigner()is synchronous on ethers v5 (5.7.2 here), unlike v6.DATABASE_URLis present in the runner container, somaybeWrap()does attach the queue — nothing else was missing.Why now
oplabs/talos#30 makes an anvil mainnet fork with an impersonated relayer the default dev environment, so branch 3 goes from rarely-exercised to the everyday path. Companion fix in arm-oeth: OriginProtocol/arm-oeth#339.
Worth a reviewer's judgement
The nonce queue does real work — rebroadcasts, fee bumps, replacements, driven by the
NONCE_QUEUE_*settings. On a fork that is arguably the point (you exercise the production path), but dev transactions become managed rather than fire-and-forget. If you would rather impersonated runs stay lightweight, the alternative is recording them without the queue.Not verified
I could not typecheck — the worktree has no
node_modules. The change is a one-line call intomaybeWrap(), already used twice in the same function, so it should be safe, but CI is the check.