Skip to content
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

fix: do not always subscribe to all column subnets #7607

Draft
wants to merge 1 commit into
base: peerDAS
Choose a base branch
from

Conversation

twoeths
Copy link
Contributor

@twoeths twoeths commented Mar 20, 2025

Motivation

  • right now we always subscribe to all column subnets which is not necessary because we only need columns that we custody/sample
  • that's why I see 128 logs like this per slot, and this could degrade performance. We only need 8 per slot
 debug: Received gossip dataColumn slot=198378, root=0x9945…49af, curentSlot=198378, peerId=16Uiu2HAm2J4ZRFnf7qWvu8VBRFT66E5XFWnRTwYTahB2oRJom5ji, delaySec=1.5910000801086426, gossipIndex=113, columnIndex=113, pending=data_column, haveColumns=31, expectedColumns=8, recvToValLatency=0.0009999275207519531, recvToValidation=0.0009999275207519531, validationTime=0

Description

  • only subscribe to custody/sampling subnets

@twoeths twoeths requested a review from a team as a code owner March 20, 2025 03:14
Copy link
Member

@matthewkeil matthewkeil left a comment

Choose a reason for hiding this comment

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

This is great!!! We were just talking about this actually on the call with the Base contributor. A couple things jump out at me that we may need to consider.

For validator custody the number that we custody/sample will be variable so we should probably use a centralized source of truth for "what count we use." There is an object called the custodyConfig that is built at startup time and that we likely could update as part of the validator custody scope. Could we pass that into the gossip sub and read the value from that instead of recalculating it?

The second is block publishing. We will need to publish all 128 columns when we produce a block so we would need to figure out how to subscribe and unsubscribe during the epoch that we have publish duties.

This is a great idea though!!

@twoeths
Copy link
Contributor Author

twoeths commented Mar 20, 2025

The second is block publishing. We will need to publish all 128 columns when we produce a block so we would need to figure out how to subscribe and unsubscribe during the epoch that we have publish duties.

to publish it's about to find peers on a topic, not about subscribe to that topic. That's a blocker for this work, will look into that part.

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