Skip to content

fix: use a valid 20-byte signer in the gasless module-sender test - #331

Merged
0xNilesh merged 1 commit into
audit-fixesfrom
fix/gasless-module-sender-test
Aug 26, 2026
Merged

fix: use a valid 20-byte signer in the gasless module-sender test#331
0xNilesh merged 1 commit into
audit-fixesfrom
fix/gasless-module-sender-test

Conversation

@0xNilesh

Copy link
Copy Markdown
Member

What's broken

The Unit tests job has been red on audit-fixes since 2026-08-24, failing on a single test:

--- FAIL: TestGaslessExecutePayloadWithModuleSender
    Error: Received unexpected error:
      failed to parse signer address: invalid address length for
      "cosmos1xpurwdecvsenyvpkxvmnge3cv93nyd34xuersef38pjnxen9xfsk2dnz8yek2drrv56qmn2ak9":
      got 42 bytes, want 20

Eleven commits have landed on top of it, so every audit-fix PR merged since then went in with tests red.

Why

A merge-order collision, 32 minutes wide. Neither production fix is wrong.

time commit what
02:04 6a1403ef F-2026-18200 merges — adds the 20-byte signer guard in utils.GetAddressPair. That guard is the fix (module impersonation via variable-length signers).
02:36 8e28eed6 F-2026-18197 merges (#317) — brings a new test whose hardcoded signer decodes to 42 bytes.

#317's branch was cut before 18200 landed, where a 42-byte signer was still accepted. It was never re-run against the updated base, so the test went red the moment it merged.

msgServer.ExecutePayload calls utils.GetAddressPair(msg.Signer) as its first statement, so the test died there — before reaching anything it was written to exercise.

The fix

Use testSigner, the valid 20-byte account already defined in the same package (execute_payload_test.go:22), instead of the ad-hoc literal.

Worth knowing

TestGaslessExecutePayloadWithModuleSender exists to assert that gasless module-sender MsgExecutePayload still executes end to end after the x/vm VerifySender hardening. Because it was failing on its first statement, it had never once exercised that invariant. This change doesn't just turn CI green — it makes the guard real for the first time.

Verification

go test -tags="ledger test_ledger_mock test" ./test/integration/uexecutor/... -run TestGaslessExecutePayloadWithModuleSender -v
=== RUN   TestGaslessExecutePayloadWithModuleSender
--- PASS: TestGaslessExecutePayloadWithModuleSender (0.05s)

go test -tags="ledger test_ledger_mock test" ./test/integration/uexecutor/...
ok  github.com/pushchain/push-chain-node/test/integration/uexecutor  10.039s

Full ./... run with the CI invocation was in progress at time of opening; will confirm on the PR.

Follow-up for maintainers

The tests job is not blocking merges on audit-fixes — that's how eleven commits landed on a red job. While the branch is red, any new PR inherits a red tests job, so genuinely new breakage looks identical to this and has to be caught by manually diffing against clean base. Worth deciding whether the job should be required.

The hardcoded literal decoded to 42 bytes, which F-2026-18200's signer-length
guard rejects in GetAddressPair before ExecutePayload runs. Red on audit-fixes
since #317.
@0xNilesh
0xNilesh merged commit 83c145f into audit-fixes Aug 26, 2026
7 checks passed
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.

1 participant