Skip to content

Conversation

@jimmygchen
Copy link
Member

Issue Addressed

Addresses this comment: #8254 (comment)

We're currently using subscribe_all_data_column_subnets here to subscribe to all subnets

if fork_name.fulu_enabled() {
if opts.subscribe_all_data_column_subnets {
for i in 0..spec.data_column_sidecar_subnet_count {
topics.push(GossipKind::DataColumnSidecar(i.into()));
}
} else {
for subnet in &opts.sampling_subnets {
topics.push(GossipKind::DataColumnSidecar(*subnet));
}
}
}

But its unnecessary because the else path also works for supernode (uses sampling_subnets instead)

The big diffs will disappear once #8254 is merged.

@jimmygchen jimmygchen requested a review from jxs as a code owner October 22, 2025 01:06
@jimmygchen jimmygchen added code-quality ready-for-review The code is ready for review v8.1.0 Post-Fulu release labels Oct 22, 2025
@jimmygchen jimmygchen force-pushed the semi-supernode-simple-cleanup branch from f3a3e2a to 029ba13 Compare October 23, 2025 05:10
@mergify
Copy link

mergify bot commented Oct 23, 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 Oct 23, 2025
Copy link
Collaborator

@dapplion dapplion left a 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

Copy link
Member

@pawanjay176 pawanjay176 left a 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

fn test_nfd_enr_encoding() {
let spec = make_fulu_spec();
let enr = build_enr_with_config(NetworkConfig::default(), None, &spec).0;
let enr = build_enr_with_config(NetworkConfig::default(), 4, &spec).0;
Copy link
Member

Choose a reason for hiding this comment

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

maybe use spec.custody_requirement here instead of hardcoding?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-quality v8.1.0 Post-Fulu release waiting-on-author The reviewer has suggested changes and awaits thier implementation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants