From e646d5ba0f6a9f79c866ed50e11bb96a1f54d371 Mon Sep 17 00:00:00 2001 From: Eric Date: Tue, 9 Jan 2024 09:46:04 -0800 Subject: [PATCH 1/3] Add signer extraction adapter to mempool --- types/mempool/priority_nonce.go | 10 ++- types/mempool/priority_nonce_test.go | 17 +++-- .../signer_extraction_adapater_test.go | 58 ++++++++++++++++ types/mempool/signer_extraction_adapter.go | 68 +++++++++++++++++++ 4 files changed, 144 insertions(+), 9 deletions(-) create mode 100644 types/mempool/signer_extraction_adapater_test.go create mode 100644 types/mempool/signer_extraction_adapter.go diff --git a/types/mempool/priority_nonce.go b/types/mempool/priority_nonce.go index 94ab96bc7f65..d87ed4faee35 100644 --- a/types/mempool/priority_nonce.go +++ b/types/mempool/priority_nonce.go @@ -41,6 +41,9 @@ type ( // (sequence number) when evicting transactions. // - if MaxTx < 0, `Insert` is a no-op. MaxTx int + + // SignerExtractor is an implementation which retrieves signer data from a sdk.Tx + SignerExtractor SignerExtractionAdapter } // PriorityNonceMempool is a mempool implementation that stores txs @@ -117,7 +120,8 @@ func NewDefaultTxPriority() TxPriority[int64] { func DefaultPriorityNonceMempoolConfig() PriorityNonceMempoolConfig[int64] { return PriorityNonceMempoolConfig[int64]{ - TxPriority: NewDefaultTxPriority(), + TxPriority: NewDefaultTxPriority(), + SignerExtractor: NewDefaultSignerExtractionAdapter(), } } @@ -205,7 +209,7 @@ func (mp *PriorityNonceMempool[C]) Insert(ctx context.Context, tx sdk.Tx) error return nil } - sigs, err := tx.(signing.SigVerifiableTx).GetSignaturesV2() + sigs, err := mp.cfg.SignerExtractor.GetSigners(tx) if err != nil { return err } @@ -214,7 +218,7 @@ func (mp *PriorityNonceMempool[C]) Insert(ctx context.Context, tx sdk.Tx) error } sig := sigs[0] - sender := sdk.AccAddress(sig.PubKey.Address()).String() + sender := sig.Signer.String() priority := mp.cfg.TxPriority.GetTxPriority(ctx, tx) nonce := sig.Sequence key := txMeta[C]{nonce: nonce, priority: priority, sender: sender} diff --git a/types/mempool/priority_nonce_test.go b/types/mempool/priority_nonce_test.go index 091e8b91d0e2..d8a4a1acb082 100644 --- a/types/mempool/priority_nonce_test.go +++ b/types/mempool/priority_nonce_test.go @@ -434,6 +434,7 @@ func (s *MempoolTestSuite) TestRandomGeneratedTxs() { OnRead: func(tx sdk.Tx) { s.iterations++ }, + SignerExtractor: mempool.NewDefaultSignerExtractionAdapter(), }, ) @@ -697,8 +698,9 @@ func TestNextSenderTx_TxLimit(t *testing.T) { // unlimited mp := mempool.NewPriorityMempool( mempool.PriorityNonceMempoolConfig[int64]{ - TxPriority: mempool.NewDefaultTxPriority(), - MaxTx: 0, + TxPriority: mempool.NewDefaultTxPriority(), + MaxTx: 0, + SignerExtractor: mempool.NewDefaultSignerExtractionAdapter(), }, ) for i, tx := range txs { @@ -717,8 +719,9 @@ func TestNextSenderTx_TxLimit(t *testing.T) { // limit: 3 mp = mempool.NewPriorityMempool( mempool.PriorityNonceMempoolConfig[int64]{ - TxPriority: mempool.NewDefaultTxPriority(), - MaxTx: 3, + TxPriority: mempool.NewDefaultTxPriority(), + MaxTx: 3, + SignerExtractor: mempool.NewDefaultSignerExtractionAdapter(), }, ) for i, tx := range txs { @@ -736,8 +739,9 @@ func TestNextSenderTx_TxLimit(t *testing.T) { // disabled mp = mempool.NewPriorityMempool( mempool.PriorityNonceMempoolConfig[int64]{ - TxPriority: mempool.NewDefaultTxPriority(), - MaxTx: -1, + TxPriority: mempool.NewDefaultTxPriority(), + MaxTx: -1, + SignerExtractor: mempool.NewDefaultSignerExtractionAdapter(), }, ) for _, tx := range txs { @@ -782,6 +786,7 @@ func TestNextSenderTx_TxReplacement(t *testing.T) { threshold := int64(100 + feeBump) return np >= op*threshold/100 }, + SignerExtractor: mempool.NewDefaultSignerExtractionAdapter(), }, ) diff --git a/types/mempool/signer_extraction_adapater_test.go b/types/mempool/signer_extraction_adapater_test.go new file mode 100644 index 000000000000..b2218dc8e511 --- /dev/null +++ b/types/mempool/signer_extraction_adapater_test.go @@ -0,0 +1,58 @@ +package mempool_test + +import ( + "fmt" + "math/rand" + "testing" + + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/mempool" + simtypes "github.com/cosmos/cosmos-sdk/types/simulation" + txsigning "github.com/cosmos/cosmos-sdk/types/tx/signing" +) + +type nonVerifiableTx struct{} + +func (n nonVerifiableTx) GetMsgs() []sdk.Msg { + panic("not implemented") +} + +func (n nonVerifiableTx) GetMsgsV2() ([]proto.Message, error) { + panic("not implemented") +} + +func TestDefaultSignerExtractor(t *testing.T) { + accounts := simtypes.RandomAccounts(rand.New(rand.NewSource(0)), 1) + sa := accounts[0].Address + ext := mempool.NewDefaultSignerExtractionAdapter() + goodTx := testTx{id: 0, priority: 0, nonce: 0, address: sa} + badTx := &sigErrTx{getSigs: func() ([]txsigning.SignatureV2, error) { + return nil, fmt.Errorf("error") + }} + nonSigVerify := nonVerifiableTx{} + + tests := []struct { + name string + tx sdk.Tx + sea mempool.SignerExtractionAdapter + err error + }{ + {name: "valid tx extracts sigs", tx: goodTx, sea: ext, err: nil}, + {name: "invalid tx fails on sig", tx: badTx, sea: ext, err: fmt.Errorf("err")}, + {name: "non-verifiable tx fails on conversion", tx: nonSigVerify, sea: ext, err: fmt.Errorf("tx of type %T does not implement SigVerifiableTx", nonSigVerify)}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + sigs, err := test.sea.GetSigners(test.tx) + if test.err != nil { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Equal(t, sigs[0].String(), mempool.SignerData{Signer: sa, Sequence: 0}.String()) + }) + } +} diff --git a/types/mempool/signer_extraction_adapter.go b/types/mempool/signer_extraction_adapter.go new file mode 100644 index 000000000000..f79d6c0693d5 --- /dev/null +++ b/types/mempool/signer_extraction_adapter.go @@ -0,0 +1,68 @@ +package mempool + +import ( + "fmt" + + "cosmossdk.io/x/auth/signing" + + sdk "github.com/cosmos/cosmos-sdk/types" +) + +// SignerData contains canonical useful information about the signer of a transaction +type SignerData struct { + Signer sdk.AccAddress + Sequence uint64 +} + +// NewSignerData returns a new SignerData instance. +func NewSignerData(signer sdk.AccAddress, sequence uint64) SignerData { + return SignerData{ + Signer: signer, + Sequence: sequence, + } +} + +// String implements the fmt.Stringer interface. +func (s SignerData) String() string { + return fmt.Sprintf("SignerData{Signer: %s, Sequence: %d}", s.Signer, s.Sequence) +} + +// SignerExtractionAdapter is an interface used to determine how the signers of a transaction should be extracted +// from the transaction. +type SignerExtractionAdapter interface { + GetSigners(sdk.Tx) ([]SignerData, error) +} + +var _ SignerExtractionAdapter = DefaultSignerExtractionAdapter{} + +// DefaultSignerExtractionAdapter is the default implementation of SignerExtractionAdapter. It extracts the signers +// from a cosmos-sdk tx via GetSignaturesV2. +type DefaultSignerExtractionAdapter struct{} + +// NewDefaultSignerExtractionAdapter constructs a new DefaultSignerExtractionAdapter instance +func NewDefaultSignerExtractionAdapter() DefaultSignerExtractionAdapter { + return DefaultSignerExtractionAdapter{} +} + +// GetSigners implements the Adapter interface +func (DefaultSignerExtractionAdapter) GetSigners(tx sdk.Tx) ([]SignerData, error) { + sigTx, ok := tx.(signing.SigVerifiableTx) + if !ok { + return nil, fmt.Errorf("tx of type %T does not implement SigVerifiableTx", tx) + } + + sigs, err := sigTx.GetSignaturesV2() + if err != nil { + return nil, err + } + + signers := make([]SignerData, len(sigs)) + for i, sig := range sigs { + signers[i] = NewSignerData( + sig.PubKey.Address().Bytes(), + sig.Sequence, + ) + } + + return signers, nil +} From 71f84348446c20682c3dbbedadc2298e9a26ca59 Mon Sep 17 00:00:00 2001 From: Eric Date: Tue, 9 Jan 2024 09:53:27 -0800 Subject: [PATCH 2/3] Ensure no segfaults --- types/mempool/priority_nonce.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/types/mempool/priority_nonce.go b/types/mempool/priority_nonce.go index d87ed4faee35..3e1c5583b45f 100644 --- a/types/mempool/priority_nonce.go +++ b/types/mempool/priority_nonce.go @@ -162,6 +162,9 @@ func skiplistComparable[C comparable](txPriority TxPriority[C]) skiplist.Compara // NewPriorityMempool returns the SDK's default mempool implementation which // returns txs in a partial order by 2 dimensions; priority, and sender-nonce. func NewPriorityMempool[C comparable](cfg PriorityNonceMempoolConfig[C]) *PriorityNonceMempool[C] { + if cfg.SignerExtractor == nil { + cfg.SignerExtractor = NewDefaultSignerExtractionAdapter() + } mp := &PriorityNonceMempool[C]{ priorityIndex: skiplist.New(skiplistComparable(cfg.TxPriority)), priorityCounts: make(map[C]int), From 8d08d3cf0bb148de6577e24e6802d099d2982cd9 Mon Sep 17 00:00:00 2001 From: Eric Date: Tue, 9 Jan 2024 09:56:46 -0800 Subject: [PATCH 3/3] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f014579d9206..e3924d6a2c48 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Features +* (types) [#18991](https://github.com/cosmos/cosmos-sdk/pull/18991) Add SignerExtractionAdapter to PriorityNonceMempool/Config and provide Default implementation matching existing behavior. * (x/auth) Support the ability to broadcast unordered transactions per ADR-070. See UPGRADING.md for more details on integration. * (client) [#18557](https://github.com/cosmos/cosmos-sdk/pull/18557) Add `--qrcode` flag to `keys show` command to support displaying keys address QR code. * (x/staking) [#18142](https://github.com/cosmos/cosmos-sdk/pull/18142) Introduce `key_rotation_fee` param to calculate fees while rotating the keys