Skip to content

Commit 9bb5194

Browse files
karalabeholiman
authored andcommitted
eth/catalyst: make engine api test time independent (#30713)
This test depends on a 100ms timer, which fails quite often, messing up our pipelines. Hook directly into the internal version of getPayload which has the capacity to wait for the full payload before returning. This might not be absolutely correct from a test perspective, but it beats failing ci. The alternative would be to expose the full build hook into the outside, but it might be a bit overkill for this scenario.
1 parent 5de5f33 commit 9bb5194

File tree

1 file changed

+12
-18
lines changed

1 file changed

+12
-18
lines changed

eth/catalyst/api_test.go

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,6 @@ func TestEth2PrepareAndGetPayload(t *testing.T) {
208208
t.Fatalf("error preparing payload, err=%v", err)
209209
}
210210
// give the payload some time to be built
211-
time.Sleep(100 * time.Millisecond)
212211
payloadID := (&miner.BuildPayloadArgs{
213212
Parent: fcState.HeadBlockHash,
214213
Timestamp: blockParams.Timestamp,
@@ -217,12 +216,12 @@ func TestEth2PrepareAndGetPayload(t *testing.T) {
217216
BeaconRoot: blockParams.BeaconRoot,
218217
Version: engine.PayloadV1,
219218
}).Id()
220-
execData, err := api.GetPayloadV1(payloadID)
219+
execData, err := api.getPayload(payloadID, true)
221220
if err != nil {
222221
t.Fatalf("error getting payload, err=%v", err)
223222
}
224-
if len(execData.Transactions) != blocks[9].Transactions().Len() {
225-
t.Fatalf("invalid number of transactions %d != 1", len(execData.Transactions))
223+
if len(execData.ExecutionPayload.Transactions) != blocks[9].Transactions().Len() {
224+
t.Fatalf("invalid number of transactions %d != 1", len(execData.ExecutionPayload.Transactions))
226225
}
227226
// Test invalid payloadID
228227
var invPayload engine.PayloadID
@@ -453,7 +452,6 @@ func startEthService(t *testing.T, genesis *core.Genesis, blocks []*types.Block)
453452
}
454453

455454
mcfg := miner.DefaultConfig
456-
mcfg.PendingFeeRecipient = testAddr
457455
ethcfg := &ethconfig.Config{Genesis: genesis, SyncMode: downloader.FullSync, TrieTimeout: time.Minute, TrieDirtyCache: 256, TrieCleanCache: 256, Miner: mcfg}
458456
ethservice, err := eth.New(n, ethcfg)
459457
if err != nil {
@@ -628,7 +626,7 @@ func TestNewPayloadOnInvalidChain(t *testing.T) {
628626
SafeBlockHash: common.Hash{},
629627
FinalizedBlockHash: common.Hash{},
630628
}
631-
payload *engine.ExecutableData
629+
payload *engine.ExecutionPayloadEnvelope
632630
resp engine.ForkChoiceResponse
633631
err error
634632
)
@@ -640,11 +638,10 @@ func TestNewPayloadOnInvalidChain(t *testing.T) {
640638
t.Fatalf("error preparing payload, invalid status: %v", resp.PayloadStatus.Status)
641639
}
642640
// give the payload some time to be built
643-
time.Sleep(50 * time.Millisecond)
644-
if payload, err = api.GetPayloadV1(*resp.PayloadID); err != nil {
641+
if payload, err = api.getPayload(*resp.PayloadID, true); err != nil {
645642
t.Fatalf("can't get payload: %v", err)
646643
}
647-
if len(payload.Transactions) > 0 {
644+
if len(payload.ExecutionPayload.Transactions) > 0 {
648645
break
649646
}
650647
// No luck this time we need to update the params and try again.
@@ -653,22 +650,22 @@ func TestNewPayloadOnInvalidChain(t *testing.T) {
653650
t.Fatalf("payload should not be empty")
654651
}
655652
}
656-
execResp, err := api.NewPayloadV1(*payload)
653+
execResp, err := api.NewPayloadV1(*payload.ExecutionPayload)
657654
if err != nil {
658655
t.Fatalf("can't execute payload: %v", err)
659656
}
660657
if execResp.Status != engine.VALID {
661658
t.Fatalf("invalid status: %v", execResp.Status)
662659
}
663660
fcState = engine.ForkchoiceStateV1{
664-
HeadBlockHash: payload.BlockHash,
665-
SafeBlockHash: payload.ParentHash,
666-
FinalizedBlockHash: payload.ParentHash,
661+
HeadBlockHash: payload.ExecutionPayload.BlockHash,
662+
SafeBlockHash: payload.ExecutionPayload.ParentHash,
663+
FinalizedBlockHash: payload.ExecutionPayload.ParentHash,
667664
}
668665
if _, err := api.ForkchoiceUpdatedV1(fcState, nil); err != nil {
669666
t.Fatalf("Failed to insert block: %v", err)
670667
}
671-
if ethservice.BlockChain().CurrentBlock().Number.Uint64() != payload.Number {
668+
if ethservice.BlockChain().CurrentBlock().Number.Uint64() != payload.ExecutionPayload.Number {
672669
t.Fatalf("Chain head should be updated")
673670
}
674671
parent = ethservice.BlockChain().CurrentBlock()
@@ -1736,9 +1733,6 @@ func TestWitnessCreationAndConsumption(t *testing.T) {
17361733
if err != nil {
17371734
t.Fatalf("error preparing payload, err=%v", err)
17381735
}
1739-
// Give the payload some time to be built
1740-
time.Sleep(100 * time.Millisecond)
1741-
17421736
payloadID := (&miner.BuildPayloadArgs{
17431737
Parent: fcState.HeadBlockHash,
17441738
Timestamp: blockParams.Timestamp,
@@ -1748,7 +1742,7 @@ func TestWitnessCreationAndConsumption(t *testing.T) {
17481742
BeaconRoot: blockParams.BeaconRoot,
17491743
Version: engine.PayloadV3,
17501744
}).Id()
1751-
envelope, err := api.GetPayloadV3(payloadID)
1745+
envelope, err := api.getPayload(payloadID, true)
17521746
if err != nil {
17531747
t.Fatalf("error getting payload, err=%v", err)
17541748
}

0 commit comments

Comments
 (0)