Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 10 commits into from
Jan 13, 2025
+1,776 −303
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
196a748
fix(client): don't marshal zero-ed fields
julienrbrt Dec 17, 2024
83b8f53
trial before heavy changes
julienrbrt Dec 17, 2024
c73aa5b
Merge branch 'main' into julien/loosen-codec
julienrbrt Jan 6, 2025
6016344
Merge branch 'main' into julien/loosen-codec
julienrbrt Jan 7, 2025
377da5f
lint
julienrbrt Jan 7, 2025
db9f6fa
updates
julienrbrt Jan 10, 2025
f09c2f7
Merge branch 'main' into julien/loosen-codec
julienrbrt Jan 10, 2025
16fec4a
rename + fix
julienrbrt Jan 10, 2025
3b379ef
Merge branch 'main' into julien/loosen-codec
julienrbrt Jan 10, 2025
fcf1ef7
tx bytes/gas change because tx body changes
julienrbrt Jan 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1,348 changes: 1,179 additions & 169 deletions api/cosmos/tx/v1beta1/tx.pulsar.go
View file
Open in desktop

Large diffs are not rendered by default.

68 changes: 37 additions & 31 deletions baseapp/abci_utils_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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},
},
Expand All @@ -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},
},
Expand Down Expand Up @@ -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()

Expand All @@ -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,
Expand Down
7 changes: 0 additions & 7 deletions codec/proto_codec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down
2 changes: 0 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
35 changes: 35 additions & 0 deletions proto/cosmos/tx/v1beta1/tx.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions simapp/v2/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ require (
// replace (
// <temporary replace>
// )
replace cosmossdk.io/api => ../../api
Copy link
Member Author

Choose a reason for hiding this comment

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

I'll have a follow-up to tag v8.0.1 and and bump it here and in the SDK.


// SimApp on main always tests the latest extracted SDK modules importing the sdk
replace (
Expand Down
2 changes: 0 additions & 2 deletions simapp/v2/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
1 change: 1 addition & 0 deletions tests/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
2 changes: 0 additions & 2 deletions tests/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/baseapp/block_gas_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading