Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/gov) [#26353](https://github.com/cosmos/cosmos-sdk/pull/26353) Fix leading comma in `proposal_messages` event attribute emitted by `SubmitProposal`.
* (telemetry) [#26390](https://github.com/cosmos/cosmos-sdk/pull/26390) Fix env var for otel telemetry initialization.
* (x/staking) [#26408](https://github.com/cosmos/cosmos-sdk/pull/26408) Fix `MsgBeginRedelegate` failure when redelegating all shares from an unbonded source validator that is removed after unbonding.
* (x/auth/tx) [#26422](https://github.com/cosmos/cosmos-sdk/pull/26422) Reuse the signing context from the codec's `InterfaceRegistry` when `ConfigOptions.SigningOptions` is unset so that `CustomGetSigners` registered via `NewInterfaceRegistryWithOptions` are honored by `NewTxConfig` / `NewTxConfigWithOptions`.

### Deprecated

Expand Down
30 changes: 22 additions & 8 deletions x/auth/tx/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,18 +187,32 @@ func NewTxConfigWithOptions(protoCodec codec.Codec, configOptions ConfigOptions)
var err error
if configOptions.SigningContext == nil {
if configOptions.SigningOptions == nil {
configOptions.SigningOptions, err = NewDefaultSigningOptions()
// Reuse the SigningContext from the codec's interface registry.
// This propagates SigningOptions (including CustomGetSigners) that
// were applied via NewInterfaceRegistryWithOptions; building a
// fresh context from NewDefaultSigningOptions would drop them. See
// https://github.com/cosmos/cosmos-sdk/issues/22200.
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
}
}
Comment on lines +195 to +206

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.

} else {
if configOptions.SigningOptions.FileResolver == nil {
configOptions.SigningOptions.FileResolver = protoCodec.InterfaceRegistry()
}
configOptions.SigningContext, err = txsigning.NewContext(*configOptions.SigningOptions)
if err != nil {
return nil, err
}
}
if configOptions.SigningOptions.FileResolver == nil {
configOptions.SigningOptions.FileResolver = protoCodec.InterfaceRegistry()
}
configOptions.SigningContext, err = txsigning.NewContext(*configOptions.SigningOptions)
if err != nil {
return nil, err
}
}
txConfig.signingContext = configOptions.SigningContext

Expand Down
62 changes: 62 additions & 0 deletions x/auth/tx/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,22 @@ package tx_test
import (
"testing"

gogoproto "github.com/cosmos/gogoproto/proto"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
protov2 "google.golang.org/protobuf/proto"
"google.golang.org/protobuf/reflect/protoreflect"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/address"
"github.com/cosmos/cosmos-sdk/codec/testutil"
"github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/std"
"github.com/cosmos/cosmos-sdk/testutil/testdata"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/auth/tx"
txtestutil "github.com/cosmos/cosmos-sdk/x/auth/tx/testutil"
txsigning "github.com/cosmos/cosmos-sdk/x/tx/signing"
)

func TestGenerator(t *testing.T) {
Expand All @@ -34,3 +39,60 @@ func TestConfigOptions(t *testing.T) {
handler := txConfig.SignModeHandler()
require.NotNil(t, handler)
}

// TestNewTxConfigPropagatesCustomGetSigners verifies that when the codec's
// interface registry was built with CustomGetSigners, those custom signer
// functions are reachable through txConfig.SigningContext(). Regression test
// for https://github.com/cosmos/cosmos-sdk/issues/22200.
func TestNewTxConfigPropagatesCustomGetSigners(t *testing.T) {
addrBytes := []byte("test-signer-addr-bytes")
customSigner := func(_ protov2.Message) ([][]byte, error) {
return [][]byte{addrBytes}, nil
}

signingOpts := txsigning.Options{
AddressCodec: address.NewBech32Codec("cosmos"),
ValidatorAddressCodec: address.NewBech32Codec("cosmosvaloper"),
}
signingOpts.DefineCustomGetSigners(protoreflect.FullName(gogoproto.MessageName(&testdata.TestMsg{})), customSigner)

interfaceRegistry, err := types.NewInterfaceRegistryWithOptions(types.InterfaceRegistryOptions{
ProtoFiles: gogoproto.HybridResolver,
SigningOptions: signingOpts,
})
require.NoError(t, err)
protoCodec := codec.NewProtoCodec(interfaceRegistry)

txConfig := tx.NewTxConfig(protoCodec, tx.DefaultSignModes)

// Sanity: txConfig must reuse the interface registry's signing context so
// custom signers carry through. With the pre-fix code, NewTxConfig built a
// fresh context from defaults and the lookup below failed.
require.Same(t, interfaceRegistry.SigningContext(), txConfig.SigningContext())
}

// TestNewTxConfigWithExplicitSigningOptions covers the branch where the caller
// supplies SigningOptions directly. The function must build a fresh signing
// context from those options (not reuse the codec's interface-registry
// context) and must default FileResolver to the interface registry when
// unset.
func TestNewTxConfigWithExplicitSigningOptions(t *testing.T) {
interfaceRegistry := types.NewInterfaceRegistry()
protoCodec := codec.NewProtoCodec(interfaceRegistry)

signingOpts := &txsigning.Options{
AddressCodec: address.NewBech32Codec("cosmos"),
ValidatorAddressCodec: address.NewBech32Codec("cosmosvaloper"),
}

txConfig, err := tx.NewTxConfigWithOptions(protoCodec, tx.ConfigOptions{
SigningOptions: signingOpts,
})
require.NoError(t, err)
require.NotNil(t, txConfig.SigningContext())
// A new context was built from the supplied SigningOptions, so it must not
// be the interface registry's own signing context.
require.NotSame(t, interfaceRegistry.SigningContext(), txConfig.SigningContext())
// FileResolver default came from the interface registry.
require.NotNil(t, signingOpts.FileResolver)
}