AllCoreDevs - Testing CL Breakout #092
Transcript
- Justin Traglia
- Nico Flaig
it's consensus breaking
- Eitan Seri-Levi
we can include
- Stefan Bratanov
will include for Teku after the call, happy with it
- Eitan Seri-Levi
we don’t need another release I think
- Justin Traglia
- Mikhail Kalinin
I think we should introduce this change in gloas/fork_choice if there is a decision to have it at all
- Nico Flaig
it will become better in gloas fwiw since no more columsn etc.
- Nico Flaig
there is MIN_EPOCHS_FOR_BLOCK_REQUESTS which is ~5 months, pretty sure that's the current time CL need to serve blocks?
- Luca | Vero
Is there a target date by which we want to have all Beacon API changes merged in? Any blockers or open questions left for https://github.com/ethereum/beacon-APIs/pull/630 ?
- Justin Traglia
Replying to "Is there a target da..." Will chat about this after this topic
- Nico Flaig
Replying to "Is there a target da..." need to give a final pass, just did some more review over the weekend but seems getting close to being mergable
- Luca | Vero
Replying to "Is there a target da..." ok thanks
- Nico Flaig
Replying to "Is there a target da..." but we reworked the pr like 5 times since it was opened so would be good if people can give it a final pass
- Nico Flaig
Replying to "Is there a target da..." (if github works again)
- Justin Traglia
Reacted to "(if github works again)" with 😅
- Kevaundray Wedderburn
Prysm: https://github.com/OffchainLabs/prysm/blob/462e52404e3965fc0ed4bc1bf7717812d9dd3ef7/cmd/beacon-chain/sync/backfill/flags/flags.go#L12, Lodestar: https://github.com/ChainSafe/lodestar/blob/ec596194e2b909e30e3c6b095fb836427af0c4fb/packages/beacon-node/src/sync/options.ts#L42, Grandine: https://github.com/grandinetech/grandine/blob/eaf220e60699cd63d4223ad2481e42fd15f67802/runtime/src/grandine_args.rs#L2027
- Nico Flaig
yeah we are not
- Luca | Vero
Reacted to "but we reworked the pr like 5 times since it was opened so would be good if people can give it a final pass" with 👍
- Mikhail Kalinin
Might be worth taking into account the worst case of non-finality
- Jason Vranek
#88 keymanagre has one open question https://github.com/ethereum/keymanager-APIs/pull/88#discussion_r3789665268
- Kevaundray Wedderburn
Replying to "Might be worth takin..." Yeah this is a good point, I think 8252 implicitly puts it as around a month
- Kevaundray Wedderburn
Replying to "Might be worth takin..." https://eips.ethereum.org/EIPS/eip-8252
- Mikhail Kalinin
Reacted to "Yeah this is a good point, I think 8252 implicitly puts it as around a month" with 👍
- Jason Vranek
will keep as is then 🙂
- Kevaundray Wedderburn
Replying to "Might be worth takin..." Though I guess one of them is related to the inactivity leak (non-finality and reorg window) whereas the other is related to the weak subjectivity period (historical block retention)
- james
Not yet e2e
- james
Km88 is implemented
- james
But not 630
- james
For prysm
- Eitan Seri-Levi
Mark did a full impl, we just haven’t reviewed/merged it yet
- Mikhail Kalinin
Replying to "Might be worth takin..." There is just one parameter in that EIP, where is the other one?
- Kevaundray Wedderburn
Replying to "Might be worth takin..." Ah I was thinking that the reorg retention window would implicitly define the worse case of non-finality
- Kevaundray Wedderburn
Replying to "Might be worth takin..." Or maybe I misunderstand?
- Nico Flaig
I need approvals from implementers on the 3 prs so we can merge it
- Mikhail Kalinin
Replying to "Might be worth takin..." Oh, yeah, according to that EIP it does so
- Stefan Bratanov
Reacted to "I need approvals from implementers on the 3 prs so we can merge it" with 👍
- Kevaundray Wedderburn
Replying to "Might be worth takin..." Though its definitely possible to only have the EL reorg say one week back, and then anything further than that would require maybe checkpoint syncing and re-executing blocks
- Mikhail Kalinin
Replying to "Might be worth takin..." But this EIP sets the lower boundary for reorg capability, not a number of blocks to retain
- Kevaundray Wedderburn
Reacted to "But this EIP sets the lower boundary for reorg capability, not a number of blocks to retain" with 👍
Call summary
Targets
- •Mid-September: Gloas state transition compliance test suite available - 00:08:07
Decisions
- •Bug fix (payment weight double count) included in DevNet 8 images; no new spec release required - 00:03:27
- •Epoch boundary reorg PR likely to be merged unless client objections raised async - 00:12:33
- •beacon-APIs PR #630 Get endpoint keeps pub key in response (as-is) - 00:25:31
Highlights
- Gloas Bug Fix:
- ·Fix Builder Payment Weight Double Count bug merged; consensus-breaking but hard to trigger - 00:00:47
- ·Bug fix to be included in DevNet 8 images; no new alpha release needed - 00:03:27
- Direct Builder Api:
- ·beacon-APIs PR #630 near mergeable; reworked 5 times, needs final review from implementers - 00:23:10
- ·Lodestar has DirectBuilder API implemented end-to-end; Lighthouse impl done but unreviewed; Teku in progress; Prysm KeyManager #88 done but not #630 - 00:25:34
- ·Goal: test direct builder bid at Gloas SLOT 0 on DevNet 8 - 00:27:05
- Epoch Boundary Reorg:
- ·Potuz PR proposes removing epoch-boundary reorg check post-Fulu; Lighthouse already allows such reorgs on mainnet - 00:09:24
- Gloas Compliance Testing:
- ·State transition compliance test suite in progress; model-based, same approach as fork choice suite - 00:04:07
- ·Small subset currently running with Teku; full suite expected mid-September - 00:06:07
- ·Client teams asked to run existing fork choice compliance tests and report blockers on Discord - 00:07:12
- Historical Block Retention:
- ·MIN_EPOCHS_FOR_BLOCK_REQUESTS safety decay of 100 (~5 months) questioned; no clear rationale found - 00:14:49
- ·Kev proposes 8192 epochs (2x safety decay of 10) to align with blob retention; Prysm, Lodestar, Grandine don't backfill to 5 months by default - 00:21:24
Action Items
- •Kevaundray Wedderburn - Write EIP to codify reduced historical block retention constant (8192 epochs) - 00:22:37
- •CL teams - Review and approve beacon-APIs PR #630, builder-spec, and keymanager-APIs PRs so they can merge - 00:29:08
Key decisions
Bug fix (payment weight double count) included in DevNet 8 images; no new spec release required
DevNet 8Fixes a consensus-breaking bug in Builder Payment Weight calculationEpoch boundary reorg PR likely to be merged unless client objections raised async
Potuz PR proposes removing epoch-boundary reorg check post-Fulubeacon-APIs PR #630 Get endpoint keeps pub key in response (as-is)
Decision regarding the structure of the DirectBuilder API response