Skip to content

Commit

Permalink
eth/catalyst: make engine api test time independent (ethereum#30713)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
karalabe authored and zfy0701 committed Dec 3, 2024
1 parent 68b046a commit 85fa95b
Showing 1 changed file with 12 additions and 18 deletions.
30 changes: 12 additions & 18 deletions eth/catalyst/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@ func TestEth2PrepareAndGetPayload(t *testing.T) {
t.Fatalf("error preparing payload, err=%v", err)
}
// give the payload some time to be built
time.Sleep(100 * time.Millisecond)
payloadID := (&miner.BuildPayloadArgs{
Parent: fcState.HeadBlockHash,
Timestamp: blockParams.Timestamp,
Expand All @@ -217,12 +216,12 @@ func TestEth2PrepareAndGetPayload(t *testing.T) {
BeaconRoot: blockParams.BeaconRoot,
Version: engine.PayloadV1,
}).Id()
execData, err := api.GetPayloadV1(payloadID)
execData, err := api.getPayload(payloadID, true)
if err != nil {
t.Fatalf("error getting payload, err=%v", err)
}
if len(execData.Transactions) != blocks[9].Transactions().Len() {
t.Fatalf("invalid number of transactions %d != 1", len(execData.Transactions))
if len(execData.ExecutionPayload.Transactions) != blocks[9].Transactions().Len() {
t.Fatalf("invalid number of transactions %d != 1", len(execData.ExecutionPayload.Transactions))
}
// Test invalid payloadID
var invPayload engine.PayloadID
Expand Down Expand Up @@ -453,7 +452,6 @@ func startEthService(t *testing.T, genesis *core.Genesis, blocks []*types.Block)
}

mcfg := miner.DefaultConfig
mcfg.PendingFeeRecipient = testAddr
ethcfg := &ethconfig.Config{Genesis: genesis, SyncMode: downloader.FullSync, TrieTimeout: time.Minute, TrieDirtyCache: 256, TrieCleanCache: 256, Miner: mcfg}
ethservice, err := eth.New(n, ethcfg)
if err != nil {
Expand Down Expand Up @@ -628,7 +626,7 @@ func TestNewPayloadOnInvalidChain(t *testing.T) {
SafeBlockHash: common.Hash{},
FinalizedBlockHash: common.Hash{},
}
payload *engine.ExecutableData
payload *engine.ExecutionPayloadEnvelope
resp engine.ForkChoiceResponse
err error
)
Expand All @@ -640,11 +638,10 @@ func TestNewPayloadOnInvalidChain(t *testing.T) {
t.Fatalf("error preparing payload, invalid status: %v", resp.PayloadStatus.Status)
}
// give the payload some time to be built
time.Sleep(50 * time.Millisecond)
if payload, err = api.GetPayloadV1(*resp.PayloadID); err != nil {
if payload, err = api.getPayload(*resp.PayloadID, true); err != nil {
t.Fatalf("can't get payload: %v", err)
}
if len(payload.Transactions) > 0 {
if len(payload.ExecutionPayload.Transactions) > 0 {
break
}
// No luck this time we need to update the params and try again.
Expand All @@ -653,22 +650,22 @@ func TestNewPayloadOnInvalidChain(t *testing.T) {
t.Fatalf("payload should not be empty")
}
}
execResp, err := api.NewPayloadV1(*payload)
execResp, err := api.NewPayloadV1(*payload.ExecutionPayload)
if err != nil {
t.Fatalf("can't execute payload: %v", err)
}
if execResp.Status != engine.VALID {
t.Fatalf("invalid status: %v", execResp.Status)
}
fcState = engine.ForkchoiceStateV1{
HeadBlockHash: payload.BlockHash,
SafeBlockHash: payload.ParentHash,
FinalizedBlockHash: payload.ParentHash,
HeadBlockHash: payload.ExecutionPayload.BlockHash,
SafeBlockHash: payload.ExecutionPayload.ParentHash,
FinalizedBlockHash: payload.ExecutionPayload.ParentHash,
}
if _, err := api.ForkchoiceUpdatedV1(fcState, nil); err != nil {
t.Fatalf("Failed to insert block: %v", err)
}
if ethservice.BlockChain().CurrentBlock().Number.Uint64() != payload.Number {
if ethservice.BlockChain().CurrentBlock().Number.Uint64() != payload.ExecutionPayload.Number {
t.Fatalf("Chain head should be updated")
}
parent = ethservice.BlockChain().CurrentBlock()
Expand Down Expand Up @@ -1736,9 +1733,6 @@ func TestWitnessCreationAndConsumption(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)

payloadID := (&miner.BuildPayloadArgs{
Parent: fcState.HeadBlockHash,
Timestamp: blockParams.Timestamp,
Expand All @@ -1748,7 +1742,7 @@ func TestWitnessCreationAndConsumption(t *testing.T) {
BeaconRoot: blockParams.BeaconRoot,
Version: engine.PayloadV3,
}).Id()
envelope, err := api.GetPayloadV3(payloadID)
envelope, err := api.getPayload(payloadID, true)
if err != nil {
t.Fatalf("error getting payload, err=%v", err)
}
Expand Down

0 comments on commit 85fa95b

Please sign in to comment.