diff --git a/app/app.go b/app/app.go index 9f26c8e5e..a78e24634 100644 --- a/app/app.go +++ b/app/app.go @@ -30,6 +30,7 @@ import ( v13_1 "github.com/classic-terra/core/v4/app/upgrades/v13_1" v14_1 "github.com/classic-terra/core/v4/app/upgrades/v14_1" v14_2 "github.com/classic-terra/core/v4/app/upgrades/v14_2" + "github.com/classic-terra/core/v4/app/upgrades/v14_3" v2 "github.com/classic-terra/core/v4/app/upgrades/v2" v3 "github.com/classic-terra/core/v4/app/upgrades/v3" v4 "github.com/classic-terra/core/v4/app/upgrades/v4" @@ -107,6 +108,7 @@ var ( v13_1.Upgrade, v14_1.Upgrade, v14_2.Upgrade, + v14_3.Upgrade, } // Forks defines forks to be applied to the network @@ -175,9 +177,9 @@ func NewTerraApp( baseAppOptions = append(baseAppOptions, func(app *baseapp.BaseApp) { var mempool *appmempool.FifoMempool if maxTxs := cast.ToInt(appOpts.Get(server.FlagMempoolMaxTxs)); maxTxs > 0 { - mempool = appmempool.NewFifoMempool(appmempool.FifoMaxTxOpt(maxTxs)) + mempool = appmempool.NewFifoMempool(appmempool.FifoMaxTxOpt(maxTxs), appmempool.FifoTxEncoderOpt(txConfig.TxEncoder())) } else { - mempool = appmempool.NewFifoMempool() + mempool = appmempool.NewFifoMempool(appmempool.FifoTxEncoderOpt(txConfig.TxEncoder())) } handler := baseapp.NewDefaultProposalHandler(mempool, app) app.SetMempool(mempool) @@ -278,6 +280,8 @@ func NewTerraApp( panic("error while reading wasm config: " + err.Error()) } + replacementTracker := customante.NewReplacementTracker() + anteHandler, err := customante.NewAnteHandler( customante.HandlerOptions{ AccountKeeper: app.AccountKeeper, @@ -298,6 +302,8 @@ func NewTerraApp( StakingKeeper: app.StakingKeeper, TaxKeeper: &app.TaxKeeper, Cdc: app.appCodec, + CommitMultiStore: app.CommitMultiStore(), + ReplacementTracker: replacementTracker, }, ) if err != nil { diff --git a/app/mempool/mempool_fifo.go b/app/mempool/mempool_fifo.go index 3a5972823..f2110c094 100644 --- a/app/mempool/mempool_fifo.go +++ b/app/mempool/mempool_fifo.go @@ -40,7 +40,9 @@ type FifoMempool struct { txsOracle *clist.CList // Oracle transactions FIFO queue txsMap sync.Map // For quick lookup of existing transactions txsMapOracle sync.Map // For quick lookup of existing transactions + txsBytes sync.Map // For distinguishing replacements with the same sender and nonce maxTx int + txEncoder sdk.TxEncoder } type FifoMempoolOptions func(mp *FifoMempool) @@ -65,13 +67,15 @@ func FifoMaxTxOpt(maxTx int) FifoMempoolOptions { } } -func (mp *FifoMempool) Insert(_ context.Context, tx sdk.Tx) error { - mp.mtx.RLock() - defer mp.mtx.RUnlock() - totalTxs := mp.txs.Len() + mp.txsOracle.Len() - if mp.maxTx >= 0 && totalTxs >= mp.maxTx { - return mempool.ErrMempoolTxMaxCapacity +func FifoTxEncoderOpt(txEncoder sdk.TxEncoder) FifoMempoolOptions { + return func(mp *FifoMempool) { + mp.txEncoder = txEncoder } +} + +func (mp *FifoMempool) Insert(_ context.Context, tx sdk.Tx) error { + mp.mtx.Lock() + defer mp.mtx.Unlock() if mp.maxTx < 0 { return nil } @@ -80,14 +84,57 @@ func (mp *FifoMempool) Insert(_ context.Context, tx sdk.Tx) error { if err != nil { return err } + txBytes, err := mp.getTxBytes(tx) + if err != nil { + return err + } + + isOracle := helper.IsOracleTx(tx.GetMsgs()) + if elem, ok := mp.txsMap.Load(txKey); ok { + if !isOracle { + // In-place replacement requires an encoder to distinguish the new tx + // from the old one at Remove time. Without one, skip the update so + // a later Remove of the superseded tx cannot accidentally evict the + // current one. + if mp.txEncoder != nil { + elem.(*clist.CElement).Value = tx + mp.txsBytes.Store(txKey, txBytes) + } + return nil + } + mp.txsMap.Delete(txKey) + mp.txsBytes.Delete(txKey) + mp.txs.Remove(elem.(*clist.CElement)) + } + if elem, ok := mp.txsMapOracle.Load(txKey); ok { + if isOracle { + if mp.txEncoder != nil { + elem.(*clist.CElement).Value = tx + mp.txsBytes.Store(txKey, txBytes) + } + return nil + } + mp.txsMapOracle.Delete(txKey) + mp.txsBytes.Delete(txKey) + mp.txsOracle.Remove(elem.(*clist.CElement)) + } + + totalTxs := mp.txs.Len() + mp.txsOracle.Len() + if mp.maxTx >= 0 && totalTxs >= mp.maxTx { + return mempool.ErrMempoolTxMaxCapacity + } + // Add to appropriate queue based on transaction type - if helper.IsOracleTx(tx.GetMsgs()) { + if isOracle { e := mp.txsOracle.PushBack(tx) mp.txsMapOracle.Store(txKey, e) } else { e := mp.txs.PushBack(tx) mp.txsMap.Store(txKey, e) } + if txBytes != "" { + mp.txsBytes.Store(txKey, txBytes) + } return nil } @@ -159,21 +206,35 @@ func (it *fifoIterator) Tx() sdk.Tx { } func (mp *FifoMempool) Remove(tx sdk.Tx) error { - mp.mtx.RLock() - defer mp.mtx.RUnlock() + mp.mtx.Lock() + defer mp.mtx.Unlock() txKey, err := getTxKey(tx) if err != nil { return err } + txBytes, err := mp.getTxBytes(tx) + if err != nil { + return err + } isOracle := helper.IsOracleTx(tx.GetMsgs()) if isOracle { - if elem, ok := mp.txsMapOracle.LoadAndDelete(txKey); ok { + if elem, ok := mp.txsMapOracle.Load(txKey); ok { + if !mp.matchesStoredTx(txKey, txBytes) { + return nil + } + mp.txsMapOracle.Delete(txKey) + mp.txsBytes.Delete(txKey) mp.txsOracle.Remove(elem.(*clist.CElement)) return nil } } else { - if elem, ok := mp.txsMap.LoadAndDelete(txKey); ok { + if elem, ok := mp.txsMap.Load(txKey); ok { + if !mp.matchesStoredTx(txKey, txBytes) { + return nil + } + mp.txsMap.Delete(txKey) + mp.txsBytes.Delete(txKey) mp.txs.Remove(elem.(*clist.CElement)) return nil } @@ -182,6 +243,13 @@ func (mp *FifoMempool) Remove(tx sdk.Tx) error { return mempool.ErrTxNotFound } +func (mp *FifoMempool) matchesStoredTx(txKey customTxKey, txBytes string) bool { + storedTxBytes, ok := mp.txsBytes.Load(txKey) + // No entry means no encoder was configured (bytes were never stored), so we + // have no discriminator and must allow the removal. + return !ok || storedTxBytes == txBytes +} + func (mp *FifoMempool) CountTx() int { mp.mtx.RLock() defer mp.mtx.RUnlock() @@ -204,6 +272,19 @@ func getTxKey(tx sdk.Tx) (customTxKey, error) { return key, nil } +func (mp *FifoMempool) getTxBytes(tx sdk.Tx) (string, error) { + if mp.txEncoder == nil { + return "", nil + } + + txBytes, err := mp.txEncoder(tx) + if err != nil { + return "", err + } + + return string(txBytes), nil +} + type customTxKey struct { address string nonce uint64 diff --git a/app/mempool/mempool_fifo_test.go b/app/mempool/mempool_fifo_test.go index fd47bf471..f763a7a89 100644 --- a/app/mempool/mempool_fifo_test.go +++ b/app/mempool/mempool_fifo_test.go @@ -391,6 +391,48 @@ func (s *MempoolTestSuite) TestTxNotFoundOnSender() { require.Equal(t, mempool.ErrTxNotFound, err) } +func (s *MempoolTestSuite) TestDuplicateSenderNonceReplacesTx() { + t := s.T() + ctx := sdk.NewContext(nil, tmproto.Header{}, false, log.NewNopLogger()) + accounts := simtypes.RandomAccounts(rand.New(rand.NewSource(0)), 1) + addr := accounts[0].Address + // Use a deterministic encoder keyed by testTx.id so replacement detection + // works correctly even when different object instances represent the same tx. + mockEnc := func(tx sdk.Tx) ([]byte, error) { + return []byte(fmt.Sprintf("%d", tx.(testTx).id)), nil + } + mp := appmempool.NewFifoMempool(appmempool.FifoTxEncoderOpt(mockEnc)) + + originalTx := testTx{ + id: 1, + nonce: 432, + address: addr, + priority: rand.Int63(), + msgs: []sdk.Msg{&banktypes.MsgSend{}}, + } + replacementTx := testTx{ + id: 2, + nonce: 432, + address: addr, + priority: rand.Int63(), + msgs: []sdk.Msg{&banktypes.MsgSend{}}, + } + + require.NoError(t, mp.Insert(ctx, originalTx)) + require.NoError(t, mp.Insert(ctx, replacementTx)) + require.Equal(t, 1, mp.CountTx()) + + itr := mp.Select(ctx, nil) + selectedTxs := fetchTxs(itr, 1000) + require.Len(t, selectedTxs, 1) + require.Equal(t, 2, selectedTxs[0].(testTx).id) + + require.NoError(t, mp.Remove(originalTx)) + require.Equal(t, 1, mp.CountTx()) + require.NoError(t, mp.Remove(replacementTx)) + require.Equal(t, 0, mp.CountTx()) +} + func (s *MempoolTestSuite) TestBatchTx_WhenEnoughMemPool() { t := s.T() ctx := sdk.NewContext(nil, tmproto.Header{}, false, log.NewNopLogger()) diff --git a/app/upgrades/v14_3/constants.go b/app/upgrades/v14_3/constants.go new file mode 100644 index 000000000..4672a7303 --- /dev/null +++ b/app/upgrades/v14_3/constants.go @@ -0,0 +1,21 @@ +//nolint:revive +package v14_3 + +import ( + store "cosmossdk.io/store/types" + "github.com/classic-terra/core/v4/app/upgrades" +) + +const UpgradeName = "v14_3" + +var Upgrade = upgrades.Upgrade{ + UpgradeName: UpgradeName, + CreateUpgradeHandler: CreateV143UpgradeHandler, + // Add new stores introduced since the last upgrade here. If there are + // no new stores for this upgrade, leave this empty. + StoreUpgrades: store.StoreUpgrades{ + Added: []string{}, + Deleted: []string{}, + Renamed: []store.StoreRename{}, + }, +} diff --git a/app/upgrades/v14_3/upgrades.go b/app/upgrades/v14_3/upgrades.go new file mode 100644 index 000000000..096da614d --- /dev/null +++ b/app/upgrades/v14_3/upgrades.go @@ -0,0 +1,24 @@ +//nolint:revive +package v14_3 + +import ( + "context" + + upgradetypes "cosmossdk.io/x/upgrade/types" + "github.com/classic-terra/core/v4/app/keepers" + "github.com/classic-terra/core/v4/app/upgrades" + "github.com/cosmos/cosmos-sdk/types/module" +) + +// CreateV143UpgradeHandler wires module migrations for v14_3. +// Add any one-off migration logic here before/after RunMigrations if needed. +func CreateV143UpgradeHandler( + mm *module.Manager, + cfg module.Configurator, + _ upgrades.BaseAppParamManager, + keepers *keepers.AppKeepers, +) upgradetypes.UpgradeHandler { + return func(ctx context.Context, _ upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { + return mm.RunMigrations(ctx, cfg, fromVM) + } +} diff --git a/custom/auth/ante/ante.go b/custom/auth/ante/ante.go index f48993579..1279ae00e 100644 --- a/custom/auth/ante/ante.go +++ b/custom/auth/ante/ante.go @@ -3,6 +3,7 @@ package ante import ( corestoretypes "cosmossdk.io/core/store" errorsmod "cosmossdk.io/errors" + storetypes "cosmossdk.io/store/types" txsigning "cosmossdk.io/x/tx/signing" wasmkeeper "github.com/CosmWasm/wasmd/x/wasm/keeper" wasmtypes "github.com/CosmWasm/wasmd/x/wasm/types" @@ -43,6 +44,8 @@ type HandlerOptions struct { StakingKeeper *stakingkeeper.Keeper TaxKeeper *taxkeeper.Keeper Cdc codec.BinaryCodec + CommitMultiStore storetypes.CommitMultiStore + ReplacementTracker *ReplacementTracker } // NewAnteHandler returns an AnteHandler that checks and increments sequence @@ -81,6 +84,14 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) { return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "tax handler is required for ante builder") } + if options.ReplacementTracker == nil { + return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "replacement tracker is required for ante builder") + } + + if options.CommitMultiStore == nil { + return nil, errorsmod.Wrap(sdkerrors.ErrLogic, "commit multi store is required for ante builder") + } + return sdk.ChainAnteDecorators( ante.NewSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first wasmkeeper.NewLimitSimulationGasDecorator(options.WasmConfig.SimulationGasLimit), @@ -96,6 +107,7 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) { // MinInitialDepositDecorator prevents submitting governance proposal low initial deposit NewMinInitialDepositDecorator(options.GovKeeper, options.TreasuryKeeper), ante.NewConsumeGasForTxSizeDecorator(options.AccountKeeper), + NewTxReplacementDecorator(options.AccountKeeper, options.CommitMultiStore, options.ReplacementTracker), NewFeeDecorator(options.AccountKeeper, options.BankKeeper, options.FeegrantKeeper, options.TaxExemptionKeeper, options.TreasuryKeeper, options.DistributionKeeper, *options.TaxKeeper), dyncommante.NewDyncommDecorator(options.Cdc, options.DyncommKeeper, options.StakingKeeper), diff --git a/custom/auth/ante/replacement.go b/custom/auth/ante/replacement.go new file mode 100644 index 000000000..9b07bf3f7 --- /dev/null +++ b/custom/auth/ante/replacement.go @@ -0,0 +1,271 @@ +package ante + +import ( + "sync" + + storetypes "cosmossdk.io/store/types" + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/cosmos/cosmos-sdk/x/auth/ante" + authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" +) + +// ReplacementTracker tracks pending tx replacements so old txs can be +// evicted during CometBFT recheck. Thread-safe. +type ReplacementTracker struct { + mu sync.RWMutex + replacements map[string]*ReplacementInfo +} + +type ReplacementInfo struct { + FromSequence uint64 + newTxBytesSet map[string]struct{} // keyed by string(txBytes) for O(1) membership +} + +// Contains reports whether txBytes is a registered replacement for this entry. +func (info *ReplacementInfo) Contains(txBytes []byte) bool { + _, ok := info.newTxBytesSet[string(txBytes)] + return ok +} + +func NewReplacementTracker() *ReplacementTracker { + return &ReplacementTracker{ + replacements: make(map[string]*ReplacementInfo), + } +} + +func (rt *ReplacementTracker) Set(sender string, fromSeq uint64, newTxBytes []byte) { + rt.mu.Lock() + defer rt.mu.Unlock() + info, exists := rt.replacements[sender] + if !exists || info.FromSequence != fromSeq { + // New sender or different sequence: start a fresh set. + rt.replacements[sender] = &ReplacementInfo{ + FromSequence: fromSeq, + newTxBytesSet: map[string]struct{}{string(newTxBytes): {}}, + } + return + } + // Same sequence: register alongside any prior ones so all rapid replacements + // are tracked and none is incorrectly evicted during recheck. + // + // Copy-on-write: callers that received a *ReplacementInfo via Get hold a + // reference to the old map and may call Contains concurrently. Mutating + // the existing map in-place while they read it would be a data race. + // Publishing a brand-new ReplacementInfo with a fresh map is safe because + // the old pointer remains valid for already-in-progress readers. + newSet := make(map[string]struct{}, len(info.newTxBytesSet)+1) + for k := range info.newTxBytesSet { + newSet[k] = struct{}{} + } + newSet[string(newTxBytes)] = struct{}{} + rt.replacements[sender] = &ReplacementInfo{ + FromSequence: fromSeq, + newTxBytesSet: newSet, + } +} + +// RemoveTxBytes removes a single replacement byte string from the tracked set. +// When the last entry is removed the sender key is deleted entirely, which +// preserves the invariant that tracker.Get returns nil once all replacements +// have been rechecked successfully. Copy-on-write is used for the same +// concurrency reason as in Set. +func (rt *ReplacementTracker) RemoveTxBytes(sender string, txBytes []byte) { + rt.mu.Lock() + defer rt.mu.Unlock() + info, ok := rt.replacements[sender] + if !ok { + return + } + key := string(txBytes) + if _, found := info.newTxBytesSet[key]; !found { + return + } + if len(info.newTxBytesSet) <= 1 { + delete(rt.replacements, sender) + return + } + newSet := make(map[string]struct{}, len(info.newTxBytesSet)-1) + for k := range info.newTxBytesSet { + if k != key { + newSet[k] = struct{}{} + } + } + rt.replacements[sender] = &ReplacementInfo{ + FromSequence: info.FromSequence, + newTxBytesSet: newSet, + } +} + +func (rt *ReplacementTracker) Get(sender string) *ReplacementInfo { + rt.mu.RLock() + defer rt.mu.RUnlock() + return rt.replacements[sender] +} + +func (rt *ReplacementTracker) Clear(sender string) { + rt.mu.Lock() + defer rt.mu.Unlock() + delete(rt.replacements, sender) +} + +// TxReplacementDecorator allows a sender to replace a stuck pending tx by +// submitting a new tx at the same (committed) sequence number. +// +// On new CheckTx: +// - If the tx sequence < pending sequence but == committed sequence, +// the account is reset to committed state so the replacement can pass +// signature verification. +// +// On recheck: +// - Old txs from the same sender (at or above the replaced sequence) are +// rejected, causing CometBFT to evict them from its mempool. +type TxReplacementDecorator struct { + ak ante.AccountKeeper + cms storetypes.CommitMultiStore + tracker *ReplacementTracker +} + +func NewTxReplacementDecorator( + ak ante.AccountKeeper, + cms storetypes.CommitMultiStore, + tracker *ReplacementTracker, +) TxReplacementDecorator { + return TxReplacementDecorator{ak: ak, cms: cms, tracker: tracker} +} + +func (d TxReplacementDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { + if simulate { + return next(ctx, tx, simulate) + } + + if ctx.IsReCheckTx() { + return d.handleRecheck(ctx, tx, next) + } + + if ctx.IsCheckTx() { + return d.handleNewTx(ctx, tx, next) + } + + return next(ctx, tx, false) +} + +// handleRecheck rejects the old tx at fromSequence that has been superseded by +// a replacement. Subsequent txs (seq > fromSequence) are left in the pool so +// CometBFT's normal sequence validation handles them after each committed block. +func (d TxReplacementDecorator) handleRecheck(ctx sdk.Context, tx sdk.Tx, next sdk.AnteHandler) (sdk.Context, error) { + sender, seq, err := firstSignerSeq(tx) + if err != nil { + return next(ctx, tx, false) + } + + info := d.tracker.Get(sender) + if info == nil { + return next(ctx, tx, false) + } + + // This is one of the registered replacement txs — reset the account and allow it. + if info.Contains(ctx.TxBytes()) { + committedAcc := d.getCommittedAccount(ctx, sdk.MustAccAddressFromBech32(sender)) + // Reset account to committed state when available so SigVerificationDecorator + // sees the correct sequence. If committedAcc is nil (very rare: pruned account), + // skip the reset and let next() try with whatever state it has. + if committedAcc != nil && seq == committedAcc.GetSequence() { + d.ak.SetAccount(ctx, committedAcc) + } + // Remove only this replacement's bytes after next() succeeds. Using + // RemoveTxBytes instead of Clear ensures that if a second replacement for + // the same sender/sequence was registered, the sender entry (and its + // fromSequence eviction rule) survives until all replacements have been + // rechecked. If next() fails keep the tracker alive so the replacement + // can be retried rather than silently losing both txs. + newCtx, err := next(ctx, tx, false) + if err == nil { + d.tracker.RemoveTxBytes(sender, ctx.TxBytes()) + } + return newCtx, err + } + + // Evict only the original stuck tx at the replaced sequence. + // Txs at seq > fromSequence are valid and will be handled by normal + // sequence validation as the committed sequence advances. + if seq == info.FromSequence { + return ctx, sdkerrors.ErrWrongSequence.Wrapf( + "tx superseded: a replacement tx was submitted for %s at sequence %d", + sender, info.FromSequence, + ) + } + + return next(ctx, tx, false) +} + +// handleNewTx detects when a tx's sequence equals the committed (on-chain) +// sequence but is below the pending (mempool-accumulated) sequence. In that +// case, the account state in the ante context is reset to the committed +// version so that the signature/sequence check can pass. +func (d TxReplacementDecorator) handleNewTx(ctx sdk.Context, tx sdk.Tx, next sdk.AnteHandler) (sdk.Context, error) { + sender, txSeq, err := firstSignerSeq(tx) + if err != nil { + return next(ctx, tx, false) + } + + acc := d.ak.GetAccount(ctx, sdk.MustAccAddressFromBech32(sender)) + if acc == nil { + return next(ctx, tx, false) + } + + pendingSeq := acc.GetSequence() + if txSeq >= pendingSeq { + return next(ctx, tx, false) + } + + // txSeq < pendingSeq → possible replacement. + // Verify it matches the committed (on-chain) sequence. + committedAcc := d.getCommittedAccount(ctx, sdk.MustAccAddressFromBech32(sender)) + if committedAcc == nil { + return next(ctx, tx, false) + } + + committedSeq := committedAcc.GetSequence() + if txSeq != committedSeq { + // Only allow replacement starting from the committed sequence. + return next(ctx, tx, false) + } + + // Reset the account in the (branched) ante context to committed state. + // If the full ante chain succeeds the branch is written; otherwise discarded. + d.ak.SetAccount(ctx, committedAcc) + + d.tracker.Set(sender, committedSeq, ctx.TxBytes()) + + newCtx, err := next(ctx, tx, false) + if err != nil { + // Remove only this tx's bytes so a previously registered replacement for + // the same sender is not accidentally wiped if this attempt fails downstream. + d.tracker.RemoveTxBytes(sender, ctx.TxBytes()) + return newCtx, err + } + return newCtx, nil +} + +func (d TxReplacementDecorator) getCommittedAccount(ctx sdk.Context, addr sdk.AccAddress) sdk.AccountI { + committedCtx := ctx.WithMultiStore(d.cms) + return d.ak.GetAccount(committedCtx, addr) +} + +// firstSignerSeq returns the bech32 address and sequence of the first signer. +func firstSignerSeq(tx sdk.Tx) (string, uint64, error) { + sigTx, ok := tx.(authsigning.SigVerifiableTx) + if !ok { + return "", 0, sdkerrors.ErrTxDecode.Wrap("tx is not SigVerifiableTx") + } + sigs, err := sigTx.GetSignaturesV2() + if err != nil { + return "", 0, err + } + if len(sigs) == 0 { + return "", 0, sdkerrors.ErrNoSignatures.Wrap("tx has no signatures") + } + addr := sdk.AccAddress(sigs[0].PubKey.Address()).String() + return addr, sigs[0].Sequence, nil +} diff --git a/custom/auth/ante/replacement_test.go b/custom/auth/ante/replacement_test.go new file mode 100644 index 000000000..0b69152fb --- /dev/null +++ b/custom/auth/ante/replacement_test.go @@ -0,0 +1,448 @@ +package ante_test + +import ( + "testing" + + sdkmath "cosmossdk.io/math" + "github.com/classic-terra/core/v4/custom/auth/ante" + core "github.com/classic-terra/core/v4/types" + abci "github.com/cometbft/cometbft/abci/types" + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" + "github.com/cosmos/cosmos-sdk/testutil/testdata" + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/cosmos/cosmos-sdk/types/mempool" + authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" + banktestutil "github.com/cosmos/cosmos-sdk/x/bank/testutil" + banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" +) + +// --------------------------------------------------------------------------- +// ReplacementTracker — pure unit tests (no app needed) +// --------------------------------------------------------------------------- + +func TestReplacementTracker(t *testing.T) { + tracker := ante.NewReplacementTracker() + const sender = "terra1abc" + orig := []byte("new-tx-bytes") + + require.Nil(t, tracker.Get(sender)) + + tracker.Set(sender, 432, orig) + + info := tracker.Get(sender) + require.NotNil(t, info) + require.Equal(t, uint64(432), info.FromSequence) + require.True(t, info.Contains(orig)) + + // Set must store a copy; mutating the slice must not affect stored bytes. + origCopy := []byte("new-tx-bytes") + orig[0] = 'X' + require.True(t, info.Contains(origCopy)) // original bytes still registered + require.False(t, info.Contains(orig)) // mutated bytes are not + + tracker.Clear(sender) + require.Nil(t, tracker.Get(sender)) + + // Clear on unknown sender is a no-op. + tracker.Clear("unknown") + + // Multiple rapid replacements at the same sequence must all be registered so + // the first replacement is not evicted when a second arrives. + rep1 := []byte("replacement-1") + rep2 := []byte("replacement-2") + tracker.Set(sender, 5, rep1) + tracker.Set(sender, 5, rep2) + info = tracker.Get(sender) + require.NotNil(t, info) + require.True(t, info.Contains(rep1)) + require.True(t, info.Contains(rep2)) + + // A new sequence must reset the set entirely. + tracker.Set(sender, 6, rep1) + info = tracker.Get(sender) + require.Equal(t, uint64(6), info.FromSequence) + require.True(t, info.Contains(rep1)) + require.False(t, info.Contains(rep2)) + + // RemoveTxBytes: removing one of two entries keeps the sender key alive + // so the original stuck tx is still evicted on the next recheck round. + tracker.Set(sender, 7, rep1) + tracker.Set(sender, 7, rep2) + tracker.RemoveTxBytes(sender, rep1) + info = tracker.Get(sender) + require.NotNil(t, info, "sender entry must survive after first of two replacements rechecks") + require.False(t, info.Contains(rep1)) + require.True(t, info.Contains(rep2)) + + // Removing the last entry must delete the whole sender key. + tracker.RemoveTxBytes(sender, rep2) + require.Nil(t, tracker.Get(sender)) + + // RemoveTxBytes on an unknown sender or unknown bytes is a no-op. + tracker.RemoveTxBytes("unknown", rep1) + tracker.Set(sender, 8, rep1) + tracker.RemoveTxBytes(sender, rep2) // rep2 not in set — must not panic + require.NotNil(t, tracker.Get(sender)) +} + +// --------------------------------------------------------------------------- +// TxReplacementDecorator — behaviour tests +// --------------------------------------------------------------------------- + +type ReplacementDecoratorSuite struct { + AnteTestSuite +} + +func TestReplacementDecoratorSuite(t *testing.T) { + suite.Run(t, new(ReplacementDecoratorSuite)) +} + +func noopHandler(ctx sdk.Context, _ sdk.Tx, _ bool) (sdk.Context, error) { + return ctx, nil +} + +func (s *ReplacementDecoratorSuite) makeDecorator(tracker *ante.ReplacementTracker) ante.TxReplacementDecorator { + return ante.NewTxReplacementDecorator(s.app.AccountKeeper, s.app.CommitMultiStore(), tracker) +} + +func (s *ReplacementDecoratorSuite) makeTxAtSeq(priv cryptotypes.PrivKey, accNum, seq uint64) sdk.Tx { + s.txBuilder = s.clientCtx.TxConfig.NewTxBuilder() + s.Require().NoError(s.txBuilder.SetMsgs( + testdata.NewTestMsg(sdk.AccAddress(priv.PubKey().Address())), + )) + s.txBuilder.SetFeeAmount(testdata.NewTestFeeAmount()) + s.txBuilder.SetGasLimit(testdata.NewTestGasLimit()) + tx, err := s.CreateTestTx([]cryptotypes.PrivKey{priv}, []uint64{accNum}, []uint64{seq}, s.ctx.ChainID()) + s.Require().NoError(err) + return tx +} + +func (s *ReplacementDecoratorSuite) makeBankSendTxAtSeqWithMemo(priv cryptotypes.PrivKey, accNum, seq uint64, memo string) sdk.Tx { + addr := sdk.AccAddress(priv.PubKey().Address()) + s.clientCtx = s.clientCtx.WithTxConfig(s.app.GetTxConfig()) + s.txBuilder = s.clientCtx.TxConfig.NewTxBuilder() + s.Require().NoError(s.txBuilder.SetMsgs( + banktypes.NewMsgSend(addr, addr, sdk.NewCoins(sdk.NewCoin(core.MicroLunaDenom, sdkmath.NewInt(1)))), + )) + s.txBuilder.SetMemo(memo) + s.txBuilder.SetFeeAmount(sdk.NewCoins(sdk.NewCoin(core.MicroLunaDenom, sdkmath.NewInt(1_000)))) + s.txBuilder.SetGasLimit(200000) + tx, err := s.CreateTestTx([]cryptotypes.PrivKey{priv}, []uint64{accNum}, []uint64{seq}, s.ctx.ChainID()) + s.Require().NoError(err) + return tx +} + +func (s *ReplacementDecoratorSuite) checkTx(tx sdk.Tx, expectedCode uint32) *abci.ResponseCheckTx { + return s.checkTxWithType(tx, abci.CheckTxType_New, expectedCode) +} + +func (s *ReplacementDecoratorSuite) checkTxWithType(tx sdk.Tx, checkType abci.CheckTxType, expectedCode uint32) *abci.ResponseCheckTx { + txBytes, err := s.clientCtx.TxConfig.TxEncoder()(tx) + s.Require().NoError(err) + + resp, err := s.app.CheckTx(&abci.RequestCheckTx{ + Tx: txBytes, + Type: checkType, + }) + s.Require().NoError(err) + s.Require().Equal(expectedCode, resp.Code, "CheckTx log: %s", resp.Log) + return resp +} + +func (s *ReplacementDecoratorSuite) selectedMempoolSequences(mp mempool.Mempool) []uint64 { + itr := mp.Select(s.ctx, nil) + var sequences []uint64 + for itr != nil { + tx := itr.Tx() + sigTx, ok := tx.(authsigning.SigVerifiableTx) + s.Require().True(ok) + sigs, err := sigTx.GetSignaturesV2() + s.Require().NoError(err) + s.Require().NotEmpty(sigs) + sequences = append(sequences, sigs[0].Sequence) + itr = itr.Next() + } + return sequences +} + +func (s *ReplacementDecoratorSuite) selectedMempoolMemos(mp mempool.Mempool) []string { + itr := mp.Select(s.ctx, nil) + var memos []string + for itr != nil { + tx := itr.Tx() + memoTx, ok := tx.(sdk.TxWithMemo) + s.Require().True(ok) + memos = append(memos, memoTx.GetMemo()) + itr = itr.Next() + } + return memos +} + +// --------------------------------------------------------------------------- +// handleRecheck tests (ctx.IsReCheckTx = true) +// --------------------------------------------------------------------------- + +// No tracker entry → decorator is transparent. +func (s *ReplacementDecoratorSuite) TestRecheck_NoTrackerEntry() { + s.SetupTest(true) + priv, _, accNum := func() (cryptotypes.PrivKey, sdk.AccAddress, uint64) { + k, _, addr := testdata.KeyTestPubAddr() + acc := s.app.AccountKeeper.NewAccountWithAddress(s.ctx, addr) + s.app.AccountKeeper.SetAccount(s.ctx, acc) + return k, addr, acc.GetAccountNumber() + }() + + dec := s.makeDecorator(ante.NewReplacementTracker()) + tx := s.makeTxAtSeq(priv, accNum, 5) + + _, err := dec.AnteHandle(s.ctx.WithIsReCheckTx(true), tx, false, noopHandler) + s.Require().NoError(err) +} + +// Old stuck tx at seq == fromSequence must be evicted (ErrWrongSequence). +func (s *ReplacementDecoratorSuite) TestRecheck_OldTxAtFromSequenceIsEvicted() { + s.SetupTest(true) + priv, _, accNum := func() (cryptotypes.PrivKey, sdk.AccAddress, uint64) { + k, _, addr := testdata.KeyTestPubAddr() + acc := s.app.AccountKeeper.NewAccountWithAddress(s.ctx, addr) + s.Require().NoError(acc.SetSequence(432)) + s.app.AccountKeeper.SetAccount(s.ctx, acc) + return k, addr, acc.GetAccountNumber() + }() + + tracker := ante.NewReplacementTracker() + tracker.Set(sdk.AccAddress(priv.PubKey().Address()).String(), 432, []byte("replacement-bytes")) + dec := s.makeDecorator(tracker) + + oldTx := s.makeTxAtSeq(priv, accNum, 432) + _, err := dec.AnteHandle(s.ctx.WithIsReCheckTx(true), oldTx, false, noopHandler) + s.Require().ErrorIs(err, sdkerrors.ErrWrongSequence) +} + +// The replacement tx itself (bytes match stored entry) must pass and clear the tracker. +func (s *ReplacementDecoratorSuite) TestRecheck_ReplacementTxClearsTracker() { + s.SetupTest(true) + priv, _, accNum := func() (cryptotypes.PrivKey, sdk.AccAddress, uint64) { + k, _, addr := testdata.KeyTestPubAddr() + acc := s.app.AccountKeeper.NewAccountWithAddress(s.ctx, addr) + s.Require().NoError(acc.SetSequence(432)) + s.app.AccountKeeper.SetAccount(s.ctx, acc) + return k, addr, acc.GetAccountNumber() + }() + + replacementTx := s.makeTxAtSeq(priv, accNum, 432) + txBytes, err := s.clientCtx.TxConfig.TxEncoder()(replacementTx) + s.Require().NoError(err) + + sender := sdk.AccAddress(priv.PubKey().Address()).String() + tracker := ante.NewReplacementTracker() + tracker.Set(sender, 432, txBytes) + dec := s.makeDecorator(tracker) + + ctx := s.ctx.WithIsReCheckTx(true).WithTxBytes(txBytes) + _, err = dec.AnteHandle(ctx, replacementTx, false, noopHandler) + s.Require().NoError(err) + s.Require().Nil(tracker.Get(sender), "tracker entry must be cleared once replacement tx passes recheck") +} + +// Txs at seq > fromSequence must NOT be evicted — they are valid queued txs that +// will be drained naturally as each block advances the committed sequence. +// This is the key behaviour introduced by the bug fix in handleRecheck. +func (s *ReplacementDecoratorSuite) TestRecheck_SubsequentTxsPassThrough() { + s.SetupTest(true) + priv, _, accNum := func() (cryptotypes.PrivKey, sdk.AccAddress, uint64) { + k, _, addr := testdata.KeyTestPubAddr() + acc := s.app.AccountKeeper.NewAccountWithAddress(s.ctx, addr) + s.Require().NoError(acc.SetSequence(433)) + s.app.AccountKeeper.SetAccount(s.ctx, acc) + return k, addr, acc.GetAccountNumber() + }() + + tracker := ante.NewReplacementTracker() + // Replacement recorded at seq 432; the txs under test are at 433, 434. + tracker.Set(sdk.AccAddress(priv.PubKey().Address()).String(), 432, []byte("replacement-bytes")) + dec := s.makeDecorator(tracker) + + for _, seq := range []uint64{433, 434, 441} { + tx := s.makeTxAtSeq(priv, accNum, seq) + _, err := dec.AnteHandle(s.ctx.WithIsReCheckTx(true), tx, false, noopHandler) + s.Require().NoError(err, "subsequent tx at seq=%d must pass through, not be evicted", seq) + } +} + +// Txs at seq < fromSequence are unaffected by the tracker. +func (s *ReplacementDecoratorSuite) TestRecheck_SeqBeforeFromSequencePassesThrough() { + s.SetupTest(true) + priv, _, accNum := func() (cryptotypes.PrivKey, sdk.AccAddress, uint64) { + k, _, addr := testdata.KeyTestPubAddr() + acc := s.app.AccountKeeper.NewAccountWithAddress(s.ctx, addr) + s.Require().NoError(acc.SetSequence(430)) + s.app.AccountKeeper.SetAccount(s.ctx, acc) + return k, addr, acc.GetAccountNumber() + }() + + tracker := ante.NewReplacementTracker() + tracker.Set(sdk.AccAddress(priv.PubKey().Address()).String(), 432, []byte("replacement-bytes")) + dec := s.makeDecorator(tracker) + + tx := s.makeTxAtSeq(priv, accNum, 430) + _, err := dec.AnteHandle(s.ctx.WithIsReCheckTx(true), tx, false, noopHandler) + s.Require().NoError(err) +} + +// --------------------------------------------------------------------------- +// handleNewTx tests (ctx.IsCheckTx = true) +// --------------------------------------------------------------------------- + +// Normal tx (txSeq >= pendingSeq) passes through without touching the tracker. +func (s *ReplacementDecoratorSuite) TestNewTx_NormalSequencePassThrough() { + s.SetupTest(true) + priv, addr, accNum := func() (cryptotypes.PrivKey, sdk.AccAddress, uint64) { + k, _, a := testdata.KeyTestPubAddr() + acc := s.app.AccountKeeper.NewAccountWithAddress(s.ctx, a) + s.Require().NoError(acc.SetSequence(432)) + s.app.AccountKeeper.SetAccount(s.ctx, acc) + return k, a, acc.GetAccountNumber() + }() + + tracker := ante.NewReplacementTracker() + dec := s.makeDecorator(tracker) + + tx := s.makeTxAtSeq(priv, accNum, 432) + _, err := dec.AnteHandle(s.ctx.WithIsCheckTx(true), tx, false, noopHandler) + s.Require().NoError(err) + s.Require().Nil(tracker.Get(addr.String()), "tracker must stay empty for a normal (non-replacement) tx") +} + +// Replacement tx: txSeq == committedSeq < pendingSeq. +// The decorator must reset the account to committed state, set the tracker entry, +// and allow the tx through so the downstream ante chain can verify it. +func (s *ReplacementDecoratorSuite) TestNewTx_ReplacementAtCommittedSeq() { + s.SetupTest(true) + priv, _, addr := testdata.KeyTestPubAddr() + + // Write the account with seq=432 directly into the committed store. + // ctx.WithMultiStore(CommitMultiStore) gives a context that reads/writes + // the committed (non-cached) store, mirroring what getCommittedAccount does. + acc := s.app.AccountKeeper.NewAccountWithAddress(s.ctx, addr) + s.Require().NoError(acc.SetSequence(432)) + committedCtx := s.ctx.WithMultiStore(s.app.CommitMultiStore()) + s.app.AccountKeeper.SetAccount(committedCtx, acc) + + // Advance the account in the test (pending) context to seq=443, + // simulating 11 queued txs (432-442) already accepted into the mempool. + s.Require().NoError(acc.SetSequence(443)) + s.app.AccountKeeper.SetAccount(s.ctx, acc) + + tracker := ante.NewReplacementTracker() + dec := s.makeDecorator(tracker) + + replacementTx := s.makeTxAtSeq(priv, acc.GetAccountNumber(), 432) + txBytes, err := s.clientCtx.TxConfig.TxEncoder()(replacementTx) + s.Require().NoError(err) + + ctx := s.ctx.WithIsCheckTx(true).WithTxBytes(txBytes) + _, err = dec.AnteHandle(ctx, replacementTx, false, noopHandler) + s.Require().NoError(err) + + sender := addr.String() + info := tracker.Get(sender) + s.Require().NotNil(info, "tracker must record the replacement") + s.Require().Equal(uint64(432), info.FromSequence) + s.Require().True(info.Contains(txBytes)) + + // The account in ctx must have been reset to committed seq=432 so that the + // downstream SigVerificationDecorator can pass. + resetAcc := s.app.AccountKeeper.GetAccount(ctx, addr) + s.Require().Equal(uint64(432), resetAcc.GetSequence()) +} + +// txSeq < pendingSeq but txSeq != committedSeq → not a valid replacement, +// decorator must pass through and leave tracker untouched. +func (s *ReplacementDecoratorSuite) TestNewTx_BelowPendingButNotAtCommitted() { + s.SetupTest(true) + priv, _, addr := testdata.KeyTestPubAddr() + + // Committed seq = 432. + acc := s.app.AccountKeeper.NewAccountWithAddress(s.ctx, addr) + s.Require().NoError(acc.SetSequence(432)) + committedCtx := s.ctx.WithMultiStore(s.app.CommitMultiStore()) + s.app.AccountKeeper.SetAccount(committedCtx, acc) + + // Pending seq = 443. + s.Require().NoError(acc.SetSequence(443)) + s.app.AccountKeeper.SetAccount(s.ctx, acc) + + tracker := ante.NewReplacementTracker() + dec := s.makeDecorator(tracker) + + // Tx at seq=430: below pending (443) but also below committed (432) → not a replacement. + tx := s.makeTxAtSeq(priv, acc.GetAccountNumber(), 430) + _, err := dec.AnteHandle(s.ctx.WithIsCheckTx(true), tx, false, noopHandler) + s.Require().NoError(err) + s.Require().Nil(tracker.Get(addr.String()), "tracker must stay empty — txSeq 430 != committedSeq 432") +} + +// This reproduces the production failure mode through the real TerraApp +// CheckTx path: queued txs advance the local CheckTx account sequence to 443 +// while the committed account remains at 432. Before TxReplacementDecorator, +// another tx at sequence 432 failed with "expected 443, got 432"; now it is +// accepted as a replacement for the stuck committed sequence. +func (s *ReplacementDecoratorSuite) TestCheckTx_ReplacesCommittedSequenceAfterPendingSequenceAdvanced() { + s.SetupTest(true) + priv, _, addr := testdata.KeyTestPubAddr() + + committedCtx := s.ctx.WithMultiStore(s.app.CommitMultiStore()) + acc := s.app.AccountKeeper.NewAccountWithAddress(committedCtx, addr) + s.Require().NoError(acc.SetSequence(432)) + s.app.AccountKeeper.SetAccount(committedCtx, acc) + s.Require().NoError(banktestutil.FundAccount( + committedCtx, + s.app.BankKeeper, + addr, + sdk.NewCoins(sdk.NewCoin(core.MicroLunaDenom, sdkmath.NewInt(1_000_000_000))), + )) + + var originalSeq432Tx sdk.Tx + for seq := uint64(432); seq <= 442; seq++ { + tx := s.makeBankSendTxAtSeqWithMemo(priv, acc.GetAccountNumber(), seq, "queued-tx") + if seq == 432 { + originalSeq432Tx = tx + } + s.checkTx(tx, 0) + } + s.Require().Equal(11, s.app.Mempool().CountTx()) + s.Require().Equal( + []uint64{432, 433, 434, 435, 436, 437, 438, 439, 440, 441, 442}, + s.selectedMempoolSequences(s.app.Mempool()), + ) + + replacementTx := s.makeBankSendTxAtSeqWithMemo(priv, acc.GetAccountNumber(), 432, "replacement-at-committed-sequence") + s.checkTx(replacementTx, 0) + s.Require().Equal(11, s.app.Mempool().CountTx()) + s.Require().Equal( + []uint64{432, 433, 434, 435, 436, 437, 438, 439, 440, 441, 442}, + s.selectedMempoolSequences(s.app.Mempool()), + ) + s.Require().Equal("replacement-at-committed-sequence", s.selectedMempoolMemos(s.app.Mempool())[0]) + + recheckResp := s.checkTxWithType(originalSeq432Tx, abci.CheckTxType_Recheck, sdkerrors.ErrWrongSequence.ABCICode()) + s.Require().Contains(recheckResp.Log, "tx superseded") + s.Require().Equal(11, s.app.Mempool().CountTx()) + s.Require().Equal( + []uint64{432, 433, 434, 435, 436, 437, 438, 439, 440, 441, 442}, + s.selectedMempoolSequences(s.app.Mempool()), + ) + s.Require().Equal("replacement-at-committed-sequence", s.selectedMempoolMemos(s.app.Mempool())[0]) + + s.checkTxWithType(replacementTx, abci.CheckTxType_Recheck, 0) + s.Require().Equal(11, s.app.Mempool().CountTx()) + s.Require().Equal( + []uint64{432, 433, 434, 435, 436, 437, 438, 439, 440, 441, 442}, + s.selectedMempoolSequences(s.app.Mempool()), + ) + s.Require().Equal("replacement-at-committed-sequence", s.selectedMempoolMemos(s.app.Mempool())[0]) +} diff --git a/go.mod b/go.mod index 4cf698979..fab13ff0e 100644 --- a/go.mod +++ b/go.mod @@ -18,7 +18,7 @@ require ( cosmossdk.io/x/upgrade v0.2.0 github.com/CosmWasm/wasmd v0.61.8 github.com/CosmWasm/wasmvm/v3 v3.0.3 - github.com/cometbft/cometbft v0.38.21 + github.com/cometbft/cometbft v0.38.23 github.com/cosmos/cosmos-db v1.1.3 github.com/cosmos/cosmos-sdk v0.53.6 github.com/cosmos/gogoproto v1.7.2 diff --git a/go.sum b/go.sum index f32ce01e0..77433c79d 100644 --- a/go.sum +++ b/go.sum @@ -802,8 +802,8 @@ github.com/cockroachdb/redact v1.1.6/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZ github.com/cockroachdb/tokenbucket v0.0.0-20250429170803-42689b6311bb h1:3bCgBvB8PbJVMX1ouCcSIxvsqKPYM7gs72o0zC76n9g= github.com/cockroachdb/tokenbucket v0.0.0-20250429170803-42689b6311bb/go.mod h1:7nc4anLGjupUW/PeY5qiNYsdNXj7zopG+eqsS7To5IQ= github.com/codahale/hdrhistogram v0.0.0-20161010025455-3a0bb77429bd/go.mod h1:sE/e/2PUdi/liOCUjSTXgM1o87ZssimdTWN964YiIeI= -github.com/cometbft/cometbft v0.38.21 h1:qcIJSH9LiwU5s6ZgKR5eRbsLNucbubfraDs5bzgjtOI= -github.com/cometbft/cometbft v0.38.21/go.mod h1:UCu8dlHqvkAsmAFmWDRWNZJPlu6ya2fTWZlDrWsivwo= +github.com/cometbft/cometbft v0.38.23 h1:jtCe5Do4EcHVf/FCyZMvkBm+AmsRGGyvswImXYAabdM= +github.com/cometbft/cometbft v0.38.23/go.mod h1:jtH//cs5e2U5dNiaYIPMBwWXZsedXJfIie77gVhuRaA= github.com/cometbft/cometbft-db v0.14.1 h1:SxoamPghqICBAIcGpleHbmoPqy+crij/++eZz3DlerQ= github.com/cometbft/cometbft-db v0.14.1/go.mod h1:KHP1YghilyGV/xjD5DP3+2hyigWx0WTp9X+0Gnx0RxQ= github.com/containerd/continuity v0.4.5 h1:ZRoN1sXq9u7V6QoHMcVWGhOwDFqZ4B9i5H6un1Wh0x4= diff --git a/tests/interchaintest/go.mod b/tests/interchaintest/go.mod index d2114a32f..156d899b3 100644 --- a/tests/interchaintest/go.mod +++ b/tests/interchaintest/go.mod @@ -72,7 +72,7 @@ require ( github.com/cockroachdb/pebble v1.1.5 // indirect github.com/cockroachdb/redact v1.1.6 // indirect github.com/cockroachdb/tokenbucket v0.0.0-20250429170803-42689b6311bb // indirect - github.com/cometbft/cometbft v0.38.21 // indirect + github.com/cometbft/cometbft v0.38.23 // indirect github.com/cometbft/cometbft-db v0.14.1 // indirect github.com/consensys/gnark-crypto v0.18.0 // indirect github.com/cosmos/btcutil v1.0.5 // indirect diff --git a/tests/interchaintest/go.sum b/tests/interchaintest/go.sum index 5c6dd8b9b..faf8c2e82 100644 --- a/tests/interchaintest/go.sum +++ b/tests/interchaintest/go.sum @@ -827,8 +827,8 @@ github.com/cockroachdb/redact v1.1.6/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZ github.com/cockroachdb/tokenbucket v0.0.0-20250429170803-42689b6311bb h1:3bCgBvB8PbJVMX1ouCcSIxvsqKPYM7gs72o0zC76n9g= github.com/cockroachdb/tokenbucket v0.0.0-20250429170803-42689b6311bb/go.mod h1:7nc4anLGjupUW/PeY5qiNYsdNXj7zopG+eqsS7To5IQ= github.com/codahale/hdrhistogram v0.0.0-20161010025455-3a0bb77429bd/go.mod h1:sE/e/2PUdi/liOCUjSTXgM1o87ZssimdTWN964YiIeI= -github.com/cometbft/cometbft v0.38.21 h1:qcIJSH9LiwU5s6ZgKR5eRbsLNucbubfraDs5bzgjtOI= -github.com/cometbft/cometbft v0.38.21/go.mod h1:UCu8dlHqvkAsmAFmWDRWNZJPlu6ya2fTWZlDrWsivwo= +github.com/cometbft/cometbft v0.38.23 h1:jtCe5Do4EcHVf/FCyZMvkBm+AmsRGGyvswImXYAabdM= +github.com/cometbft/cometbft v0.38.23/go.mod h1:jtH//cs5e2U5dNiaYIPMBwWXZsedXJfIie77gVhuRaA= github.com/cometbft/cometbft-db v0.14.1 h1:SxoamPghqICBAIcGpleHbmoPqy+crij/++eZz3DlerQ= github.com/cometbft/cometbft-db v0.14.1/go.mod h1:KHP1YghilyGV/xjD5DP3+2hyigWx0WTp9X+0Gnx0RxQ= github.com/consensys/gnark-crypto v0.18.0 h1:vIye/FqI50VeAr0B3dx+YjeIvmc3LWz4yEfbWBpTUf0= diff --git a/tests/interchaintest/tx_replacement_test.go b/tests/interchaintest/tx_replacement_test.go new file mode 100644 index 000000000..5fae5f598 --- /dev/null +++ b/tests/interchaintest/tx_replacement_test.go @@ -0,0 +1,184 @@ +package interchaintest + +import ( + "context" + "encoding/json" + "fmt" + "testing" + "time" + + sdkmath "cosmossdk.io/math" + "github.com/cosmos/interchaintest/v10" + "github.com/cosmos/interchaintest/v10/chain/cosmos" + "github.com/cosmos/interchaintest/v10/testreporter" + "github.com/cosmos/interchaintest/v10/testutil" + "github.com/stretchr/testify/require" + "go.uber.org/zap/zaptest" +) + +func TestTxReplacementAtCommittedSequence(t *testing.T) { + if testing.Short() { + t.Skip() + } + + t.Parallel() + + numVals := 1 + numFullNodes := 0 + + config, err := createConfig() + require.NoError(t, err) + config.ConfigFileOverrides = map[string]any{ + "config/config.toml": testutil.Toml{ + "consensus": testutil.Toml{ + "timeout_commit": "20s", + }, + }, + } + + cf := interchaintest.NewBuiltinChainFactory(zaptest.NewLogger(t), []*interchaintest.ChainSpec{ + { + Name: "terra", + ChainConfig: config, + NumValidators: &numVals, + NumFullNodes: &numFullNodes, + }, + }) + + chains, err := cf.Chains(t.Name()) + require.NoError(t, err) + + terra := chains[0].(*cosmos.CosmosChain) + ic := interchaintest.NewInterchain().AddChain(terra) + + rep := testreporter.NewNopReporter() + eRep := rep.RelayerExecReporter(t) + + ctx := context.Background() + client, network := interchaintest.DockerSetup(t) + + err = ic.Build(ctx, eRep, interchaintest.InterchainBuildOptions{ + TestName: t.Name(), + Client: client, + NetworkID: network, + SkipPathCreation: true, + }) + require.NoError(t, err) + + t.Cleanup(func() { + _ = ic.Close() + }) + + require.NoError(t, testutil.WaitForBlocks(ctx, 2, terra)) + + users := interchaintest.GetAndFundTestUsers( + t, + ctx, + "replacement", + sdkmath.NewInt(genesisWalletAmount), + terra, + terra, + ) + sender := users[0] + receiver := users[1] + + require.NoError(t, testutil.WaitForBlocks(ctx, 2, terra)) + + accountInfo, err := terra.AuthQueryAccountInfo(ctx, sender.FormattedAddress()) + require.NoError(t, err) + committedSeq := accountInfo.GetSequence() + accountNumber := accountInfo.GetAccountNumber() + + initialReceiverBalance, err := terra.GetBalance(ctx, receiver.FormattedAddress(), terra.Config().Denom) + require.NoError(t, err) + + node := terra.GetNode() + for seq := committedSeq; seq <= committedSeq+10; seq++ { + require.NoError(t, broadcastSyncBankSendAtSequence( + ctx, + node, + sender.KeyName(), + receiver.FormattedAddress(), + terra.Config().Denom, + sdkmath.OneInt(), + accountNumber, + seq, + fmt.Sprintf("queued-%d", seq), + )) + } + + replacementAmount := sdkmath.NewInt(9) + require.NoError(t, broadcastSyncBankSendAtSequence( + ctx, + node, + sender.KeyName(), + receiver.FormattedAddress(), + terra.Config().Denom, + replacementAmount, + accountNumber, + committedSeq, + "replacement-at-committed-sequence", + )) + + require.NoError(t, testutil.WaitForBlocks(ctx, 2, terra)) + + expectedFinalSeq := committedSeq + 11 + require.Eventually(t, func() bool { + accountInfo, err := terra.AuthQueryAccountInfo(ctx, sender.FormattedAddress()) + return err == nil && accountInfo.GetSequence() == expectedFinalSeq + }, time.Minute, 2*time.Second) + + finalReceiverBalance, err := terra.GetBalance(ctx, receiver.FormattedAddress(), terra.Config().Denom) + require.NoError(t, err) + + // The original tx at committedSeq sent 1uluna. If it was not replaced, the + // receiver would only gain 11uluna. The replacement sends 9uluna, followed + // by the ten queued txs at committedSeq+1..+10. + require.Equal(t, initialReceiverBalance.Add(sdkmath.NewInt(19)), finalReceiverBalance) +} + +func broadcastSyncBankSendAtSequence( + ctx context.Context, + node *cosmos.ChainNode, + keyName string, + toAddress string, + denom string, + amount sdkmath.Int, + accountNumber uint64, + sequence uint64, + memo string, +) error { + stdout, _, err := node.Exec(ctx, node.TxCommand( + keyName, + "bank", "send", + keyName, + toAddress, + fmt.Sprintf("%s%s", amount.String(), denom), + "--account-number", fmt.Sprintf("%d", accountNumber), + "--sequence", fmt.Sprintf("%d", sequence), + "--offline", + "--broadcast-mode", "sync", + "--fees", fmt.Sprintf("6000000%s", denom), + "--gas", "200000", + "--note", memo, + ), node.Chain.Config().Env) + if err != nil { + return err + } + + var txResp struct { + Code uint32 `json:"code"` + RawLog string `json:"raw_log"` + TxHash string `json:"txhash"` + } + if err := json.Unmarshal(stdout, &txResp); err != nil { + return err + } + if txResp.Code != 0 { + return fmt.Errorf("sync broadcast failed with code %d: %s", txResp.Code, txResp.RawLog) + } + if txResp.TxHash == "" { + return fmt.Errorf("sync broadcast returned empty txhash") + } + return nil +}