-
Notifications
You must be signed in to change notification settings - Fork 957
Remove redundant subscribe_all_data_column_subnets field from network
#8259
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
Remove redundant subscribe_all_data_column_subnets field from network
#8259
Conversation
Co-authored-by: pawanjay176 <[email protected]>
… validator should have `validator_custody_at_head` set to 0.
f3a3e2a to
029ba13
Compare
|
Some required checks have failed. Could you please take a look @jimmygchen? 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@jimmygchen this commit fixes the failing test 4084d11
pawanjay176
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just one nit
Co-Authored-By: dapplion <[email protected]>
fe778b3 to
c19e963
Compare
|
@dapplion thanks for the fix, I've pushed your changes. |
…rk (sigp#8259) Addresses this comment: sigp#8254 (comment) We're currently using `subscribe_all_data_column_subnets` here to subscribe to all subnets https://github.com/sigp/lighthouse/blob/522bd9e9c6ac167f2231525e937c9ebbcb86cf6e/beacon_node/lighthouse_network/src/types/topics.rs#L82-L92 But its unnecessary because the else path also works for supernode (uses `sampling_subnets` instead) The big diffs will disappear once sigp#8254 is merged. Co-Authored-By: Jimmy Chen <[email protected]>
…rk (sigp#8259) Addresses this comment: sigp#8254 (comment) We're currently using `subscribe_all_data_column_subnets` here to subscribe to all subnets https://github.com/sigp/lighthouse/blob/522bd9e9c6ac167f2231525e937c9ebbcb86cf6e/beacon_node/lighthouse_network/src/types/topics.rs#L82-L92 But its unnecessary because the else path also works for supernode (uses `sampling_subnets` instead) The big diffs will disappear once sigp#8254 is merged. Co-Authored-By: Jimmy Chen <[email protected]>
Issue Addressed
Addresses this comment: #8254 (comment)
We're currently using
subscribe_all_data_column_subnetshere to subscribe to all subnetslighthouse/beacon_node/lighthouse_network/src/types/topics.rs
Lines 82 to 92 in 522bd9e
But its unnecessary because the else path also works for supernode (uses
sampling_subnetsinstead)The big diffs will disappear once #8254 is merged.