fix: marshal old tx body if unordered not used (#22723)

cosmos · Jan 13, 2025 · 9ca815c · 9ca815c
1 parent 064c9ba
commit 9ca815c
Show file tree

Hide file tree

Showing 13 changed files with 1,776 additions and 303 deletions.
diff --git a/api/cosmos/tx/v1beta1/tx.pulsar.go b/api/cosmos/tx/v1beta1/tx.pulsar.go
diff --git a/baseapp/abci_utils_test.go b/baseapp/abci_utils_test.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"sort"
 	"testing"
+	"time"
 
 	abci "github.com/cometbft/cometbft/api/cometbft/abci/v1"
 	cmtprotocrypto "github.com/cometbft/cometbft/api/cometbft/crypto/v1"
@@ -499,10 +500,10 @@ func (s *ABCIUtilsTestSuite) TestDefaultProposalHandler_NoOpMempoolTxSelection()
 	tx := builder.GetTx()
 	txBz, err := txConfig.TxEncoder()(tx)
 	s.Require().NoError(err)
-	s.Require().Len(txBz, 165)
+	s.Require().Len(txBz, 152)
 
 	txDataSize := int(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{txBz}))
-	s.Require().Equal(txDataSize, 168)
+	s.Require().Equal(155, txDataSize)
 
 	testCases := map[string]struct {
 		ctx         sdk.Context
@@ -533,7 +534,7 @@ func (s *ABCIUtilsTestSuite) TestDefaultProposalHandler_NoOpMempoolTxSelection()
 			ctx: s.ctx,
 			req: &abci.PrepareProposalRequest{
 				Txs:        [][]byte{txBz, txBz, txBz, txBz, txBz},
-				MaxTxBytes: 465,
+				MaxTxBytes: 464,
 			},
 			expectedTxs: 2,
 		},
@@ -595,24 +596,24 @@ func (s *ABCIUtilsTestSuite) TestDefaultProposalHandler_PriorityNonceMempoolTxSe
 
 	testTxs := []testTx{
 		// test 1
-		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`0`), [][]byte{secret1}, []uint64{1}), priority: 10},
-		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`12345678910`), [][]byte{secret1}, []uint64{2}), priority: 10},
-		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`22`), [][]byte{secret1}, []uint64{3}), priority: 10},
-		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`32`), [][]byte{secret2}, []uint64{1}), priority: 8},
+		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`0`), [][]byte{secret1}, []uint64{1}, false), priority: 10},
+		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`12345678910`), [][]byte{secret1}, []uint64{2}, false), priority: 10},
+		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`22`), [][]byte{secret1}, []uint64{3}, false), priority: 10},
+		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`32`), [][]byte{secret2}, []uint64{1}, false), priority: 8},
 		// test 2
-		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`4`), [][]byte{secret1, secret2}, []uint64{3, 3}), priority: 10},
-		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`52345678910`), [][]byte{secret1, secret3}, []uint64{4, 3}), priority: 10},
-		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`62`), [][]byte{secret1, secret4}, []uint64{5, 3}), priority: 8},
-		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`72`), [][]byte{secret3, secret5}, []uint64{4, 3}), priority: 8},
-		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`82`), [][]byte{secret2, secret6}, []uint64{4, 3}), priority: 8},
+		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`4`), [][]byte{secret1, secret2}, []uint64{3, 3}, false), priority: 10},
+		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`52345678910`), [][]byte{secret1, secret3}, []uint64{4, 3}, false), priority: 10},
+		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`62`), [][]byte{secret1, secret4}, []uint64{5, 3}, false), priority: 8},
+		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`72`), [][]byte{secret3, secret5}, []uint64{4, 3}, false), priority: 8},
+		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`82`), [][]byte{secret2, secret6}, []uint64{4, 3}, false), priority: 8},
 		// test 3
-		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`9`), [][]byte{secret3, secret4}, []uint64{3, 3}), priority: 10},
-		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`1052345678910`), [][]byte{secret1, secret2}, []uint64{4, 4}), priority: 8},
-		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`11`), [][]byte{secret1, secret2}, []uint64{5, 5}), priority: 8},
+		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`9`), [][]byte{secret3, secret4}, []uint64{3, 3}, false), priority: 10},
+		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`1052345678910`), [][]byte{secret1, secret2}, []uint64{4, 4}, false), priority: 8},
+		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`11`), [][]byte{secret1, secret2}, []uint64{5, 5}, false), priority: 8},
 		// test 4
