From ff8789a0287147a658b7976a47e34c7e0e35a13a Mon Sep 17 00:00:00 2001 From: Matt Kocubinski Date: Thu, 3 Oct 2024 06:10:02 -0500 Subject: [PATCH 1/7] 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 } From 09bf0ff77b94161fcdac7a6e91724f813bf92d25 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Thu, 3 Oct 2024 13:48:25 +0200 Subject: [PATCH 2/7] fix half conflicts and revert tests --- go.mod | 2 +- go.sum | 4 +- simapp/go.mod | 2 +- simapp/go.sum | 4 +- tests/go.mod | 2 +- tests/go.sum | 4 +- 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/tx/CHANGELOG.md | 8 +- 11 files changed, 272 insertions(+), 414 deletions(-) delete mode 100644 tests/integration/tx/internal/util.go diff --git a/go.mod b/go.mod index 93e473bc5dc1..54cb97e3f63d 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( cosmossdk.io/log v1.4.1 cosmossdk.io/math v1.3.0 cosmossdk.io/store v1.1.1 - cosmossdk.io/x/tx v0.13.5 + cosmossdk.io/x/tx v0.13.6-0.20241003112805-ff8789a02871 github.com/99designs/keyring v1.2.1 github.com/bgentry/speakeasy v0.1.1-0.20220910012023-760eaf8b6816 github.com/bits-and-blooms/bitset v1.8.0 diff --git a/go.sum b/go.sum index 7a4cfedc0a8a..57e9bc9eab38 100644 --- a/go.sum +++ b/go.sum @@ -16,8 +16,8 @@ cosmossdk.io/math v1.3.0 h1:RC+jryuKeytIiictDslBP9i1fhkVm6ZDmZEoNP316zE= cosmossdk.io/math v1.3.0/go.mod h1:vnRTxewy+M7BtXBNFybkuhSH4WfedVAAnERHgVFhp3k= cosmossdk.io/store v1.1.1 h1:NA3PioJtWDVU7cHHeyvdva5J/ggyLDkyH0hGHl2804Y= cosmossdk.io/store v1.1.1/go.mod h1:8DwVTz83/2PSI366FERGbWSH7hL6sB7HbYp8bqksNwM= -cosmossdk.io/x/tx v0.13.5 h1:FdnU+MdmFWn1pTsbfU0OCf2u6mJ8cqc1H4OMG418MLw= -cosmossdk.io/x/tx v0.13.5/go.mod h1:V6DImnwJMTq5qFjeGWpXNiT/fjgE4HtmclRmTqRVM3w= +cosmossdk.io/x/tx v0.13.6-0.20241003112805-ff8789a02871 h1:+lRwWQRVvB3jgRgdqrgeFUJ45BoXZh/UeeAV5f/m2Gk= +cosmossdk.io/x/tx v0.13.6-0.20241003112805-ff8789a02871/go.mod h1:V6DImnwJMTq5qFjeGWpXNiT/fjgE4HtmclRmTqRVM3w= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= filippo.io/edwards25519 v1.0.0 h1:0wAIcmJUqRdI8IJ/3eGi5/HwXZWPujYXXlkrQogz0Ek= filippo.io/edwards25519 v1.0.0/go.mod h1:N1IkdkCkiLB6tki+MYJoSx2JTY9NUlxZE7eHn5EwJns= diff --git a/simapp/go.mod b/simapp/go.mod index 685d5f570066..3c7157da8d78 100644 --- a/simapp/go.mod +++ b/simapp/go.mod @@ -16,7 +16,7 @@ require ( cosmossdk.io/x/evidence v0.1.1 cosmossdk.io/x/feegrant v0.1.1 cosmossdk.io/x/nft v0.1.1 - cosmossdk.io/x/tx v0.13.5 + cosmossdk.io/x/tx v0.13.6-0.20241003112805-ff8789a02871 cosmossdk.io/x/upgrade v0.1.4 github.com/cometbft/cometbft v0.38.12 github.com/cosmos/cosmos-db v1.0.2 diff --git a/simapp/go.sum b/simapp/go.sum index ad8dbdeb39e4..b7f3422a1af2 100644 --- a/simapp/go.sum +++ b/simapp/go.sum @@ -210,8 +210,8 @@ cosmossdk.io/x/feegrant v0.1.1 h1:EKFWOeo/pup0yF0svDisWWKAA9Zags6Zd0P3nRvVvw8= cosmossdk.io/x/feegrant v0.1.1/go.mod h1:2GjVVxX6G2fta8LWj7pC/ytHjryA6MHAJroBWHFNiEQ= cosmossdk.io/x/nft v0.1.1 h1:pslAVS8P5NkW080+LWOamInjDcq+v2GSCo+BjN9sxZ8= cosmossdk.io/x/nft v0.1.1/go.mod h1:Kac6F6y2gsKvoxU+fy8uvxRTi4BIhLOor2zgCNQwVgY= -cosmossdk.io/x/tx v0.13.5 h1:FdnU+MdmFWn1pTsbfU0OCf2u6mJ8cqc1H4OMG418MLw= -cosmossdk.io/x/tx v0.13.5/go.mod h1:V6DImnwJMTq5qFjeGWpXNiT/fjgE4HtmclRmTqRVM3w= +cosmossdk.io/x/tx v0.13.6-0.20241003112805-ff8789a02871 h1:+lRwWQRVvB3jgRgdqrgeFUJ45BoXZh/UeeAV5f/m2Gk= +cosmossdk.io/x/tx v0.13.6-0.20241003112805-ff8789a02871/go.mod h1:V6DImnwJMTq5qFjeGWpXNiT/fjgE4HtmclRmTqRVM3w= cosmossdk.io/x/upgrade v0.1.4 h1:/BWJim24QHoXde8Bc64/2BSEB6W4eTydq0X/2f8+g38= cosmossdk.io/x/upgrade v0.1.4/go.mod h1:9v0Aj+fs97O+Ztw+tG3/tp5JSlrmT7IcFhAebQHmOPo= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= diff --git a/tests/go.mod b/tests/go.mod index be3db502fe7a..58f871b12fa6 100644 --- a/tests/go.mod +++ b/tests/go.mod @@ -14,7 +14,7 @@ require ( cosmossdk.io/x/evidence v0.1.1 cosmossdk.io/x/feegrant v0.1.1 cosmossdk.io/x/nft v0.1.1 // indirect - cosmossdk.io/x/tx v0.13.5 + cosmossdk.io/x/tx v0.13.6-0.20241003112805-ff8789a02871 cosmossdk.io/x/upgrade v0.1.4 github.com/cometbft/cometbft v0.38.12 github.com/cosmos/cosmos-db v1.0.2 diff --git a/tests/go.sum b/tests/go.sum index 0521bf1ba569..079779a509ed 100644 --- a/tests/go.sum +++ b/tests/go.sum @@ -208,8 +208,8 @@ cosmossdk.io/x/feegrant v0.1.1 h1:EKFWOeo/pup0yF0svDisWWKAA9Zags6Zd0P3nRvVvw8= cosmossdk.io/x/feegrant v0.1.1/go.mod h1:2GjVVxX6G2fta8LWj7pC/ytHjryA6MHAJroBWHFNiEQ= cosmossdk.io/x/nft v0.1.1 h1:pslAVS8P5NkW080+LWOamInjDcq+v2GSCo+BjN9sxZ8= cosmossdk.io/x/nft v0.1.1/go.mod h1:Kac6F6y2gsKvoxU+fy8uvxRTi4BIhLOor2zgCNQwVgY= -cosmossdk.io/x/tx v0.13.5 h1:FdnU+MdmFWn1pTsbfU0OCf2u6mJ8cqc1H4OMG418MLw= -cosmossdk.io/x/tx v0.13.5/go.mod h1:V6DImnwJMTq5qFjeGWpXNiT/fjgE4HtmclRmTqRVM3w= +cosmossdk.io/x/tx v0.13.6-0.20241003112805-ff8789a02871 h1:+lRwWQRVvB3jgRgdqrgeFUJ45BoXZh/UeeAV5f/m2Gk= +cosmossdk.io/x/tx v0.13.6-0.20241003112805-ff8789a02871/go.mod h1:V6DImnwJMTq5qFjeGWpXNiT/fjgE4HtmclRmTqRVM3w= cosmossdk.io/x/upgrade v0.1.4 h1:/BWJim24QHoXde8Bc64/2BSEB6W4eTydq0X/2f8+g38= cosmossdk.io/x/upgrade v0.1.4/go.mod h1:9v0Aj+fs97O+Ztw+tG3/tp5JSlrmT7IcFhAebQHmOPo= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= diff --git a/tests/integration/rapidgen/rapidgen.go b/tests/integration/rapidgen/rapidgen.go index bd293de062a3..08f2346ab523 100644 --- a/tests/integration/rapidgen/rapidgen.go +++ b/tests/integration/rapidgen/rapidgen.go @@ -231,6 +231,7 @@ 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), @@ -271,9 +272,7 @@ var ( GenType(&slashingtypes.Params{}, &slashingapi.Params{}, GenOpts.WithDisallowNil()), - // 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(&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 b48b78a5418c..eb271308f8d6 100644 --- a/tests/integration/tx/aminojson/aminojson_test.go +++ b/tests/integration/tx/aminojson/aminojson_test.go @@ -1,13 +1,9 @@ package aminojson import ( - "bytes" + "context" "fmt" -<<<<<<< HEAD "reflect" -======= - stdmath "math" ->>>>>>> 2d40cc1ab (fix(x/tx): fix amino json drift from legacy spec (#21825)) "testing" "time" @@ -15,16 +11,31 @@ 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" @@ -32,13 +43,17 @@ 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" @@ -76,7 +91,6 @@ 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{}, @@ -84,25 +98,6 @@ 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() @@ -110,7 +105,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(r *rapid.T) { + rapid.Check(t, func(t *rapid.T) { // uncomment to debug; catch a panic and inspect application state // defer func() { // if r := recover(); r != nil { @@ -119,28 +114,26 @@ func TestAminoJSON_Equivalence(t *testing.T) { // } // }() - msg := gen.Draw(r, "msg") + msg := gen.Draw(t, "msg") postFixPulsarMessage(msg) gogo := tt.Gogo sanity := tt.Pulsar protoBz, err := proto.Marshal(msg) - require.NoError(r, err) + require.NoError(t, err) err = proto.Unmarshal(protoBz, sanity) - require.NoError(r, err) + require.NoError(t, err) - err = fixture.UnmarshalGogoProto(protoBz, gogo) - require.NoError(r, err) + err = encCfg.Codec.Unmarshal(protoBz, gogo) + require.NoError(t, err) - legacyAminoJSON := fixture.MarshalLegacyAminoJSON(t, gogo) + legacyAminoJSON, err := encCfg.Amino.MarshalJSON(gogo) + require.NoError(t, err) aminoJSON, err := aj.Marshal(msg) - 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)) - } + require.NoError(t, err) + require.Equal(t, string(legacyAminoJSON), string(aminoJSON)) // test amino json signer handler equivalence if !proto.HasExtension(desc.Options(), msgv1.E_Signer) { @@ -148,13 +141,47 @@ func TestAminoJSON_Equivalence(t *testing.T) { return } - fixture.RequireLegacyAminoEquivalent(t, gogo) + 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)) }) }) } } -<<<<<<< HEAD func newAny(t *testing.T, msg proto.Message) *anypb.Any { bz, err := proto.Marshal(msg) require.NoError(t, err) @@ -171,139 +198,157 @@ 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")}) - dec5point4 := math.LegacyMustNewDecFromStr("5.4") - failingBaseAccount := authtypes.NewBaseAccountWithAddress(addr1) - failingBaseAccount.AccountNumber = stdmath.MaxUint64 + pubkeyAnyPulsar := newAny(t, &secp256k1.PubKey{Key: []byte("foo")}) + dec10bz, _ := math.LegacyNewDec(10).Marshal() + int123bz, _ := math.NewInt(123).Marshal() cases := map[string]struct { - gogo gogoproto.Message - fails bool + 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 }{ - "auth/params": { - gogo: &authtypes.Params{TxSigLimit: 10}, - }, - "auth/module_account_nil_permissions": { + "auth/params": {gogo: &authtypes.Params{TxSigLimit: 10}, pulsar: &authapi.Params{TxSigLimit: 10}}, + "auth/module_account": { gogo: &authtypes.ModuleAccount{ - BaseAccount: authtypes.NewBaseAccountWithAddress( - addr1, - ), + BaseAccount: authtypes.NewBaseAccountWithAddress(addr1), 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{}, + pulsar: &authapi.ModuleAccount{ + BaseAccount: &authapi.BaseAccount{Address: addr1.String()}, Permissions: []string{}, }, - fails: true, + roundTripUnequal: true, }, "auth/base_account": { - gogo: &authtypes.BaseAccount{Address: addr1.String(), PubKey: pubkeyAny, AccountNumber: 1, Sequence: 2}, + gogo: &authtypes.BaseAccount{Address: addr1.String(), PubKey: pubkeyAny}, + pulsar: &authapi.BaseAccount{Address: addr1.String(), PubKey: pubkeyAnyPulsar}, }, "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}}, + gogo: &authtypes.MsgUpdateParams{Params: authtypes.Params{TxSigLimit: 10}}, + pulsar: &authapi.MsgUpdateParams{Params: &authapi.Params{TxSigLimit: 10}}, }, "authz/msg_exec/empty_msgs": { - gogo: &authztypes.MsgExec{Msgs: []*codectypes.Any{}}, + gogo: &authztypes.MsgExec{Msgs: []*codectypes.Any{}}, + pulsar: &authzapi.MsgExec{Msgs: []*anypb.Any{}}, }, "distribution/delegator_starting_info": { - gogo: &disttypes.DelegatorStartingInfo{Stake: math.LegacyNewDec(10)}, + gogo: &disttypes.DelegatorStartingInfo{}, + pulsar: &distapi.DelegatorStartingInfo{}, }, "distribution/delegator_starting_info/non_zero_dec": { - gogo: &disttypes.DelegatorStartingInfo{Stake: math.LegacyNewDec(10)}, + gogo: &disttypes.DelegatorStartingInfo{Stake: math.LegacyNewDec(10)}, + pulsar: &distapi.DelegatorStartingInfo{Stake: "10.000000000000000000"}, + protoUnmarshalFails: true, }, "distribution/delegation_delegator_reward": { - gogo: &disttypes.DelegationDelegatorReward{}, + gogo: &disttypes.DelegationDelegatorReward{}, + pulsar: &distapi.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"}, + gogo: &disttypes.MsgWithdrawDelegatorReward{DelegatorAddress: "foo"}, + pulsar: &distapi.MsgWithdrawDelegatorReward{DelegatorAddress: "foo"}, }, "crypto/ed25519": { - gogo: &ed25519types.PubKey{Key: []byte("key")}, + gogo: &ed25519types.PubKey{Key: []byte("key")}, + pulsar: &ed25519.PubKey{Key: []byte("key")}, }, "crypto/secp256k1": { - gogo: &secp256k1types.PubKey{Key: []byte("key")}, + gogo: &secp256k1types.PubKey{Key: []byte("key")}, + pulsar: &secp256k1.PubKey{Key: []byte("key")}, }, "crypto/legacy_amino_pubkey": { - gogo: &multisig.LegacyAminoPubKey{PubKeys: []*codectypes.Any{pubkeyAny}}, + gogo: &multisig.LegacyAminoPubKey{PubKeys: []*codectypes.Any{pubkeyAny}}, + pulsar: &multisigapi.LegacyAminoPubKey{PublicKeys: []*anypb.Any{pubkeyAnyPulsar}}, }, - "crypto/legacy_amino_pubkey_empty": { - gogo: &multisig.LegacyAminoPubKey{}, + "crypto/legacy_amino_pubkey/empty": { + gogo: &multisig.LegacyAminoPubKey{}, + pulsar: &multisigapi.LegacyAminoPubKey{}, }, "consensus/evidence_params/duration": { - gogo: &gov_v1beta1_types.VotingParams{VotingPeriod: 1e9 + 7}, + gogo: &gov_v1beta1_types.VotingParams{VotingPeriod: 1e9 + 7}, + pulsar: &gov_v1beta1_api.VotingParams{VotingPeriod: &durationpb.Duration{Seconds: 1, Nanos: 7}}, }, "consensus/evidence_params/big_duration": { - gogo: &gov_v1beta1_types.VotingParams{ - VotingPeriod: time.Duration(rapidproto.MaxDurationSeconds*1e9) + 999999999, - }, + 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, + }}, }, "consensus/evidence_params/too_big_duration": { - gogo: &gov_v1beta1_types.VotingParams{ - VotingPeriod: time.Duration(rapidproto.MaxDurationSeconds*1e9) + 999999999, - }, + 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, }, // amino.dont_omitempty + empty/nil lists produce some surprising results "bank/send_authorization/empty_coins": { - gogo: &banktypes.SendAuthorization{SpendLimit: []types.Coin{}}, + gogo: &banktypes.SendAuthorization{SpendLimit: []types.Coin{}}, + pulsar: &bankapi.SendAuthorization{SpendLimit: []*v1beta1.Coin{}}, }, "bank/send_authorization/nil_coins": { - gogo: &banktypes.SendAuthorization{SpendLimit: nil}, + gogo: &banktypes.SendAuthorization{SpendLimit: nil}, + pulsar: &bankapi.SendAuthorization{SpendLimit: nil}, }, "bank/send_authorization/empty_list": { - gogo: &banktypes.SendAuthorization{AllowList: []string{}}, + gogo: &banktypes.SendAuthorization{AllowList: []string{}}, + pulsar: &bankapi.SendAuthorization{AllowList: []string{}}, }, "bank/send_authorization/nil_list": { - gogo: &banktypes.SendAuthorization{AllowList: nil}, + gogo: &banktypes.SendAuthorization{AllowList: nil}, + pulsar: &bankapi.SendAuthorization{AllowList: nil}, }, "bank/msg_multi_send/nil_everything": { - gogo: &banktypes.MsgMultiSend{}, + gogo: &banktypes.MsgMultiSend{}, + pulsar: &bankapi.MsgMultiSend{}, }, "gov/v1_msg_submit_proposal": { - gogo: &gov_v1_types.MsgSubmitProposal{}, + gogo: &gov_v1_types.MsgSubmitProposal{}, + pulsar: &gov_v1_api.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), - SlashFractionDoubleSign: math.LegacyZeroDec(), - SlashFractionDowntime: math.LegacyZeroDec(), + DowntimeJailDuration: 1e9 + 7, + MinSignedPerWindow: math.LegacyNewDec(10), + }, + pulsar: &slashingapi.Params{ + DowntimeJailDuration: &durationpb.Duration{Seconds: 1, Nanos: 7}, + MinSignedPerWindow: dec10bz, }, }, -<<<<<<< HEAD "staking/create_validator": { gogo: &stakingtypes.MsgCreateValidator{Pubkey: pubkeyAny}, pulsar: &stakingapi.MsgCreateValidator{ @@ -311,115 +356,140 @@ 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{}, + gogo: &stakingtypes.MsgCancelUnbondingDelegationResponse{}, + pulsar: &stakingapi.MsgCancelUnbondingDelegationResponse{}, }, "staking/stake_authorization_empty": { - gogo: &stakingtypes.StakeAuthorization{}, + gogo: &stakingtypes.StakeAuthorization{}, + pulsar: &stakingapi.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, }, - }, - "staking/stake_authorization_deny": { - gogo: &stakingtypes.StakeAuthorization{ - MaxTokens: &types.Coin{Denom: "foo", Amount: math.NewInt(123)}, - Validators: &stakingtypes.StakeAuthorization_DenyList{ - DenyList: &stakingtypes.StakeAuthorization_Validators{}, + pulsar: &stakingapi.StakeAuthorization{ + Validators: &stakingapi.StakeAuthorization_AllowList{ + AllowList: &stakingapi.StakeAuthorization_Validators{Address: []string{"foo"}}, }, - 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{}}, + gogo: &vestingtypes.BaseVestingAccount{BaseAccount: &authtypes.BaseAccount{}}, + pulsar: &vestingapi.BaseVestingAccount{BaseAccount: &authapi.BaseAccount{}}, }, "vesting/base_account_pubkey": { - gogo: &vestingtypes.BaseVestingAccount{ - BaseAccount: &authtypes.BaseAccount{PubKey: pubkeyAny}, - }, + gogo: &vestingtypes.BaseVestingAccount{BaseAccount: &authtypes.BaseAccount{PubKey: pubkeyAny}}, + pulsar: &vestingapi.BaseVestingAccount{BaseAccount: &authapi.BaseAccount{PubKey: pubkeyAnyPulsar}}, }, "math/int_as_string": { - gogo: &gogo_testpb.IntAsString{IntAsString: math.NewInt(123)}, + gogo: &gogo_testpb.IntAsString{IntAsString: math.NewInt(123)}, + pulsar: &pulsar_testpb.IntAsString{IntAsString: "123"}, }, "math/int_as_string/empty": { - gogo: &gogo_testpb.IntAsString{}, + gogo: &gogo_testpb.IntAsString{}, + pulsar: &pulsar_testpb.IntAsString{}, }, "math/int_as_bytes": { - gogo: &gogo_testpb.IntAsBytes{IntAsBytes: math.NewInt(123)}, + gogo: &gogo_testpb.IntAsBytes{IntAsBytes: math.NewInt(123)}, + pulsar: &pulsar_testpb.IntAsBytes{IntAsBytes: int123bz}, }, "math/int_as_bytes/empty": { - gogo: &gogo_testpb.IntAsBytes{}, + gogo: &gogo_testpb.IntAsBytes{}, + pulsar: &pulsar_testpb.IntAsBytes{}, }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { - legacyBytes := fixture.MarshalLegacyAminoJSON(t, tc.gogo) - dynamicBytes, err := aj.Marshal(fixture.DynamicMessage(t, tc.gogo)) + 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 + } require.NoError(t, err) - t.Logf("legacy: %s\n", string(legacyBytes)) - t.Logf(" sut: %s\n", string(dynamicBytes)) - if tc.fails { - require.NotEqual(t, string(legacyBytes), string(dynamicBytes)) + newGogoBytes, err := encCfg.Amino.MarshalJSON(newGogo) + require.NoError(t, err) + if tc.roundTripUnequal { + require.NotEqual(t, string(gogoBytes), string(newGogoBytes)) return } - require.Equal(t, string(legacyBytes), string(dynamicBytes)) + require.Equal(t, string(gogoBytes), string(newGogoBytes)) // test amino json signer handler equivalence - if !proto.HasExtension(fixture.MessageDescriptor(t, tc.gogo).Options(), msgv1.E_Signer) { + msg, ok := tc.gogo.(legacytx.LegacyMsg) + if !ok { // not signable return } - fixture.RequireLegacyAminoEquivalent(t, tc.gogo) + + 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)) }) } } 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 e2814de77c69..2210c9c934fd 100644 --- a/tests/integration/tx/context_test.go +++ b/tests/integration/tx/context_test.go @@ -3,19 +3,29 @@ 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 ProvideCustomGetSigner() signing.CustomGetSigner { - return internal.TestRepeatedFieldsSigner +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 TestDefineCustomGetSigners(t *testing.T) { @@ -30,7 +40,7 @@ func TestDefineCustomGetSigners(t *testing.T) { configurator.ConsensusModule(), ), depinject.Supply(log.NewNopLogger()), - depinject.Provide(ProvideCustomGetSigner), + depinject.Provide(ProvideCustomGetSigners), ), &interfaceRegistry, ) diff --git a/tests/integration/tx/internal/util.go b/tests/integration/tx/internal/util.go deleted file mode 100644 index e3f6a5828997..000000000000 --- a/tests/integration/tx/internal/util.go +++ /dev/null @@ -1,221 +0,0 @@ -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/tx/CHANGELOG.md b/x/tx/CHANGELOG.md index 12d1b368e861..1d2d3f502622 100644 --- a/x/tx/CHANGELOG.md +++ b/x/tx/CHANGELOG.md @@ -33,12 +33,12 @@ Since v0.13.0, x/tx follows Cosmos SDK semver: https://github.com/cosmos/cosmos- ## [Unreleased] -<<<<<<< HEAD -======= +## [v0.13.6](https://github.com/cosmos/cosmos-sdk/releases/tag/x/tx/v0.13.6) - 2024-10-XX + +### Bug Fixes + * [#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 From ff0e4c8907e1d6929970dbc26ff45cd32f77c0ce Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Thu, 3 Oct 2024 13:53:05 +0200 Subject: [PATCH 3/7] conflicts --- x/auth/migrations/legacytx/stdtx_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/x/auth/migrations/legacytx/stdtx_test.go b/x/auth/migrations/legacytx/stdtx_test.go index be3295df19ca..f49357ca6398 100644 --- a/x/auth/migrations/legacytx/stdtx_test.go +++ b/x/auth/migrations/legacytx/stdtx_test.go @@ -44,11 +44,7 @@ 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 From ea53f32bfe443c13c69ab3771a683d92c005a314 Mon Sep 17 00:00:00 2001 From: Matt Kocubinski Date: Tue, 8 Oct 2024 04:21:25 -0500 Subject: [PATCH 4/7] fix(x/tx/amino): special case for string represented decimals (#22161) --- tests/go.mod | 1 + tests/go.sum | 2 -- .../tx/aminojson/aminojson_test.go | 5 ++++ x/tx/CHANGELOG.md | 1 + x/tx/signing/aminojson/encoder.go | 4 +-- x/tx/signing/aminojson/json_marshal.go | 8 +++--- x/tx/signing/aminojson/options.go | 26 ++++++++++++++++++- 7 files changed, 39 insertions(+), 8 deletions(-) diff --git a/tests/go.mod b/tests/go.mod index 58f871b12fa6..41c99ad3e895 100644 --- a/tests/go.mod +++ b/tests/go.mod @@ -202,6 +202,7 @@ require ( // replace ( // // ) +replace cosmossdk.io/x/tx => ../x/tx // Below are the long-lived replace for tests. replace ( diff --git a/tests/go.sum b/tests/go.sum index 079779a509ed..6062dc6ad7e2 100644 --- a/tests/go.sum +++ b/tests/go.sum @@ -208,8 +208,6 @@ cosmossdk.io/x/feegrant v0.1.1 h1:EKFWOeo/pup0yF0svDisWWKAA9Zags6Zd0P3nRvVvw8= cosmossdk.io/x/feegrant v0.1.1/go.mod h1:2GjVVxX6G2fta8LWj7pC/ytHjryA6MHAJroBWHFNiEQ= cosmossdk.io/x/nft v0.1.1 h1:pslAVS8P5NkW080+LWOamInjDcq+v2GSCo+BjN9sxZ8= cosmossdk.io/x/nft v0.1.1/go.mod h1:Kac6F6y2gsKvoxU+fy8uvxRTi4BIhLOor2zgCNQwVgY= -cosmossdk.io/x/tx v0.13.6-0.20241003112805-ff8789a02871 h1:+lRwWQRVvB3jgRgdqrgeFUJ45BoXZh/UeeAV5f/m2Gk= -cosmossdk.io/x/tx v0.13.6-0.20241003112805-ff8789a02871/go.mod h1:V6DImnwJMTq5qFjeGWpXNiT/fjgE4HtmclRmTqRVM3w= cosmossdk.io/x/upgrade v0.1.4 h1:/BWJim24QHoXde8Bc64/2BSEB6W4eTydq0X/2f8+g38= cosmossdk.io/x/upgrade v0.1.4/go.mod h1:9v0Aj+fs97O+Ztw+tG3/tp5JSlrmT7IcFhAebQHmOPo= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= diff --git a/tests/integration/tx/aminojson/aminojson_test.go b/tests/integration/tx/aminojson/aminojson_test.go index eb271308f8d6..ec834df31b47 100644 --- a/tests/integration/tx/aminojson/aminojson_test.go +++ b/tests/integration/tx/aminojson/aminojson_test.go @@ -339,6 +339,11 @@ func TestAminoJSON_LegacyParity(t *testing.T) { // 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() + "gov/v1_params": { + gogo: &gov_v1_types.Params{ + Quorum: math.LegacyMustNewDecFromStr("0.33").String(), + }, + }, "slashing/params/dec": { gogo: &slashingtypes.Params{ DowntimeJailDuration: 1e9 + 7, diff --git a/x/tx/CHANGELOG.md b/x/tx/CHANGELOG.md index f24da2178b93..201e1a07b488 100644 --- a/x/tx/CHANGELOG.md +++ b/x/tx/CHANGELOG.md @@ -37,6 +37,7 @@ Since v0.13.0, x/tx follows Cosmos SDK semver: https://github.com/cosmos/cosmos- ### Bug Fixes +* [#22161](https://github.com/cosmos/cosmos-sdk/pull/22161) Add special case for string represented decimals. * [#21825](https://github.com/cosmos/cosmos-sdk/pull/21825) Fix decimal encoding and field ordering in Amino JSON encoder. * [#21782](https://github.com/cosmos/cosmos-sdk/pull/21782) Fix JSON attribute sort order on messages with oneof fields. diff --git a/x/tx/signing/aminojson/encoder.go b/x/tx/signing/aminojson/encoder.go index e954c2651e30..6c26bdb7ed99 100644 --- a/x/tx/signing/aminojson/encoder.go +++ b/x/tx/signing/aminojson/encoder.go @@ -43,7 +43,7 @@ func cosmosIntEncoder(_ *Encoder, v protoreflect.Value, w io.Writer) error { } } -// cosmosDecEncoder provides legacy compatible encoding for cosmos.Dec and cosmos.Int types. These are sometimes +// cosmosDecEncoder provides legacy compatible encoding for cosmos.Dec types. These are sometimes // represented as strings in pulsar messages and sometimes as bytes. This encoder handles both cases. func cosmosDecEncoder(_ *Encoder, v protoreflect.Value, w io.Writer) error { switch val := v.Interface().(type) { @@ -54,7 +54,7 @@ func cosmosDecEncoder(_ *Encoder, v protoreflect.Value, w io.Writer) error { var dec math.LegacyDec err := dec.Unmarshal([]byte(val)) if err != nil { - return err + return fmt.Errorf("failed to unmarshal for Amino JSON encoding; string %q into Dec: %w", val, err) } return jsonMarshal(w, dec.String()) case []byte: diff --git a/x/tx/signing/aminojson/json_marshal.go b/x/tx/signing/aminojson/json_marshal.go index f53e351b6ea6..1e5d1b59af81 100644 --- a/x/tx/signing/aminojson/json_marshal.go +++ b/x/tx/signing/aminojson/json_marshal.go @@ -16,6 +16,8 @@ import ( "cosmossdk.io/x/tx/signing" ) +const cosmosDecType = "cosmos.Dec" + // MessageEncoder is a function that can encode a protobuf protoreflect.Message to JSON. type MessageEncoder func(*Encoder, protoreflect.Message, io.Writer) error @@ -68,8 +70,8 @@ func NewEncoder(options EncoderOptions) Encoder { } enc := Encoder{ cosmosProtoScalarEncoders: map[string]FieldEncoder{ - "cosmos.Dec": cosmosDecEncoder, - "cosmos.Int": cosmosIntEncoder, + cosmosDecType: cosmosDecEncoder, + "cosmos.Int": cosmosIntEncoder, }, aminoMessageEncoders: map[string]MessageEncoder{ "key_field": keyFieldEncoder, @@ -387,7 +389,7 @@ func (enc Encoder) marshalMessage(msg protoreflect.Message, writer io.Writer) er } // encode value - if encoder := enc.getFieldEncoding(f); encoder != nil { + if encoder := enc.getFieldEncoder(f); encoder != nil { err = encoder(&enc, v, writer) if err != nil { return err diff --git a/x/tx/signing/aminojson/options.go b/x/tx/signing/aminojson/options.go index cf9110aef3ae..c5c9462ef106 100644 --- a/x/tx/signing/aminojson/options.go +++ b/x/tx/signing/aminojson/options.go @@ -2,11 +2,14 @@ package aminojson import ( cosmos_proto "github.com/cosmos/cosmos-proto" + gogo "github.com/cosmos/gogoproto/gogoproto" gogoproto "github.com/cosmos/gogoproto/proto" "github.com/iancoleman/strcase" "github.com/pkg/errors" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" + "google.golang.org/protobuf/runtime/protoimpl" + "google.golang.org/protobuf/types/descriptorpb" "cosmossdk.io/api/amino" ) @@ -100,7 +103,16 @@ func (enc Encoder) getMessageEncoder(message protoreflect.Message) MessageEncode return nil } -func (enc Encoder) getFieldEncoding(field protoreflect.FieldDescriptor) FieldEncoder { +var customTypeExtension = protoimpl.ExtensionInfo{ + ExtendedType: (*descriptorpb.FieldOptions)(nil), + ExtensionType: gogo.E_Customtype.ExtensionType, + Field: gogo.E_Customtype.Field, + Name: gogo.E_Customtype.Name, + Tag: gogo.E_Customtype.Tag, + Filename: gogo.E_Customtype.Filename, +} + +func (enc Encoder) getFieldEncoder(field protoreflect.FieldDescriptor) FieldEncoder { opts := field.Options() if proto.HasExtension(opts, amino.E_Encoding) { encoding := proto.GetExtension(opts, amino.E_Encoding).(string) @@ -110,6 +122,18 @@ func (enc Encoder) getFieldEncoding(field protoreflect.FieldDescriptor) FieldEnc } if proto.HasExtension(opts, cosmos_proto.E_Scalar) { scalar := proto.GetExtension(opts, cosmos_proto.E_Scalar).(string) + // do not handle encoding of fields tagged only with scalar which are not backed by a + // LegacyDec custom type. This types are handled by the default encoding, as they are + // expected to already be encoded as their human readable string representation + // containing a radix, i.e. "1.2345". + // For example: + // https://github.com/cosmos/cosmos-sdk/blob/9076487d035e43d39fe54e8498da1ce31b9c845c/x/gov/proto/cosmos/gov/v1/gov.proto#L274 + if scalar == cosmosDecType { + customType := proto.GetExtension(opts, &customTypeExtension) + if customType != "cosmossdk.io/math.LegacyDec" { + return nil + } + } if fn, ok := enc.cosmosProtoScalarEncoders[scalar]; ok { return fn } From 565b711705e84d4eea1f77a34de8b21716db701c Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Thu, 14 Nov 2024 10:43:31 +0100 Subject: [PATCH 5/7] updates --- tests/integration/tx/aminojson/aminojson_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/integration/tx/aminojson/aminojson_test.go b/tests/integration/tx/aminojson/aminojson_test.go index ec834df31b47..44c2b670236a 100644 --- a/tests/integration/tx/aminojson/aminojson_test.go +++ b/tests/integration/tx/aminojson/aminojson_test.go @@ -343,6 +343,9 @@ func TestAminoJSON_LegacyParity(t *testing.T) { gogo: &gov_v1_types.Params{ Quorum: math.LegacyMustNewDecFromStr("0.33").String(), }, + pulsar: &gov_v1_api.Params{ + Quorum: "0.33", + }, }, "slashing/params/dec": { gogo: &slashingtypes.Params{ From 675e57b66f5945b9a12ccb3eb1b35b3d68adcb3b Mon Sep 17 00:00:00 2001 From: Matt Kocubinski Date: Thu, 14 Nov 2024 16:23:01 -0600 Subject: [PATCH 6/7] add registration --- x/tx/signing/aminojson/options.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/x/tx/signing/aminojson/options.go b/x/tx/signing/aminojson/options.go index c5c9462ef106..9a87cd56726b 100644 --- a/x/tx/signing/aminojson/options.go +++ b/x/tx/signing/aminojson/options.go @@ -8,6 +8,7 @@ import ( "github.com/pkg/errors" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" + "google.golang.org/protobuf/reflect/protoregistry" "google.golang.org/protobuf/runtime/protoimpl" "google.golang.org/protobuf/types/descriptorpb" @@ -103,7 +104,7 @@ func (enc Encoder) getMessageEncoder(message protoreflect.Message) MessageEncode return nil } -var customTypeExtension = protoimpl.ExtensionInfo{ +var customTypeExtension = &protoimpl.ExtensionInfo{ ExtendedType: (*descriptorpb.FieldOptions)(nil), ExtensionType: gogo.E_Customtype.ExtensionType, Field: gogo.E_Customtype.Field, @@ -112,6 +113,10 @@ var customTypeExtension = protoimpl.ExtensionInfo{ Filename: gogo.E_Customtype.Filename, } +func init() { + protoregistry.GlobalTypes.RegisterExtension(customTypeExtension) +} + func (enc Encoder) getFieldEncoder(field protoreflect.FieldDescriptor) FieldEncoder { opts := field.Options() if proto.HasExtension(opts, amino.E_Encoding) { @@ -129,7 +134,7 @@ func (enc Encoder) getFieldEncoder(field protoreflect.FieldDescriptor) FieldEnco // For example: // https://github.com/cosmos/cosmos-sdk/blob/9076487d035e43d39fe54e8498da1ce31b9c845c/x/gov/proto/cosmos/gov/v1/gov.proto#L274 if scalar == cosmosDecType { - customType := proto.GetExtension(opts, &customTypeExtension) + customType := proto.GetExtension(opts, customTypeExtension) if customType != "cosmossdk.io/math.LegacyDec" { return nil } From bcfea6b02f1996618e7a8d1867c8f9a883f89c19 Mon Sep 17 00:00:00 2001 From: Matt Kocubinski Date: Mon, 18 Nov 2024 20:23:22 -0500 Subject: [PATCH 7/7] fix test --- tests/integration/tx/aminojson/aminojson_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/tx/aminojson/aminojson_test.go b/tests/integration/tx/aminojson/aminojson_test.go index 44c2b670236a..ca245d6c4fb5 100644 --- a/tests/integration/tx/aminojson/aminojson_test.go +++ b/tests/integration/tx/aminojson/aminojson_test.go @@ -344,7 +344,7 @@ func TestAminoJSON_LegacyParity(t *testing.T) { Quorum: math.LegacyMustNewDecFromStr("0.33").String(), }, pulsar: &gov_v1_api.Params{ - Quorum: "0.33", + Quorum: math.LegacyMustNewDecFromStr("0.33").String(), }, }, "slashing/params/dec": {