fix: F-2026-18189 | [Dual Defense] Manual Module EVM Nonce Desync on Reverted Inbound Execution

[0xNilesh]

ModuleAccountNonce fed the manual nonce for every module-sender DerivedEVMCall and was incremented on the outer ctx before the call, so a failed attempt still burned it. Nothing ever moved the module account's own EVM nonce, so the two were never in step at all: the counter climbs 0, 1, 2, … while eth_getTransactionCount(ue-module) stays 0 forever — not only after a revert.

Call sites. moduleSenderNonce from develop does not exist on audit-fixes; the prologue is inlined five times, all in x/uexecutor/keeper/evm.go: CallPRC20Deposit, CallPRC20DepositAutoSwap, CallUniversalCoreSetChainMeta, CallUniversalCoreRefundUnusedGas, CallExecuteUniversalTx. The UEA helpers (CallFactoryToDeployUEA, CallUEAExecutePayload, CallUEAMigrateUEA) pass isModuleSender=false and no manual nonce, so they read accountKeeper.GetSequence(from) — and the three inbound executors deploy with from = ue-module, which meant a frozen 0.

Which recommendation, and why

Rec 2, not rec 1 — settled by the rec-4 test rather than by argument.

Rec 1 (read evm.GetNonce(module) immediately before each call, retire the counter) was implemented and measured. Test 1 passes under it. Test 2 does not:

--- FAIL: TestModuleSenderNonceDistinctHashesAcrossFailedAttempt
    Error:    Should not be: "0xec8a0b2d90f12cb9e3f58e94f2d17b1821861d863880bf7094e7e5d463b18dad"
    Messages: consecutive byte-identical module calls produced the same derived tx hash

The residual is worse than "conditional, after a revert". ApplyMessageWithConfig advances a sender's nonce only in its contractCreation branch, and every module call is a plain CALL, so evm.GetNonce(module) is a constant. Under rec 1, byte-identical module calls collide whether or not anything reverted. The unconditional counter is load-bearing, as the write-up said.

Shipped

Keep the counter and its unconditional advance — that is what keeps derived tx hashes distinct — and remove the desync by moving both values together.

derivedModuleCall is now the only place a module-sender DerivedEVMCall is made from. It takes the nonce from nextModuleSenderNonce (max(counter, evm.GetNonce(module))), makes the call, and burns exactly one nonce whether the call committed, reverted, or never reached the EVM — writing the new value to both the counter and the module account's sequence. Collapsing five copies of the prologue into one also means a new module call site cannot forget the increment, which is the failure mode fa6ffe1e / dde0f870 fix on develop.

ModuleAccountNonce == evm.GetNonce(ue-module) now holds by construction, so rec 3's invariant is structural rather than something to register and check, and drift is impossible in both directions. That makes those two develop commits moot for these five call sites; they are not cherry-picked here.

One assumption this fork does not hold up

SkipNonceChecks is set on the core.Message but never read — cosmos/evm does not run geth's preCheck for ApplyMessageWithConfig. So the reported impact ("later module calls supply N+1 with SkipNonceChecks=false and fail") does not reproduce: on unpatched audit-fixes the module-sender call after a reverted deposit succeeds. The manual nonce's only consumer is the derived tx identity — ethtypes.NewTx(&DynamicFeeTx{Nonce, …}).Hash() → res.Hash → the ethereum_tx event — which is why hash uniqueness is the property that had to be protected, and why test 2 is the one that decided the design.

Tests

test/integration/uexecutor/module_nonce_test.go, against the real EVM with the real contracts.

• TestModuleSenderNonceSurvivesRevertedDeposit — forces depositPRC20Token into a codeless PRC20, then asserts the next module-sender call still succeeds, that the failed attempt still burned its nonce, and that counter and account nonce still agree. Fails on unpatched audit-fixes: expected 0x1, actual 0x0.
• TestModuleSenderNonceDistinctHashesAcrossFailedAttempt — three byte-identical deposits with a forced failure wedged between the first and second, asserting all three ethereum_tx hashes differ. Passes on unpatched audit-fixes (the counter was already doing this) and fails under rec 1.

Mutation-checked both directions: dropping the account-nonce mirroring fails test 1 on the sync assertion (expected 0x1, actual 0x0); burning the nonce only on success fails test 1 on the burn assertion (expected 0x2, actual 0x1). Restored, both green.

A note that is in the test as well: a module-sender call passes gasLimit == nil, so DerivedEVMCallWithData runs EstimateGasInternal first, and for an always-reverting call that returns {Gas: 0, VmError: "execution reverted"}. The call then dies in ApplyMessageWithConfig with "intrinsic gas too low" before any ethereum_tx is emitted — so a failing module deposit burns a nonce and produces no derived transaction at all.