Skip to content

Run forkChoice to get correct head for proposal #4058

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

Merged
g11tech merged 3 commits into from
May 26, 2022
Merged

Run forkChoice to get correct head for proposal #4058

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d9d64e4
Run forkChoice to get correct head for proposal
g11tech May 25, 2022
77cad12
add a small comment
g11tech May 25, 2022
c424e4b
return early if already updated
g11tech May 26, 2022
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: 3 additions & 1 deletion packages/fork-choice/src/forkChoice/forkChoice.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,7 @@ export class ForkChoice implements IForkChoice {
* Call `onTick` for all slots between `fcStore.getCurrentSlot()` and the provided `currentSlot`.
*/
updateTime(currentSlot: Slot): void {
if (this.fcStore.currentSlot >= currentSlot) return;
while (this.fcStore.currentSlot < currentSlot) {
const previousSlot = this.fcStore.currentSlot;
// Note: we are relying upon `onTick` to update `fcStore.time` to ensure we don't get stuck in a loop.
Expand Down Expand Up @@ -951,7 +952,8 @@ export class ForkChoice implements IForkChoice {
private processAttestationQueue(): void {
const currentSlot = this.fcStore.currentSlot;
for (const attestation of this.queuedAttestations.values()) {
if (attestation.slot <= currentSlot) {
// Delay consideration in the fork choice until their slot is in the past.
if (attestation.slot < currentSlot) {
this.queuedAttestations.delete(attestation);
const {blockRoot, targetEpoch} = attestation;
const blockRootHex = blockRoot;
Expand Down
6 changes: 6 additions & 0 deletions packages/lodestar/src/api/impl/validator/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,12 @@ export function getValidatorApi({chain, config, logger, metrics, network, sync}:
notWhileSyncing();
await waitForSlot(slot); // Must never request for a future slot > currentSlot

// Process the queued attestations in the forkchoice for correct head estimation
// forkChoice.updateTime() might have already been called by the onSlot clock
// handler, in which case this should just return.
chain.forkChoice.updateTime(slot);

@twoeths twoeths May 26, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

calling forkChoice.updateTime() multiple times would calling processAttestationQueue() multiple times.

I think we should do early return in forkChoice.updateTime(slot) if this.fcStore.currentSlot >= slot

@g11tech g11tech May 26, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

makes sense, updating! 👍

chain.forkChoice.updateHead();

timer = metrics?.blockProductionTime.startTimer();
const block = await assembleBlock(
{chain, metrics},
Expand Down