Skip to content

DHT bootnodes: get randomness from the next epoch descriptor #8405

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.

Sign up for GitHub

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

Merged
dmitry-markin merged 6 commits into from
May 6, 2025
Merged

DHT bootnodes: get randomness from the next epoch descriptor #8405

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
735296a
Reduce re-advertisements of next epoch key
dmitry-markin May 2, 2025
854afbe
Get epoch randomness from next epoch descriptor
dmitry-markin May 2, 2025
f19bc18
Apply suggestions from code review
dmitry-markin May 5, 2025
74b1473
Merge branch 'master' into dm-dht-bootnodes-runtime-optimization
dmitry-markin May 5, 2025
4a0d903
`header.log()` -> `header.digest().convert_first()`
dmitry-markin May 5, 2025
ba21eea
Merge branch 'master' into dm-dht-bootnodes-runtime-optimization
dmitry-markin May 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 36 additions & 68 deletions cumulus/client/bootnodes/src/advertisement.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,20 @@ impl BootnodeAdvertisement {
async fn handle_import_notification(&mut self, header: RelayHeader) {

@bkchr bkchr May 2, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some edge case that is not covered is when the notification stream skipped blocks (because of major sync). Then you would maybe advertise too long with an invalid key.

@dmitry-markin dmitry-markin May 5, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, stop_providing is a local-only action only (Kademlia DHT doesn't have a way to tell target nodes we are not providing the key anymore), so remote nodes will advertise every key until it expires in 48 hours anyway.

@bkchr bkchr May 5, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This was not what I wanted to say :) The problem I raised there is when you for example sync across an epoch change, but the node was doing this in "major sync mode". Then you would not get a notification for these blocks.

@dmitry-markin dmitry-markin May 6, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the explanation, this makes sense. How common is such situation except during the initial sync?

if let Some(ref old_current_epoch_key) = self.current_epoch_key {
// Readvertise on start of new epoch only.
let is_start_of_epoch =
header.digest().logs.iter().any(|v| v.as_next_epoch_descriptor().is_some());
if !is_start_of_epoch {
let Some(next_epoch_descriptor) =
header.digest().convert_first(|v| v.as_next_epoch_descriptor())
else {
return;
};

let next_epoch_key = self.epoch_key(next_epoch_descriptor.randomness);

if Some(&next_epoch_key) == self.next_epoch_key.as_ref() {
trace!(
target: LOG_TARGET,
"Next epoch descriptor contains the same randomness as the previous one, \
not considering this as epoch change (switched fork?)",
);
return;
}

Expand All @@ -219,77 +230,34 @@ impl BootnodeAdvertisement {

debug!(target: LOG_TARGET, "New epoch started, readvertising parachain bootnode.");

let (current_epoch_key, next_epoch_key) =
self.current_and_next_epoch_keys(header).await;
// Stop advertisement of the obsolete key.
debug!(
target: LOG_TARGET,
"Stopping advertisement of bootnode for old current epoch key {}",
hex::encode(old_current_epoch_key.as_ref()),
);
self.relay_chain_network.stop_providing(old_current_epoch_key.clone());

// Readvertise for current epoch.
if let Some(ref current_epoch_key) = current_epoch_key {
if current_epoch_key == old_current_epoch_key {
debug!(
target: LOG_TARGET,
"Re-advertising bootnode for current epoch key {}",
hex::encode(current_epoch_key.as_ref()),
);
} else {
self.relay_chain_network.stop_providing(old_current_epoch_key.clone());
debug!(
target: LOG_TARGET,
"Stopped advertising bootnode for past epoch key {}",
hex::encode(old_current_epoch_key.as_ref()),
);

match self.next_epoch_key {
Some(ref old_next_key) if old_next_key == current_epoch_key => debug!(
target: LOG_TARGET,
"Advertising bootnode for current (old next) epoch key {}",
hex::encode(current_epoch_key.as_ref()),
),
_ => debug!(
target: LOG_TARGET,
"Advertising bootnode for current epoch key {}",
hex::encode(current_epoch_key.as_ref()),
),
}
}
// Advertise current keys.
self.current_epoch_key = self.next_epoch_key.clone();
self.next_epoch_key = Some(next_epoch_key);

if let Some(ref current_epoch_key) = self.current_epoch_key {
debug!(
target: LOG_TARGET,
"Advertising bootnode for current (old next) epoch key {}",
hex::encode(current_epoch_key.as_ref()),
);
self.relay_chain_network.start_providing(current_epoch_key.clone());
self.current_epoch_key = Some(current_epoch_key.clone());
}

// Readvertise for next epoch.
if let Some(next_epoch_key) = next_epoch_key {
match (current_epoch_key, &self.next_epoch_key) {
(Some(current_epoch_key), Some(old_next_epoch_key)) =>
if *old_next_epoch_key != current_epoch_key {
self.relay_chain_network.stop_providing(old_next_epoch_key.clone());

debug!(
target: LOG_TARGET,
"Stopped advertising bootnode for discarded next epoch key {}",
hex::encode(old_next_epoch_key.as_ref()),
);
},
// In all other cases we keep the old next epoch key advertised, as it either
// became a current epoch key, or in odd cases will just expire.
_ => {},
}

if Some(&next_epoch_key) == self.next_epoch_key.as_ref() {
debug!(
target: LOG_TARGET,
"Re-advertising bootnode for next epoch key {}",
hex::encode(next_epoch_key.as_ref()),
);
} else {
debug!(
target: LOG_TARGET,
"Advertising bootnode for next epoch key {}",
hex::encode(next_epoch_key.as_ref()),
);
}

if let Some(ref next_epoch_key) = self.next_epoch_key {
debug!(
target: LOG_TARGET,
"Advertising bootnode for next epoch key {}",
hex::encode(next_epoch_key.as_ref()),
);
self.relay_chain_network.start_providing(next_epoch_key.clone());
self.next_epoch_key = Some(next_epoch_key.clone());
}
} else {
// First advertisement on startup.
Expand Down
Loading