From d880c816174a180b573cf49314a1e04af7bec7b1 Mon Sep 17 00:00:00 2001 From: Adrian Sutton Date: Wed, 13 Dec 2023 12:19:40 +1000 Subject: [PATCH] Remove global variable to change miner behaviour. Use a longer wait in tests for the payload to build. --- eth/catalyst/api_test.go | 15 +++++++-------- miner/payload_building.go | 16 ++-------------- miner/payload_building_test.go | 1 - 3 files changed, 9 insertions(+), 23 deletions(-) diff --git a/eth/catalyst/api_test.go b/eth/catalyst/api_test.go index 93e2d0bf09..82e679bced 100644 --- a/eth/catalyst/api_test.go +++ b/eth/catalyst/api_test.go @@ -51,10 +51,6 @@ import ( "github.com/mattn/go-colorable" ) -func init() { - miner.IsPayloadBuildingTest = true -} - var ( // testKey is a private key to use for funding a tester account. testKey, _ = crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291") @@ -206,8 +202,7 @@ func TestEth2PrepareAndGetPayload(t *testing.T) { if err != nil { t.Fatalf("error preparing payload, err=%v", err) } - // give the payload some time to be built - time.Sleep(100 * time.Millisecond) + waitForPayloadToBuild() payloadID := (&miner.BuildPayloadArgs{ Parent: fcState.HeadBlockHash, Timestamp: blockParams.Timestamp, @@ -639,8 +634,7 @@ func TestNewPayloadOnInvalidChain(t *testing.T) { if resp.PayloadStatus.Status != engine.VALID { t.Fatalf("error preparing payload, invalid status: %v", resp.PayloadStatus.Status) } - // give the payload some time to be built - time.Sleep(50 * time.Millisecond) + waitForPayloadToBuild() if payload, err = api.GetPayloadV1(*resp.PayloadID); err != nil { t.Fatalf("can't get payload: %v", err) } @@ -1651,3 +1645,8 @@ func TestParentBeaconBlockRoot(t *testing.T) { t.Fatalf("incorrect root stored: want %s, got %s", *blockParams.BeaconRoot, root) } } + +func waitForPayloadToBuild() { + // give the payload some time to be built + time.Sleep(1 * time.Second) +} diff --git a/miner/payload_building.go b/miner/payload_building.go index 18b28fe76b..29e9a50182 100644 --- a/miner/payload_building.go +++ b/miner/payload_building.go @@ -33,8 +33,6 @@ import ( "github.com/ethereum/go-ethereum/rlp" ) -var IsPayloadBuildingTest = false - // BuildPayloadArgs contains the provided parameters for building payload. // Check engine-api specification for more details. // https://github.com/ethereum/execution-apis/blob/main/src/engine/cancun.md#payloadattributesv3 @@ -98,7 +96,7 @@ type Payload struct { err error stopOnce sync.Once - interrupt *atomic.Int32 // interrup signal shared with worker + interrupt *atomic.Int32 // interrupt signal shared with worker } // newPayload initializes the payload object. @@ -219,9 +217,7 @@ func (payload *Payload) stopBuilding() { // Concurrent Resolve calls should only stop once. payload.stopOnce.Do(func() { log.Debug("Interrupt payload building.", "id", payload.id) - if !IsPayloadBuildingTest { - payload.interrupt.Store(commitInterruptResolve) - } + payload.interrupt.Store(commitInterruptResolve) close(payload.stop) }) } @@ -321,14 +317,6 @@ func (w *worker) buildPayload(args *BuildPayloadArgs) (*Payload, error) { return dur } - if IsPayloadBuildingTest { - // engine api tests assume that getPayload can be called immediately after calling - // forkchoiceUpdated. This races with closing the payload building process `stop` - // channel and setting the interrupt, so in tests we always build one full - // payload. - updatePayload() - } - var lastDuration time.Duration for { select { diff --git a/miner/payload_building_test.go b/miner/payload_building_test.go index 9c26775a64..84ee0fb15c 100644 --- a/miner/payload_building_test.go +++ b/miner/payload_building_test.go @@ -30,7 +30,6 @@ import ( ) func TestBuildPayload(t *testing.T) { - IsPayloadBuildingTest = true t.Run("with-tx-pool", func(t *testing.T) { testBuildPayload(t, false) }) t.Run("no-tx-pool", func(t *testing.T) { testBuildPayload(t, true) }) }