track authorities from aura digests #9272

sistemd · 2025-07-21T00:15:39Z

Closes #9064.

Tracks AURA authorities in a ForkTree. The fork tree is updated whenever there is an authorities change log in the digest. If the fork tree doesn't contain the authorities, they are fetched for the runtime (should only happen at startup, or if something weird is going on with forks maybe).

paritytech-workflow-stopper · 2025-07-21T00:21:21Z

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/16405762024
Failed job name: fmt

sistemd · 2025-07-21T16:56:56Z

/cmd prdoc --audience node_dev --audience node_operator --bump patch

…e_dev --audience node_operator --bump patch'

skunert

We need the same changes also for

polkadot-sdk/cumulus/client/consensus/aura/src/equivocation_import_queue.rs

Line 112 in 76b685a

    
           impl<P, Client, Block, CIDP> VerifierT<Block> for Verifier<P, Client, Block, CIDP>

.

Are you planning to do this in a follow up? Also It would be really cool to have some tests introduced for this.

substrate/client/consensus/aura/src/import_queue.rs

sistemd · 2025-07-28T13:16:26Z

@skunert

Are you planning to do this in a follow up?

Yes and no - the follow up will be moving the authorities tracking code (and related equivocation checks and the inherents checks) into the import queue. At that point the code you linked will not need any changes (other than removing code). So I decided to not change that verifier at all in this PR. Let me know if you disagree - it's possible I'm missing something.

substrate/client/consensus/aura/src/authorities_tracker.rs

cumulus/client/consensus/aura/src/equivocation_import_queue.rs

skunert

Looks nice!

lexnv

Looks good to me! 🚀

Maybe we could add a few tiny tests to ensure we captured the authorities in the tracker, but offhand that seems quite involved

iulianbarbu · 2025-08-22T15:00:40Z

Have a question, can't say that I am 100% on top of the full picture, but saw this comment from Basti in the associated issue:

The idea is that we change the aura pallet to also announce the new authority set via a digest,

Was this done? I can't remember. If no, should it be still done (maybe in this PR if yes)?

iulianbarbu · 2025-08-22T15:05:46Z

We need the same changes also for

polkadot-sdk/cumulus/client/consensus/aura/src/equivocation_import_queue.rs

Line 112 in 76b685a

impl<P, Client, Block, CIDP> VerifierT<Block> for Verifier<P, Client, Block, CIDP>

.

Are you planning to do this in a follow up? Also It would be really cool to have some tests introduced for this.

I think this update will be relevant for parachain-template too (besides omni-node). Might be worth verifying both at the time it is done.

sistemd · 2025-08-25T13:02:31Z

Was this done? I can't remember. If no, should it be still done (maybe in this PR if yes)?

Yes sir - this was done way before. (This predates me 😄).

iulianbarbu · 2025-08-25T14:39:33Z

Was this done? I can't remember. If no, should it be still done (maybe in this PR if yes)?

Yes sir - this was done way before. (This predates me 😄).

Seems that I misread the comment then. I think this corresponds to babe equivalent Basti linked:

polkadot-sdk/substrate/frame/aura/src/lib.rs

Line 197 in 7ecf3f7

