-
-
Notifications
You must be signed in to change notification settings - Fork 405
feat: implement EIP-7928 BALs #8426
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
Draft
nflaig
wants to merge
13
commits into
unstable
Choose a base branch
from
bal-devnet-0
base: unstable
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## unstable #8426 +/- ##
============================================
- Coverage 52.26% 52.24% -0.03%
============================================
Files 853 853
Lines 64784 64810 +26
Branches 4766 4770 +4
============================================
Hits 33858 33858
- Misses 30856 30882 +26
Partials 70 70 🚀 New features to boost your workflow:
|
Performance Report🚀🚀 Significant benchmark improvement detected
Full benchmark results
|
#8444) `Date.now()` returns current unit timestamp in milliseconds but we want seconds here, ie. need to divide by 1000.
**Motivation** - we see different slots for the same root in `BlockInputSync` logs, this causes confusion - this was added last week in #8416 **Description** - the [root, slot] is not trusted in network processor because messages could be originated from an out-of-sync node - we actually don't need the slot in BlockInputSync, it's just a nice to have but caused confusion. So I reverted that part: do not emit slot to `BlockInputSync` from network processor - log the full root hex Closes #8465 --------- Co-authored-by: Tuyen Nguyen <[email protected]>
**Motivation** - when investigating #8457 I found that we did not set `hashAllColumn()` correctly at the 1st time **Description** - correct `hasAllData` when creating IBlockInput from column - nice to have: in `verifyBlocksDataAvailability`, only wait for all data if not has all data. This is to make it consistent to `writeBlockInputToDb()` where the error did not happen, see #8457 (comment) Co-authored-by: Tuyen Nguyen <[email protected]>
**Motivation** - when we remove an eagerly persisted block input, it's very normal that we did not have any columns persisted - but right now it threw this error as shown in #8457 ``` Sep-23 19:16:55.002[chain] �[33mwarn�[39m: Error pruning eagerly imported block inputs, DB may grow in size if this error happens frequently slot=30 - Invalid dataColumnSidecars=0 for custody expected custodyColumnsLen=128 Error: Invalid dataColumnSidecars=0 for custody expected custodyColumnsLen=128 ``` **Description** - do not throw error in this case part of #8457 Co-authored-by: Tuyen Nguyen <[email protected]>
**Motivation** Since #7927 we started to use parent slot (the slot of parent block) to get sync aggregate for block but this can cause `BLOCK_ERROR_INVALID_SIGNATURE` during fork transition as might include sync aggregates into our block that are older than block slot - 1 which is used to compute domain during signature verification. https://github.com/ChainSafe/lodestar/blob/8961b06c11ac67baab79fe774e0a3d1bfb217ea1/packages/state-transition/src/block/processSyncCommittee.ts#L81 This means we will reject our own block due to invalid signature if there are missed slots during the epoch transition. > we currently hard code parent slot as producedSlot - 1 which is incorrect in reorg scenario This was the motivation from the previous change but this doesn't seem right, sync committee messages are produced at latest 4 seconds into the slot so if we re-org the block due to it being weak/late then sync committee votes will also be for parent block root of that block and not the block itself. So irrespective of block being reorged or not we should always include sync committee messages from previous slot. **Description** Use previous slot to get sync aggregate for block to avoid `BLOCK_ERROR_INVALID_SIGNATURE` in case there are missed slots.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.