Skip to content

fix(x/auth/tx): propagate signing options from codec to NewTxConfig#26422

Open
ozpool wants to merge 2 commits into
cosmos:mainfrom
ozpool:fix/22200-newtxconfig-propagate-signing-options
Open

fix(x/auth/tx): propagate signing options from codec to NewTxConfig#26422
ozpool wants to merge 2 commits into
cosmos:mainfrom
ozpool:fix/22200-newtxconfig-propagate-signing-options

Conversation

@ozpool
Copy link
Copy Markdown
Contributor

@ozpool ozpool commented May 14, 2026

Summary

NewTxConfig built a fresh SigningContext from NewDefaultSigningOptions whenever the caller did not pass ConfigOptions.SigningOptions, dropping any CustomGetSigners that had been applied to the codec's InterfaceRegistry via NewInterfaceRegistryWithOptions. txConfig.SigningContext().Validate() then failed for messages relying on custom getters with:

```
no cosmos.msg.v1.signer option found for message ; use DefineCustomGetSigners to specify a custom getter
```

Reuse the InterfaceRegistry's SigningContext when neither SigningOptions nor SigningContext were supplied, so options applied at registry construction time carry through to txConfig consumers without forcing them to switch to NewTxConfigWithOptions.

Closes #22200

Test plan

  • Added regression test `TestNewTxConfigPropagatesCustomGetSigners` that builds an InterfaceRegistry with a CustomGetSigners entry, runs it through `NewTxConfig`, and asserts `txConfig.SigningContext()` is the same context as `interfaceRegistry.SigningContext()`. With the pre-fix code the assertion fails because a fresh context was built from defaults.
  • Full `x/auth/tx` suite passes locally.

NewTxConfig built a fresh SigningContext from NewDefaultSigningOptions
whenever the caller did not pass ConfigOptions.SigningOptions, dropping
any CustomGetSigners that had been applied to the codec's
InterfaceRegistry via NewInterfaceRegistryWithOptions. The resulting
txConfig.SigningContext().Validate() then failed for messages that
relied on custom getters.

Reuse the InterfaceRegistry's SigningContext when neither
SigningOptions nor SigningContext were supplied, so options applied at
registry construction time carry through to txConfig consumers without
forcing them to switch to NewTxConfigWithOptions.

Closes cosmos#22200
@aljo242
Copy link
Copy Markdown
Contributor

aljo242 commented May 22, 2026

@greptile review

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 22, 2026

Greptile Summary

This PR fixes a bug where NewTxConfig discarded CustomGetSigners entries applied to the codec's InterfaceRegistry via NewInterfaceRegistryWithOptions, because it always built a fresh SigningContext from NewDefaultSigningOptions rather than reusing the one already embedded in the registry.

  • NewTxConfigWithOptions now checks for the IR's SigningContext first (when neither SigningOptions nor SigningContext are supplied), so custom signer functions carry through to all txConfig consumers automatically.
  • A regression test TestNewTxConfigPropagatesCustomGetSigners verifies pointer-identity of the context before and after the fix, confirming the exact same context object is threaded through.

Confidence Score: 4/5

Safe to merge; the core bug is correctly fixed and the regression test is solid.

The fix correctly threads the IR's signing context through to txConfig, resolving the CustomGetSigners propagation failure. The one side-effect to watch is that the new path leaves SigningOptions nil, so NewSigningHandlerMap provisions a fresh set of default options for the amino JSON and textual handlers — those handlers get HybridResolver as their file resolver rather than the IR's ProtoFiles. For every known caller this is equivalent, but callers using a custom ProtoFiles in NewInterfaceRegistryWithOptions would silently lose that resolver for amino/textual signing.

x/auth/tx/config.go — specifically the interaction between the new signing-context reuse path and the downstream NewSigningHandlerMap call, which leaves SigningOptions nil.

Important Files Changed

Filename Overview
x/auth/tx/config.go Reuses IR's SigningContext instead of building a fresh one, correctly propagating CustomGetSigners; minor: SigningOptions remains nil on the new path, so NewSigningHandlerMap falls back to HybridResolver for amino JSON/textual FileResolver instead of the IR's ProtoFiles
x/auth/tx/config_test.go Adds regression test that verifies pointer identity of the SigningContext between IR and txConfig after calling NewTxConfig with a custom signer

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["NewTxConfigWithOptions"] --> B{"SigningContext set?"}
    B -- yes --> G["txConfig.signingContext = SigningContext"]
    B -- no --> C{"SigningOptions set?"}
    C -- yes --> D["Set FileResolver=IR if nil\nNewContext(SigningOptions)"]
    D --> G
    C -- no --> E{"ir = InterfaceRegistry\nnot nil?"}
    E -- yes --> F["SigningContext = ir.SigningContext()\ncarries CustomGetSigners"]
    F --> G
    E -- no --> H["NewDefaultSigningOptions\nNewContext(defaults)"]
    H --> G
    G --> I["NewSigningHandlerMap(configOptions)"]
    I --> J{"SigningOptions nil?"}
    J -- yes --> K["Fresh NewDefaultSigningOptions\nFileResolver = nil"]
    K --> L["aminojson: HybridResolver fallback\ndirectaux: uses SigningContext.FileResolver"]
    J -- no --> M["Use provided SigningOptions\nFileResolver = IR"]
    M --> L
Loading

Reviews (1): Last reviewed commit: "Merge branch 'main' into fix/22200-newtx..." | Re-trigger Greptile

Comment thread x/auth/tx/config.go
Comment on lines +195 to +206
if ir := protoCodec.InterfaceRegistry(); ir != nil {
configOptions.SigningContext = ir.SigningContext()
} else {
configOptions.SigningOptions, err = NewDefaultSigningOptions()
if err != nil {
return nil, err
}
configOptions.SigningContext, err = txsigning.NewContext(*configOptions.SigningOptions)
if err != nil {
return nil, err
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Amino JSON / Textual sign modes lose IR file resolver

When ir != nil the new path sets configOptions.SigningContext but leaves configOptions.SigningOptions as nil. NewSigningHandlerMap (called at line 224) checks SigningOptions == nil first and allocates a fresh NewDefaultSigningOptions() — with a nil FileResolver — before it consults SigningContext. The SIGN_MODE_LEGACY_AMINO_JSON and SIGN_MODE_TEXTUAL handlers therefore receive a nil FileResolver and fall back to gogoproto.HybridResolver, whereas before this PR they received protoCodec.InterfaceRegistry() as their resolver.

For the common case (NewInterfaceRegistry() embeds HybridResolver as its ProtoFiles) the two are functionally equivalent, but callers who pass a custom ProtoFiles via NewInterfaceRegistryWithOptions will silently lose that resolver for amino/textual signing. The SigningContext itself retains the correct resolver (via ir.SigningContext()), so SIGN_MODE_DIRECT_AUX — which reads the resolver directly from the context via signersContext.FileResolver() — is unaffected.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 22, 2026

Codecov Report

❌ Patch coverage is 46.15385% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.88%. Comparing base (efabbb3) to head (05f461b).

Files with missing lines Patch % Lines
x/auth/tx/config.go 46.15% 7 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #26422      +/-   ##
==========================================
- Coverage   64.89%   64.88%   -0.02%     
==========================================
  Files         792      792              
  Lines       54970    54977       +7     
==========================================
- Hits        35674    35673       -1     
- Misses      19296    19304       +8     
Files with missing lines Coverage Δ
x/auth/tx/config.go 82.50% <46.15%> (-2.46%) ⬇️

... and 5 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

Signing options (e.g. CustomGetSigners) should be passed down to NewTxConfig

2 participants