x/auth: auth fee nil guard#91
Conversation
|
@claude review once |
There was a problem hiding this comment.
Pull request overview
This PR prevents recovered panics caused by protobuf-decoded transactions where AuthInfo.Fee is legitimately omitted (nil) by guarding fee-related accessors in the x/auth/tx wrapper, and adds a regression test reproducing the nil-fee decode scenario.
Changes:
- Add nil checks to
GetGasandGetFeeso fee-less decoded txs return zero values instead of panicking. - Update
FeePayeraccessor logic to avoid a direct nil dereference. - Add a new regression test that decodes a tx whose
AuthInfoomits thefeefield and asserts accessor behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| x/auth/tx/builder.go | Adds nil guards for fee-related wrapper accessors to prevent nil dereference panics on decoded txs missing AuthInfo.Fee. |
| x/auth/tx/nilfee_repro_test.go | New regression tests covering decoding of fee-less AuthInfo and validating accessor behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
manav2401
left a comment
manav2401
left a comment
There was a problem hiding this comment.
LGTM - good to include the copilot suggestions.
|
|
Summary
The
wrapperaccessorsGetGas,GetFee, andFeePayerdereferencew.tx.AuthInfo.Feewithout a nil check. A protobuf-decoded tx can legitimately haveAuthInfo.Fee == nil— thefeefield (AuthInfo field 2) is optional andDefaultTxDecoderdoes not require it. When such a tx reaches the ante chain, the first decorator (SetUpContextDecorator) callsGetGas(), which then hits a nil pointer dereference.runTx's recovery middleware catches it, so it is not fatal — but every fee-less tx becomes a recovered panic (with a full stack capture) instead of a clean rejection. On a node that receives such txs over the mempool this is noisy and wasteful.This guards the three accessors against a nil
Fee, mirroring the guard the wrapper's setters (SetGasLimit,SetFeeAmount,SetFeePayer,SetFeeGranter) already apply. A fee-less tx now reports gas0/ no fee and is rejected through the normal ante path (out-of-gas / fee checks) rather than panicking. A well-formed tx (Fee present) is completely unaffected.Executed tests
x/auth/tx/nilfee_repro_test.go: decodes a tx whoseAuthInfoomits the fee field and assertsGetGas/GetFeeno longer panic and return zero values; plus round-trips confirming an empty&Fee{}and a populatedFeeare unchanged.go test ./x/auth/tx/... ./x/auth/ante/...— pass.x/auth/tx/builder.go: 100% mutation kill on the changed accessors, all gates green.Rollout notes
go.modreplace bump once this merges and the fork is tagged.