-		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`1252345678910`), [][]byte{secret1}, []uint64{3}), priority: 10},
-		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`13`), [][]byte{secret1}, []uint64{5}), priority: 10},
-		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`14`), [][]byte{secret1}, []uint64{6}), priority: 8},
+		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`1252345678910`), [][]byte{secret1}, []uint64{3}, false), priority: 10},
+		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`13`), [][]byte{secret1}, []uint64{5}, false), priority: 10},
+		{tx: buildMsg(s.T(), txConfig, signingCtx.AddressCodec(), []byte(`14`), [][]byte{secret1}, []uint64{6}, false), priority: 8},
 	}
 
 	for i := range testTxs {
@@ -622,15 +623,15 @@ func (s *ABCIUtilsTestSuite) TestDefaultProposalHandler_PriorityNonceMempoolTxSe
 		testTxs[i].size = int(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{bz}))
 	}
 
-	s.Require().Equal(193, testTxs[0].size)
-	s.Require().Equal(203, testTxs[1].size)
-	s.Require().Equal(194, testTxs[2].size)
-	s.Require().Equal(194, testTxs[3].size)
-	s.Require().Equal(276, testTxs[4].size)
-	s.Require().Equal(286, testTxs[5].size)
-	s.Require().Equal(277, testTxs[6].size)
-	s.Require().Equal(277, testTxs[7].size)
-	s.Require().Equal(277, testTxs[8].size)
+	s.Require().Equal(180, testTxs[0].size)
+	s.Require().Equal(190, testTxs[1].size)
+	s.Require().Equal(181, testTxs[2].size)
+	s.Require().Equal(181, testTxs[3].size)
+	s.Require().Equal(263, testTxs[4].size)
+	s.Require().Equal(273, testTxs[5].size)
+	s.Require().Equal(264, testTxs[6].size)
+	s.Require().Equal(264, testTxs[7].size)
+	s.Require().Equal(264, testTxs[8].size)
 
 	testCases := map[string]struct {
 		ctx         sdk.Context
@@ -643,15 +644,15 @@ func (s *ABCIUtilsTestSuite) TestDefaultProposalHandler_PriorityNonceMempoolTxSe
 			ctx:      s.ctx,
 			txInputs: []testTx{testTxs[0], testTxs[1], testTxs[2], testTxs[3]},
 			req: &abci.PrepareProposalRequest{
-				MaxTxBytes: 193 + 194,
+				MaxTxBytes: 180 + 181,
 			},
 			expectedTxs: []int{0, 3},
 		},
 		"skip multi-signers msg non-sequential sequence": {
 			ctx:      s.ctx,
 			txInputs: []testTx{testTxs[4], testTxs[5], testTxs[6], testTxs[7], testTxs[8]},
 			req: &abci.PrepareProposalRequest{
-				MaxTxBytes: 276 + 277,
+				MaxTxBytes: 263 + 264,
 			},
 			expectedTxs: []int{4, 8},
 		},
@@ -660,7 +661,7 @@ func (s *ABCIUtilsTestSuite) TestDefaultProposalHandler_PriorityNonceMempoolTxSe
 			ctx:      s.ctx,
 			txInputs: []testTx{testTxs[9], testTxs[10], testTxs[11]},
 			req: &abci.PrepareProposalRequest{
-				MaxTxBytes: 276 + 277,
+				MaxTxBytes: 263 + 264,
 			},
 			expectedTxs: []int{9},
 		},
@@ -722,7 +723,7 @@ func marshalDelimitedFn(msg proto.Message) ([]byte, error) {
 	return buf.Bytes(), nil
 }
 