pub fn change_authorities(new: BoundedVec<T::AuthorityId, T::MaxAuthorities>) {

.

sistemd · 2025-08-25T15:41:18Z

Also, here's a test for this: 251544c. I think this is a nice solution, it's simple because it doesn't add a new zombienet test (CI is slow enough as it is 😄) but it does check that 1) authorities are fetched from the runtime exactly once and 2) they are fetched from the cache many times, and since the sync proceeds without errors this proves that we must be tracking authorities correctly.

iulianbarbu · 2025-08-26T10:34:43Z

Also, here's a test for this: 251544c

Test lgtm! Looks like we're a check-semver fix away from merging this :D. Please do that before

bkchr · 2025-09-01T19:42:29Z

substrate/client/consensus/aura/src/authorities_tracker.rs

+			None => {
+				// Authorities are missing from the cache. Fetch them from the runtime and cache
+				// them.
+				log::debug!(
+					target: LOG_TARGET,
+					"Authorities for block {:?} at number {} not found in cache, fetching from runtime",
+					hash,
+					number
+				);
+				let authorities = fetch_authorities_from_runtime(
+					&*self.client,
+					parent_hash,
+					number,
+					compatibility_mode,
+				)
+				.map_err(|e| format!("Could not fetch authorities at {:?}: {}", parent_hash, e))?;
+				let is_descendent_of = sc_client_api::utils::is_descendent_of(&*self.client, None);
+				let mut authorities_cache = self.authorities.write();
+				authorities_cache
+					.import(
+						parent_hash,
+						number - 1u32.into(),
+						authorities.clone(),
+						&is_descendent_of,
+					)
+					.map_err(|e| {
+						format!("Could not import authorities for block {parent_hash:?} at number {}: {e}", number - 1u32.into())
+					})?;
+				Ok(authorities)
+			},


This code path should not exist in this function. We should warm up the cache at startup and fetch the authorities for all blocks from the leaves back to the finalized block.

bkchr · 2025-09-01T19:48:17Z

substrate/client/consensus/aura/src/authorities_tracker.rs

+	for log in header.digest().logs() {
+		log::trace!(target: LOG_TARGET, "Checking log {:?}, looking for authorities change digest.", log);
+		let log = log
+			.try_to::<ConsensusLog<AuthorityId<P>>>(OpaqueDigestItemId::Consensus(&AURA_ENGINE_ID));
+		if let Some(ConsensusLog::AuthoritiesChange(authorities)) = log {
+			return Some(authorities);
+		}
+	}
+	None


Suggested change

for log in header.digest().logs() {

log::trace!(target: LOG_TARGET, "Checking log {:?}, looking for authorities change digest.", log);

let log = log

.try_to::<ConsensusLog<AuthorityId<P>>>(OpaqueDigestItemId::Consensus(&AURA_ENGINE_ID));

if let Some(ConsensusLog::AuthoritiesChange(authorities)) = log {

return Some(authorities);

}

}

None

header.digest().convert_first(|log|

log::trace!(target: LOG_TARGET, "Checking log {:?}, looking for authorities change digest.", log);

let (engine, log) = log.as_consensus()?

if engine != AURA_ENGINE_ID { return None; }

if let Some(ConsensusLog::AuthoritiesChange(authorities)) = ConsensusLog<AuthorityId<P>>>::decode_all(&mut &log[..] {

return Some(authorities);

}

}

None

as_consensus is very important here. You could also extend

polkadot-sdk/substrate/primitives/consensus/aura/src/digests.rs

Line 29 in 3726493

pub trait CompatibleDigestItem<Signature>: Sized {

to support the consensuslog.

Closes #9064. Tracks AURA authorities in a `ForkTree`. The fork tree is updated whenever there is an authorities change log in the digest. If the fork tree doesn't contain the authorities, they are fetched for the runtime (should only happen at startup, or if something weird is going on with forks maybe). --------- Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>

sistemd added 2 commits July 21, 2025 02:11

track authorities from aura digests

a43dd3c

fmt

5ae6a6e

sistemd requested review from bkchr, iulianbarbu and skunert July 21, 2025 00:37

sistemd added the T9-cumulus This PR/Issue is related to cumulus. label Jul 21, 2025

sistemd and others added 2 commits July 21, 2025 18:57

Merge branch 'master' into sistemd/aura-authorities-in-digest

5d84aba

Update from github-actions[bot] running command 'prdoc --audience nod…

76b685a

…e_dev --audience node_operator --bump patch'

skunert reviewed Jul 28, 2025

View reviewed changes

substrate/client/consensus/aura/src/import_queue.rs Outdated Show resolved Hide resolved

sistemd added 3 commits August 1, 2025 00:21

AuthoritiesTracker

805d24c

use AuthoritiesTracker in import_queue

16bd58c

use AuthoritiesTracker in equivocation_import_queue

97c2189

sistemd requested a review from skunert July 31, 2025 23:11

skunert reviewed Aug 13, 2025

View reviewed changes

sistemd added 4 commits August 14, 2025 13:59

add license header

387f952

small simplification

0e6cd39

move client into authorities tracker

abede1e

import authorities after equivocation check

d69fe13

skunert approved these changes Aug 14, 2025

View reviewed changes

lexnv approved these changes Aug 21, 2025

View reviewed changes

Merge branch 'master' into sistemd/aura-authorities-in-digest

6936fdc

sistemd added 2 commits August 25, 2025 15:03

Merge branch 'master' into sistemd/aura-authorities-in-digest

9e820b1

update zombienet test to verify authorities tracker works

251544c

iulianbarbu approved these changes Aug 25, 2025

View reviewed changes

fix semver

c00fef6

sistemd added this pull request to the merge queue Aug 26, 2025

Merged via the queue into master with commit dcd9cac Aug 26, 2025
285 of 292 checks passed

sistemd deleted the sistemd/aura-authorities-in-digest branch August 26, 2025 17:55

bkchr reviewed Sep 1, 2025

View reviewed changes

sistemd mentioned this pull request Sep 10, 2025

feat: @bkchr feedback for #9272 #9707

Open

track authorities from aura digests #9272

track authorities from aura digests #9272

Uh oh!

Conversation

sistemd commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

paritytech-workflow-stopper bot commented Jul 21, 2025

Uh oh!

sistemd commented Jul 21, 2025

Uh oh!

skunert left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sistemd commented Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

skunert left a comment

Choose a reason for hiding this comment

Uh oh!

lexnv left a comment

Choose a reason for hiding this comment

Uh oh!

iulianbarbu commented Aug 22, 2025

Uh oh!

iulianbarbu commented Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sistemd commented Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iulianbarbu commented Aug 25, 2025

Uh oh!

sistemd commented Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iulianbarbu commented Aug 26, 2025

Uh oh!

Uh oh!

bkchr Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

bkchr Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

bkchr Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sistemd commented Jul 21, 2025 •

edited

Loading

sistemd commented Jul 28, 2025 •

edited

Loading

iulianbarbu commented Aug 22, 2025 •

edited

Loading

sistemd commented Aug 25, 2025 •

edited

Loading

sistemd commented Aug 25, 2025 •

edited

Loading