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.

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

fix: enforce strict timeout for builder to provide bid #7151

Merged
merged 3 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
4 changes: 2 additions & 2 deletions packages/beacon-node/src/api/impl/validator/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ import {computeSubnetForCommitteesAtSlot, getPubkeysForIndices, selectBlockProdu
export const SYNC_TOLERANCE_EPOCHS = 1;

/**
* Cutoff time to wait for execution and builder block production apis to resolve
* Cutoff time to wait from start of the slot for execution and builder block production apis to resolve
* Post this time, race execution and builder to pick whatever resolves first
*
* Empirically the builder block resolves in ~1.5+ seconds, and execution should resolve <1 sec.
Expand Down Expand Up @@ -631,7 +631,7 @@ export function getValidatorApi(
: Promise.reject(new Error("Engine disabled"));

const [builder, engine] = await resolveOrRacePromises([builderPromise, enginePromise], {
resolveTimeoutMs: BLOCK_PRODUCTION_RACE_CUTOFF_MS,
resolveTimeoutMs: Math.max(0, BLOCK_PRODUCTION_RACE_CUTOFF_MS - chain.clock.secFromSlot(slot) * 1000),
raceTimeoutMs: BLOCK_PRODUCTION_RACE_TIMEOUT_MS,
signal: controller.signal,
});
Expand Down
10 changes: 9 additions & 1 deletion packages/beacon-node/src/execution/builder/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ export const defaultExecutionBuilderHttpOpts: ExecutionBuilderHttpOpts = {
timeout: 12000,
};

/**
* Duration given to the builder to provide a `SignedBuilderBid` before the deadline
* is reached, aborting the external builder flow in favor of the local build process.
*/
const BUILDER_PROPOSAL_DELAY_TOLERANCE = 1000;
Copy link
Contributor

Choose a reason for hiding this comment

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

this is too tight, 1.5 sec seems more reasonable

Copy link
Member Author

Choose a reason for hiding this comment

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

mev-boost uses a default value of 950ms (slightly lower to account for passing response to CL), this is what all other CLs use and the spec defines

Copy link
Member Author

Choose a reason for hiding this comment

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

as discussed on discord, this is as per spec


export class ExecutionBuilderHttp implements IExecutionBuilder {
readonly api: BuilderApi;
readonly config: ChainForkConfig;
Expand Down Expand Up @@ -115,7 +121,9 @@ export class ExecutionBuilderHttp implements IExecutionBuilder {
executionPayloadValue: Wei;
blobKzgCommitments?: deneb.BlobKzgCommitments;
}> {
const signedBuilderBid = (await this.api.getHeader({slot, parentHash, proposerPubkey})).value();
const signedBuilderBid = (
await this.api.getHeader({slot, parentHash, proposerPubkey}, {timeoutMs: BUILDER_PROPOSAL_DELAY_TOLERANCE})
).value();

if (!signedBuilderBid) {
throw Error("No bid received");
Expand Down
Loading