-func buildMsg(t *testing.T, txConfig client.TxConfig, ac address.Codec, value []byte, secrets [][]byte, nonces []uint64) sdk.Tx {
+func buildMsg(t *testing.T, txConfig client.TxConfig, ac address.Codec, value []byte, secrets [][]byte, nonces []uint64, unordered bool) sdk.Tx {
 	t.Helper()
 	builder := txConfig.NewTxBuilder()
 
@@ -742,6 +743,11 @@ func buildMsg(t *testing.T, txConfig client.TxConfig, ac address.Codec, value []
 	addr, err := ac.BytesToString(signatures[0].PubKey.Bytes())
 	require.NoError(t, err)
 
+	builder.SetUnordered(unordered)
+	if unordered {
+		builder.SetTimeoutTimestamp(time.Now().Add(time.Hour))
+	}
+
 	_ = builder.SetMsgs(
 		&baseapptestutil.MsgKeyValue{
 			Signer: addr,

diff --git a/codec/proto_codec.go b/codec/proto_codec.go
@@ -21,13 +21,6 @@ import (
 	"github.com/cosmos/cosmos-sdk/codec/types"
 )
 
-// ProtoCodecMarshaler defines an interface for codecs that utilize Protobuf for both
-// binary and JSON encoding.
-// Deprecated: Use Codec instead.
-type ProtoCodecMarshaler interface {
-	Codec
-}
-
 // ProtoCodec defines a codec that utilizes Protobuf for both binary and JSON
 // encoding.
 type ProtoCodec struct {

diff --git a/go.mod b/go.mod
@@ -176,6 +176,7 @@ require (
 
 // TODO remove after all modules have their own go.mods
 replace (
+	cosmossdk.io/api => ./api
 	cosmossdk.io/x/bank => ./x/bank
 	cosmossdk.io/x/staking => ./x/staking
 )

diff --git a/go.sum b/go.sum
@@ -4,8 +4,6 @@ buf.build/gen/go/cosmos/gogo-proto/protocolbuffers/go v1.36.1-20240130113600-88e
 buf.build/gen/go/cosmos/gogo-proto/protocolbuffers/go v1.36.1-20240130113600-88ef6483f90f.1/go.mod h1:GB5hdNJd6cahKmHcLArJo5wnV9TeZGMSz7ysK4YLvag=
 cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
 cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
-cosmossdk.io/api v0.8.0 h1:E5Xifxu/3mPTLF79si9fyq4rR0wagubeVNmOz5duTUo=
-cosmossdk.io/api v0.8.0/go.mod h1:hgJ83P0ZUu0rS1SZoVM6abk6ADOkiM259BVVlYtAPP0=
 cosmossdk.io/collections v1.0.0 h1:YCYIe/pIMtc1iLDD0OrVdfWCnIkpwdy7k9NSQpaR5mg=
 cosmossdk.io/collections v1.0.0/go.mod h1:mFfLxnYT1fV+B3Lx9GLap1qxmffIPqQCND4xBExerps=
 cosmossdk.io/core v1.0.0 h1:e7XBbISOytLBOXMVwpRPixThXqEkeLGlg8no/qpgS8U=

diff --git a/proto/cosmos/tx/v1beta1/tx.proto b/proto/cosmos/tx/v1beta1/tx.proto
@@ -143,6 +143,41 @@ message TxBody {
   repeated google.protobuf.Any non_critical_extension_options = 2047;
 }
 
+// TxBodyCompat is the body of a transaction that all signers sign over.
+// This message is used to represent the transaction body before the unordered
+// features was added. It is used to support backwards compatibility when user a newer
+// client with an older chain.
+message TxBodyCompat {
+  // messages is a list of messages to be executed. The required signers of
+  // those messages define the number and order of elements in AuthInfo's
+  // signer_infos and Tx's signatures. Each required signer address is added to
+  // the list only the first time it occurs.
+  // By convention, the first required signer (usually from the first message)
+  // is referred to as the primary signer and pays the fee for the whole
+  // transaction.
+  repeated google.protobuf.Any messages = 1;
+
+  // memo is any arbitrary note/comment to be added to the transaction.
+  // WARNING: in clients, any publicly exposed text should not be called memo,
+  // but should be called `note` instead (see
+  // https://github.com/cosmos/cosmos-sdk/issues/9122).
+  string memo = 2;
+
+  // timeout_height is the block height after which this transaction will not
+  // be processed by the chain.
+  uint64 timeout_height = 3;
+
+  // extension_options are arbitrary options that can be added by chains
+  // when the default options are not sufficient. If any of these are present
+  // and can't be handled, the transaction will be rejected
+  repeated google.protobuf.Any extension_options = 1023;
+
+  // extension_options are arbitrary options that can be added by chains
+  // when the default options are not sufficient. If any of these are present
+  // and can't be handled, they will be ignored
+  repeated google.protobuf.Any non_critical_extension_options = 2047;
+}
+
 // AuthInfo describes the fee and signer modes that are used to sign a
 // transaction.
 message AuthInfo {

diff --git a/simapp/v2/go.mod b/simapp/v2/go.mod
@@ -257,6 +257,7 @@ require (
 // replace (
 // 	<temporary replace>
 // )
+replace cosmossdk.io/api => ../../api
 
 // SimApp on main always tests the latest extracted SDK modules importing the sdk
 replace (

diff --git a/simapp/v2/go.sum b/simapp/v2/go.sum
@@ -192,8 +192,6 @@ cloud.google.com/go/webrisk v1.4.0/go.mod h1:Hn8X6Zr+ziE2aNd8SliSDWpEnSS1u4R9+xX
 cloud.google.com/go/webrisk v1.5.0/go.mod h1:iPG6fr52Tv7sGk0H6qUFzmL3HHZev1htXuWDEEsqMTg=
 cloud.google.com/go/workflows v1.6.0/go.mod h1:6t9F5h/unJz41YqfBmqSASJSXccBLtD1Vwf+KmJENM0=
 cloud.google.com/go/workflows v1.7.0/go.mod h1:JhSrZuVZWuiDfKEFxU0/F1PQjmpnpcoISEXH2bcHC3M=
-cosmossdk.io/api v0.8.0 h1:E5Xifxu/3mPTLF79si9fyq4rR0wagubeVNmOz5duTUo=
-cosmossdk.io/api v0.8.0/go.mod h1:hgJ83P0ZUu0rS1SZoVM6abk6ADOkiM259BVVlYtAPP0=
 cosmossdk.io/collections v1.0.0 h1:YCYIe/pIMtc1iLDD0OrVdfWCnIkpwdy7k9NSQpaR5mg=
 cosmossdk.io/collections v1.0.0/go.mod h1:mFfLxnYT1fV+B3Lx9GLap1qxmffIPqQCND4xBExerps=
 cosmossdk.io/core v1.0.0 h1:e7XBbISOytLBOXMVwpRPixThXqEkeLGlg8no/qpgS8U=

diff --git a/tests/go.mod b/tests/go.mod
@@ -235,6 +235,7 @@ require (
 // replace (
 // 	<temporary replace>
 // )
+replace cosmossdk.io/api => ../api
 
 // SimApp on main always tests the latest extracted SDK modules importing the sdk
 replace (

diff --git a/tests/go.sum b/tests/go.sum
@@ -192,8 +192,6 @@ cloud.google.com/go/webrisk v1.4.0/go.mod h1:Hn8X6Zr+ziE2aNd8SliSDWpEnSS1u4R9+xX
 cloud.google.com/go/webrisk v1.5.0/go.mod h1:iPG6fr52Tv7sGk0H6qUFzmL3HHZev1htXuWDEEsqMTg=
 cloud.google.com/go/workflows v1.6.0/go.mod h1:6t9F5h/unJz41YqfBmqSASJSXccBLtD1Vwf+KmJENM0=
 cloud.google.com/go/workflows v1.7.0/go.mod h1:JhSrZuVZWuiDfKEFxU0/F1PQjmpnpcoISEXH2bcHC3M=
-cosmossdk.io/api v0.8.0 h1:E5Xifxu/3mPTLF79si9fyq4rR0wagubeVNmOz5duTUo=
-cosmossdk.io/api v0.8.0/go.mod h1:hgJ83P0ZUu0rS1SZoVM6abk6ADOkiM259BVVlYtAPP0=
 cosmossdk.io/collections v1.0.0 h1:YCYIe/pIMtc1iLDD0OrVdfWCnIkpwdy7k9NSQpaR5mg=
 cosmossdk.io/collections v1.0.0/go.mod h1:mFfLxnYT1fV+B3Lx9GLap1qxmffIPqQCND4xBExerps=
 cosmossdk.io/core v1.0.0 h1:e7XBbISOytLBOXMVwpRPixThXqEkeLGlg8no/qpgS8U=

diff --git a/tests/integration/baseapp/block_gas_test.go b/tests/integration/baseapp/block_gas_test.go
@@ -172,7 +172,7 @@ func TestBaseApp_BlockGas(t *testing.T) {
 				require.Equal(t, []byte("ok"), okValue)
 			}
 			// check block gas is always consumed
-			baseGas := uint64(39205) // baseGas is the gas consumed before tx msg
+			baseGas := uint64(39075) // baseGas is the gas consumed before tx msg
 			expGasConsumed := addUint64Saturating(tc.gasToConsume, baseGas)
 			if expGasConsumed > uint64(simtestutil.DefaultConsensusParams.Block.MaxGas) {
 				// capped by gasLimit