Skip to content

Conversation

@jimmygchen
Copy link
Member

@jimmygchen jimmygchen commented Sep 16, 2025

Addresses #8018 and more below:

  • Fixes a bug where a validator custody supernode never trigger reconstruction (currently only cli supernodes do)
  • Remove code to trigger immediate reconstruction if the message fails to be sent to beacon processor.
  • Trigger reconstruction if sampling column is > 50% of columns.
  • Add tracing to data column reconstruction.

@jimmygchen jimmygchen requested a review from jxs as a code owner September 16, 2025 02:11
@jimmygchen jimmygchen added ready-for-review The code is ready for review low-hanging-fruit Easy to resolve, get it before someone else does! tracing labels Sep 16, 2025
commit a209f72
Author: Eitan Seri- Levi <[email protected]>
Date:   Mon Sep 15 20:18:40 2025 -0700

    remove publish flag

commit a4044a9
Author: Eitan Seri- Levi <[email protected]>
Date:   Mon Sep 15 20:14:34 2025 -0700

    remove publish flag

commit eab7693
Author: Eitan Seri- Levi <[email protected]>
Date:   Mon Sep 15 18:56:19 2025 -0700

    linting

commit a3e0aec
Author: Eitan Seri- Levi <[email protected]>
Date:   Mon Sep 15 18:53:23 2025 -0700

    Delete reconstruction when processesing column rpc requests
@mergify
Copy link

mergify bot commented Sep 16, 2025

Some required checks have failed. Could you please take a look @jimmygchen? 🙏

@mergify mergify bot added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Sep 16, 2025
…e trigger to attempt reconstruction if column count is >= 50%.
@jimmygchen jimmygchen changed the title Add tracing span to reconstruction Enable reconstruction for nodes custodying more than 50% of columns and instrument tracing Sep 16, 2025
@jimmygchen jimmygchen added ready-for-review The code is ready for review das Data Availability Sampling and removed low-hanging-fruit Easy to resolve, get it before someone else does! waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Sep 16, 2025
@mergify
Copy link

mergify bot commented Sep 16, 2025

This pull request has merge conflicts. Could you please resolve them @jimmygchen? 🙏

@mergify mergify bot added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Sep 16, 2025
# Conflicts:
#	beacon_node/network/src/network_beacon_processor/gossip_methods.rs
#	beacon_node/network/src/network_beacon_processor/mod.rs
@jimmygchen jimmygchen added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Sep 16, 2025
Comment on lines 1056 to 1063
if !self
.chain
.data_availability_checker
.custody_context()
.should_attempt_reconstruction(
slot.epoch(T::EthSpec::slots_per_epoch()),
&self.chain.spec,
)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if !self
.chain
.data_availability_checker
.custody_context()
.should_attempt_reconstruction(
slot.epoch(T::EthSpec::slots_per_epoch()),
&self.chain.spec,
)
if self
.chain
.data_availability_checker
.custody_context()
.should_attempt_reconstruction(
slot.epoch(T::EthSpec::slots_per_epoch()),
&self.chain.spec,
)

@eserilev
Copy link
Member

other than my comment, this looks good to me

@jimmygchen jimmygchen added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Sep 16, 2025
@jimmygchen
Copy link
Member Author

@mergify dequeue

@mergify
Copy link

mergify bot commented Sep 16, 2025

This pull request has been removed from the queue for the following reason: pull request manually updated.

The pull request #8052 has been manually updated.

If you want to requeue this pull request, you can post a @mergifyio requeue comment.

@mergify
Copy link

mergify bot commented Sep 16, 2025

dequeue

☑️ The pull request is not queued

@mergify mergify bot merged commit 3de646c into sigp:unstable Sep 16, 2025
37 checks passed
jtraglia pushed a commit to jtraglia/lighthouse that referenced this pull request Sep 16, 2025
…nd instrument tracing (sigp#8052)

Co-Authored-By: Jimmy Chen <[email protected]>

Co-Authored-By: Jimmy Chen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

das Data Availability Sampling ready-for-merge This PR is ready to merge. tracing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants