From ff8789a0287147a658b7976a47e34c7e0e35a13a Mon Sep 17 00:00:00 2001 From: Matt Kocubinski Date: Thu, 3 Oct 2024 06:10:02 -0500 Subject: [PATCH] fix(x/tx): fix amino json drift from legacy spec (#21825) (cherry picked from commit 2d40cc1ab6bea7ebc44af3859f6b353954149685) # Conflicts: # tests/integration/tx/aminojson/aminojson_test.go # x/auth/migrations/legacytx/stdtx_test.go # x/tx/CHANGELOG.md --- tests/integration/rapidgen/rapidgen.go | 5 +- .../tx/aminojson/aminojson_test.go | 414 ++++++++---------- tests/integration/tx/context_test.go | 20 +- tests/integration/tx/internal/util.go | 221 ++++++++++ x/auth/migrations/legacytx/stdtx_test.go | 4 + x/tx/CHANGELOG.md | 6 + x/tx/signing/aminojson/encoder.go | 77 ++-- 7 files changed, 461 insertions(+), 286 deletions(-) create mode 100644 tests/integration/tx/internal/util.go diff --git a/tests/integration/rapidgen/rapidgen.go b/tests/integration/rapidgen/rapidgen.go index 08f2346ab523..bd293de062a3 100644 --- a/tests/integration/rapidgen/rapidgen.go +++ b/tests/integration/rapidgen/rapidgen.go @@ -231,7 +231,6 @@ var ( NonsignableTypes = []GeneratedType{ GenType(&authtypes.Params{}, &authapi.Params{}, GenOpts), GenType(&authtypes.BaseAccount{}, &authapi.BaseAccount{}, GenOpts.WithAnyTypes(&ed25519.PubKey{})), - GenType(&authtypes.ModuleAccount{}, &authapi.ModuleAccount{}, GenOpts.WithAnyTypes(&ed25519.PubKey{})), GenType(&authtypes.ModuleCredential{}, &authapi.ModuleCredential{}, GenOpts), GenType(&authztypes.GenericAuthorization{}, &authzapi.GenericAuthorization{}, GenOpts), @@ -272,7 +271,9 @@ var ( GenType(&slashingtypes.Params{}, &slashingapi.Params{}, GenOpts.WithDisallowNil()), - GenType(&stakingtypes.StakeAuthorization{}, &stakingapi.StakeAuthorization{}, GenOpts), + // JSON ordering of one of fields to be fixed in https://github.com/cosmos/cosmos-sdk/pull/21782 + // TODO uncomment once merged + // GenType(&stakingtypes.StakeAuthorization{}, &stakingapi.StakeAuthorization{}, GenOpts), GenType(&upgradetypes.CancelSoftwareUpgradeProposal{}, &upgradeapi.CancelSoftwareUpgradeProposal{}, GenOpts), // nolint:staticcheck // testing legacy code path GenType(&upgradetypes.SoftwareUpgradeProposal{}, &upgradeapi.SoftwareUpgradeProposal{}, GenOpts.WithDisallowNil()), // nolint:staticcheck // testing legacy code path diff --git a/tests/integration/tx/aminojson/aminojson_test.go b/tests/integration/tx/aminojson/aminojson_test.go index eb271308f8d6..b48b78a5418c 100644 --- a/tests/integration/tx/aminojson/aminojson_test.go +++ b/tests/integration/tx/aminojson/aminojson_test.go @@ -1,9 +1,13 @@ package aminojson import ( - "context" + "bytes" "fmt" +<<<<<<< HEAD "reflect" +======= + stdmath "math" +>>>>>>> 2d40cc1ab (fix(x/tx): fix amino json drift from legacy spec (#21825)) "testing" "time" @@ -11,31 +15,16 @@ import ( gogoproto "github.com/cosmos/gogoproto/proto" "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" - "google.golang.org/protobuf/types/known/anypb" - "google.golang.org/protobuf/types/known/durationpb" - "google.golang.org/protobuf/types/known/timestamppb" "pgregory.net/rapid" authapi "cosmossdk.io/api/cosmos/auth/v1beta1" - authzapi "cosmossdk.io/api/cosmos/authz/v1beta1" bankapi "cosmossdk.io/api/cosmos/bank/v1beta1" v1beta1 "cosmossdk.io/api/cosmos/base/v1beta1" - "cosmossdk.io/api/cosmos/crypto/ed25519" - multisigapi "cosmossdk.io/api/cosmos/crypto/multisig" - "cosmossdk.io/api/cosmos/crypto/secp256k1" - distapi "cosmossdk.io/api/cosmos/distribution/v1beta1" - gov_v1_api "cosmossdk.io/api/cosmos/gov/v1" - gov_v1beta1_api "cosmossdk.io/api/cosmos/gov/v1beta1" msgv1 "cosmossdk.io/api/cosmos/msg/v1" - slashingapi "cosmossdk.io/api/cosmos/slashing/v1beta1" - stakingapi "cosmossdk.io/api/cosmos/staking/v1beta1" - txv1beta1 "cosmossdk.io/api/cosmos/tx/v1beta1" - vestingapi "cosmossdk.io/api/cosmos/vesting/v1beta1" "cosmossdk.io/math" "cosmossdk.io/x/evidence" feegrantmodule "cosmossdk.io/x/feegrant/module" "cosmossdk.io/x/tx/signing/aminojson" - signing_testutil "cosmossdk.io/x/tx/signing/testutil" "cosmossdk.io/x/upgrade" codectypes "github.com/cosmos/cosmos-sdk/codec/types" @@ -43,17 +32,13 @@ import ( "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" secp256k1types "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" "github.com/cosmos/cosmos-sdk/tests/integration/rapidgen" + "github.com/cosmos/cosmos-sdk/tests/integration/tx/internal" gogo_testpb "github.com/cosmos/cosmos-sdk/tests/integration/tx/internal/gogo/testpb" - pulsar_testpb "github.com/cosmos/cosmos-sdk/tests/integration/tx/internal/pulsar/testpb" "github.com/cosmos/cosmos-sdk/testutil/testdata" "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/bech32" "github.com/cosmos/cosmos-sdk/types/module/testutil" - signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing" "github.com/cosmos/cosmos-sdk/x/auth" - "github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx" - "github.com/cosmos/cosmos-sdk/x/auth/signing" - "github.com/cosmos/cosmos-sdk/x/auth/tx" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" "github.com/cosmos/cosmos-sdk/x/auth/vesting" vestingtypes "github.com/cosmos/cosmos-sdk/x/auth/vesting/types" @@ -91,6 +76,7 @@ import ( // In order for step 3 to work certain restrictions on the data generated in step 1 must be enforced and are described // by the mutation of genOpts passed to the generator. func TestAminoJSON_Equivalence(t *testing.T) { +<<<<<<< HEAD encCfg := testutil.MakeTestEncodingConfig( auth.AppModuleBasic{}, authzmodule.AppModuleBasic{}, bank.AppModuleBasic{}, consensus.AppModuleBasic{}, distribution.AppModuleBasic{}, evidence.AppModuleBasic{}, feegrantmodule.AppModuleBasic{}, @@ -98,6 +84,25 @@ func TestAminoJSON_Equivalence(t *testing.T) { slashing.AppModuleBasic{}, staking.AppModuleBasic{}, upgrade.AppModuleBasic{}, vesting.AppModuleBasic{}) legacytx.RegressionTestingAminoCodec = encCfg.Amino aj := aminojson.NewEncoder(aminojson.EncoderOptions{DoNotSortFields: true}) +======= + fixture := internal.NewSigningFixture(t, internal.SigningFixtureOptions{}, + auth.AppModule{}, + authzmodule.AppModule{}, + bank.AppModule{}, + consensus.AppModule{}, + distribution.AppModule{}, + evidence.AppModule{}, + feegrantmodule.AppModule{}, + gov.AppModule{}, + groupmodule.AppModule{}, + mint.AppModule{}, + slashing.AppModule{}, + staking.AppModule{}, + upgrade.AppModule{}, + vesting.AppModule{}, + ) + aj := aminojson.NewEncoder(aminojson.EncoderOptions{}) +>>>>>>> 2d40cc1ab (fix(x/tx): fix amino json drift from legacy spec (#21825)) for _, tt := range rapidgen.DefaultGeneratedTypes { desc := tt.Pulsar.ProtoReflect().Descriptor() @@ -105,7 +110,7 @@ func TestAminoJSON_Equivalence(t *testing.T) { t.Run(name, func(t *testing.T) { gen := rapidproto.MessageGenerator(tt.Pulsar, tt.Opts) fmt.Printf("testing %s\n", tt.Pulsar.ProtoReflect().Descriptor().FullName()) - rapid.Check(t, func(t *rapid.T) { + rapid.Check(t, func(r *rapid.T) { // uncomment to debug; catch a panic and inspect application state // defer func() { // if r := recover(); r != nil { @@ -114,26 +119,28 @@ func TestAminoJSON_Equivalence(t *testing.T) { // } // }() - msg := gen.Draw(t, "msg") + msg := gen.Draw(r, "msg") postFixPulsarMessage(msg) gogo := tt.Gogo sanity := tt.Pulsar protoBz, err := proto.Marshal(msg) - require.NoError(t, err) + require.NoError(r, err) err = proto.Unmarshal(protoBz, sanity) - require.NoError(t, err) + require.NoError(r, err) - err = encCfg.Codec.Unmarshal(protoBz, gogo) - require.NoError(t, err) + err = fixture.UnmarshalGogoProto(protoBz, gogo) + require.NoError(r, err) - legacyAminoJSON, err := encCfg.Amino.MarshalJSON(gogo) - require.NoError(t, err) + legacyAminoJSON := fixture.MarshalLegacyAminoJSON(t, gogo) aminoJSON, err := aj.Marshal(msg) - require.NoError(t, err) - require.Equal(t, string(legacyAminoJSON), string(aminoJSON)) + require.NoError(r, err) + if !bytes.Equal(legacyAminoJSON, aminoJSON) { + require.Failf(r, "JSON mismatch", "legacy: %s\n x/tx: %s\n", + string(legacyAminoJSON), string(aminoJSON)) + } // test amino json signer handler equivalence if !proto.HasExtension(desc.Options(), msgv1.E_Signer) { @@ -141,47 +148,13 @@ func TestAminoJSON_Equivalence(t *testing.T) { return } - handlerOptions := signing_testutil.HandlerArgumentOptions{ - ChainID: "test-chain", - Memo: "sometestmemo", - Msg: tt.Pulsar, - AccNum: 1, - AccSeq: 2, - SignerAddress: "signerAddress", - Fee: &txv1beta1.Fee{ - Amount: []*v1beta1.Coin{{Denom: "uatom", Amount: "1000"}}, - }, - } - - signerData, txData, err := signing_testutil.MakeHandlerArguments(handlerOptions) - require.NoError(t, err) - - handler := aminojson.NewSignModeHandler(aminojson.SignModeHandlerOptions{}) - signBz, err := handler.GetSignBytes(context.Background(), signerData, txData) - require.NoError(t, err) - - legacyHandler := tx.NewSignModeLegacyAminoJSONHandler() - txBuilder := encCfg.TxConfig.NewTxBuilder() - require.NoError(t, txBuilder.SetMsgs([]types.Msg{tt.Gogo}...)) - txBuilder.SetMemo(handlerOptions.Memo) - txBuilder.SetFeeAmount(types.Coins{types.NewInt64Coin("uatom", 1000)}) - theTx := txBuilder.GetTx() - - legacySigningData := signing.SignerData{ - ChainID: handlerOptions.ChainID, - Address: handlerOptions.SignerAddress, - AccountNumber: handlerOptions.AccNum, - Sequence: handlerOptions.AccSeq, - } - legacySignBz, err := legacyHandler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, - legacySigningData, theTx) - require.NoError(t, err) - require.Equal(t, string(legacySignBz), string(signBz)) + fixture.RequireLegacyAminoEquivalent(t, gogo) }) }) } } +<<<<<<< HEAD func newAny(t *testing.T, msg proto.Message) *anypb.Any { bz, err := proto.Marshal(msg) require.NoError(t, err) @@ -198,157 +171,139 @@ func TestAminoJSON_LegacyParity(t *testing.T) { bank.AppModuleBasic{}, distribution.AppModuleBasic{}, slashing.AppModuleBasic{}, staking.AppModuleBasic{}, vesting.AppModuleBasic{}, gov.AppModuleBasic{}) legacytx.RegressionTestingAminoCodec = encCfg.Amino +======= +// TestAminoJSON_LegacyParity tests that the Encoder encoder produces the same output as the Encoder encoder. +func TestAminoJSON_LegacyParity(t *testing.T) { + fixture := internal.NewSigningFixture(t, internal.SigningFixtureOptions{}, + auth.AppModule{}, authzmodule.AppModule{}, + bank.AppModule{}, distribution.AppModule{}, slashing.AppModule{}, staking.AppModule{}, + vesting.AppModule{}, gov.AppModule{}) + aj := aminojson.NewEncoder(aminojson.EncoderOptions{}) +>>>>>>> 2d40cc1ab (fix(x/tx): fix amino json drift from legacy spec (#21825)) - aj := aminojson.NewEncoder(aminojson.EncoderOptions{DoNotSortFields: true}) addr1 := types.AccAddress("addr1") now := time.Now() - genericAuth, _ := codectypes.NewAnyWithValue(&authztypes.GenericAuthorization{Msg: "foo"}) - genericAuthPulsar := newAny(t, &authzapi.GenericAuthorization{Msg: "foo"}) pubkeyAny, _ := codectypes.NewAnyWithValue(&secp256k1types.PubKey{Key: []byte("foo")}) - pubkeyAnyPulsar := newAny(t, &secp256k1.PubKey{Key: []byte("foo")}) - dec10bz, _ := math.LegacyNewDec(10).Marshal() - int123bz, _ := math.NewInt(123).Marshal() + dec5point4 := math.LegacyMustNewDecFromStr("5.4") + failingBaseAccount := authtypes.NewBaseAccountWithAddress(addr1) + failingBaseAccount.AccountNumber = stdmath.MaxUint64 cases := map[string]struct { - gogo gogoproto.Message - pulsar proto.Message - pulsarMarshalFails bool - - // this will fail in cases where a lossy encoding of an empty array to protobuf occurs. the unmarshalled bytes - // represent the array as nil, and a subsequent marshal to JSON represent the array as null instead of empty. - roundTripUnequal bool - - // pulsar does not support marshaling a math.Dec as anything except a string. Therefore, we cannot unmarshal - // a pulsar encoded Math.dec (the string representation of a Decimal) into a gogo Math.dec (expecting an int64). - protoUnmarshalFails bool + gogo gogoproto.Message + fails bool }{ - "auth/params": {gogo: &authtypes.Params{TxSigLimit: 10}, pulsar: &authapi.Params{TxSigLimit: 10}}, - "auth/module_account": { + "auth/params": { + gogo: &authtypes.Params{TxSigLimit: 10}, + }, + "auth/module_account_nil_permissions": { gogo: &authtypes.ModuleAccount{ - BaseAccount: authtypes.NewBaseAccountWithAddress(addr1), Permissions: []string{}, + BaseAccount: authtypes.NewBaseAccountWithAddress( + addr1, + ), }, - pulsar: &authapi.ModuleAccount{ - BaseAccount: &authapi.BaseAccount{Address: addr1.String()}, Permissions: []string{}, + }, + "auth/module_account/max_uint64": { + gogo: &authtypes.ModuleAccount{ + BaseAccount: failingBaseAccount, + }, + fails: true, + }, + "auth/module_account_empty_permissions": { + gogo: &authtypes.ModuleAccount{ + BaseAccount: authtypes.NewBaseAccountWithAddress( + addr1, + ), + // empty set and nil are indistinguishable from the protoreflect API since they both + // marshal to zero proto bytes, there empty set is not supported. + Permissions: []string{}, }, - roundTripUnequal: true, + fails: true, }, "auth/base_account": { - gogo: &authtypes.BaseAccount{Address: addr1.String(), PubKey: pubkeyAny}, - pulsar: &authapi.BaseAccount{Address: addr1.String(), PubKey: pubkeyAnyPulsar}, + gogo: &authtypes.BaseAccount{Address: addr1.String(), PubKey: pubkeyAny, AccountNumber: 1, Sequence: 2}, }, "authz/msg_grant": { gogo: &authztypes.MsgGrant{ + Granter: addr1.String(), Grantee: addr1.String(), Grant: authztypes.Grant{Expiration: &now, Authorization: genericAuth}, }, - pulsar: &authzapi.MsgGrant{ - Grant: &authzapi.Grant{Expiration: timestamppb.New(now), Authorization: genericAuthPulsar}, - }, }, "authz/msg_update_params": { - gogo: &authtypes.MsgUpdateParams{Params: authtypes.Params{TxSigLimit: 10}}, - pulsar: &authapi.MsgUpdateParams{Params: &authapi.Params{TxSigLimit: 10}}, + gogo: &authtypes.MsgUpdateParams{Params: authtypes.Params{TxSigLimit: 10}}, }, "authz/msg_exec/empty_msgs": { - gogo: &authztypes.MsgExec{Msgs: []*codectypes.Any{}}, - pulsar: &authzapi.MsgExec{Msgs: []*anypb.Any{}}, + gogo: &authztypes.MsgExec{Msgs: []*codectypes.Any{}}, }, "distribution/delegator_starting_info": { - gogo: &disttypes.DelegatorStartingInfo{}, - pulsar: &distapi.DelegatorStartingInfo{}, + gogo: &disttypes.DelegatorStartingInfo{Stake: math.LegacyNewDec(10)}, }, "distribution/delegator_starting_info/non_zero_dec": { - gogo: &disttypes.DelegatorStartingInfo{Stake: math.LegacyNewDec(10)}, - pulsar: &distapi.DelegatorStartingInfo{Stake: "10.000000000000000000"}, - protoUnmarshalFails: true, + gogo: &disttypes.DelegatorStartingInfo{Stake: math.LegacyNewDec(10)}, }, "distribution/delegation_delegator_reward": { - gogo: &disttypes.DelegationDelegatorReward{}, - pulsar: &distapi.DelegationDelegatorReward{}, + gogo: &disttypes.DelegationDelegatorReward{}, }, "distribution/community_pool_spend_proposal_with_deposit": { gogo: &disttypes.CommunityPoolSpendProposalWithDeposit{}, pulsar: &distapi.CommunityPoolSpendProposalWithDeposit{}, //nolint:staticcheck // keep test as is testing legacy parity }, "distribution/msg_withdraw_delegator_reward": { - gogo: &disttypes.MsgWithdrawDelegatorReward{DelegatorAddress: "foo"}, - pulsar: &distapi.MsgWithdrawDelegatorReward{DelegatorAddress: "foo"}, + gogo: &disttypes.MsgWithdrawDelegatorReward{DelegatorAddress: "foo"}, }, "crypto/ed25519": { - gogo: &ed25519types.PubKey{Key: []byte("key")}, - pulsar: &ed25519.PubKey{Key: []byte("key")}, + gogo: &ed25519types.PubKey{Key: []byte("key")}, }, "crypto/secp256k1": { - gogo: &secp256k1types.PubKey{Key: []byte("key")}, - pulsar: &secp256k1.PubKey{Key: []byte("key")}, + gogo: &secp256k1types.PubKey{Key: []byte("key")}, }, "crypto/legacy_amino_pubkey": { - gogo: &multisig.LegacyAminoPubKey{PubKeys: []*codectypes.Any{pubkeyAny}}, - pulsar: &multisigapi.LegacyAminoPubKey{PublicKeys: []*anypb.Any{pubkeyAnyPulsar}}, + gogo: &multisig.LegacyAminoPubKey{PubKeys: []*codectypes.Any{pubkeyAny}}, }, - "crypto/legacy_amino_pubkey/empty": { - gogo: &multisig.LegacyAminoPubKey{}, - pulsar: &multisigapi.LegacyAminoPubKey{}, + "crypto/legacy_amino_pubkey_empty": { + gogo: &multisig.LegacyAminoPubKey{}, }, "consensus/evidence_params/duration": { - gogo: &gov_v1beta1_types.VotingParams{VotingPeriod: 1e9 + 7}, - pulsar: &gov_v1beta1_api.VotingParams{VotingPeriod: &durationpb.Duration{Seconds: 1, Nanos: 7}}, + gogo: &gov_v1beta1_types.VotingParams{VotingPeriod: 1e9 + 7}, }, "consensus/evidence_params/big_duration": { - gogo: &gov_v1beta1_types.VotingParams{VotingPeriod: time.Duration(rapidproto.MaxDurationSeconds*1e9) + 999999999}, - pulsar: &gov_v1beta1_api.VotingParams{VotingPeriod: &durationpb.Duration{ - Seconds: rapidproto.MaxDurationSeconds, Nanos: 999999999, - }}, + gogo: &gov_v1beta1_types.VotingParams{ + VotingPeriod: time.Duration(rapidproto.MaxDurationSeconds*1e9) + 999999999, + }, }, "consensus/evidence_params/too_big_duration": { - gogo: &gov_v1beta1_types.VotingParams{VotingPeriod: time.Duration(rapidproto.MaxDurationSeconds*1e9) + 999999999}, - pulsar: &gov_v1beta1_api.VotingParams{VotingPeriod: &durationpb.Duration{ - Seconds: rapidproto.MaxDurationSeconds + 1, Nanos: 999999999, - }}, - pulsarMarshalFails: true, + gogo: &gov_v1beta1_types.VotingParams{ + VotingPeriod: time.Duration(rapidproto.MaxDurationSeconds*1e9) + 999999999, + }, }, // amino.dont_omitempty + empty/nil lists produce some surprising results "bank/send_authorization/empty_coins": { - gogo: &banktypes.SendAuthorization{SpendLimit: []types.Coin{}}, - pulsar: &bankapi.SendAuthorization{SpendLimit: []*v1beta1.Coin{}}, + gogo: &banktypes.SendAuthorization{SpendLimit: []types.Coin{}}, }, "bank/send_authorization/nil_coins": { - gogo: &banktypes.SendAuthorization{SpendLimit: nil}, - pulsar: &bankapi.SendAuthorization{SpendLimit: nil}, + gogo: &banktypes.SendAuthorization{SpendLimit: nil}, }, "bank/send_authorization/empty_list": { - gogo: &banktypes.SendAuthorization{AllowList: []string{}}, - pulsar: &bankapi.SendAuthorization{AllowList: []string{}}, + gogo: &banktypes.SendAuthorization{AllowList: []string{}}, }, "bank/send_authorization/nil_list": { - gogo: &banktypes.SendAuthorization{AllowList: nil}, - pulsar: &bankapi.SendAuthorization{AllowList: nil}, + gogo: &banktypes.SendAuthorization{AllowList: nil}, }, "bank/msg_multi_send/nil_everything": { - gogo: &banktypes.MsgMultiSend{}, - pulsar: &bankapi.MsgMultiSend{}, + gogo: &banktypes.MsgMultiSend{}, }, "gov/v1_msg_submit_proposal": { - gogo: &gov_v1_types.MsgSubmitProposal{}, - pulsar: &gov_v1_api.MsgSubmitProposal{}, + gogo: &gov_v1_types.MsgSubmitProposal{}, }, - "slashing/params/empty_dec": { - gogo: &slashingtypes.Params{DowntimeJailDuration: 1e9 + 7}, - pulsar: &slashingapi.Params{DowntimeJailDuration: &durationpb.Duration{Seconds: 1, Nanos: 7}}, - }, - // This test cases demonstrates the expected contract and proper way to set a cosmos.Dec field represented - // as bytes in protobuf message, namely: - // dec10bz, _ := types.NewDec(10).Marshal() "slashing/params/dec": { gogo: &slashingtypes.Params{ - DowntimeJailDuration: 1e9 + 7, - MinSignedPerWindow: math.LegacyNewDec(10), - }, - pulsar: &slashingapi.Params{ - DowntimeJailDuration: &durationpb.Duration{Seconds: 1, Nanos: 7}, - MinSignedPerWindow: dec10bz, + DowntimeJailDuration: 1e9 + 7, + MinSignedPerWindow: math.LegacyNewDec(10), + SlashFractionDoubleSign: math.LegacyZeroDec(), + SlashFractionDowntime: math.LegacyZeroDec(), }, }, +<<<<<<< HEAD "staking/create_validator": { gogo: &stakingtypes.MsgCreateValidator{Pubkey: pubkeyAny}, pulsar: &stakingapi.MsgCreateValidator{ @@ -356,140 +311,115 @@ func TestAminoJSON_LegacyParity(t *testing.T) { Description: &stakingapi.Description{}, Commission: &stakingapi.CommissionRates{}, Value: &v1beta1.Coin{}, +======= + "staking/msg_update_params": { + gogo: &stakingtypes.MsgUpdateParams{ + Params: stakingtypes.Params{ + UnbondingTime: 0, + KeyRotationFee: types.Coin{}, + MinCommissionRate: math.LegacyZeroDec(), + }, + }, + }, + "staking/create_validator": { + gogo: &stakingtypes.MsgCreateValidator{ + Pubkey: pubkeyAny, + Commission: stakingtypes.CommissionRates{ + Rate: dec5point4, + MaxRate: math.LegacyZeroDec(), + MaxChangeRate: math.LegacyZeroDec(), + }, + MinSelfDelegation: math.NewIntFromUint64(10), +>>>>>>> 2d40cc1ab (fix(x/tx): fix amino json drift from legacy spec (#21825)) }, }, "staking/msg_cancel_unbonding_delegation_response": { - gogo: &stakingtypes.MsgCancelUnbondingDelegationResponse{}, - pulsar: &stakingapi.MsgCancelUnbondingDelegationResponse{}, + gogo: &stakingtypes.MsgCancelUnbondingDelegationResponse{}, }, "staking/stake_authorization_empty": { - gogo: &stakingtypes.StakeAuthorization{}, - pulsar: &stakingapi.StakeAuthorization{}, + gogo: &stakingtypes.StakeAuthorization{}, }, "staking/stake_authorization_allow": { gogo: &stakingtypes.StakeAuthorization{ + MaxTokens: &types.Coin{Denom: "foo", Amount: math.NewInt(123)}, Validators: &stakingtypes.StakeAuthorization_AllowList{ - AllowList: &stakingtypes.StakeAuthorization_Validators{Address: []string{"foo"}}, + AllowList: &stakingtypes.StakeAuthorization_Validators{ + Address: []string{"foo"}, + }, }, + AuthorizationType: stakingtypes.AuthorizationType_AUTHORIZATION_TYPE_DELEGATE, }, - pulsar: &stakingapi.StakeAuthorization{ - Validators: &stakingapi.StakeAuthorization_AllowList{ - AllowList: &stakingapi.StakeAuthorization_Validators{Address: []string{"foo"}}, + }, + "staking/stake_authorization_deny": { + gogo: &stakingtypes.StakeAuthorization{ + MaxTokens: &types.Coin{Denom: "foo", Amount: math.NewInt(123)}, + Validators: &stakingtypes.StakeAuthorization_DenyList{ + DenyList: &stakingtypes.StakeAuthorization_Validators{}, }, + AuthorizationType: stakingtypes.AuthorizationType_AUTHORIZATION_TYPE_DELEGATE, }, + // to be fixed in https://github.com/cosmos/cosmos-sdk/pull/21782 + // TODO remove once merged + fails: true, }, "vesting/base_account_empty": { - gogo: &vestingtypes.BaseVestingAccount{BaseAccount: &authtypes.BaseAccount{}}, - pulsar: &vestingapi.BaseVestingAccount{BaseAccount: &authapi.BaseAccount{}}, + gogo: &vestingtypes.BaseVestingAccount{BaseAccount: &authtypes.BaseAccount{}}, }, "vesting/base_account_pubkey": { - gogo: &vestingtypes.BaseVestingAccount{BaseAccount: &authtypes.BaseAccount{PubKey: pubkeyAny}}, - pulsar: &vestingapi.BaseVestingAccount{BaseAccount: &authapi.BaseAccount{PubKey: pubkeyAnyPulsar}}, + gogo: &vestingtypes.BaseVestingAccount{ + BaseAccount: &authtypes.BaseAccount{PubKey: pubkeyAny}, + }, }, "math/int_as_string": { - gogo: &gogo_testpb.IntAsString{IntAsString: math.NewInt(123)}, - pulsar: &pulsar_testpb.IntAsString{IntAsString: "123"}, + gogo: &gogo_testpb.IntAsString{IntAsString: math.NewInt(123)}, }, "math/int_as_string/empty": { - gogo: &gogo_testpb.IntAsString{}, - pulsar: &pulsar_testpb.IntAsString{}, + gogo: &gogo_testpb.IntAsString{}, }, "math/int_as_bytes": { - gogo: &gogo_testpb.IntAsBytes{IntAsBytes: math.NewInt(123)}, - pulsar: &pulsar_testpb.IntAsBytes{IntAsBytes: int123bz}, + gogo: &gogo_testpb.IntAsBytes{IntAsBytes: math.NewInt(123)}, }, "math/int_as_bytes/empty": { - gogo: &gogo_testpb.IntAsBytes{}, - pulsar: &pulsar_testpb.IntAsBytes{}, + gogo: &gogo_testpb.IntAsBytes{}, }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { - gogoBytes, err := encCfg.Amino.MarshalJSON(tc.gogo) - require.NoError(t, err) - - pulsarBytes, err := aj.Marshal(tc.pulsar) - if tc.pulsarMarshalFails { - require.Error(t, err) - return - } - require.NoError(t, err) - - fmt.Printf("pulsar: %s\n", string(pulsarBytes)) - fmt.Printf(" gogo: %s\n", string(gogoBytes)) - require.Equal(t, string(gogoBytes), string(pulsarBytes)) - - pulsarProtoBytes, err := proto.Marshal(tc.pulsar) - require.NoError(t, err) - - gogoType := reflect.TypeOf(tc.gogo).Elem() - newGogo := reflect.New(gogoType).Interface().(gogoproto.Message) - - err = encCfg.Codec.Unmarshal(pulsarProtoBytes, newGogo) - if tc.protoUnmarshalFails { - require.Error(t, err) - return - } + legacyBytes := fixture.MarshalLegacyAminoJSON(t, tc.gogo) + dynamicBytes, err := aj.Marshal(fixture.DynamicMessage(t, tc.gogo)) require.NoError(t, err) - newGogoBytes, err := encCfg.Amino.MarshalJSON(newGogo) - require.NoError(t, err) - if tc.roundTripUnequal { - require.NotEqual(t, string(gogoBytes), string(newGogoBytes)) + t.Logf("legacy: %s\n", string(legacyBytes)) + t.Logf(" sut: %s\n", string(dynamicBytes)) + if tc.fails { + require.NotEqual(t, string(legacyBytes), string(dynamicBytes)) return } - require.Equal(t, string(gogoBytes), string(newGogoBytes)) + require.Equal(t, string(legacyBytes), string(dynamicBytes)) // test amino json signer handler equivalence - msg, ok := tc.gogo.(legacytx.LegacyMsg) - if !ok { + if !proto.HasExtension(fixture.MessageDescriptor(t, tc.gogo).Options(), msgv1.E_Signer) { // not signable return } - - handlerOptions := signing_testutil.HandlerArgumentOptions{ - ChainID: "test-chain", - Memo: "sometestmemo", - Msg: tc.pulsar, - AccNum: 1, - AccSeq: 2, - SignerAddress: "signerAddress", - Fee: &txv1beta1.Fee{ - Amount: []*v1beta1.Coin{{Denom: "uatom", Amount: "1000"}}, - }, - } - - signerData, txData, err := signing_testutil.MakeHandlerArguments(handlerOptions) - require.NoError(t, err) - - handler := aminojson.NewSignModeHandler(aminojson.SignModeHandlerOptions{}) - signBz, err := handler.GetSignBytes(context.Background(), signerData, txData) - require.NoError(t, err) - - legacyHandler := tx.NewSignModeLegacyAminoJSONHandler() - txBuilder := encCfg.TxConfig.NewTxBuilder() - require.NoError(t, txBuilder.SetMsgs([]types.Msg{msg}...)) - txBuilder.SetMemo(handlerOptions.Memo) - txBuilder.SetFeeAmount(types.Coins{types.NewInt64Coin("uatom", 1000)}) - theTx := txBuilder.GetTx() - - legacySigningData := signing.SignerData{ - ChainID: handlerOptions.ChainID, - Address: handlerOptions.SignerAddress, - AccountNumber: handlerOptions.AccNum, - Sequence: handlerOptions.AccSeq, - } - legacySignBz, err := legacyHandler.GetSignBytes(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, - legacySigningData, theTx) - require.NoError(t, err) - require.Equal(t, string(legacySignBz), string(signBz)) + fixture.RequireLegacyAminoEquivalent(t, tc.gogo) }) } } func TestSendAuthorization(t *testing.T) { +<<<<<<< HEAD encCfg := testutil.MakeTestEncodingConfig(auth.AppModuleBasic{}, authzmodule.AppModuleBasic{}, distribution.AppModuleBasic{}, bank.AppModuleBasic{}) +======= + encCfg := testutil.MakeTestEncodingConfig( + codectestutil.CodecOptions{}, + auth.AppModule{}, + authzmodule.AppModule{}, + distribution.AppModule{}, + bank.AppModule{}, + ) +>>>>>>> 2d40cc1ab (fix(x/tx): fix amino json drift from legacy spec (#21825)) aj := aminojson.NewEncoder(aminojson.EncoderOptions{}) diff --git a/tests/integration/tx/context_test.go b/tests/integration/tx/context_test.go index 2210c9c934fd..e2814de77c69 100644 --- a/tests/integration/tx/context_test.go +++ b/tests/integration/tx/context_test.go @@ -3,29 +3,19 @@ package tx import ( "testing" - "github.com/stretchr/testify/require" - "google.golang.org/protobuf/proto" - "cosmossdk.io/depinject" "cosmossdk.io/log" "cosmossdk.io/x/tx/signing" - codectypes "github.com/cosmos/cosmos-sdk/codec/types" + "github.com/cosmos/cosmos-sdk/tests/integration/tx/internal" "github.com/cosmos/cosmos-sdk/tests/integration/tx/internal/pulsar/testpb" "github.com/cosmos/cosmos-sdk/testutil/configurator" simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" + "github.com/stretchr/testify/require" ) -func ProvideCustomGetSigners() signing.CustomGetSigner { - return signing.CustomGetSigner{ - MsgType: proto.MessageName(&testpb.TestRepeatedFields{}), - Fn: func(msg proto.Message) ([][]byte, error) { - testMsg := msg.(*testpb.TestRepeatedFields) - // arbitrary logic - signer := testMsg.NullableDontOmitempty[1].Value - return [][]byte{[]byte(signer)}, nil - }, - } +func ProvideCustomGetSigner() signing.CustomGetSigner { + return internal.TestRepeatedFieldsSigner } func TestDefineCustomGetSigners(t *testing.T) { @@ -40,7 +30,7 @@ func TestDefineCustomGetSigners(t *testing.T) { configurator.ConsensusModule(), ), depinject.Supply(log.NewNopLogger()), - depinject.Provide(ProvideCustomGetSigners), + depinject.Provide(ProvideCustomGetSigner), ), &interfaceRegistry, ) diff --git a/tests/integration/tx/internal/util.go b/tests/integration/tx/internal/util.go new file mode 100644 index 000000000000..e3f6a5828997 --- /dev/null +++ b/tests/integration/tx/internal/util.go @@ -0,0 +1,221 @@ +package internal + +import ( + "bytes" + "context" + "encoding/json" + "testing" + + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/reflect/protoreflect" + "google.golang.org/protobuf/types/dynamicpb" + + "cosmossdk.io/core/transaction" + "cosmossdk.io/x/tx/signing" + "cosmossdk.io/x/tx/signing/aminojson" + + "github.com/cosmos/cosmos-sdk/client" + "github.com/cosmos/cosmos-sdk/codec" + codectypes "github.com/cosmos/cosmos-sdk/codec/types" + "github.com/cosmos/cosmos-sdk/std" + "github.com/cosmos/cosmos-sdk/tests/integration/tx/internal/pulsar/testpb" + "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/module" + signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing" + "github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx" + authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" + "github.com/cosmos/cosmos-sdk/x/auth/tx" + gogoproto "github.com/cosmos/gogoproto/proto" +) + +var TestRepeatedFieldsSigner = signing.CustomGetSigner{ + MsgType: proto.MessageName(&testpb.TestRepeatedFields{}), + Fn: func(msg proto.Message) ([][]byte, error) { + testMsg := msg.(*testpb.TestRepeatedFields) + // arbitrary logic + signer := testMsg.NullableDontOmitempty[1].Value + return [][]byte{[]byte(signer)}, nil + }, +} + +type noOpAddressCodec struct{} + +func (a noOpAddressCodec) StringToBytes(text string) ([]byte, error) { + return []byte(text), nil +} + +func (a noOpAddressCodec) BytesToString(bz []byte) (string, error) { + return string(bz), nil +} + +type SigningFixture struct { + txConfig client.TxConfig + legacy *codec.LegacyAmino + protoCodec *codec.ProtoCodec + options SigningFixtureOptions + registry codectypes.InterfaceRegistry +} + +type SigningFixtureOptions struct { + DoNotSortFields bool +} + +func NewSigningFixture( + t *testing.T, + options SigningFixtureOptions, + modules ...module.AppModule, +) *SigningFixture { + t.Helper() + // set up transaction and signing infra + addressCodec, valAddressCodec := noOpAddressCodec{}, noOpAddressCodec{} + customGetSigners := []signing.CustomGetSigner{TestRepeatedFieldsSigner} + interfaceRegistry, _, err := codec.ProvideInterfaceRegistry( + addressCodec, + valAddressCodec, + customGetSigners, + ) + require.NoError(t, err) + protoCodec := codec.ProvideProtoCodec(interfaceRegistry) + signingOptions := &signing.Options{ + FileResolver: interfaceRegistry, + AddressCodec: addressCodec, + ValidatorAddressCodec: valAddressCodec, + } + for _, customGetSigner := range customGetSigners { + signingOptions.DefineCustomGetSigners(customGetSigner.MsgType, customGetSigner.Fn) + } + txConfig, err := tx.NewTxConfigWithOptions( + protoCodec, + tx.ConfigOptions{ + EnabledSignModes: []signingtypes.SignMode{ + signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, + }, + SigningOptions: signingOptions, + }) + require.NoError(t, err) + + legacyAminoCodec := codec.NewLegacyAmino() + mb := module.NewManager(modules...) + std.RegisterLegacyAminoCodec(legacyAminoCodec) + std.RegisterInterfaces(interfaceRegistry) + mb.RegisterLegacyAminoCodec(legacyAminoCodec) + mb.RegisterInterfaces(interfaceRegistry) + + return &SigningFixture{ + txConfig: txConfig, + legacy: legacyAminoCodec, + options: options, + protoCodec: protoCodec, + registry: interfaceRegistry, + } +} + +func (s *SigningFixture) RequireLegacyAminoEquivalent(t *testing.T, msg transaction.Msg) { + t.Helper() + // create tx envelope + txBuilder := s.txConfig.NewTxBuilder() + err := txBuilder.SetMsgs([]types.Msg{msg}...) + require.NoError(t, err) + builtTx := txBuilder.GetTx() + + // round trip it to simulate application usage + txBz, err := s.txConfig.TxEncoder()(builtTx) + require.NoError(t, err) + theTx, err := s.txConfig.TxDecoder()(txBz) + require.NoError(t, err) + + // create signing envelope + signerData := signing.SignerData{ + Address: "sender-address", + ChainID: "test-chain", + AccountNumber: 0, + Sequence: 0, + } + adaptableTx, ok := theTx.(authsigning.V2AdaptableTx) + require.True(t, ok) + + legacytx.RegressionTestingAminoCodec = s.legacy + defer func() { + legacytx.RegressionTestingAminoCodec = nil + }() + legacyAminoSignHandler := tx.NewSignModeLegacyAminoJSONHandler() + legacyBz, err := legacyAminoSignHandler.GetSignBytes( + signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, + authsigning.SignerData{ + ChainID: signerData.ChainID, + Address: signerData.Address, + AccountNumber: signerData.AccountNumber, + Sequence: signerData.Sequence, + }, + theTx) + require.NoError(t, err) + + handler := aminojson.NewSignModeHandler(aminojson.SignModeHandlerOptions{}) + signBz, err := handler.GetSignBytes( + context.Background(), + signerData, + adaptableTx.GetSigningTxData(), + ) + require.NoError(t, err) + + require.Truef(t, + bytes.Equal(legacyBz, signBz), + "legacy: %s\n x/tx: %s", string(legacyBz), string(signBz)) +} + +func (s *SigningFixture) MarshalLegacyAminoJSON(t *testing.T, o any) []byte { + t.Helper() + bz, err := s.legacy.MarshalJSON(o) + require.NoError(t, err) + if s.options.DoNotSortFields { + return bz + } + sortedBz, err := sortJson(bz) + require.NoError(t, err) + return sortedBz +} + +func (s *SigningFixture) UnmarshalGogoProto(bz []byte, ptr transaction.Msg) error { + return s.protoCodec.Unmarshal(bz, ptr) +} + +func (s *SigningFixture) MessageDescriptor(t *testing.T, msg transaction.Msg) protoreflect.MessageDescriptor { + t.Helper() + typeName := gogoproto.MessageName(msg) + msgDesc, err := s.registry.FindDescriptorByName(protoreflect.FullName(typeName)) + require.NoError(t, err) + return msgDesc.(protoreflect.MessageDescriptor) +} + +// DynamicMessage is identical to the Decoder implementation in +// https://github.com/cosmos/cosmos-sdk/blob/6d2f6ff068c81c5783e01319beaa51c7dbb43edd/x/tx/decode/decode.go#L136 +// It is duplicated here to test dynamic message implementations specifically. +// The code path linked above is also covered in this package. +func (s *SigningFixture) DynamicMessage(t *testing.T, msg transaction.Msg) proto.Message { + t.Helper() + msgDesc := s.MessageDescriptor(t, msg) + protoBz, err := gogoproto.Marshal(msg) + require.NoError(t, err) + dynamicMsg := dynamicpb.NewMessageType(msgDesc).New().Interface() + err = proto.Unmarshal(protoBz, dynamicMsg) + require.NoError(t, err) + return dynamicMsg +} + +// sortJson sorts the JSON bytes by way of the side effect of unmarshalling and remarshalling +// the JSON using encoding/json. This hacky way of sorting JSON fields was used by the legacy +// amino JSON encoding x/auth/migrations/legacytx.StdSignBytes. It is used here ensure the x/tx +// JSON encoding is equivalent to the legacy amino JSON encoding. +func sortJson(bz []byte) ([]byte, error) { + var c any + err := json.Unmarshal(bz, &c) + if err != nil { + return nil, err + } + js, err := json.Marshal(c) + if err != nil { + return nil, err + } + return js, nil +} diff --git a/x/auth/migrations/legacytx/stdtx_test.go b/x/auth/migrations/legacytx/stdtx_test.go index f49357ca6398..be3295df19ca 100644 --- a/x/auth/migrations/legacytx/stdtx_test.go +++ b/x/auth/migrations/legacytx/stdtx_test.go @@ -44,7 +44,11 @@ func TestStdSignBytes(t *testing.T) { Amount: []*basev1beta1.Coin{{Denom: "atom", Amount: "150"}}, GasLimit: 100000, } +<<<<<<< HEAD msgStr := fmt.Sprintf(`{"type":"testpb/TestMsg","value":{"signers":["%s"]}}`, addr) +======= + msgStr := fmt.Sprintf(`{"type":"testpb/TestMsg","value":{"decField":"0.000000000000000000","signers":["%s"]}}`, addr) +>>>>>>> 2d40cc1ab (fix(x/tx): fix amino json drift from legacy spec (#21825)) tests := []struct { name string args args diff --git a/x/tx/CHANGELOG.md b/x/tx/CHANGELOG.md index 058b9b7d99a2..12d1b368e861 100644 --- a/x/tx/CHANGELOG.md +++ b/x/tx/CHANGELOG.md @@ -33,6 +33,12 @@ Since v0.13.0, x/tx follows Cosmos SDK semver: https://github.com/cosmos/cosmos- ## [Unreleased] +<<<<<<< HEAD +======= +* [#21825](https://github.com/cosmos/cosmos-sdk/pull/21825) Fix decimal encoding and field ordering in Amino JSON encoder. +* [#21850](https://github.com/cosmos/cosmos-sdk/pull/21850) Support bytes field as signer. + +>>>>>>> 2d40cc1ab (fix(x/tx): fix amino json drift from legacy spec (#21825)) ## [v0.13.5](https://github.com/cosmos/cosmos-sdk/releases/tag/x/tx/v0.13.5) - 2024-09-18 ### Improvements diff --git a/x/tx/signing/aminojson/encoder.go b/x/tx/signing/aminojson/encoder.go index c1e37ec0723c..e954c2651e30 100644 --- a/x/tx/signing/aminojson/encoder.go +++ b/x/tx/signing/aminojson/encoder.go @@ -51,7 +51,12 @@ func cosmosDecEncoder(_ *Encoder, v protoreflect.Value, w io.Writer) error { if val == "" { return jsonMarshal(w, "0") } - return jsonMarshal(w, val) + var dec math.LegacyDec + err := dec.Unmarshal([]byte(val)) + if err != nil { + return err + } + return jsonMarshal(w, dec.String()) case []byte: if len(val) == 0 { return jsonMarshal(w, "0") @@ -125,27 +130,40 @@ func keyFieldEncoder(_ *Encoder, msg protoreflect.Message, w io.Writer) error { } type moduleAccountPretty struct { - Address string `json:"address"` - PubKey string `json:"public_key"` AccountNumber uint64 `json:"account_number"` - Sequence uint64 `json:"sequence"` + Address string `json:"address"` Name string `json:"name"` Permissions []string `json:"permissions"` + PubKey string `json:"public_key"` + Sequence uint64 `json:"sequence"` } // moduleAccountEncoder replicates the behavior in // https://github.com/cosmos/cosmos-sdk/blob/41a3dfeced2953beba3a7d11ec798d17ee19f506/x/auth/types/account.go#L230-L254 func moduleAccountEncoder(_ *Encoder, msg protoreflect.Message, w io.Writer) error { - ma := msg.Interface().(*authapi.ModuleAccount) + ma := &authapi.ModuleAccount{} + msgDesc := msg.Descriptor() + if msgDesc.FullName() != ma.ProtoReflect().Descriptor().FullName() { + return errors.New("moduleAccountEncoder: msg not a auth.ModuleAccount") + } + fields := msgDesc.Fields() + pretty := moduleAccountPretty{ - PubKey: "", - Name: ma.Name, - Permissions: ma.Permissions, - } - if ma.BaseAccount != nil { - pretty.Address = ma.BaseAccount.Address - pretty.AccountNumber = ma.BaseAccount.AccountNumber - pretty.Sequence = ma.BaseAccount.Sequence + PubKey: "", + Name: msg.Get(fields.ByName("name")).String(), + } + permissions := msg.Get(fields.ByName("permissions")).List() + for i := 0; i < permissions.Len(); i++ { + pretty.Permissions = append(pretty.Permissions, permissions.Get(i).String()) + } + + if msg.Has(fields.ByName("base_account")) { + baseAccount := msg.Get(fields.ByName("base_account")) + baMsg := baseAccount.Message() + bamdFields := baMsg.Descriptor().Fields() + pretty.Address = baMsg.Get(bamdFields.ByName("address")).String() + pretty.AccountNumber = baMsg.Get(bamdFields.ByName("account_number")).Uint() + pretty.Sequence = baMsg.Get(bamdFields.ByName("sequence")).Uint() } else { pretty.Address = "" pretty.AccountNumber = 0 @@ -166,29 +184,34 @@ func moduleAccountEncoder(_ *Encoder, msg protoreflect.Message, w io.Writer) err // also see: // https://github.com/cosmos/cosmos-sdk/blob/b49f948b36bc991db5be431607b475633aed697e/proto/cosmos/crypto/multisig/keys.proto#L15/ func thresholdStringEncoder(enc *Encoder, msg protoreflect.Message, w io.Writer) error { - pk, ok := msg.Interface().(*multisig.LegacyAminoPubKey) - if !ok { + pk := &multisig.LegacyAminoPubKey{} + msgDesc := msg.Descriptor() + fields := msgDesc.Fields() + if msgDesc.FullName() != pk.ProtoReflect().Descriptor().FullName() { return errors.New("thresholdStringEncoder: msg not a multisig.LegacyAminoPubKey") } - _, err := fmt.Fprintf(w, `{"threshold":"%d","pubkeys":`, pk.Threshold) - if err != nil { - return err - } - if len(pk.PublicKeys) == 0 { - _, err = io.WriteString(w, `[]}`) - return err - } - - fields := msg.Descriptor().Fields() pubkeysField := fields.ByName("public_keys") pubkeys := msg.Get(pubkeysField).List() - err = enc.marshalList(pubkeys, pubkeysField, w) + _, err := io.WriteString(w, `{"pubkeys":`) if err != nil { return err } - _, err = io.WriteString(w, `}`) + if pubkeys.Len() == 0 { + _, err := io.WriteString(w, `[]`) + if err != nil { + return err + } + } else { + err := enc.marshalList(pubkeys, pubkeysField, w) + if err != nil { + return err + } + } + + threshold := fields.ByName("threshold") + _, err = fmt.Fprintf(w, `,"threshold":"%d"}`, msg.Get(threshold).Uint()) return err }