Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(mempool): set pre and post tx check funcs after genesis, not only after first block #691

Merged
merged 3 commits into from
Apr 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion block/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,10 +186,12 @@ func (e *Executor) commit(ctx context.Context, state *types.State, block *types.

maxBytes := state.ConsensusParams.Block.MaxBytes
maxGas := state.ConsensusParams.Block.MaxGas
err = e.mempool.Update(int64(block.Header.Height), fromDymintTxs(block.Data.Txs), deliverTxs, mempool.PreCheckMaxBytes(maxBytes), mempool.PostCheckMaxGas(maxGas))
err = e.mempool.Update(int64(block.Header.Height), fromDymintTxs(block.Data.Txs), deliverTxs)
if err != nil {
return nil, 0, err
}
e.mempool.SetPreCheckFn(mempool.PreCheckMaxBytes(maxBytes))
e.mempool.SetPostCheckFn(mempool.PostCheckMaxGas(maxGas))

return resp.Data, resp.RetainHeight, err
}
Expand Down
2 changes: 2 additions & 0 deletions block/initchain.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ func (m *Manager) RunInitChain(ctx context.Context) error {

// update the state with only the consensus pubkey
m.executor.UpdateStateAfterInitChain(&m.lastState, res, gensisValSet)
m.executor.UpdateMempoolAfterInitChain(&m.lastState)

if _, err := m.store.UpdateState(m.lastState, nil); err != nil {
return err
}
Expand Down
6 changes: 6 additions & 0 deletions block/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
tmstate "github.com/tendermint/tendermint/proto/tendermint/state"
tmtypes "github.com/tendermint/tendermint/types"

"github.com/dymensionxyz/dymint/mempool"
"github.com/dymensionxyz/dymint/types"
)

Expand Down Expand Up @@ -103,6 +104,11 @@ func (e *Executor) UpdateStateAfterInitChain(s *types.State, res *abci.ResponseI
s.LastValidators = s.Validators.Copy()
}

func (e *Executor) UpdateMempoolAfterInitChain(s *types.State) {
e.mempool.SetPreCheckFn(mempool.PreCheckMaxBytes(s.ConsensusParams.Block.MaxBytes))
e.mempool.SetPostCheckFn(mempool.PostCheckMaxGas(s.ConsensusParams.Block.MaxGas))
}

