Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(blockchain) : non optimistic client should not request build optimistically #2240

Merged
merged 8 commits into from
Dec 16, 2024
Merged

chore(blockchain) : non optimistic client should not request build optimistically #2240

Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
514f98e
non optimistic client should not sendnextFCU without attributes
nidhi-singh02 Dec 11, 2024
1b68882
linter
nidhi-singh02 Dec 11, 2024
395fdea
Merge branch 'main' into optimistic-builder
nidhi-singh02 Dec 12, 2024
a2c93c1
e2e fix
nidhi-singh02 Dec 12, 2024
75c0952
Merge branch 'main' into optimistic-builder
nidhi-singh02 Dec 13, 2024
7040e7e
Merge branch 'main' into optimistic-builder
nidhi-singh02 Dec 16, 2024
d3ca039
moved func into the calling func
nidhi-singh02 Dec 16, 2024
5ab9446
lint fix
nidhi-singh02 Dec 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 8 additions & 67 deletions beacon/blockchain/execution_engine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,15 @@ package blockchain

import (
"context"
"time"

payloadtime "github.com/berachain/beacon-kit/beacon/payload-time"
"github.com/berachain/beacon-kit/config/spec"
engineprimitives "github.com/berachain/beacon-kit/engine-primitives/engine-primitives"
)

// sendPostBlockFCU sends a forkchoice update to the execution client.
// sendPostBlockFCU sends a forkchoice update to the execution client after a
// block is finalized.This function should only be used to notify
// the EL client of the new head and should not request optimistic builds, as:
// Optimistic clients already request builds in handleOptimisticPayloadBuild()
// Non-optimistic clients should never request optimistic builds.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: this function is still called in its own goroutine. Is this necessary? Can we call it synchronously? @nidhi-singh02 @shotes

func (s *Service[
_, _, ConsensusBlockT, _, _, BeaconStateT, _, _, _, _, _, _, _, _,
]) sendPostBlockFCU(
Expand All @@ -46,69 +47,9 @@ func (s *Service[
return
}

if !s.shouldBuildOptimisticPayloads() && s.localBuilder.Enabled() {
s.sendNextFCUWithAttributes(ctx, st, blk, lph)
} else {
s.sendNextFCUWithoutAttributes(ctx, blk, lph)
}
}

// sendNextFCUWithAttributes sends a forkchoice update to the execution
// client with attributes.
func (s *Service[
_, _, ConsensusBlockT, _, _, BeaconStateT, _,
_, _, ExecutionPayloadHeaderT, _, _, _, _,
]) sendNextFCUWithAttributes(
ctx context.Context,
st BeaconStateT,
blk ConsensusBlockT,
lph ExecutionPayloadHeaderT,
) {
beaconBlk := blk.GetBeaconBlock()

stCopy := st.Copy()
if _, err := s.stateProcessor.ProcessSlots(
stCopy, beaconBlk.GetSlot()+1,
); err != nil {
s.logger.Error(
"failed to process slots in non-optimistic payload",
"error", err,
)
return
}

nextPayloadTime := payloadtime.Next(
blk.GetConsensusTime(),
lph.GetTimestamp(),
true, // buildOptimistically
).Unwrap()

// We set timestamp check on Bartio for backward compatibility reasons
// TODO: drop this we drop other Bartio special cases.
if s.chainSpec.DepositEth1ChainID() == spec.BartioChainID {
nextPayloadTime = max(
//#nosec:G701
uint64(time.Now().Unix()+1),
uint64((lph.GetTimestamp() + 1)),
)
}

prevBlockRoot := beaconBlk.HashTreeRoot()
if _, err := s.localBuilder.RequestPayloadAsync(
ctx,
stCopy,
beaconBlk.GetSlot()+1,
nextPayloadTime,
prevBlockRoot,
lph.GetBlockHash(),
lph.GetParentHash(),
); err != nil {
s.logger.Error(
"failed to send forkchoice update with attributes in non-optimistic payload",
"error",
err,
)
}
// Send a forkchoice update without payload attributes to notify
// EL of the new head.
s.sendNextFCUWithoutAttributes(ctx, blk, lph)
nidhi-singh02 marked this conversation as resolved.
Show resolved Hide resolved
abi87 marked this conversation as resolved.
Show resolved Hide resolved
}

// sendNextFCUWithoutAttributes sends a forkchoice update to the
Expand Down
Loading