diff --git a/miner/payload_building.go b/miner/payload_building.go index 29e9a50182..9d6f105760 100644 --- a/miner/payload_building.go +++ b/miner/payload_building.go @@ -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: @@ -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 { @@ -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) }) } diff --git a/miner/payload_building_test.go b/miner/payload_building_test.go index 84ee0fb15c..ecbb249999 100644 --- a/miner/payload_building_test.go +++ b/miner/payload_building_test.go @@ -17,6 +17,7 @@ package miner import ( + "math/big" "reflect" "testing" "time" @@ -30,11 +31,14 @@ 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") @@ -42,6 +46,14 @@ func testBuildPayload(t *testing.T, noTxPool bool) { 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(), @@ -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) } } @@ -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)) } @@ -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{