// UpdateStateFromResponses updates state based on the ABCIResponses.
func (e *Executor) UpdateStateFromResponses(resp *tmstate.ABCIResponses, state types.State, block *types.Block) (types.State, error) {
// Dymint ignores any setValidator responses from the app, as it is manages the validator set based on the settlement consensus
Expand Down
8 changes: 6 additions & 2 deletions mempool/mempool.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,14 @@ type Mempool interface {
blockHeight int64,
blockTxs types.Txs,
deliverTxResponses []*abci.ResponseDeliverTx,
newPreFn PreCheckFunc,
newPostFn PostCheckFunc,
) error

// SetPreCheckFn sets the pre-check function.
SetPreCheckFn(fn PreCheckFunc)

// SetPostCheckFn sets the post-check function.
SetPostCheckFn(fn PostCheckFunc)

// FlushAppConn flushes the mempool connection to ensure async callback calls
// are done, e.g. from CheckTx.
//
Expand Down
19 changes: 9 additions & 10 deletions mempool/v1/mempool.go
Original file line number Diff line number Diff line change
Expand Up @@ -392,8 +392,6 @@ func (txmp *TxMempool) Update(
blockHeight int64,
blockTxs types.Txs,
deliverTxResponses []*abci.ResponseDeliverTx,
newPreFn mempool.PreCheckFunc,
newPostFn mempool.PostCheckFunc,
) error {
// Safety sanity check: The caller is required to hold the lock.
if txmp.mtx.TryLock() {
Expand All @@ -409,13 +407,6 @@ func (txmp *TxMempool) Update(
txmp.height = blockHeight
txmp.notifiedTxsAvailable = false

if newPreFn != nil {
txmp.preCheck = newPreFn
}
if newPostFn != nil {
txmp.postCheck = newPostFn
}

for i, tx := range blockTxs {
// Add successful committed transactions to the cache (if they are not
// already present). Transactions that failed to commit are removed from
Expand All @@ -432,7 +423,7 @@ func (txmp *TxMempool) Update(

txmp.purgeExpiredTxs(blockHeight)

// If there any uncommitted transactions left in the mempool, we either
// If there are any uncommitted transactions left in the mempool, we either
// initiate re-CheckTx per remaining transaction or notify that remaining
// transactions are left.
size := txmp.Size()
Expand All @@ -447,6 +438,14 @@ func (txmp *TxMempool) Update(
return nil
}

func (txmp *TxMempool) SetPreCheckFn(fn mempool.PreCheckFunc) {
txmp.preCheck = fn
}

func (txmp *TxMempool) SetPostCheckFn(fn mempool.PostCheckFunc) {
txmp.postCheck = fn
}

// initialTxCallback handles the ABCI CheckTx response for the first time a
// transaction is added to the mempool. A recheck after a block is committed
// goes to the default callback (see recheckTxCallback).
Expand Down
14 changes: 7 additions & 7 deletions mempool/v1/mempool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ func TestTxMempool_TxsAvailable(t *testing.T) {

// commit half the transactions and ensure we fire an event
txmp.Lock()
require.NoError(t, txmp.Update(1, rawTxs[:50], responses, nil, nil))
require.NoError(t, txmp.Update(1, rawTxs[:50], responses))
txmp.Unlock()
ensureTxFire()
ensureNoTxFire()
Expand Down Expand Up @@ -203,7 +203,7 @@ func TestTxMempool_Size(t *testing.T) {
}

txmp.Lock()
require.NoError(t, txmp.Update(1, rawTxs[:50], responses, nil, nil))
require.NoError(t, txmp.Update(1, rawTxs[:50], responses))
txmp.Unlock()

require.Equal(t, len(rawTxs)/2, txmp.Size())
Expand Down Expand Up @@ -298,7 +298,7 @@ func TestTxMempool_Flush(t *testing.T) {
}

txmp.Lock()
require.NoError(t, txmp.Update(1, rawTxs[:50], responses, nil, nil))
require.NoError(t, txmp.Update(1, rawTxs[:50], responses))
txmp.Unlock()

txmp.Flush()
Expand Down Expand Up @@ -503,7 +503,7 @@ func TestTxMempool_ConcurrentTxs(t *testing.T) {
}

txmp.Lock()
require.NoError(t, txmp.Update(height, reapedTxs, responses, nil, nil))
require.NoError(t, txmp.Update(height, reapedTxs, responses))
txmp.Unlock()

height++
Expand Down Expand Up @@ -551,7 +551,7 @@ func TestTxMempool_ExpiredTxs_Timestamp(t *testing.T) {
// Trigger an update so that pruning will occur.
txmp.Lock()
defer txmp.Unlock()
require.NoError(t, txmp.Update(txmp.height+1, nil, nil, nil, nil))
require.NoError(t, txmp.Update(txmp.height+1, nil, nil))

// All the transactions in the original set should have been purged.
for _, tx := range added1 {
Expand Down Expand Up @@ -588,7 +588,7 @@ func TestTxMempool_ExpiredTxs_NumBlocks(t *testing.T) {
}

txmp.Lock()
require.NoError(t, txmp.Update(txmp.height+1, reapedTxs, responses, nil, nil))
require.NoError(t, txmp.Update(txmp.height+1, reapedTxs, responses))
txmp.Unlock()

require.Equal(t, 95, txmp.Size())
Expand All @@ -612,7 +612,7 @@ func TestTxMempool_ExpiredTxs_NumBlocks(t *testing.T) {
}

txmp.Lock()
require.NoError(t, txmp.Update(txmp.height+10, reapedTxs, responses, nil, nil))
require.NoError(t, txmp.Update(txmp.height+10, reapedTxs, responses))
txmp.Unlock()

require.GreaterOrEqual(t, txmp.Size(), 45)
Expand Down
Loading