LaconicNetwork/block-sdk
7
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(compare): Adding Sequence Number check on Compare Priority (#159)

Browse Source 
* adding seq num check on compare

* nit

* adding debug logging
This commit is contained in:
David Terpay
2023-10-17 16:50:28 -04:00
committed by GitHub
parent 339b927323
commit aff0e228a3
13 changed files with 842 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files
block/base/handlers.go
+4 -2
View File
@@ -124,8 +124,10 @@ func (l *BaseLane) DefaultProcessLaneHandler() ProcessLaneHandler {
// If the transactions do not respect the priority defined by the mempool, we consider the proposal
// to be invalid
if index > 0 && l.Compare(ctx, partialProposal[index-1], tx) == -1 {
return fmt.Errorf("transaction at index %d has a higher priority than %d", index, index-1)
if index > 0 {
if v, err := l.Compare(ctx, partialProposal[index-1], tx); v == -1 || err != nil {
return fmt.Errorf("transaction at index %d has a higher priority than %d", index, index-1)
}
}
if err := l.VerifyTx(ctx, tx, false); err != nil {
block/base/mempool.go
+47 -2
View File
@@ -22,6 +22,10 @@ type (
// index defines an index of transactions.
index sdkmempool.Mempool
// signerExtractor defines the signer extraction adapter that allows us to
// extract the signer from a transaction.
extractor signer_extraction.Adapter
// txPriority defines the transaction priority function. It is used to
// retrieve the priority of a given transaction and to compare the priority
// of two transactions. The index utilizes this struct to order transactions
@@ -91,6 +95,7 @@ func NewMempool[C comparable](txPriority TxPriority[C], txEncoder sdk.TxEncoder,
},
extractor,
),
extractor: extractor,
txPriority: txPriority,
txEncoder: txEncoder,
txCache: make(map[string]struct{}),
@@ -155,8 +160,48 @@ func (cm *Mempool[C]) Contains(tx sdk.Tx) bool {
}
// Compare determines the relative priority of two transactions belonging in the same lane.
func (cm *Mempool[C]) Compare(ctx sdk.Context, this sdk.Tx, other sdk.Tx) int {
// There are two cases to consider:
// 1. The transactions have the same signer. In this case, we compare the sequence numbers.
// 2. The transactions have different signers. In this case, we compare the priorities of the
// transactions.
//
// Compare will return -1 if this transaction has a lower priority than the other transaction, 0 if
// they have the same priority, and 1 if this transaction has a higher priority than the other transaction.
func (cm *Mempool[C]) Compare(ctx sdk.Context, this sdk.Tx, other sdk.Tx) (int, error) {
signers, err := cm.extractor.GetSigners(this)
if err != nil {
return 0, err
}
if len(signers) == 0 {
return 0, fmt.Errorf("expected one signer for the first transaction")
}
// The priority nonce mempool uses the first tx signer so this is a safe operation.
thisSignerInfo := signers[0]
signers, err = cm.extractor.GetSigners(other)
if err != nil {
return 0, err
}
if len(signers) == 0 {
return 0, fmt.Errorf("expected one signer for the second transaction")
}
otherSignerInfo := signers[0]
// If the signers are the same, we compare the sequence numbers.
if thisSignerInfo.Signer.Equals(otherSignerInfo.Signer) {
switch {
case thisSignerInfo.Sequence < otherSignerInfo.Sequence:
return 1, nil
case thisSignerInfo.Sequence > otherSignerInfo.Sequence:
return -1, nil
default:
// This case should never happen but we add in the case for completeness.
return 0, fmt.Errorf("the two transactions have the same sequence number")
}
}
// Determine the priority and compare the priorities.
firstPriority := cm.txPriority.GetTxPriority(ctx, this)
secondPriority := cm.txPriority.GetTxPriority(ctx, other)
return cm.txPriority.Compare(firstPriority, secondPriority)
return cm.txPriority.Compare(firstPriority, secondPriority), nil
}
block/lane.go
+1 -1
View File
@@ -18,7 +18,7 @@ type LaneMempool interface {
// Compare determines the relative priority of two transactions belonging in the same lane. Compare
// will return -1 if this transaction has a lower priority than the other transaction, 0 if they have
// the same priority, and 1 if this transaction has a higher priority than the other transaction.
Compare(ctx sdk.Context, this, other sdk.Tx) int
Compare(ctx sdk.Context, this, other sdk.Tx) (int, error)
// Contains returns true if the transaction is contained in the mempool.
Contains(tx sdk.Tx) bool
block/proposals/proposals.go
+7 -3
View File
@@ -1,6 +1,7 @@
package proposals
import (
"cosmossdk.io/log"
"cosmossdk.io/math"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/skip-mev/block-sdk/block/proposals/types"
@@ -9,6 +10,8 @@ import (
type (
// Proposal defines a block proposal type.
Proposal struct {
Logger log.Logger
// Txs is the list of transactions in the proposal.
Txs [][]byte
// Cache is a cache of the selected transactions in the proposal.
@@ -21,15 +24,16 @@ type (
)
// NewProposalWithContext returns a new empty proposal.
func NewProposalWithContext(ctx sdk.Context, txEncoder sdk.TxEncoder) Proposal {
func NewProposalWithContext(logger log.Logger, ctx sdk.Context, txEncoder sdk.TxEncoder) Proposal {
maxBlockSize, maxGasLimit := GetBlockLimits(ctx)
return NewProposal(txEncoder, maxBlockSize, maxGasLimit)
return NewProposal(logger, txEncoder, maxBlockSize, maxGasLimit)
}
// NewProposal returns a new empty proposal. Any transactions added to the proposal
// will be subject to the given max block size and max gas limit.
func NewProposal(txEncoder sdk.TxEncoder, maxBlockSize int64, maxGasLimit uint64) Proposal {
func NewProposal(logger log.Logger, txEncoder sdk.TxEncoder, maxBlockSize int64, maxGasLimit uint64) Proposal {
return Proposal{
Logger: logger,
TxEncoder: txEncoder,
Txs: make([][]byte, 0),
Cache: make(map[string]struct{}),
block/proposals/proposals_test.go
+11 -10
View File
@@ -4,6 +4,7 @@ import (
"math/rand"
"testing"
"cosmossdk.io/log"
"cosmossdk.io/math"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/skip-mev/block-sdk/block/mocks"
@@ -27,7 +28,7 @@ func TestUpdateProposal(t *testing.T) {
lane.On("GetMaxBlockSpace").Return(math.LegacyNewDec(1)).Maybe()
t.Run("can update with no transactions", func(t *testing.T) {
proposal := proposals.NewProposal(nil, 100, 100)
proposal := proposals.NewProposal(log.NewTestLogger(t), nil, 100, 100)
err := proposal.UpdateProposal(lane, nil)
require.NoError(t, err)
@@ -59,7 +60,7 @@ func TestUpdateProposal(t *testing.T) {
size := len(txBzs[0])
gasLimit := 100
proposal := proposals.NewProposal(encodingConfig.TxConfig.TxEncoder(), int64(size), uint64(gasLimit))
proposal := proposals.NewProposal(log.NewTestLogger(t), encodingConfig.TxConfig.TxEncoder(), int64(size), uint64(gasLimit))
err = proposal.UpdateProposal(lane, []sdk.Tx{tx})
require.NoError(t, err)
@@ -105,7 +106,7 @@ func TestUpdateProposal(t *testing.T) {
gasLimit += 100
}
proposal := proposals.NewProposal(encodingConfig.TxConfig.TxEncoder(), int64(size), gasLimit)
proposal := proposals.NewProposal(log.NewTestLogger(t), encodingConfig.TxConfig.TxEncoder(), int64(size), gasLimit)
err = proposal.UpdateProposal(lane, txs)
require.NoError(t, err)
@@ -142,7 +143,7 @@ func TestUpdateProposal(t *testing.T) {
size := int64(len(txBzs[0]))
gasLimit := uint64(100)
proposal := proposals.NewProposal(encodingConfig.TxConfig.TxEncoder(), size, gasLimit)
proposal := proposals.NewProposal(log.NewTestLogger(t), encodingConfig.TxConfig.TxEncoder(), size, gasLimit)
err = proposal.UpdateProposal(lane, []sdk.Tx{tx})
require.NoError(t, err)
@@ -202,7 +203,7 @@ func TestUpdateProposal(t *testing.T) {
size := len(txBzs[0]) + len(txBzs[1])
gasLimit := 200
proposal := proposals.NewProposal(encodingConfig.TxConfig.TxEncoder(), int64(size), uint64(gasLimit))
proposal := proposals.NewProposal(log.NewTestLogger(t), encodingConfig.TxConfig.TxEncoder(), int64(size), uint64(gasLimit))
err = proposal.UpdateProposal(lane, []sdk.Tx{tx})
require.NoError(t, err)
@@ -240,7 +241,7 @@ func TestUpdateProposal(t *testing.T) {
size := len(txBzs[0])
gasLimit := 100
proposal := proposals.NewProposal(encodingConfig.TxConfig.TxEncoder(), int64(size), uint64(gasLimit))
proposal := proposals.NewProposal(log.NewTestLogger(t), encodingConfig.TxConfig.TxEncoder(), int64(size), uint64(gasLimit))
lane := mocks.NewLane(t)
@@ -278,7 +279,7 @@ func TestUpdateProposal(t *testing.T) {
size := len(txBzs[0])
gasLimit := 100
proposal := proposals.NewProposal(encodingConfig.TxConfig.TxEncoder(), int64(size), uint64(gasLimit))
proposal := proposals.NewProposal(log.NewTestLogger(t), encodingConfig.TxConfig.TxEncoder(), int64(size), uint64(gasLimit))
lane := mocks.NewLane(t)
@@ -316,7 +317,7 @@ func TestUpdateProposal(t *testing.T) {
size := len(txBzs[0])
gasLimit := 100
proposal := proposals.NewProposal(encodingConfig.TxConfig.TxEncoder(), int64(size)-1, uint64(gasLimit))
proposal := proposals.NewProposal(log.NewTestLogger(t), encodingConfig.TxConfig.TxEncoder(), int64(size)-1, uint64(gasLimit))
err = proposal.UpdateProposal(lane, []sdk.Tx{tx})
require.Error(t, err)
@@ -349,7 +350,7 @@ func TestUpdateProposal(t *testing.T) {
size := len(txBzs[0])
gasLimit := 100
proposal := proposals.NewProposal(encodingConfig.TxConfig.TxEncoder(), int64(size), uint64(gasLimit)-1)
proposal := proposals.NewProposal(log.NewTestLogger(t), encodingConfig.TxConfig.TxEncoder(), int64(size), uint64(gasLimit)-1)
err = proposal.UpdateProposal(lane, []sdk.Tx{tx})
require.Error(t, err)
@@ -390,7 +391,7 @@ func TestUpdateProposal(t *testing.T) {
txBzs, err := utils.GetEncodedTxs(encodingConfig.TxConfig.TxEncoder(), []sdk.Tx{tx, tx2})
require.NoError(t, err)
proposal := proposals.NewProposal(encodingConfig.TxConfig.TxEncoder(), 10000, 10000)
proposal := proposals.NewProposal(log.NewTestLogger(t), encodingConfig.TxConfig.TxEncoder(), 10000, 10000)
err = proposal.UpdateProposal(lane, []sdk.Tx{tx})
require.NoError(t, err)
block/proposals/update.go
+12
View File
@@ -1,6 +1,7 @@
package proposals
import (
"encoding/base64"
"fmt"
"cosmossdk.io/math"
@@ -46,6 +47,17 @@ func (p *Proposal) UpdateProposal(lane Lane, partialProposal []sdk.Tx) error {
return fmt.Errorf("err retrieving transaction info: %s", err)
}
p.Logger.Debug(
"updating proposal with tx",
"index", index,
"lane", lane.Name(),
"tx_hash", txInfo.Hash,
"tx_size", txInfo.Size,
"tx_gas_limit", txInfo.GasLimit,
"tx_bytes", txInfo.TxBytes,
"raw_tx", base64.StdEncoding.EncodeToString(txInfo.TxBytes),
)
// invariant check: Ensure that the transaction is not already in the proposal.
if _, ok := p.Cache[txInfo.Hash]; ok {
return fmt.Errorf("transaction %s is already in the proposal", txInfo.Hash)