Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Refactor: extract most aura logic out to standalone module, make use of these #13764

Merged
merged 5 commits into from
Mar 31, 2023
Merged

Refactor: extract most aura logic out to standalone module, make use of these #13764

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2686d95
Extract most aura logic out to standalone module, make use of these
rphmeier Mar 29, 2023
af4182b
Update client/consensus/aura/src/standalone.rs
rphmeier Mar 30, 2023
e1a46dc
add slot_duration_at
rphmeier Mar 30, 2023
a18f21e
Merge remote-tracking branch 'origin/master' into rh-aura-standalone-…
Mar 31, 2023
b5e4271
".git/.scripts/commands/fmt/fmt.sh"
Mar 31, 2023
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
46 changes: 20 additions & 26 deletions client/consensus/aura/src/import_queue.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
//! Module implementing the logic for verifying and importing AuRa blocks.

use crate::{
aura_err, authorities, find_pre_digest, slot_author, AuthorityId, CompatibilityMode, Error,
authorities, standalone::SealVerificationError, AuthorityId, CompatibilityMode, Error,
LOG_TARGET,
};
use codec::{Codec, Decode, Encode};
Expand All @@ -36,7 +36,7 @@ use sp_api::{ApiExt, ProvideRuntimeApi};
use sp_block_builder::BlockBuilder as BlockBuilderApi;
use sp_blockchain::HeaderBackend;
use sp_consensus::Error as ConsensusError;
use sp_consensus_aura::{digests::CompatibleDigestItem, inherents::AuraInherentData, AuraApi};
use sp_consensus_aura::{inherents::AuraInherentData, AuraApi};
use sp_consensus_slots::Slot;
use sp_core::{crypto::Pair, ExecutionContext};
use sp_inherents::{CreateInherentDataProviders, InherentDataProvider as _};
Expand All @@ -54,7 +54,7 @@ use std::{fmt::Debug, hash::Hash, marker::PhantomData, sync::Arc};
fn check_header<C, B: BlockT, P: Pair>(
client: &C,
slot_now: Slot,
mut header: B::Header,
header: B::Header,
hash: B::Hash,
authorities: &[AuthorityId<P>],
check_for_equivocation: CheckForEquivocation,
Expand All @@ -64,27 +64,16 @@ where
C: sc_client_api::backend::AuxStore,
P::Public: Encode + Decode + PartialEq + Clone,
{
let seal = header.digest_mut().pop().ok_or(Error::HeaderUnsealed(hash))?;

let sig = seal.as_aura_seal().ok_or_else(|| aura_err(Error::HeaderBadSeal(hash)))?;

let slot = find_pre_digest::<B, P::Signature>(&header)?;

if slot > slot_now {
header.digest_mut().push(seal);
Ok(CheckedHeader::Deferred(header, slot))
} else {
// check the signature is valid under the expected authority and
// chain state.
let expected_author =
slot_author::<P>(slot, authorities).ok_or(Error::SlotAuthorNotFound)?;

let pre_hash = header.hash();

if P::verify(&sig, pre_hash.as_ref(), expected_author) {
if check_for_equivocation.check_for_equivocation() {
let check_result =
crate::standalone::check_header_slot_and_seal::<B, P>(slot_now, header, authorities);

match check_result {
Ok((header, slot, seal)) => {
let expected_author = crate::standalone::slot_author::<P>(slot, &authorities);
let should_equiv_check = check_for_equivocation.check_for_equivocation();
if let (true, Some(expected)) = (should_equiv_check, expected_author) {
if let Some(equivocation_proof) =
check_equivocation(client, slot_now, slot, &header, expected_author)
check_equivocation(client, slot_now, slot, &header, expected)
.map_err(Error::Client)?
{
info!(
Expand All @@ -98,9 +87,14 @@ where
}

Ok(CheckedHeader::Checked(header, (slot, seal)))
} else {
Err(Error::BadSignature(hash))
}
},
Err(SealVerificationError::Deferred(header, slot)) =>
davxy marked this conversation as resolved.
Show resolved Hide resolved
Ok(CheckedHeader::Deferred(header, slot)),
Err(SealVerificationError::Unsealed) => Err(Error::HeaderUnsealed(hash)),
Err(SealVerificationError::BadSeal) => Err(Error::HeaderBadSeal(hash)),
Err(SealVerificationError::BadSignature) => Err(Error::BadSignature(hash)),
Err(SealVerificationError::SlotAuthorNotFound) => Err(Error::SlotAuthorNotFound),
Err(SealVerificationError::InvalidPreDigest(e)) => Err(Error::from(e)),
Comment on lines +91 to +97
Copy link
Member

Choose a reason for hiding this comment

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

Please add SealVerificationError as variant to Error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would break the API, so I don't think it's a good idea.

SealVerificationError is basically just a copy of the Error variants that could be produced within seal verification.

}
}

Expand Down
127 changes: 16 additions & 111 deletions client/consensus/aura/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,32 +33,30 @@
use std::{fmt::Debug, hash::Hash, marker::PhantomData, pin::Pin, sync::Arc};

use futures::prelude::*;
use log::{debug, trace};

use codec::{Codec, Decode, Encode};

use sc_client_api::{backend::AuxStore, BlockOf, UsageProvider};
use sc_client_api::{backend::AuxStore, BlockOf};
use sc_consensus::{BlockImport, BlockImportParams, ForkChoiceStrategy, StateAction};
use sc_consensus_slots::{
BackoffAuthoringBlocksStrategy, InherentDataProviderExt, SimpleSlotWorkerToSlotWorker,
SlotInfo, StorageChanges,
};
use sc_telemetry::TelemetryHandle;
use sp_api::{Core, ProvideRuntimeApi};
use sp_application_crypto::{AppCrypto, AppPublic};
use sp_blockchain::{HeaderBackend, Result as CResult};
use sp_application_crypto::AppPublic;
use sp_blockchain::HeaderBackend;
use sp_consensus::{BlockOrigin, Environment, Error as ConsensusError, Proposer, SelectChain};
use sp_consensus_slots::Slot;
use sp_core::crypto::{ByteArray, Pair, Public};
use sp_core::crypto::{Pair, Public};
use sp_inherents::CreateInherentDataProviders;
use sp_keystore::KeystorePtr;
use sp_runtime::{
traits::{Block as BlockT, Header, Member, NumberFor, Zero},
DigestItem,
};
use sp_runtime::traits::{Block as BlockT, Header, Member, NumberFor};

mod import_queue;
pub mod standalone;
davxy marked this conversation as resolved.
Show resolved Hide resolved

pub use crate::standalone::{find_pre_digest, slot_duration};
pub use import_queue::{
build_verifier, import_queue, AuraVerifier, BuildVerifierParams, CheckForEquivocation,
ImportQueueParams,
Expand Down Expand Up @@ -112,39 +110,6 @@ impl<N> Default for CompatibilityMode<N> {
}
}

/// Get the slot duration for Aura.
pub fn slot_duration<A, B, C>(client: &C) -> CResult<SlotDuration>
where
A: Codec,
B: BlockT,
C: AuxStore + ProvideRuntimeApi<B> + UsageProvider<B>,
C::Api: AuraApi<B, A>,
{
client
.runtime_api()
.slot_duration(client.usage_info().chain.best_hash)
.map_err(|err| err.into())
}

/// Get slot author for given block along with authorities.
fn slot_author<P: Pair>(slot: Slot, authorities: &[AuthorityId<P>]) -> Option<&AuthorityId<P>> {
if authorities.is_empty() {
return None
}

let idx = *slot % (authorities.len() as u64);
assert!(
idx <= usize::MAX as u64,
"It is impossible to have a vector with length beyond the address space; qed",
);

let current_author = authorities.get(idx as usize).expect(
"authorities not empty; index constrained to list length;this is a valid index; qed",
);

Some(current_author)
}

/// Parameters of [`start_aura`].
pub struct StartAuraParams<C, SC, I, PF, SO, L, CIDP, BS, N> {
/// The duration of a slot.
Expand Down Expand Up @@ -412,21 +377,11 @@ where
slot: Slot,
authorities: &Self::AuxData,
) -> Option<Self::Claim> {
let expected_author = slot_author::<P>(slot, authorities);
expected_author.and_then(|p| {
if self
.keystore
.has_keys(&[(p.to_raw_vec(), sp_application_crypto::key_types::AURA)])
{
Some(p.clone())
} else {
None
}
})
crate::standalone::claim_slot::<P>(slot, authorities, &self.keystore).await
}

fn pre_digest_data(&self, slot: Slot, _claim: &Self::Claim) -> Vec<sp_runtime::DigestItem> {
vec![<DigestItem as CompatibleDigestItem<P::Signature>>::aura_pre_digest(slot)]
vec![crate::standalone::pre_digest::<P>(slot)]
}

async fn block_import_params(
Expand All @@ -441,28 +396,8 @@ where
sc_consensus::BlockImportParams<B, <Self::BlockImport as BlockImport<B>>::Transaction>,
ConsensusError,
> {
let signature = self
.keystore
.sign_with(
<AuthorityId<P> as AppCrypto>::ID,
<AuthorityId<P> as AppCrypto>::CRYPTO_ID,
public.as_slice(),
header_hash.as_ref(),
)
.map_err(|e| ConsensusError::CannotSign(format!("{}. Key: {:?}", e, public)))?
.ok_or_else(|| {
ConsensusError::CannotSign(format!(
"Could not find key in keystore. Key: {:?}",
public
))
})?;
let signature = signature
.clone()
.try_into()
.map_err(|_| ConsensusError::InvalidSignature(signature, public.to_raw_vec()))?;

let signature_digest_item =
<DigestItem as CompatibleDigestItem<P::Signature>>::aura_seal(signature);
crate::standalone::seal::<_, P>(header_hash, &public, &self.keystore)?;

let mut import_block = BlockImportParams::new(BlockOrigin::Own, header);
import_block.post_digests.push(signature_digest_item);
Expand Down Expand Up @@ -526,11 +461,6 @@ where
}
}

fn aura_err<B: BlockT>(error: Error<B>) -> Error<B> {
debug!(target: LOG_TARGET, "{}", error);
error
}

/// Aura Errors
#[derive(Debug, thiserror::Error)]
pub enum Error<B: BlockT> {
Expand Down Expand Up @@ -569,22 +499,13 @@ impl<B: BlockT> From<Error<B>> for String {
}
}

/// Get pre-digests from the header
pub fn find_pre_digest<B: BlockT, Signature: Codec>(header: &B::Header) -> Result<Slot, Error<B>> {
if header.number().is_zero() {
return Ok(0.into())
}

let mut pre_digest: Option<Slot> = None;
for log in header.digest().logs() {
trace!(target: LOG_TARGET, "Checking log {:?}", log);
match (CompatibleDigestItem::<Signature>::as_aura_pre_digest(log), pre_digest.is_some()) {
(Some(_), true) => return Err(aura_err(Error::MultipleHeaders)),
(None, _) => trace!(target: LOG_TARGET, "Ignoring digest not meant for us"),
(s, false) => pre_digest = s,
impl<B: BlockT> From<crate::standalone::PreDigestLookupError> for Error<B> {
fn from(e: crate::standalone::PreDigestLookupError) -> Self {
match e {
crate::standalone::PreDigestLookupError::MultipleHeaders => Error::MultipleHeaders,
crate::standalone::PreDigestLookupError::NoDigestFound => Error::NoDigestFound,
}
}
pre_digest.ok_or_else(|| aura_err(Error::NoDigestFound))
}

fn authorities<A, B, C>(
Expand Down Expand Up @@ -637,7 +558,7 @@ mod tests {
use sc_consensus_slots::{BackoffAuthoringOnFinalizedHeadLagging, SimpleSlotWorker};
use sc_keystore::LocalKeystore;
use sc_network_test::{Block as TestBlock, *};
use sp_application_crypto::key_types::AURA;
use sp_application_crypto::{key_types::AURA, AppCrypto};
use sp_consensus::{DisableProofRecording, NoNetwork as DummyOracle, Proposal};
use sp_consensus_aura::sr25519::AuthorityPair;
use sp_inherents::InherentData;
Expand Down Expand Up @@ -851,22 +772,6 @@ mod tests {
.await;
}

#[test]
fn authorities_call_works() {
let client = substrate_test_runtime_client::new();

assert_eq!(client.chain_info().best_number, 0);
assert_eq!(
authorities(&client, client.chain_info().best_hash, 1, &CompatibilityMode::None)
.unwrap(),
vec![
Keyring::Alice.public().into(),
Keyring::Bob.public().into(),
Keyring::Charlie.public().into()
]
);
}

#[tokio::test]
async fn current_node_authority_should_claim_slot() {
let net = AuraTestNet::new(4);
Expand Down
Loading