From 7aba2e72ce2ac33315e5c661cbcfd23273cdf3bc Mon Sep 17 00:00:00 2001 From: Facundo Date: Wed, 28 Aug 2024 11:58:53 +0200 Subject: [PATCH 01/20] small fixes --- simapp/app.go | 9 +++++++++ simapp/app_di.go | 10 ++++++++++ x/auth/ante/unorderedtx/manager.go | 4 +++- 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/simapp/app.go b/simapp/app.go index 933149336ea6..58181a8f6756 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -589,6 +589,7 @@ func NewSimApp( app.SetBeginBlocker(app.BeginBlocker) app.SetEndBlocker(app.EndBlocker) app.setAnteHandler(txConfig) + app.SetPrecommiter(app.Precommiter) // In v0.46, the SDK introduces _postHandlers_. PostHandlers are like // antehandlers, but are run _after_ the `runMsgs` execution. They are also @@ -686,6 +687,14 @@ func (app *SimApp) EndBlocker(ctx sdk.Context) (sdk.EndBlock, error) { return app.ModuleManager.EndBlock(ctx) } +func (app *SimApp) Precommiter(ctx sdk.Context) { + if err := app.ModuleManager.Precommit(ctx); err != nil { + panic(err) + } + + app.UnorderedTxManager.OnNewBlock(ctx.BlockTime()) +} + func (a *SimApp) Configurator() module.Configurator { // nolint:staticcheck // SA1019: Configurator is deprecated but still used in runtime v1. return a.configurator } diff --git a/simapp/app_di.go b/simapp/app_di.go index c66eeef8e68e..063f39977f86 100644 --- a/simapp/app_di.go +++ b/simapp/app_di.go @@ -53,6 +53,7 @@ import ( "github.com/cosmos/cosmos-sdk/server/config" servertypes "github.com/cosmos/cosmos-sdk/server/types" testdata_pulsar "github.com/cosmos/cosmos-sdk/testutil/testdata/testpb" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" ) @@ -295,6 +296,7 @@ func NewSimApp( utxDataDir := filepath.Join(cast.ToString(appOpts.Get(flags.FlagHome)), "data") app.UnorderedTxManager = unorderedtx.NewManager(utxDataDir) app.UnorderedTxManager.Start() + app.SetPrecommiter(app.Precommiter) if err := app.UnorderedTxManager.OnInit(); err != nil { panic(fmt.Errorf("failed to initialize unordered tx manager: %w", err)) @@ -345,6 +347,14 @@ func (app *SimApp) setCustomAnteHandler() { app.SetAnteHandler(anteHandler) } +func (app *SimApp) Precommiter(ctx sdk.Context) { + if err := app.ModuleManager.Precommit(ctx); err != nil { + panic(err) + } + + app.UnorderedTxManager.OnNewBlock(ctx.BlockTime()) +} + // Close implements the Application interface and closes all necessary application // resources. func (app *SimApp) Close() error { diff --git a/x/auth/ante/unorderedtx/manager.go b/x/auth/ante/unorderedtx/manager.go index 8b5a91ed2a01..427af68d2844 100644 --- a/x/auth/ante/unorderedtx/manager.go +++ b/x/auth/ante/unorderedtx/manager.go @@ -57,7 +57,9 @@ type Manager struct { func NewManager(dataDir string) *Manager { path := filepath.Join(dataDir, dirName) if _, err := os.Stat(path); errors.Is(err, os.ErrNotExist) { - _ = os.Mkdir(path, os.ModePerm) + if err = os.Mkdir(path, os.ModePerm); err != nil { + panic(fmt.Errorf("failed to create unordered txs directory: %w", err)) + } } m := &Manager{ From 8f3e2cb1b3040daeb7d33016bcbe0107b655bab6 Mon Sep 17 00:00:00 2001 From: Facundo Date: Thu, 29 Aug 2024 15:35:45 +0200 Subject: [PATCH 02/20] update upgrading md --- UPGRADING.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/UPGRADING.md b/UPGRADING.md index 7218b4e5161b..0c8df7e9f981 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -166,6 +166,23 @@ transactions in your application: } ``` +* Create or update the App's `Precommiter()` method to call the unordered tx + manager's `OnNewBlock()` method. + + ```go + ... + app.SetPrecommiter(app.Precommiter) + ... + + func (app *SimApp) Precommiter(ctx sdk.Context) { + if err := app.ModuleManager.Precommit(ctx); err != nil { + panic(err) + } + + app.UnorderedTxManager.OnNewBlock(ctx.BlockTime()) + } + ``` + * Create or update the App's `Close()` method to close the unordered tx manager. Note, this is critical as it ensures the manager's state is written to file such that when the node restarts, it can recover the state to provide replay From 83fa1f0cc78f50be1a49ffc0651baf2ceab63fe6 Mon Sep 17 00:00:00 2001 From: Facundo Date: Fri, 30 Aug 2024 16:38:49 +0200 Subject: [PATCH 03/20] do not chekc the sequence in prepareproposal --- baseapp/abci_utils.go | 81 ++++++++++++++++++++++++------------------- 1 file changed, 45 insertions(+), 36 deletions(-) diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go index 6da80906fab5..15851e430db4 100644 --- a/baseapp/abci_utils.go +++ b/baseapp/abci_utils.go @@ -290,34 +290,41 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan var selectedTxsNums int for iterator != nil { memTx := iterator.Tx() - signerData, err := h.signerExtAdapter.GetSigners(memTx) - if err != nil { - return nil, err - } - - // If the signers aren't in selectedTxsSignersSeqs then we haven't seen them before - // so we add them and continue given that we don't need to check the sequence. - shouldAdd := true txSignersSeqs := make(map[string]uint64) - for _, signer := range signerData { - seq, ok := selectedTxsSignersSeqs[signer.Signer.String()] - if !ok { - txSignersSeqs[signer.Signer.String()] = signer.Sequence - continue + + unorderedTx, ok := memTx.(sdk.TxWithUnordered) + isUnordered := ok && unorderedTx.GetUnordered() + + if !isUnordered { + var err error + signerData, err := h.signerExtAdapter.GetSigners(memTx) + if err != nil { + return nil, err } - // If we have seen this signer before in this block, we must make - // sure that the current sequence is seq+1; otherwise is invalid - // and we skip it. - if seq+1 != signer.Sequence { - shouldAdd = false - break + // If the signers aren't in selectedTxsSignersSeqs then we haven't seen them before + // so we add them and continue given that we don't need to check the sequence. + shouldAdd := true + for _, signer := range signerData { + seq, ok := selectedTxsSignersSeqs[signer.Signer.String()] + if !ok { + txSignersSeqs[signer.Signer.String()] = signer.Sequence + continue + } + + // If we have seen this signer before in this block, we must make + // sure that the current sequence is seq+1; otherwise is invalid + // and we skip it. + if seq+1 != signer.Sequence { + shouldAdd = false + break + } + txSignersSeqs[signer.Signer.String()] = signer.Sequence + } + if !shouldAdd { + iterator = iterator.Next() + continue } - txSignersSeqs[signer.Signer.String()] = signer.Sequence - } - if !shouldAdd { - iterator = iterator.Next() - continue } // NOTE: Since transaction verification was already executed in CheckTx, @@ -337,18 +344,20 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan } txsLen := len(h.txSelector.SelectedTxs(ctx)) - for sender, seq := range txSignersSeqs { - // If txsLen != selectedTxsNums is true, it means that we've - // added a new tx to the selected txs, so we need to update - // the sequence of the sender. - if txsLen != selectedTxsNums { - selectedTxsSignersSeqs[sender] = seq - } else if _, ok := selectedTxsSignersSeqs[sender]; !ok { - // The transaction hasn't been added but it passed the - // verification, so we know that the sequence is correct. - // So we set this sender's sequence to seq-1, in order - // to avoid unnecessary calls to PrepareProposalVerifyTx. - selectedTxsSignersSeqs[sender] = seq - 1 + if !isUnordered { + for sender, seq := range txSignersSeqs { + // If txsLen != selectedTxsNums is true, it means that we've + // added a new tx to the selected txs, so we need to update + // the sequence of the sender. + if txsLen != selectedTxsNums { + selectedTxsSignersSeqs[sender] = seq + } else if _, ok := selectedTxsSignersSeqs[sender]; !ok { + // The transaction hasn't been added but it passed the + // verification, so we know that the sequence is correct. + // So we set this sender's sequence to seq-1, in order + // to avoid unnecessary calls to PrepareProposalVerifyTx. + selectedTxsSignersSeqs[sender] = seq - 1 + } } } selectedTxsNums = txsLen From 33665d581934ec07a2ff088ef92d17eb2b4ffe59 Mon Sep 17 00:00:00 2001 From: Facundo Date: Tue, 3 Sep 2024 15:35:12 +0200 Subject: [PATCH 04/20] double-check this, not sure --- types/mempool/priority_nonce.go | 19 +++++++++++++++++++ types/mempool/sender_nonce.go | 18 ++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/types/mempool/priority_nonce.go b/types/mempool/priority_nonce.go index a927693410ef..fe9e9daf7165 100644 --- a/types/mempool/priority_nonce.go +++ b/types/mempool/priority_nonce.go @@ -223,6 +223,16 @@ func (mp *PriorityNonceMempool[C]) Insert(ctx context.Context, tx sdk.Tx) error sender := sig.Signer.String() priority := mp.cfg.TxPriority.GetTxPriority(ctx, tx) nonce := sig.Sequence + + // if it's an unordered tx, we use the gas instead of the nonce + if unordered, ok := tx.(sdk.TxWithUnordered); ok && unordered.GetUnordered() { + gasLimit, err := unordered.GetGasLimit() + nonce = gasLimit + if err != nil { + return err + } + } + key := txMeta[C]{nonce: nonce, priority: priority, sender: sender} senderIndex, ok := mp.senderIndices[sender] @@ -444,6 +454,15 @@ func (mp *PriorityNonceMempool[C]) Remove(tx sdk.Tx) error { sender := sig.Signer.String() nonce := sig.Sequence + // if it's an unordered tx, we use the gas instead of the nonce + if unordered, ok := tx.(sdk.TxWithUnordered); ok && unordered.GetUnordered() { + gasLimit, err := unordered.GetGasLimit() + nonce = gasLimit + if err != nil { + return err + } + } + scoreKey := txMeta[C]{nonce: nonce, sender: sender} score, ok := mp.scores[scoreKey] if !ok { diff --git a/types/mempool/sender_nonce.go b/types/mempool/sender_nonce.go index fc4902f64792..2ce4a9766900 100644 --- a/types/mempool/sender_nonce.go +++ b/types/mempool/sender_nonce.go @@ -146,6 +146,15 @@ func (snm *SenderNonceMempool) Insert(_ context.Context, tx sdk.Tx) error { snm.senders[sender] = senderTxs } + // if it's an unordered tx, we use the gas instead of the nonce + if unordered, ok := tx.(sdk.TxWithUnordered); ok && unordered.GetUnordered() { + gasLimit, err := unordered.GetGasLimit() + nonce = gasLimit + if err != nil { + return err + } + } + senderTxs.Set(nonce, tx) key := txKey{nonce: nonce, address: sender} @@ -213,6 +222,15 @@ func (snm *SenderNonceMempool) Remove(tx sdk.Tx) error { sender := sdk.AccAddress(sig.PubKey.Address()).String() nonce := sig.Sequence + // if it's an unordered tx, we use the gas instead of the nonce + if unordered, ok := tx.(sdk.TxWithUnordered); ok && unordered.GetUnordered() { + gasLimit, err := unordered.GetGasLimit() + nonce = gasLimit + if err != nil { + return err + } + } + senderTxs, found := snm.senders[sender] if !found { return ErrTxNotFound From 0661845ddbc3708ebb1bdacf876df2425772545a Mon Sep 17 00:00:00 2001 From: Facundo Date: Wed, 4 Sep 2024 12:36:43 +0200 Subject: [PATCH 05/20] update --- x/auth/ante/unorderedtx/manager.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/auth/ante/unorderedtx/manager.go b/x/auth/ante/unorderedtx/manager.go index 427af68d2844..9fab3041efd9 100644 --- a/x/auth/ante/unorderedtx/manager.go +++ b/x/auth/ante/unorderedtx/manager.go @@ -187,8 +187,8 @@ func (m *Manager) exportSnapshot(height uint64, snapshotWriter func([]byte) erro return snapshotWriter(buf.Bytes()) } -// flushToFile writes all unexpired unordered transactions along with their TTL -// to file, overwriting the existing file if it exists. +// flushToFile writes all unordered transactions (including expired if not pruned yet) +// along with their TTL to file, overwriting the existing file if it exists. func (m *Manager) flushToFile() error { f, err := os.Create(filepath.Join(m.dataDir, dirName, fileName)) if err != nil { From af42e6c8934dd7b55b06426c208f9b44ecedbd9c Mon Sep 17 00:00:00 2001 From: Facundo Date: Wed, 4 Sep 2024 13:22:42 +0200 Subject: [PATCH 06/20] use mkdirall instead of just mkdir --- x/auth/ante/unorderedtx/manager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/auth/ante/unorderedtx/manager.go b/x/auth/ante/unorderedtx/manager.go index 9fab3041efd9..3d5b9cad1e37 100644 --- a/x/auth/ante/unorderedtx/manager.go +++ b/x/auth/ante/unorderedtx/manager.go @@ -57,7 +57,7 @@ type Manager struct { func NewManager(dataDir string) *Manager { path := filepath.Join(dataDir, dirName) if _, err := os.Stat(path); errors.Is(err, os.ErrNotExist) { - if err = os.Mkdir(path, os.ModePerm); err != nil { + if err = os.MkdirAll(path, os.ModePerm); err != nil { panic(fmt.Errorf("failed to create unordered txs directory: %w", err)) } } From a06cc3f0e95fb1da013332486cac868f07d96489 Mon Sep 17 00:00:00 2001 From: Facundo Date: Wed, 4 Sep 2024 13:38:32 +0200 Subject: [PATCH 07/20] re-add skip of seq in prepareproposal --- baseapp/abci_utils.go | 83 ++++++++++++++++++++++++------------------- 1 file changed, 46 insertions(+), 37 deletions(-) diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go index 4fa068b3c9a4..7263afb30ddc 100644 --- a/baseapp/abci_utils.go +++ b/baseapp/abci_utils.go @@ -292,35 +292,41 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan invalidTxs []sdk.Tx // invalid txs to be removed out of the loop to avoid dead lock ) h.mempool.SelectBy(ctx, req.Txs, func(memTx sdk.Tx) bool { - signerData, err := h.signerExtAdapter.GetSigners(memTx) - if err != nil { - // propagate the error to the caller - resError = err - return false - } - - // If the signers aren't in selectedTxsSignersSeqs then we haven't seen them before - // so we add them and continue given that we don't need to check the sequence. - shouldAdd := true + unorderedTx, ok := memTx.(sdk.TxWithUnordered) + isUnordered := ok && unorderedTx.GetUnordered() txSignersSeqs := make(map[string]uint64) - for _, signer := range signerData { - seq, ok := selectedTxsSignersSeqs[signer.Signer.String()] - if !ok { - txSignersSeqs[signer.Signer.String()] = signer.Sequence - continue + + // if the tx is unordered, we don't need to check the sequence, we just add it + if !isUnordered { + signerData, err := h.signerExtAdapter.GetSigners(memTx) + if err != nil { + // propagate the error to the caller + resError = err + return false } - // If we have seen this signer before in this block, we must make - // sure that the current sequence is seq+1; otherwise is invalid - // and we skip it. - if seq+1 != signer.Sequence { - shouldAdd = false - break + // If the signers aren't in selectedTxsSignersSeqs then we haven't seen them before + // so we add them and continue given that we don't need to check the sequence. + shouldAdd := true + for _, signer := range signerData { + seq, ok := selectedTxsSignersSeqs[signer.Signer.String()] + if !ok { + txSignersSeqs[signer.Signer.String()] = signer.Sequence + continue + } + + // If we have seen this signer before in this block, we must make + // sure that the current sequence is seq+1; otherwise is invalid + // and we skip it. + if seq+1 != signer.Sequence { + shouldAdd = false + break + } + txSignersSeqs[signer.Signer.String()] = signer.Sequence + } + if !shouldAdd { + return true } - txSignersSeqs[signer.Signer.String()] = signer.Sequence - } - if !shouldAdd { - return true } // NOTE: Since transaction verification was already executed in CheckTx, @@ -337,18 +343,21 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan } txsLen := len(h.txSelector.SelectedTxs(ctx)) - for sender, seq := range txSignersSeqs { - // If txsLen != selectedTxsNums is true, it means that we've - // added a new tx to the selected txs, so we need to update - // the sequence of the sender. - if txsLen != selectedTxsNums { - selectedTxsSignersSeqs[sender] = seq - } else if _, ok := selectedTxsSignersSeqs[sender]; !ok { - // The transaction hasn't been added but it passed the - // verification, so we know that the sequence is correct. - // So we set this sender's sequence to seq-1, in order - // to avoid unnecessary calls to PrepareProposalVerifyTx. - selectedTxsSignersSeqs[sender] = seq - 1 + // If the tx is unordered, we don't need to update the sender sequence. + if !isUnordered { + for sender, seq := range txSignersSeqs { + // If txsLen != selectedTxsNums is true, it means that we've + // added a new tx to the selected txs, so we need to update + // the sequence of the sender. + if txsLen != selectedTxsNums { + selectedTxsSignersSeqs[sender] = seq + } else if _, ok := selectedTxsSignersSeqs[sender]; !ok { + // The transaction hasn't been added but it passed the + // verification, so we know that the sequence is correct. + // So we set this sender's sequence to seq-1, in order + // to avoid unnecessary calls to PrepareProposalVerifyTx. + selectedTxsSignersSeqs[sender] = seq - 1 + } } } selectedTxsNums = txsLen From 8a5018e21ea66e8c2de0da60e5483a28b93ef37c Mon Sep 17 00:00:00 2001 From: Facundo Date: Wed, 4 Sep 2024 13:41:33 +0200 Subject: [PATCH 08/20] fix close --- simapp/app_di.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/simapp/app_di.go b/simapp/app_di.go index cf78ee03b49b..6f86b9040c8e 100644 --- a/simapp/app_di.go +++ b/simapp/app_di.go @@ -337,6 +337,9 @@ func (app *SimApp) Precommiter(ctx sdk.Context) { // Close implements the Application interface and closes all necessary application // resources. func (app *SimApp) Close() error { + if err := app.BaseApp.Close(); err != nil { + return err + } return app.UnorderedTxManager.Close() } From 5109b4520449d86f98c6f76aa36e5d4174971c42 Mon Sep 17 00:00:00 2001 From: Facundo Date: Thu, 5 Sep 2024 13:23:07 +0200 Subject: [PATCH 09/20] fix --- runtime/app.go | 3 ++ simapp/app_di.go | 31 --------------------- x/auth/ante/unordered_test.go | 52 +++++++++++++++++++++++++++++++++-- 3 files changed, 52 insertions(+), 34 deletions(-) diff --git a/runtime/app.go b/runtime/app.go index c5533d79ce1b..9b6cd5f9599b 100644 --- a/runtime/app.go +++ b/runtime/app.go @@ -173,6 +173,9 @@ func (a *App) Close() error { // PreBlocker application updates every pre block func (a *App) PreBlocker(ctx sdk.Context, _ *abci.FinalizeBlockRequest) error { + if a.UnorderedTxManager != nil { + a.UnorderedTxManager.OnNewBlock(ctx.BlockTime()) + } return a.ModuleManager.PreBlock(ctx) } diff --git a/simapp/app_di.go b/simapp/app_di.go index 6f86b9040c8e..94764c72eeb3 100644 --- a/simapp/app_di.go +++ b/simapp/app_di.go @@ -6,7 +6,6 @@ import ( _ "embed" "fmt" "io" - "path/filepath" clienthelpers "cosmossdk.io/client/v2/helpers" "cosmossdk.io/core/address" @@ -36,7 +35,6 @@ import ( "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/client" - "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/codec" codectypes "github.com/cosmos/cosmos-sdk/codec/types" "github.com/cosmos/cosmos-sdk/runtime" @@ -45,7 +43,6 @@ import ( "github.com/cosmos/cosmos-sdk/server/config" servertypes "github.com/cosmos/cosmos-sdk/server/types" testdata_pulsar "github.com/cosmos/cosmos-sdk/testutil/testdata/testpb" - sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" "github.com/cosmos/cosmos-sdk/x/auth" "github.com/cosmos/cosmos-sdk/x/auth/ante" @@ -53,7 +50,6 @@ import ( authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" authsims "github.com/cosmos/cosmos-sdk/x/auth/simulation" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" - "github.com/spf13/cast" ) // DefaultNodeHome default home directories for the application daemon @@ -272,16 +268,6 @@ func NewSimApp( // return app.App.InitChainer(ctx, req) // }) - // create, start, and load the unordered tx manager - utxDataDir := filepath.Join(cast.ToString(appOpts.Get(flags.FlagHome)), "data") - app.UnorderedTxManager = unorderedtx.NewManager(utxDataDir) - app.UnorderedTxManager.Start() - app.SetPrecommiter(app.Precommiter) - - if err := app.UnorderedTxManager.OnInit(); err != nil { - panic(fmt.Errorf("failed to initialize unordered tx manager: %w", err)) - } - // register custom snapshot extensions (if any) if manager := app.SnapshotManager(); manager != nil { if err := manager.RegisterExtensions( @@ -326,23 +312,6 @@ func (app *SimApp) setCustomAnteHandler() { app.SetAnteHandler(anteHandler) } -func (app *SimApp) Precommiter(ctx sdk.Context) { - if err := app.ModuleManager.Precommit(ctx); err != nil { - panic(err) - } - - app.UnorderedTxManager.OnNewBlock(ctx.BlockTime()) -} - -// Close implements the Application interface and closes all necessary application -// resources. -func (app *SimApp) Close() error { - if err := app.BaseApp.Close(); err != nil { - return err - } - return app.UnorderedTxManager.Close() -} - // LegacyAmino returns SimApp's amino codec. // // NOTE: This is solely to be used for testing purposes as it may be desirable diff --git a/x/auth/ante/unordered_test.go b/x/auth/ante/unordered_test.go index 41100856b578..a5038c2b8f74 100644 --- a/x/auth/ante/unordered_test.go +++ b/x/auth/ante/unordered_test.go @@ -8,13 +8,13 @@ import ( "cosmossdk.io/core/header" storetypes "cosmossdk.io/store/types" + "cosmossdk.io/x/auth/ante" + "cosmossdk.io/x/auth/ante/unorderedtx" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/tx/signing" - "github.com/cosmos/cosmos-sdk/x/auth/ante" - "github.com/cosmos/cosmos-sdk/x/auth/ante/unorderedtx" ) const gasConsumed = uint64(25) @@ -141,7 +141,7 @@ func TestUnorderedTxDecorator_UnorderedTx_ValidDeliverTx(t *testing.T) { require.True(t, txm.Contains(bz)) } -func genUnorderedTx(t *testing.T, unordered bool, timestamp time.Time) (sdk.Tx, []byte) { +func genUnorderedTx(t testing.TB, unordered bool, timestamp time.Time) (sdk.Tx, []byte) { t.Helper() s := SetupTestSuite(t, true) @@ -171,3 +171,49 @@ func genUnorderedTx(t *testing.T, unordered bool, timestamp time.Time) (sdk.Tx, return tx, txBz[:] } + +// Benchmark function for genUnorderedTx +func BenchmarkGenUnorderedTxOld(b *testing.B) { + // tx, _ := genUnorderedTx(b, true, time.Now().Add(time.Minute)) + // b.ResetTimer() + // for _, iterations := range []int{1000, 10000, 100000, 100000} { + // b.Run("Iterations_"+strconv.Itoa(iterations), func(b *testing.B) { + // for i := 0; i < iterations; i++ { + // createAllocations(12000000) + // _, err := ante.TxIdentifier(uint64(time.Now().Unix()+int64(i)), tx) + // if err != nil { + // b.Fatal(err) + // } + // } + // }) + // } +} + +func BenchmarkGenUnorderedTx(b *testing.B) { + tx, _ := genUnorderedTx(b, true, time.Now().Add(time.Minute)) + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, _ = ante.TxIdentifier(uint64(time.Now().Unix()+int64(i)), tx) + + } +} + +// goos: darwin +// goarch: arm64 +// pkg: cosmossdk.io/x/auth/ante +// cpu: Apple M2 +// BenchmarkGenUnorderedTx-8 5807298 205.9 ns/op 88 B/op 5 allocs/op +// BenchmarkGenUnorderedTx-8 5373726 207.8 ns/op 88 B/op 5 allocs/op +// BenchmarkGenUnorderedTx-8 5718591 204.9 ns/op 88 B/op 5 allocs/op +// PASS +// ok cosmossdk.io/x/auth/ante 2.066s + +// goos: darwin +// goarch: arm64 +// pkg: cosmossdk.io/x/auth/ante +// cpu: Apple M2 +// BenchmarkGenUnorderedTx-8 5298655 225.0 ns/op 248 B/op 7 allocs/op +// BenchmarkGenUnorderedTx-8 5279116 226.7 ns/op 248 B/op 7 allocs/op +// BenchmarkGenUnorderedTx-8 5361398 221.6 ns/op 248 B/op 7 allocs/op +// PASS +// ok cosmossdk.io/x/auth/ante 2.230s From d86bfd48b1b0cad8f5bd9d367b983e7a5bc6e10b Mon Sep 17 00:00:00 2001 From: Facundo Date: Thu, 5 Sep 2024 13:25:17 +0200 Subject: [PATCH 10/20] remove benchmark --- x/auth/ante/unordered_test.go | 46 ----------------------------------- 1 file changed, 46 deletions(-) diff --git a/x/auth/ante/unordered_test.go b/x/auth/ante/unordered_test.go index a5038c2b8f74..70a63df7f0d7 100644 --- a/x/auth/ante/unordered_test.go +++ b/x/auth/ante/unordered_test.go @@ -171,49 +171,3 @@ func genUnorderedTx(t testing.TB, unordered bool, timestamp time.Time) (sdk.Tx, return tx, txBz[:] } - -// Benchmark function for genUnorderedTx -func BenchmarkGenUnorderedTxOld(b *testing.B) { - // tx, _ := genUnorderedTx(b, true, time.Now().Add(time.Minute)) - // b.ResetTimer() - // for _, iterations := range []int{1000, 10000, 100000, 100000} { - // b.Run("Iterations_"+strconv.Itoa(iterations), func(b *testing.B) { - // for i := 0; i < iterations; i++ { - // createAllocations(12000000) - // _, err := ante.TxIdentifier(uint64(time.Now().Unix()+int64(i)), tx) - // if err != nil { - // b.Fatal(err) - // } - // } - // }) - // } -} - -func BenchmarkGenUnorderedTx(b *testing.B) { - tx, _ := genUnorderedTx(b, true, time.Now().Add(time.Minute)) - b.ResetTimer() - for i := 0; i < b.N; i++ { - _, _ = ante.TxIdentifier(uint64(time.Now().Unix()+int64(i)), tx) - - } -} - -// goos: darwin -// goarch: arm64 -// pkg: cosmossdk.io/x/auth/ante -// cpu: Apple M2 -// BenchmarkGenUnorderedTx-8 5807298 205.9 ns/op 88 B/op 5 allocs/op -// BenchmarkGenUnorderedTx-8 5373726 207.8 ns/op 88 B/op 5 allocs/op -// BenchmarkGenUnorderedTx-8 5718591 204.9 ns/op 88 B/op 5 allocs/op -// PASS -// ok cosmossdk.io/x/auth/ante 2.066s - -// goos: darwin -// goarch: arm64 -// pkg: cosmossdk.io/x/auth/ante -// cpu: Apple M2 -// BenchmarkGenUnorderedTx-8 5298655 225.0 ns/op 248 B/op 7 allocs/op -// BenchmarkGenUnorderedTx-8 5279116 226.7 ns/op 248 B/op 7 allocs/op -// BenchmarkGenUnorderedTx-8 5361398 221.6 ns/op 248 B/op 7 allocs/op -// PASS -// ok cosmossdk.io/x/auth/ante 2.230s From 80319606214d75dcbfbb8fc0eefdcfb998b190f5 Mon Sep 17 00:00:00 2001 From: Facundo Date: Thu, 5 Sep 2024 13:28:24 +0200 Subject: [PATCH 11/20] update upgrading.md --- UPGRADING.md | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/UPGRADING.md b/UPGRADING.md index 53aaaa1233cb..573b884b7b57 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -277,20 +277,17 @@ If you are still using the legacy wiring, you must enable unordered transactions } ``` -* Create or update the App's `Precommiter()` method to call the unordered tx +* Create or update the App's `Preblocker()` method to call the unordered tx manager's `OnNewBlock()` method. ```go ... - app.SetPrecommiter(app.Precommiter) + app.SetPreblocker(app.PreBlocker) ... - func (app *SimApp) Precommiter(ctx sdk.Context) { - if err := app.ModuleManager.Precommit(ctx); err != nil { - panic(err) - } - + func (app *SimApp) PreBlocker(ctx sdk.Context, req *abci.RequestFinalizeBlock) (*sdk.ResponsePreBlock, error) { app.UnorderedTxManager.OnNewBlock(ctx.BlockTime()) + return app.ModuleManager.PreBlock(ctx, req) } ``` From 9be58bd92987cdbb3646f3904a379d76f97578d7 Mon Sep 17 00:00:00 2001 From: Facundo Date: Thu, 5 Sep 2024 15:44:54 +0200 Subject: [PATCH 12/20] use preblock on simapp v1 --- simapp/app.go | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/simapp/app.go b/simapp/app.go index adb3b7b48875..a3baf5aa14e8 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -598,7 +598,6 @@ func NewSimApp( app.SetBeginBlocker(app.BeginBlocker) app.SetEndBlocker(app.EndBlocker) app.setAnteHandler(txConfig) - app.SetPrecommiter(app.Precommiter) // In v0.46, the SDK introduces _postHandlers_. PostHandlers are like // antehandlers, but are run _after_ the `runMsgs` execution. They are also @@ -687,6 +686,7 @@ func (app *SimApp) Name() string { return app.BaseApp.Name() } // PreBlocker application updates every pre block func (app *SimApp) PreBlocker(ctx sdk.Context, _ *abci.FinalizeBlockRequest) error { + app.UnorderedTxManager.OnNewBlock(ctx.BlockTime()) return app.ModuleManager.PreBlock(ctx) } @@ -700,14 +700,6 @@ func (app *SimApp) EndBlocker(ctx sdk.Context) (sdk.EndBlock, error) { return app.ModuleManager.EndBlock(ctx) } -func (app *SimApp) Precommiter(ctx sdk.Context) { - if err := app.ModuleManager.Precommit(ctx); err != nil { - panic(err) - } - - app.UnorderedTxManager.OnNewBlock(ctx.BlockTime()) -} - func (a *SimApp) Configurator() module.Configurator { // nolint:staticcheck // SA1019: Configurator is deprecated but still used in runtime v1. return a.configurator } From 7ccca5de9443b26adfe2c38f977a06911eeb42b5 Mon Sep 17 00:00:00 2001 From: Facundo Date: Thu, 5 Sep 2024 16:10:23 +0200 Subject: [PATCH 13/20] rollback change in test --- x/auth/ante/unordered_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/auth/ante/unordered_test.go b/x/auth/ante/unordered_test.go index 70a63df7f0d7..b02fcc28c471 100644 --- a/x/auth/ante/unordered_test.go +++ b/x/auth/ante/unordered_test.go @@ -141,7 +141,7 @@ func TestUnorderedTxDecorator_UnorderedTx_ValidDeliverTx(t *testing.T) { require.True(t, txm.Contains(bz)) } -func genUnorderedTx(t testing.TB, unordered bool, timestamp time.Time) (sdk.Tx, []byte) { +func genUnorderedTx(t *testing.T, unordered bool, timestamp time.Time) (sdk.Tx, []byte) { t.Helper() s := SetupTestSuite(t, true) From 0b9141a337248b13535a03a942d173081f49c74b Mon Sep 17 00:00:00 2001 From: Facundo Date: Mon, 9 Sep 2024 11:46:41 +0200 Subject: [PATCH 14/20] timestamp and height --- UPGRADING.md | 2 +- .../adr-070-unordered-transactions.md | 4 ++-- runtime/app.go | 2 +- simapp/app.go | 2 +- x/auth/ante/unorderedtx/manager.go | 23 +++++++++++-------- x/auth/ante/unorderedtx/manager_test.go | 4 +++- 6 files changed, 22 insertions(+), 15 deletions(-) diff --git a/UPGRADING.md b/UPGRADING.md index 654e8a7d9272..4eeda61ad27d 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -286,7 +286,7 @@ If you are still using the legacy wiring, you must enable unordered transactions ... func (app *SimApp) PreBlocker(ctx sdk.Context, req *abci.RequestFinalizeBlock) (*sdk.ResponsePreBlock, error) { - app.UnorderedTxManager.OnNewBlock(ctx.BlockTime()) + app.UnorderedTxManager.OnNewBlock(ctx.BlockTime(), ctx.BlockHeight()) return app.ModuleManager.PreBlock(ctx, req) } ``` diff --git a/docs/architecture/adr-070-unordered-transactions.md b/docs/architecture/adr-070-unordered-transactions.md index abcf63e76dd6..8be95a53411a 100644 --- a/docs/architecture/adr-070-unordered-transactions.md +++ b/docs/architecture/adr-070-unordered-transactions.md @@ -144,8 +144,8 @@ func (m *UnorderedTxManager) Add(hash TxHash, expire time.Time) { m.txHashes[hash] = expire } -// OnNewBlock send the latest block time to the background purge loop, which -// should be called in ABCI Commit event. +// OnNewBlock send the latest block time and height to the background purge loop, +// which should be called in the Preblock method. func (m *UnorderedTxManager) OnNewBlock(blockTime time.Time) { m.blockCh <- blockTime } diff --git a/runtime/app.go b/runtime/app.go index f9a74a29a208..eccba955533a 100644 --- a/runtime/app.go +++ b/runtime/app.go @@ -174,7 +174,7 @@ func (a *App) Close() error { // PreBlocker application updates every pre block func (a *App) PreBlocker(ctx sdk.Context, _ *abci.FinalizeBlockRequest) error { if a.UnorderedTxManager != nil { - a.UnorderedTxManager.OnNewBlock(ctx.BlockTime()) + a.UnorderedTxManager.OnNewBlock(ctx.BlockTime(), uint64(ctx.BlockHeight())) } return a.ModuleManager.PreBlock(ctx) } diff --git a/simapp/app.go b/simapp/app.go index e000faede658..cf910193940d 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -689,7 +689,7 @@ func (app *SimApp) Name() string { return app.BaseApp.Name() } // PreBlocker application updates every pre block func (app *SimApp) PreBlocker(ctx sdk.Context, _ *abci.FinalizeBlockRequest) error { - app.UnorderedTxManager.OnNewBlock(ctx.BlockTime()) + app.UnorderedTxManager.OnNewBlock(ctx.BlockTime(), uint64(ctx.BlockHeight())) return app.ModuleManager.PreBlock(ctx) } diff --git a/x/auth/ante/unorderedtx/manager.go b/x/auth/ante/unorderedtx/manager.go index 3d5b9cad1e37..3a836e0ce36c 100644 --- a/x/auth/ante/unorderedtx/manager.go +++ b/x/auth/ante/unorderedtx/manager.go @@ -28,11 +28,16 @@ const ( // TxHash defines a transaction hash type alias, which is a fixed array of 32 bytes. type TxHash [32]byte +type BlockInfo struct { + Time time.Time + Height uint64 +} + // Manager contains the tx hash dictionary for duplicates checking, and expire // them when block production progresses. type Manager struct { // blockCh defines a channel to receive newly committed block time - blockCh chan time.Time + blockCh chan BlockInfo // doneCh allows us to ensure the purgeLoop has gracefully terminated prior to closing doneCh chan struct{} @@ -64,7 +69,7 @@ func NewManager(dataDir string) *Manager { m := &Manager{ dataDir: dataDir, - blockCh: make(chan time.Time, 16), + blockCh: make(chan BlockInfo, 16), doneCh: make(chan struct{}), txHashes: make(map[TxHash]time.Time), } @@ -153,10 +158,10 @@ func (m *Manager) OnInit() error { return nil } -// OnNewBlock sends the latest block time to the background purge loop, which -// should be called in ABCI Commit event. -func (m *Manager) OnNewBlock(blockTime time.Time) { - m.blockCh <- blockTime +// OnNewBlock send the latest block time and height to the background purge loop, +// which should be called in the Preblock method. +func (m *Manager) OnNewBlock(blockTime time.Time, blockHeight uint64) { + m.blockCh <- BlockInfo{Time: blockTime, Height: blockHeight} } func (m *Manager) exportSnapshot(height uint64, snapshotWriter func([]byte) error) error { @@ -263,14 +268,14 @@ func (m *Manager) batchReceive() (time.Time, bool) { case <-ctx.Done(): return latestTime, true - case blockTime, ok := <-m.blockCh: + case blockInfo, ok := <-m.blockCh: if !ok { // channel is closed return time.Time{}, false } - if blockTime.After(latestTime) { - latestTime = blockTime + if blockInfo.Time.After(latestTime) { + latestTime = blockInfo.Time } } } diff --git a/x/auth/ante/unorderedtx/manager_test.go b/x/auth/ante/unorderedtx/manager_test.go index 8c16933fce7f..b48d40956908 100644 --- a/x/auth/ante/unorderedtx/manager_test.go +++ b/x/auth/ante/unorderedtx/manager_test.go @@ -116,12 +116,14 @@ func TestUnorderedTxManager_Flow(t *testing.T) { // start a goroutine that mimics new blocks being made every 500ms doneBlockCh := make(chan bool) + blockHeight := 1 go func() { ticker := time.NewTicker(time.Millisecond * 500) defer ticker.Stop() for t := range ticker.C { - txm.OnNewBlock(t) + txm.OnNewBlock(t, uint64(blockHeight)) + blockHeight++ if t.After(currentTime.Add(time.Millisecond * 500 * time.Duration(25))) { doneBlockCh <- true From dcc970523ffb5f411b429e571791a9198e2c7535 Mon Sep 17 00:00:00 2001 From: Facundo Date: Mon, 9 Sep 2024 12:22:28 +0200 Subject: [PATCH 15/20] Revert "timestamp and height" This reverts commit 0b9141a337248b13535a03a942d173081f49c74b. --- UPGRADING.md | 2 +- .../adr-070-unordered-transactions.md | 4 ++-- runtime/app.go | 2 +- simapp/app.go | 2 +- x/auth/ante/unorderedtx/manager.go | 23 ++++++++----------- x/auth/ante/unorderedtx/manager_test.go | 4 +--- 6 files changed, 15 insertions(+), 22 deletions(-) diff --git a/UPGRADING.md b/UPGRADING.md index 4eeda61ad27d..654e8a7d9272 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -286,7 +286,7 @@ If you are still using the legacy wiring, you must enable unordered transactions ... func (app *SimApp) PreBlocker(ctx sdk.Context, req *abci.RequestFinalizeBlock) (*sdk.ResponsePreBlock, error) { - app.UnorderedTxManager.OnNewBlock(ctx.BlockTime(), ctx.BlockHeight()) + app.UnorderedTxManager.OnNewBlock(ctx.BlockTime()) return app.ModuleManager.PreBlock(ctx, req) } ``` diff --git a/docs/architecture/adr-070-unordered-transactions.md b/docs/architecture/adr-070-unordered-transactions.md index 8be95a53411a..abcf63e76dd6 100644 --- a/docs/architecture/adr-070-unordered-transactions.md +++ b/docs/architecture/adr-070-unordered-transactions.md @@ -144,8 +144,8 @@ func (m *UnorderedTxManager) Add(hash TxHash, expire time.Time) { m.txHashes[hash] = expire } -// OnNewBlock send the latest block time and height to the background purge loop, -// which should be called in the Preblock method. +// OnNewBlock send the latest block time to the background purge loop, which +// should be called in ABCI Commit event. func (m *UnorderedTxManager) OnNewBlock(blockTime time.Time) { m.blockCh <- blockTime } diff --git a/runtime/app.go b/runtime/app.go index eccba955533a..f9a74a29a208 100644 --- a/runtime/app.go +++ b/runtime/app.go @@ -174,7 +174,7 @@ func (a *App) Close() error { // PreBlocker application updates every pre block func (a *App) PreBlocker(ctx sdk.Context, _ *abci.FinalizeBlockRequest) error { if a.UnorderedTxManager != nil { - a.UnorderedTxManager.OnNewBlock(ctx.BlockTime(), uint64(ctx.BlockHeight())) + a.UnorderedTxManager.OnNewBlock(ctx.BlockTime()) } return a.ModuleManager.PreBlock(ctx) } diff --git a/simapp/app.go b/simapp/app.go index cf910193940d..e000faede658 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -689,7 +689,7 @@ func (app *SimApp) Name() string { return app.BaseApp.Name() } // PreBlocker application updates every pre block func (app *SimApp) PreBlocker(ctx sdk.Context, _ *abci.FinalizeBlockRequest) error { - app.UnorderedTxManager.OnNewBlock(ctx.BlockTime(), uint64(ctx.BlockHeight())) + app.UnorderedTxManager.OnNewBlock(ctx.BlockTime()) return app.ModuleManager.PreBlock(ctx) } diff --git a/x/auth/ante/unorderedtx/manager.go b/x/auth/ante/unorderedtx/manager.go index 3a836e0ce36c..3d5b9cad1e37 100644 --- a/x/auth/ante/unorderedtx/manager.go +++ b/x/auth/ante/unorderedtx/manager.go @@ -28,16 +28,11 @@ const ( // TxHash defines a transaction hash type alias, which is a fixed array of 32 bytes. type TxHash [32]byte -type BlockInfo struct { - Time time.Time - Height uint64 -} - // Manager contains the tx hash dictionary for duplicates checking, and expire // them when block production progresses. type Manager struct { // blockCh defines a channel to receive newly committed block time - blockCh chan BlockInfo + blockCh chan time.Time // doneCh allows us to ensure the purgeLoop has gracefully terminated prior to closing doneCh chan struct{} @@ -69,7 +64,7 @@ func NewManager(dataDir string) *Manager { m := &Manager{ dataDir: dataDir, - blockCh: make(chan BlockInfo, 16), + blockCh: make(chan time.Time, 16), doneCh: make(chan struct{}), txHashes: make(map[TxHash]time.Time), } @@ -158,10 +153,10 @@ func (m *Manager) OnInit() error { return nil } -// OnNewBlock send the latest block time and height to the background purge loop, -// which should be called in the Preblock method. -func (m *Manager) OnNewBlock(blockTime time.Time, blockHeight uint64) { - m.blockCh <- BlockInfo{Time: blockTime, Height: blockHeight} +// OnNewBlock sends the latest block time to the background purge loop, which +// should be called in ABCI Commit event. +func (m *Manager) OnNewBlock(blockTime time.Time) { + m.blockCh <- blockTime } func (m *Manager) exportSnapshot(height uint64, snapshotWriter func([]byte) error) error { @@ -268,14 +263,14 @@ func (m *Manager) batchReceive() (time.Time, bool) { case <-ctx.Done(): return latestTime, true - case blockInfo, ok := <-m.blockCh: + case blockTime, ok := <-m.blockCh: if !ok { // channel is closed return time.Time{}, false } - if blockInfo.Time.After(latestTime) { - latestTime = blockInfo.Time + if blockTime.After(latestTime) { + latestTime = blockTime } } } diff --git a/x/auth/ante/unorderedtx/manager_test.go b/x/auth/ante/unorderedtx/manager_test.go index b48d40956908..8c16933fce7f 100644 --- a/x/auth/ante/unorderedtx/manager_test.go +++ b/x/auth/ante/unorderedtx/manager_test.go @@ -116,14 +116,12 @@ func TestUnorderedTxManager_Flow(t *testing.T) { // start a goroutine that mimics new blocks being made every 500ms doneBlockCh := make(chan bool) - blockHeight := 1 go func() { ticker := time.NewTicker(time.Millisecond * 500) defer ticker.Stop() for t := range ticker.C { - txm.OnNewBlock(t, uint64(blockHeight)) - blockHeight++ + txm.OnNewBlock(t) if t.After(currentTime.Add(time.Millisecond * 500 * time.Duration(25))) { doneBlockCh <- true From 4b304923f1109b7d0a1315f391a1a814ab348a8f Mon Sep 17 00:00:00 2001 From: Facundo Date: Mon, 9 Sep 2024 15:46:56 +0200 Subject: [PATCH 16/20] we don't care about timestamp in snapshot --- x/auth/ante/unorderedtx/manager.go | 6 +----- x/auth/ante/unorderedtx/snapshotter.go | 5 +++-- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/x/auth/ante/unorderedtx/manager.go b/x/auth/ante/unorderedtx/manager.go index 3d5b9cad1e37..7c66ef2f23f7 100644 --- a/x/auth/ante/unorderedtx/manager.go +++ b/x/auth/ante/unorderedtx/manager.go @@ -164,13 +164,9 @@ func (m *Manager) exportSnapshot(height uint64, snapshotWriter func([]byte) erro w := bufio.NewWriter(&buf) keys := slices.SortedFunc(maps.Keys(m.txHashes), func(i, j TxHash) int { return bytes.Compare(i[:], j[:]) }) - timestamp := time.Unix(int64(height), 0) for _, txHash := range keys { timeoutTime := m.txHashes[txHash] - if timestamp.After(timeoutTime) { - // skip expired txs that have yet to be purged - continue - } + // right now we dont have access block time at this flow, so we would just include the expired txs // and let it be purge during purge loop chunk := unorderedTxToBytes(txHash, uint64(timeoutTime.Unix())) diff --git a/x/auth/ante/unorderedtx/snapshotter.go b/x/auth/ante/unorderedtx/snapshotter.go index 39142e956bb0..4c855c9e3167 100644 --- a/x/auth/ante/unorderedtx/snapshotter.go +++ b/x/auth/ante/unorderedtx/snapshotter.go @@ -81,8 +81,9 @@ func (s *Snapshotter) restore(height uint64, payloadReader snapshot.ExtensionPay timestamp := binary.BigEndian.Uint64(payload[i+txHashSize : i+chunkSize]) - // purge any expired txs - if timestamp != 0 && timestamp > height { + // add all txs, we don't care at this point if they are expired, + // we'll let the purge loop handle that + if timestamp != 0 { s.m.Add(txHash, time.Unix(int64(timestamp), 0)) } From cf9fd707b2cfc01eec7c7bd1a54451bfcccb75a2 Mon Sep 17 00:00:00 2001 From: Facundo Date: Tue, 10 Sep 2024 15:16:46 +0200 Subject: [PATCH 17/20] finally fix this --- x/auth/ante/unordered_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/auth/ante/unordered_test.go b/x/auth/ante/unordered_test.go index b02fcc28c471..41100856b578 100644 --- a/x/auth/ante/unordered_test.go +++ b/x/auth/ante/unordered_test.go @@ -8,13 +8,13 @@ import ( "cosmossdk.io/core/header" storetypes "cosmossdk.io/store/types" - "cosmossdk.io/x/auth/ante" - "cosmossdk.io/x/auth/ante/unorderedtx" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/tx/signing" + "github.com/cosmos/cosmos-sdk/x/auth/ante" + "github.com/cosmos/cosmos-sdk/x/auth/ante/unorderedtx" ) const gasConsumed = uint64(25) From 274c5965ea364c3859f1683d2b401b499ff09067 Mon Sep 17 00:00:00 2001 From: Facundo Date: Wed, 11 Sep 2024 12:18:03 +0200 Subject: [PATCH 18/20] fix test --- x/auth/ante/unorderedtx/manager.go | 4 +++- x/auth/ante/unorderedtx/snapshotter_test.go | 14 +++++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/x/auth/ante/unorderedtx/manager.go b/x/auth/ante/unorderedtx/manager.go index 7c66ef2f23f7..4dbcbbac3ffc 100644 --- a/x/auth/ante/unorderedtx/manager.go +++ b/x/auth/ante/unorderedtx/manager.go @@ -98,6 +98,7 @@ func (m *Manager) Contains(hash TxHash) bool { } func (m *Manager) Size() int { + fmt.Println("Size") m.mu.RLock() defer m.mu.RUnlock() @@ -159,7 +160,7 @@ func (m *Manager) OnNewBlock(blockTime time.Time) { m.blockCh <- blockTime } -func (m *Manager) exportSnapshot(height uint64, snapshotWriter func([]byte) error) error { +func (m *Manager) exportSnapshot(_ uint64, snapshotWriter func([]byte) error) error { var buf bytes.Buffer w := bufio.NewWriter(&buf) @@ -243,6 +244,7 @@ func (m *Manager) purgeLoop() { } hashes := m.expiredTxs(latestTime) + fmt.Println("Purging", len(hashes)) if len(hashes) > 0 { m.purge(hashes) } diff --git a/x/auth/ante/unorderedtx/snapshotter_test.go b/x/auth/ante/unorderedtx/snapshotter_test.go index a2e66bfd869f..fc5ce003d896 100644 --- a/x/auth/ante/unorderedtx/snapshotter_test.go +++ b/x/auth/ante/unorderedtx/snapshotter_test.go @@ -39,12 +39,20 @@ func TestSnapshotter(t *testing.T) { err = s.RestoreExtension(50, 2, pr) require.Error(t, err) - // restore with timestamp > timeout time which should result in no unordered txs synced + // restore with timestamp > timeout time which should result in all unordered txs synced, + // even the ones that have timed out. txm2 := unorderedtx.NewManager(dataDir) s2 := unorderedtx.NewSnapshotter(txm2) - err = s2.RestoreExtension(uint64(currentTime.Add(time.Second*200).Unix()), unorderedtx.SnapshotFormat, pr) + err = s2.RestoreExtension(1, unorderedtx.SnapshotFormat, pr) require.NoError(t, err) - require.Empty(t, txm2.Size()) + require.Equal(t, 100, txm2.Size()) + + // start the manager and wait a bit for the background purge loop to run + txm2.Start() + time.Sleep(time.Millisecond * 5) + txm2.OnNewBlock(currentTime.Add(time.Second * 200)) + time.Sleep(time.Second * 5) // the loop runs every 5 seconds, so we need to wait for that + require.Equal(t, 0, txm2.Size()) // restore with timestamp < timeout time which should result in all unordered txs synced txm3 := unorderedtx.NewManager(dataDir) From 0fcc166662de1c9aabe11c5597ada44af5c600be Mon Sep 17 00:00:00 2001 From: Facundo Date: Wed, 11 Sep 2024 12:18:57 +0200 Subject: [PATCH 19/20] comment --- x/auth/ante/unorderedtx/manager.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x/auth/ante/unorderedtx/manager.go b/x/auth/ante/unorderedtx/manager.go index 4dbcbbac3ffc..6c7afbac5fe3 100644 --- a/x/auth/ante/unorderedtx/manager.go +++ b/x/auth/ante/unorderedtx/manager.go @@ -251,6 +251,8 @@ func (m *Manager) purgeLoop() { } } +// batchReceive receives block time from the channel until the context is done +// or the channel is closed. func (m *Manager) batchReceive() (time.Time, bool) { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() From 9974f61ce817558c192433609868e9c799139017 Mon Sep 17 00:00:00 2001 From: Facundo Date: Wed, 11 Sep 2024 13:28:48 +0200 Subject: [PATCH 20/20] remove test prints --- x/auth/ante/unorderedtx/manager.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/x/auth/ante/unorderedtx/manager.go b/x/auth/ante/unorderedtx/manager.go index 6c7afbac5fe3..2d103b709ec8 100644 --- a/x/auth/ante/unorderedtx/manager.go +++ b/x/auth/ante/unorderedtx/manager.go @@ -98,7 +98,6 @@ func (m *Manager) Contains(hash TxHash) bool { } func (m *Manager) Size() int { - fmt.Println("Size") m.mu.RLock() defer m.mu.RUnlock() @@ -244,7 +243,6 @@ func (m *Manager) purgeLoop() { } hashes := m.expiredTxs(latestTime) - fmt.Println("Purging", len(hashes)) if len(hashes) > 0 { m.purge(hashes) }