From bc88555aafcd93733768b09f199983ab17865978 Mon Sep 17 00:00:00 2001 From: redhdx Date: Thu, 8 Aug 2024 15:29:22 +0800 Subject: [PATCH 1/4] feature(op-geth): fix bundlepool drop issue --- core/txpool/bundlepool/bundlepool.go | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/core/txpool/bundlepool/bundlepool.go b/core/txpool/bundlepool/bundlepool.go index 1f5589badd..f6849e28ff 100644 --- a/core/txpool/bundlepool/bundlepool.go +++ b/core/txpool/bundlepool/bundlepool.go @@ -200,7 +200,7 @@ func (p *BundlePool) AddBundle(bundle *types.Bundle, originBundle *types.SendBun } for p.slots+numSlots(bundle) > p.config.GlobalSlots { - p.drop() + p.drop(bundle) } p.bundles[hash] = bundle heap.Push(&p.bundleHeap, bundle) @@ -397,14 +397,24 @@ func (p *BundlePool) deleteBundle(hash common.Hash) { } // drop removes the bundle with the lowest gas price from the pool. -func (p *BundlePool) drop() { +func (p *BundlePool) drop(bundle *types.Bundle) { + dropBundles := make([]*types.Bundle, 0, numSlots(bundle)) + dropSlots := uint64(0) for len(p.bundleHeap) > 0 { + if dropSlots >= numSlots(bundle) { + for _, dropBundle := range dropBundles { + p.deleteBundle(dropBundle.Hash()) + } + break + } // Pop the bundle with the lowest gas price // the min element in the heap may not exist in the pool as it may be pruned leastPriceBundleHash := heap.Pop(&p.bundleHeap).(*types.Bundle).Hash() - if _, ok := p.bundles[leastPriceBundleHash]; ok { - p.deleteBundle(leastPriceBundleHash) - break + leastPriceBundle, _ := p.bundles[leastPriceBundleHash] + if leastPriceBundle != nil && leastPriceBundle.Price.Cmp(bundle.Price) < 0 { + dropBundles = append(dropBundles, leastPriceBundle) + dropSlots = dropSlots + numSlots(leastPriceBundle) + continue } } } From 18aa2896819bf5d6b9d14a2a4a8b926b19439688 Mon Sep 17 00:00:00 2001 From: redhdx Date: Mon, 12 Aug 2024 09:53:34 +0800 Subject: [PATCH 2/4] feature(op-geth): optimize bundlepool drop logic --- core/txpool/bundlepool/bundlepool.go | 29 +++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/core/txpool/bundlepool/bundlepool.go b/core/txpool/bundlepool/bundlepool.go index f6849e28ff..21f39aa701 100644 --- a/core/txpool/bundlepool/bundlepool.go +++ b/core/txpool/bundlepool/bundlepool.go @@ -175,8 +175,10 @@ func (p *BundlePool) AddBundle(bundle *types.Bundle, originBundle *types.SendBun p.mu.Lock() defer p.mu.Unlock() - if price.Cmp(p.minimalBundleGasPrice()) <= 0 && p.slots+numSlots(bundle) > p.config.GlobalSlots { - return ErrBundleGasPriceLow + if p.slots+numSlots(bundle) > p.config.GlobalSlots { + if !p.drop(bundle) { + return ErrBundleGasPriceLow + } } bundle.Price = price @@ -199,9 +201,6 @@ func (p *BundlePool) AddBundle(bundle *types.Bundle, originBundle *types.SendBun bundleDeliverAll.Inc(1) } - for p.slots+numSlots(bundle) > p.config.GlobalSlots { - p.drop(bundle) - } p.bundles[hash] = bundle heap.Push(&p.bundleHeap, bundle) p.slots += numSlots(bundle) @@ -397,7 +396,7 @@ func (p *BundlePool) deleteBundle(hash common.Hash) { } // drop removes the bundle with the lowest gas price from the pool. -func (p *BundlePool) drop(bundle *types.Bundle) { +func (p *BundlePool) drop(bundle *types.Bundle) bool { dropBundles := make([]*types.Bundle, 0, numSlots(bundle)) dropSlots := uint64(0) for len(p.bundleHeap) > 0 { @@ -405,18 +404,26 @@ func (p *BundlePool) drop(bundle *types.Bundle) { for _, dropBundle := range dropBundles { p.deleteBundle(dropBundle.Hash()) } - break + return true } // Pop the bundle with the lowest gas price // the min element in the heap may not exist in the pool as it may be pruned leastPriceBundleHash := heap.Pop(&p.bundleHeap).(*types.Bundle).Hash() leastPriceBundle, _ := p.bundles[leastPriceBundleHash] - if leastPriceBundle != nil && leastPriceBundle.Price.Cmp(bundle.Price) < 0 { - dropBundles = append(dropBundles, leastPriceBundle) - dropSlots = dropSlots + numSlots(leastPriceBundle) - continue + if _, ok := p.bundles[leastPriceBundleHash]; ok { + if leastPriceBundle.Price.Cmp(bundle.Price) < 0 { + dropBundles = append(dropBundles, leastPriceBundle) + dropSlots = dropSlots + numSlots(leastPriceBundle) + continue + } else { + for _, dropBundle := range dropBundles { + heap.Push(&p.bundleHeap, dropBundle) + } + return false + } } } + return false } // minimalBundleGasPrice return the lowest gas price from the pool. From 2e0a629be0a6a3942c55f9c7d40da64a3dae62a7 Mon Sep 17 00:00:00 2001 From: redhdx Date: Wed, 14 Aug 2024 15:51:32 +0800 Subject: [PATCH 3/4] feature(op-geth): add bundlepool unit test and fix drop issue --- core/txpool/bundlepool/bundlepool.go | 4 +- core/txpool/bundlepool/bundlepool_test.go | 136 ++++++++++++++++++++++ 2 files changed, 138 insertions(+), 2 deletions(-) create mode 100644 core/txpool/bundlepool/bundlepool_test.go diff --git a/core/txpool/bundlepool/bundlepool.go b/core/txpool/bundlepool/bundlepool.go index 21f39aa701..97c545ff29 100644 --- a/core/txpool/bundlepool/bundlepool.go +++ b/core/txpool/bundlepool/bundlepool.go @@ -409,13 +409,13 @@ func (p *BundlePool) drop(bundle *types.Bundle) bool { // Pop the bundle with the lowest gas price // the min element in the heap may not exist in the pool as it may be pruned leastPriceBundleHash := heap.Pop(&p.bundleHeap).(*types.Bundle).Hash() - leastPriceBundle, _ := p.bundles[leastPriceBundleHash] - if _, ok := p.bundles[leastPriceBundleHash]; ok { + if leastPriceBundle, ok := p.bundles[leastPriceBundleHash]; ok { if leastPriceBundle.Price.Cmp(bundle.Price) < 0 { dropBundles = append(dropBundles, leastPriceBundle) dropSlots = dropSlots + numSlots(leastPriceBundle) continue } else { + heap.Push(&p.bundleHeap, leastPriceBundle) for _, dropBundle := range dropBundles { heap.Push(&p.bundleHeap, dropBundle) } diff --git a/core/txpool/bundlepool/bundlepool_test.go b/core/txpool/bundlepool/bundlepool_test.go new file mode 100644 index 0000000000..bc32499a62 --- /dev/null +++ b/core/txpool/bundlepool/bundlepool_test.go @@ -0,0 +1,136 @@ +package bundlepool + +import ( + "container/heap" + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/rawdb" + "github.com/ethereum/go-ethereum/core/state" + "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/miner" + "github.com/ethereum/go-ethereum/params" + "math/big" + "testing" +) + +type testBlockChain struct { + config *params.ChainConfig + gasLimit uint64 + statedb *state.StateDB +} + +func (bc *testBlockChain) Config() *params.ChainConfig { + return bc.config +} + +func (bc *testBlockChain) CurrentBlock() *types.Header { + return &types.Header{ + Number: new(big.Int), + } +} + +func (bc *testBlockChain) GetBlock(hash common.Hash, number uint64) *types.Block { + return nil +} + +func (bc *testBlockChain) StateAt(root common.Hash) (*state.StateDB, error) { + return bc.statedb, nil +} + +func setupBundlePool(config Config, mevConfig miner.MevConfig) *BundlePool { + return setupBundlePoolWithConfig(config, mevConfig) +} + +func setupBundlePoolWithConfig(config Config, mevConfig miner.MevConfig) *BundlePool { + statedb, _ := state.New(types.EmptyRootHash, state.NewDatabase(rawdb.NewMemoryDatabase()), nil) + blockchain := newTestBlockChain(params.TestChainConfig, 100000000, statedb) + + pool := New(config, mevConfig, blockchain) + return pool +} + +func newTestBlockChain(config *params.ChainConfig, gasLimit uint64, statedb *state.StateDB) *testBlockChain { + bc := testBlockChain{config: config, gasLimit: gasLimit, statedb: statedb} + return &bc +} + +func TestBundlePoolDrop(t *testing.T) { + bundleConfig := Config{GlobalSlots: 5} + mevConfig := miner.MevConfig{MevEnabled: true} + bundlepool := setupBundlePool(bundleConfig, mevConfig) + for i := 0; i < 5; i++ { + bundle := &types.Bundle{ + Price: big.NewInt(int64(i)), + } + hash := bundle.Hash() + bundlepool.bundles[hash] = bundle + heap.Push(&bundlepool.bundleHeap, bundle) + bundlepool.slots += numSlots(bundle) + } + + // now bundle pool order by price [0, 1, 2, 3, 4] + // test drop, one bundle with one slot replace success + bundle := &types.Bundle{ + Txs: nil, + Price: big.NewInt(5), + } + if !bundlepool.drop(bundle) { + t.Errorf("bundle pool drop expect success, but failed") + } + // check old least price bundle (bundle price is 0) not exist in bundle pool + leastPriceBundleHash := heap.Pop(&bundlepool.bundleHeap).(*types.Bundle).Hash() + if leastPriceBundle, ok := bundlepool.bundles[leastPriceBundleHash]; ok { + if leastPriceBundle.Price.Uint64() == 0 { + t.Errorf("old bundle expect not in bundlepool, but in") + } + heap.Push(&bundlepool.bundleHeap, leastPriceBundle) + } + hash := bundle.Hash() + bundlepool.bundles[hash] = bundle + heap.Push(&bundlepool.bundleHeap, bundle) + bundlepool.slots += numSlots(bundle) + + // now bundle pool as price [1, 2, 3, 4, 5] + // test drop, one bundle with 2 slot replace failed + data := make([]byte, bundleSlotSize+1) + for i := range data { + data[i] = 0xFF + } + txs := types.Transactions{ + types.NewTx(&types.LegacyTx{ + Data: data, + }), + } + bundle = &types.Bundle{ + Txs: txs, + Price: big.NewInt(2), + } + if bundlepool.drop(bundle) { + t.Errorf("bundle pool drop expect failed, but success") + } + // check old least price bundle (bundle price is 1) exist in bundle pool, not dropped + leastPriceBundleHash = heap.Pop(&bundlepool.bundleHeap).(*types.Bundle).Hash() + if leastPriceBundle, ok := bundlepool.bundles[leastPriceBundleHash]; ok { + if leastPriceBundle.Price.Uint64() != 1 { + t.Errorf("old bundle expect in bundlepool, but dropped") + } + heap.Push(&bundlepool.bundleHeap, leastPriceBundle) + } + + // now bundle pool as price [1, 2, 3, 4, 5] + // test drop, one bundle with 2 slot replace success + bundle = &types.Bundle{ + Txs: txs, + Price: big.NewInt(3), + } + if !bundlepool.drop(bundle) { + t.Errorf("bundle pool drop expect success, but failed") + } + // check old least bundle (bundle price is 1 and 2) not exist in bundle pool, dropped + leastPriceBundleHash = heap.Pop(&bundlepool.bundleHeap).(*types.Bundle).Hash() + if leastPriceBundle, ok := bundlepool.bundles[leastPriceBundleHash]; ok { + if leastPriceBundle.Price.Uint64() != 3 { + t.Errorf("old bundle expect not in bundlepool, but in") + } + heap.Push(&bundlepool.bundleHeap, leastPriceBundle) + } +} From 059cf8cdad7ab6f5b4613135b5df2e5999dc42b0 Mon Sep 17 00:00:00 2001 From: redhdx Date: Thu, 15 Aug 2024 15:31:41 +0800 Subject: [PATCH 4/4] feature(op-geth): refine addBundle logic --- core/txpool/bundlepool/bundlepool.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/core/txpool/bundlepool/bundlepool.go b/core/txpool/bundlepool/bundlepool.go index 97c545ff29..b9b30dbc4a 100644 --- a/core/txpool/bundlepool/bundlepool.go +++ b/core/txpool/bundlepool/bundlepool.go @@ -171,21 +171,21 @@ func (p *BundlePool) AddBundle(bundle *types.Bundle, originBundle *types.SendBun if err != nil { return err } + bundle.Price = price p.mu.Lock() defer p.mu.Unlock() + hash := bundle.Hash() + if _, ok := p.bundles[hash]; ok { + return ErrBundleAlreadyExist + } + if p.slots+numSlots(bundle) > p.config.GlobalSlots { if !p.drop(bundle) { return ErrBundleGasPriceLow } } - bundle.Price = price - - hash := bundle.Hash() - if _, ok := p.bundles[hash]; ok { - return ErrBundleAlreadyExist - } for url, cli := range p.bundleReceiverClients { cli := cli