From ab1cc531a8d2c6e1975306381b057f58f184fe11 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Fri, 15 Mar 2024 09:13:48 +0800 Subject: [PATCH 1/4] align signer extraction adapter for mempool remove this change should have been included in 545258668753e4defc2aa3912362a42df6a7c652 --- CHANGELOG.md | 1 + types/mempool/priority_nonce.go | 6 ++---- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc5cd5d519e3..38a468e9e4fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i * (types) [#18768](https://github.com/cosmos/cosmos-sdk/pull/18768) Add MustValAddressFromBech32 function. * (gRPC) [#19049](https://github.com/cosmos/cosmos-sdk/pull/19049) Add debug log prints for each gRPC request. * (x/consensus) [#19483](https://github.com/cosmos/cosmos-sdk/pull/19483) Add consensus messages registration to consensus module. +* (types) [#](https://github.com/cosmos/cosmos-sdk/pull/) Align SignerExtractionAdapter in PriorityNonceMempool Remove. ### Improvements diff --git a/types/mempool/priority_nonce.go b/types/mempool/priority_nonce.go index 3e1c5583b45f..f0df79e70882 100644 --- a/types/mempool/priority_nonce.go +++ b/types/mempool/priority_nonce.go @@ -8,8 +8,6 @@ import ( "github.com/huandu/skiplist" - "cosmossdk.io/x/auth/signing" - sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -433,7 +431,7 @@ func (mp *PriorityNonceMempool[C]) CountTx() int { func (mp *PriorityNonceMempool[C]) Remove(tx sdk.Tx) error { mp.mtx.Lock() defer mp.mtx.Unlock() - sigs, err := tx.(signing.SigVerifiableTx).GetSignaturesV2() + sigs, err := mp.cfg.SignerExtractor.GetSigners(tx) if err != nil { return err } @@ -442,7 +440,7 @@ func (mp *PriorityNonceMempool[C]) Remove(tx sdk.Tx) error { } sig := sigs[0] - sender := sdk.AccAddress(sig.PubKey.Address()).String() + sender := sig.Signer.String() nonce := sig.Sequence scoreKey := txMeta[C]{nonce: nonce, sender: sender} From a88eb0076511fb2970a0356de1f3f8c000d1d8fb Mon Sep 17 00:00:00 2001 From: mmsqe Date: Fri, 15 Mar 2024 09:17:48 +0800 Subject: [PATCH 2/4] add test --- types/mempool/priority_nonce_test.go | 82 ++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/types/mempool/priority_nonce_test.go b/types/mempool/priority_nonce_test.go index d8a4a1acb082..f6759128e017 100644 --- a/types/mempool/priority_nonce_test.go +++ b/types/mempool/priority_nonce_test.go @@ -59,6 +59,88 @@ func TestOutOfOrder(t *testing.T) { require.Error(t, validateOrder(rmtxs)) } +type signerExtractionAdapter struct { + UseOld bool +} + +func (a signerExtractionAdapter) GetSigners(tx sdk.Tx) ([]mempool.SignerData, error) { + if a.UseOld { + sigs, err := tx.(signing.SigVerifiableTx).GetSignaturesV2() + if err != nil { + return nil, err + } + signerData := make([]mempool.SignerData, len(sigs)) + for _, sig := range sigs { + signerData = append(signerData, mempool.SignerData{ + Signer: sig.PubKey.Address().Bytes(), + Sequence: sig.Sequence, + }) + } + return signerData, nil + } + signerData, err := mempool.NewDefaultSignerExtractionAdapter().GetSigners(tx) + return signerData, err +} + +func (s *MempoolTestSuite) TestPriorityNonceTxOrderWithAdapter() { + t := s.T() + ctx := sdk.NewContext(nil, false, log.NewNopLogger()) + accounts := simtypes.RandomAccounts(rand.New(rand.NewSource(0)), 5) + sa := accounts[0].Address + sb := accounts[1].Address + + tests := []struct { + txs []txSpec + order []int + fail bool + }{ + { + txs: []txSpec{ + {p: 21, n: 4, a: sa}, + {p: 8, n: 3, a: sa}, + {p: 6, n: 2, a: sa}, + {p: 15, n: 1, a: sb}, + {p: 20, n: 1, a: sa}, + }, + order: []int{4, 3, 2, 1, 0}, + }, + } + for i, tt := range tests { + t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) { + adapter := signerExtractionAdapter{} + pool := mempool.NewPriorityMempool(mempool.PriorityNonceMempoolConfig[int64]{ + TxPriority: mempool.NewDefaultTxPriority(), + SignerExtractor: adapter, + }) + + // create test txs and insert into mempool + for i, ts := range tt.txs { + tx := testTx{id: i, priority: int64(ts.p), nonce: uint64(ts.n), address: ts.a} + c := ctx.WithPriority(tx.priority) + err := pool.Insert(c, tx) + require.NoError(t, err) + } + + orderedTxs := fetchTxs(pool.Select(ctx, nil), 1000) + + var txOrder []int + for _, tx := range orderedTxs { + txOrder = append(txOrder, tx.(testTx).id) + } + + require.Equal(t, tt.order, txOrder) + require.NoError(t, validateOrder(orderedTxs)) + + adapter.UseOld = true + for _, tx := range orderedTxs { + require.NoError(t, pool.Remove(tx)) + } + + require.NoError(t, mempool.IsEmpty[int64](pool)) + }) + } +} + func (s *MempoolTestSuite) TestPriorityNonceTxOrder() { t := s.T() ctx := sdk.NewContext(nil, false, log.NewNopLogger()) From 7762759d61797633be930287013d112c0320b8ba Mon Sep 17 00:00:00 2001 From: mmsqe Date: Fri, 15 Mar 2024 09:27:32 +0800 Subject: [PATCH 3/4] update doc --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38a468e9e4fd..b543d29b8a7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,7 +54,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i * (types) [#18768](https://github.com/cosmos/cosmos-sdk/pull/18768) Add MustValAddressFromBech32 function. * (gRPC) [#19049](https://github.com/cosmos/cosmos-sdk/pull/19049) Add debug log prints for each gRPC request. * (x/consensus) [#19483](https://github.com/cosmos/cosmos-sdk/pull/19483) Add consensus messages registration to consensus module. -* (types) [#](https://github.com/cosmos/cosmos-sdk/pull/) Align SignerExtractionAdapter in PriorityNonceMempool Remove. +* (types) [#19759](https://github.com/cosmos/cosmos-sdk/pull/19759) Align SignerExtractionAdapter in PriorityNonceMempool Remove. ### Improvements From d6abbbe2427eda051fc9dc04a1d770eda919cd18 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Fri, 15 Mar 2024 19:08:00 +0800 Subject: [PATCH 4/4] Apply suggestions from code review --- types/mempool/priority_nonce_test.go | 29 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/types/mempool/priority_nonce_test.go b/types/mempool/priority_nonce_test.go index f6759128e017..0a2f40355fbd 100644 --- a/types/mempool/priority_nonce_test.go +++ b/types/mempool/priority_nonce_test.go @@ -64,22 +64,21 @@ type signerExtractionAdapter struct { } func (a signerExtractionAdapter) GetSigners(tx sdk.Tx) ([]mempool.SignerData, error) { - if a.UseOld { - sigs, err := tx.(signing.SigVerifiableTx).GetSignaturesV2() - if err != nil { - return nil, err - } - signerData := make([]mempool.SignerData, len(sigs)) - for _, sig := range sigs { - signerData = append(signerData, mempool.SignerData{ - Signer: sig.PubKey.Address().Bytes(), - Sequence: sig.Sequence, - }) - } - return signerData, nil + if !a.UseOld { + return mempool.NewDefaultSignerExtractionAdapter().GetSigners(tx) + } + sigs, err := tx.(signing.SigVerifiableTx).GetSignaturesV2() + if err != nil { + return nil, err + } + signerData := make([]mempool.SignerData, len(sigs)) + for _, sig := range sigs { + signerData = append(signerData, mempool.SignerData{ + Signer: sig.PubKey.Address().Bytes(), + Sequence: sig.Sequence, + }) } - signerData, err := mempool.NewDefaultSignerExtractionAdapter().GetSigners(tx) - return signerData, err + return signerData, nil } func (s *MempoolTestSuite) TestPriorityNonceTxOrderWithAdapter() {