Skip to content

Commit

Permalink
miner: Interrupt first payload building job
Browse files Browse the repository at this point in the history
Also added interrupt test.
Had to add sleep to make non-interrupt test work.
  • Loading branch information
sebastianst committed Dec 15, 2023
1 parent d880c81 commit 597ba58
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 9 deletions.
31 changes: 27 additions & 4 deletions miner/payload_building.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,10 @@ func (payload *Payload) resolve(onlyFull bool) *engine.ExecutionPayloadEnvelope
payload.lock.Lock()
defer payload.lock.Unlock()

// We interrupt any active building block to prevent it from adding more transactions,
// and if it is an update, don't attempt to seal the block.
payload.interruptBuilding()

if payload.full == nil && (onlyFull || payload.empty == nil) {
select {
case <-payload.stop:
Expand All @@ -198,6 +202,7 @@ func (payload *Payload) resolve(onlyFull bool) *engine.ExecutionPayloadEnvelope
payload.cond.Wait()
}

// Now we can signal the building routine to stop.
payload.stopBuilding()

if payload.full != nil {
Expand All @@ -210,14 +215,32 @@ func (payload *Payload) resolve(onlyFull bool) *engine.ExecutionPayloadEnvelope
return nil
}

// stopBuilding sets an interrupt for any potentially ongoing
// block building process and signals stop to the block updating routine.
// interruptBuilding sets an interrupt for a potentially ongoing
// block building process.
// This will prevent it from adding new transactions to the block, and if it is
// building an update, the block will also not be sealed, as we would discard
// the update anyways.
// interruptBuilding is safe to be called concurrently.
func (payload *Payload) interruptBuilding() {
// Set the interrupt if not interrupted already.
// It's ok if it has either already been interrupted by payload resolution earlier,
// or by the timeout timer set to commitInterruptTimeout.
if payload.interrupt.CompareAndSwap(commitInterruptNone, commitInterruptResolve) {
log.Debug("Interrupted payload building.", "id", payload.id)
} else {
log.Debug("Payload building already interrupted.",
"id", payload.id, "interrupt", payload.interrupt.Load())
}
}

// stopBuilding signals to the block updating routine to stop. An ongoing payload
// building job will still complete. It can be interrupted to stop filling new
// transactions with interruptBuilding.
// stopBuilding is safe to be called concurrently.
func (payload *Payload) stopBuilding() {
// Concurrent Resolve calls should only stop once.
payload.stopOnce.Do(func() {
log.Debug("Interrupt payload building.", "id", payload.id)
payload.interrupt.Store(commitInterruptResolve)
log.Debug("Stop payload building.", "id", payload.id)
close(payload.stop)
})
}
Expand Down
46 changes: 41 additions & 5 deletions miner/payload_building_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package miner

import (
"math/big"
"reflect"
"testing"
"time"
Expand All @@ -30,18 +31,29 @@ import (
)

func TestBuildPayload(t *testing.T) {
t.Run("with-tx-pool", func(t *testing.T) { testBuildPayload(t, false) })
t.Run("no-tx-pool", func(t *testing.T) { testBuildPayload(t, true) })
t.Run("no-tx-pool", func(t *testing.T) { testBuildPayload(t, true, false) })
// no-tx-pool case with interrupt not interesting because no-tx-pool doesn't run
// the builder routine
t.Run("with-tx-pool", func(t *testing.T) { testBuildPayload(t, false, false) })
t.Run("with-tx-pool-interrupt", func(t *testing.T) { testBuildPayload(t, false, true) })
}

func testBuildPayload(t *testing.T, noTxPool bool) {
func testBuildPayload(t *testing.T, noTxPool, interrupt bool) {
var (
db = rawdb.NewMemoryDatabase()
recipient = common.HexToAddress("0xdeadbeef")
)
w, b := newTestWorker(t, params.TestChainConfig, ethash.NewFaker(), db, 0)
defer w.close()

const numInterruptTxs = 256
if interrupt {
// when donig interrupt testing, create a large pool so interruption will
// definitely be visible.
txs := genTxs(1, numInterruptTxs)
b.txPool.Add(txs, true, false)
}

timestamp := uint64(time.Now().Unix())
args := &BuildPayloadArgs{
Parent: b.chain.CurrentBlock().Hash(),
Expand Down Expand Up @@ -74,8 +86,10 @@ func testBuildPayload(t *testing.T, noTxPool bool) {
if payload.FeeRecipient != recipient {
t.Fatal("Unexpect fee recipient")
}
if len(payload.Transactions) != txs {
if !interrupt && len(payload.Transactions) != txs {
t.Fatalf("Unexpect transaction set: got %d, expected %d", len(payload.Transactions), txs)
} else if interrupt && len(payload.Transactions) >= txs {
t.Fatalf("Unexpect transaction set: got %d, expected less than %d", len(payload.Transactions), txs)
}
}

Expand All @@ -85,7 +99,13 @@ func testBuildPayload(t *testing.T, noTxPool bool) {
verify(empty, 0)
full := payload.ResolveFull()
verify(full, 0)
} else {
} else if interrupt {
full := payload.ResolveFull()
verify(full, len(pendingTxs)+numInterruptTxs)
} else { // tx-pool and no interrupt
// wait to fully build the first block
// if we call ResolveFull too fast, it interrupts it and doesn't add any tx
time.Sleep(time.Second)
full := payload.ResolveFull()
verify(full, len(pendingTxs))
}
Expand All @@ -99,6 +119,22 @@ func testBuildPayload(t *testing.T, noTxPool bool) {
}
}

func genTxs(startNonce, count uint64) types.Transactions {
txs := make(types.Transactions, 0, count)
signer := types.LatestSigner(params.TestChainConfig)
for nonce := startNonce; nonce < startNonce+count; nonce++ {
txs = append(txs, types.MustSignNewTx(testBankKey, signer, &types.AccessListTx{
ChainID: params.TestChainConfig.ChainID,
Nonce: nonce,
To: &testUserAddress,
Value: big.NewInt(1000),
Gas: params.TxGas,
GasPrice: big.NewInt(params.InitialBaseFee),
}))
}
return txs
}

func TestPayloadId(t *testing.T) {
ids := make(map[string]int)
for i, tt := range []*BuildPayloadArgs{
Expand Down

0 comments on commit 597ba58

Please sign in to comment.