fix: do not allow unordered txs to have sequence values set (#24581)
Co-authored-by: Alex | Interchain Labs <alex@interchainlabs.io> Co-authored-by: Aaron Craelius <aaronc@users.noreply.github.com>
This commit is contained in:
co-authored by
Alex | Interchain Labs
Aaron Craelius
parent
a158c24b38
commit
f9f3bfb066
@@ -0,0 +1,26 @@
|
||||
package mempool
|
||||
|
||||
import (
|
||||
"errors"
|
||||
|
||||
sdk "github.com/cosmos/cosmos-sdk/types"
|
||||
)
|
||||
|
||||
// ChooseNonce gets the nonce from a transaction. If the transaction is unordered,
|
||||
// it uses the timeout timestamp as the nonce. Sequence values must be zero in this case.
|
||||
// If the transaction is ordered, it uses the sequence number as the nonce.
|
||||
func ChooseNonce(seq uint64, tx sdk.Tx) (uint64, error) {
|
||||
// if it's an unordered tx, we use the timeout timestamp instead of the nonce
|
||||
if unordered, ok := tx.(sdk.TxWithUnordered); ok && unordered.GetUnordered() {
|
||||
if seq > 0 {
|
||||
return 0, errors.New("unordered txs must not have sequence set")
|
||||
}
|
||||
timestamp := unordered.GetTimeoutTimeStamp().UnixNano()
|
||||
if timestamp < 0 {
|
||||
return 0, errors.New("invalid timestamp value")
|
||||
}
|
||||
return uint64(timestamp), nil
|
||||
}
|
||||
// otherwise, use the sequence as normal.
|
||||
return seq, nil
|
||||
}
|
||||
@@ -0,0 +1,58 @@
|
||||
package mempool_test
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/cosmos/cosmos-sdk/types/mempool"
|
||||
)
|
||||
|
||||
func TestChooseNonce(t *testing.T) {
|
||||
testCases := []struct {
|
||||
name string
|
||||
seq uint64
|
||||
unordered bool
|
||||
timeout time.Time
|
||||
expErr string
|
||||
expNonce int64
|
||||
}{
|
||||
{
|
||||
name: "unordered nonce chosen",
|
||||
unordered: true,
|
||||
timeout: time.Unix(100, 15),
|
||||
expNonce: time.Unix(100, 15).UnixNano(),
|
||||
},
|
||||
{
|
||||
name: "sequence chosen",
|
||||
seq: 15,
|
||||
expNonce: 15,
|
||||
},
|
||||
{
|
||||
name: "timeout invalid",
|
||||
unordered: true,
|
||||
timeout: time.Time{},
|
||||
expErr: "invalid timestamp value",
|
||||
},
|
||||
{
|
||||
name: "invalid if sequence and unordered set",
|
||||
unordered: true,
|
||||
seq: 15,
|
||||
expErr: "unordered txs must not have sequence set",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
tx := testTx{unordered: tc.unordered, nonce: tc.seq, timeout: &tc.timeout}
|
||||
nonce, err := mempool.ChooseNonce(tc.seq, tx)
|
||||
if tc.expErr != "" {
|
||||
require.ErrorContains(t, err, tc.expErr)
|
||||
} else {
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, nonce, uint64(tc.expNonce))
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -2,7 +2,6 @@ package mempool
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"math"
|
||||
"sync"
|
||||
@@ -222,15 +221,9 @@ func (mp *PriorityNonceMempool[C]) Insert(ctx context.Context, tx sdk.Tx) error
|
||||
sig := sigs[0]
|
||||
sender := sig.Signer.String()
|
||||
priority := mp.cfg.TxPriority.GetTxPriority(ctx, tx)
|
||||
nonce := sig.Sequence
|
||||
|
||||
// if it's an unordered tx, we use the timeout timestamp instead of the nonce
|
||||
if unordered, ok := tx.(sdk.TxWithUnordered); ok && unordered.GetUnordered() {
|
||||
timestamp := unordered.GetTimeoutTimeStamp().UnixNano()
|
||||
if timestamp < 0 {
|
||||
return errors.New("invalid timestamp value")
|
||||
}
|
||||
nonce = uint64(timestamp)
|
||||
nonce, err := ChooseNonce(sig.Sequence, tx)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
key := txMeta[C]{nonce: nonce, priority: priority, sender: sender}
|
||||
@@ -467,15 +460,9 @@ func (mp *PriorityNonceMempool[C]) Remove(tx sdk.Tx) error {
|
||||
|
||||
sig := sigs[0]
|
||||
sender := sig.Signer.String()
|
||||
nonce := sig.Sequence
|
||||
|
||||
// if it's an unordered tx, we use the timeout timestamp instead of the nonce
|
||||
if unordered, ok := tx.(sdk.TxWithUnordered); ok && unordered.GetUnordered() {
|
||||
timestamp := unordered.GetTimeoutTimeStamp().UnixNano()
|
||||
if timestamp < 0 {
|
||||
return errors.New("invalid timestamp value")
|
||||
}
|
||||
nonce = uint64(timestamp)
|
||||
nonce, err := ChooseNonce(sig.Sequence, tx)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
scoreKey := txMeta[C]{nonce: nonce, sender: sender}
|
||||
|
||||
@@ -973,6 +973,14 @@ func TestNextSenderTx_TxReplacement(t *testing.T) {
|
||||
require.Equal(t, txs[3], iter.Tx())
|
||||
}
|
||||
|
||||
func TestPriorityNonceMempool_UnorderedTx_FailsForSequence(t *testing.T) {
|
||||
mp := mempool.DefaultPriorityMempool()
|
||||
accounts := simtypes.RandomAccounts(rand.New(rand.NewSource(0)), 1)
|
||||
tx := testTx{id: 1, priority: 0, address: accounts[0].Address, nonce: 1, unordered: true}
|
||||
err := mp.Insert(sdk.NewContext(nil, cmtproto.Header{}, false, log.NewNopLogger()), tx)
|
||||
require.ErrorContains(t, err, "unordered txs must not have sequence set")
|
||||
}
|
||||
|
||||
func TestPriorityNonceMempool_UnorderedTx(t *testing.T) {
|
||||
ctx := sdk.NewContext(nil, cmtproto.Header{}, false, log.NewNopLogger())
|
||||
accounts := simtypes.RandomAccounts(rand.New(rand.NewSource(0)), 2)
|
||||
|
||||
@@ -4,7 +4,6 @@ import (
|
||||
"context"
|
||||
crand "crypto/rand" // #nosec // crypto/rand is used for seed generation
|
||||
"encoding/binary"
|
||||
"errors"
|
||||
"fmt"
|
||||
"math/rand" // #nosec // math/rand is used for random selection and seeded from crypto/rand
|
||||
"slices"
|
||||
@@ -139,15 +138,9 @@ func (snm *SenderNonceMempool) Insert(_ context.Context, tx sdk.Tx) error {
|
||||
|
||||
sig := sigs[0]
|
||||
sender := sdk.AccAddress(sig.PubKey.Address()).String()
|
||||
nonce := sig.Sequence
|
||||
|
||||
// if it's an unordered tx, we use the timeout timestamp instead of the nonce
|
||||
if unordered, ok := tx.(sdk.TxWithUnordered); ok && unordered.GetUnordered() {
|
||||
timestamp := unordered.GetTimeoutTimeStamp().UnixNano()
|
||||
if timestamp < 0 {
|
||||
return errors.New("invalid timestamp value")
|
||||
}
|
||||
nonce = uint64(timestamp)
|
||||
nonce, err := ChooseNonce(sig.Sequence, tx)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
senderTxs, found := snm.senders[sender]
|
||||
@@ -236,15 +229,9 @@ func (snm *SenderNonceMempool) Remove(tx sdk.Tx) error {
|
||||
|
||||
sig := sigs[0]
|
||||
sender := sdk.AccAddress(sig.PubKey.Address()).String()
|
||||
nonce := sig.Sequence
|
||||
|
||||
// if it's an unordered tx, we use the timeout timestamp instead of the nonce
|
||||
if unordered, ok := tx.(sdk.TxWithUnordered); ok && unordered.GetUnordered() {
|
||||
timestamp := unordered.GetTimeoutTimeStamp().UnixNano()
|
||||
if timestamp < 0 {
|
||||
return errors.New("invalid timestamp value")
|
||||
}
|
||||
nonce = uint64(timestamp)
|
||||
nonce, err := ChooseNonce(sig.Sequence, tx)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
senderTxs, found := snm.senders[sender]
|
||||
|
||||
@@ -170,6 +170,22 @@ func (s *MempoolTestSuite) TestMaxTx() {
|
||||
require.Equal(t, mempool.ErrMempoolTxMaxCapacity, err)
|
||||
}
|
||||
|
||||
func (s *MempoolTestSuite) TestTxRejectedWithUnorderedAndSequence() {
|
||||
t := s.T()
|
||||
ctx := sdk.NewContext(nil, cmtproto.Header{}, false, log.NewNopLogger())
|
||||
accounts := simtypes.RandomAccounts(rand.New(rand.NewSource(0)), 1)
|
||||
mp := mempool.NewSenderNonceMempool(mempool.SenderNonceMaxTxOpt(5000))
|
||||
|
||||
txSender := testTx{
|
||||
nonce: 15,
|
||||
address: accounts[0].Address,
|
||||
priority: rand.Int63(),
|
||||
unordered: true,
|
||||
}
|
||||
err := mp.Insert(ctx, txSender)
|
||||
require.ErrorContains(t, err, "unordered txs must not have sequence set")
|
||||
}
|
||||
|
||||
func (s *MempoolTestSuite) TestTxNotFoundOnSender() {
|
||||
t := s.T()
|
||||
ctx := sdk.NewContext(nil, cmtproto.Header{}, false, log.NewNopLogger())
|
||||
|
||||
@@ -373,6 +373,10 @@ type TxBody struct {
|
||||
// Note, when set to true, the existing 'timeout_timestamp' value must
|
||||
// be set and will be used to correspond to a timestamp in which the transaction is deemed
|
||||
// valid.
|
||||
//
|
||||
// When true, the sequence value MUST be 0, and any transaction with unordered=true and a non-zero sequence value will
|
||||
// be rejected.
|
||||
// External services that make assumptions about sequence values may need to be updated because of this.
|
||||
Unordered bool `protobuf:"varint,4,opt,name=unordered,proto3" json:"unordered,omitempty"`
|
||||
// timeout_timestamp is the block time after which this transaction will not
|
||||
// be processed by the chain.
|
||||
|
||||
Reference in New Issue
Block a user