Skip to content

fix: only start sending StatusV2 requests after fulu#8169

Merged
wemeetagain merged 11 commits into
peerDASfrom
nflaig/use-after-fulu
Aug 11, 2025
Merged

fix: only start sending StatusV2 requests after fulu#8169
wemeetagain merged 11 commits into
peerDASfrom
nflaig/use-after-fulu

Conversation

@nflaig

@nflaig nflaig commented Aug 9, 2025

Copy link
Copy Markdown
Member

There is no reason to use or allow StatusV2 pre-fulu and we can't properly handle it right now (see code comments for more details).

@nflaig nflaig changed the title fix: only start sending StatusV2 and MetadataV3 request after fulu fix: only start sending StatusV2 and MetadataV3 requests after fulu Aug 9, 2025
@nflaig nflaig changed the title fix: only start sending StatusV2 and MetadataV3 requests after fulu fix: only start sending StatusV2 requests after fulu Aug 9, 2025
Comment on lines +863 to -876
earliestAvailableSlot: this._earliestAvailableSlot,
};

if (isForkPostFulu(boundary.fork)) {
(status as fulu.Status).earliestAvailableSlot = this._earliestAvailableSlot;
}

return status;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this isn't strictly required anymore if we don't use v2 pre-fulu but there is also no downside to it as suggested in #8173 (comment)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

seemed unused so I deleted the file

@wemeetagain wemeetagain left a comment

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.

This looks good. What is needed to move from draft?

@nflaig

nflaig commented Aug 11, 2025

Copy link
Copy Markdown
Member Author

This looks good. What is needed to move from draft?

was trying to figure out why sim tests failed but it seems good now

@nflaig nflaig marked this pull request as ready for review August 11, 2025 13:59
@nflaig nflaig requested a review from a team as a code owner August 11, 2025 13:59
peerId,
ReqRespMethod.BeaconBlocksByRange,
// Before altair, prioritize V2. After altair only request V2
this.config.getForkSeq(this.clock.currentSlot) >= ForkSeq.altair ? [Version.V2] : [(Version.V2, Version.V1)],

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.

hahah this was evil

@wemeetagain wemeetagain merged commit 6a45c48 into peerDAS Aug 11, 2025
14 of 17 checks passed
@wemeetagain wemeetagain deleted the nflaig/use-after-fulu branch August 11, 2025 14:08
@wemeetagain

Copy link
Copy Markdown
Member

🎉 This PR is included in v1.34.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants