Skip to content
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

Cherry-pick: Add consensus_block_info digest to domain block header and use it to load ER #1903

Merged
merged 5 commits into from
Aug 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions crates/subspace-fraud-proof/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ async fn execution_proof_creation_and_verification_should_work() {
let create_block_builder = || {
let primary_hash = ferdie.client.hash(*header.number()).unwrap().unwrap();
let digest = Digest {
logs: vec![DigestItem::primary_block_info((
logs: vec![DigestItem::consensus_block_info((
*header.number(),
primary_hash,
))],
Expand Down Expand Up @@ -218,7 +218,7 @@ async fn execution_proof_creation_and_verification_should_work() {
Default::default(),
parent_header.hash(),
Digest {
logs: vec![DigestItem::primary_block_info((
logs: vec![DigestItem::consensus_block_info((
*header.number(),
primary_hash,
))],
Expand Down Expand Up @@ -487,7 +487,7 @@ async fn invalid_execution_proof_should_not_work() {
*parent_header.number(),
RecordProof::No,
Digest {
logs: vec![DigestItem::primary_block_info((
logs: vec![DigestItem::consensus_block_info((
*header.number(),
header.hash(),
))],
Expand Down
36 changes: 0 additions & 36 deletions domains/client/domain-operator/src/aux_schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,6 @@ const BAD_RECEIPT_MISMATCH_INFO: &[u8] = b"bad_receipt_mismatch_info";
/// NOTE: Unbounded but the size is not expected to be large.
const BAD_RECEIPT_NUMBERS: &[u8] = b"bad_receipt_numbers";

/// domain_block_hash => Vec<consensus_block_hash>
///
/// Updated only when there is a new domain block produced
///
/// NOTE: different consensus blocks may derive the exact same domain block, thus one domain block may
/// mapping to multiple consensus block.
const CONSENSUS_HASH: &[u8] = b"consensus_block_hash";

/// domain_block_hash => latest_consensus_block_hash
///
/// It's important to note that a consensus block could possibly contain no bundles for a specific domain,
Expand Down Expand Up @@ -93,7 +85,6 @@ where
{
let block_number = execution_receipt.consensus_block_number;
let consensus_hash = execution_receipt.consensus_block_hash;
let domain_hash = execution_receipt.domain_block_hash;

let block_number_key = (EXECUTION_RECEIPT_BLOCK_NUMBER, block_number).encode();
let mut hashes_at_block_number =
Expand Down Expand Up @@ -131,24 +122,12 @@ where
}
}

let consensus_hashes = {
let mut hashes =
consensus_block_hash_for::<Backend, Block::Hash, CBlock::Hash>(backend, domain_hash)?;
hashes.push(consensus_hash);
hashes
};

backend.insert_aux(
&[
(
execution_receipt_key(consensus_hash).as_slice(),
execution_receipt.encode().as_slice(),
),
// TODO: prune the stale mappings.
(
(CONSENSUS_HASH, domain_hash).encode().as_slice(),
consensus_hashes.encode().as_slice(),
),
(
block_number_key.as_slice(),
hashes_at_block_number.encode().as_slice(),
Expand Down Expand Up @@ -242,21 +221,6 @@ where
)
}

pub(super) fn consensus_block_hash_for<Backend, Hash, PHash>(
backend: &Backend,
domain_hash: Hash,
) -> ClientResult<Vec<PHash>>
where
Backend: AuxStore,
Hash: Encode,
PHash: Decode,
{
Ok(
load_decode(backend, (CONSENSUS_HASH, domain_hash).encode().as_slice())?
.unwrap_or_default(),
)
}

// TODO: Unlock once domain test infra is workable again.
#[allow(dead_code)]
pub(super) fn target_receipt_is_pruned(
Expand Down
9 changes: 7 additions & 2 deletions domains/client/domain-operator/src/bundle_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@ use sp_api::{NumberFor, ProvideRuntimeApi};
use sp_blockchain::{HeaderBackend, HeaderMetadata};
use sp_consensus::BlockOrigin;
use sp_core::traits::CodeExecutor;
use sp_domain_digests::AsPredigest;
use sp_domains::{DomainId, DomainsApi, InvalidReceipt, ReceiptValidity};
use sp_keystore::KeystorePtr;
use sp_messenger::MessengerApi;
use sp_runtime::traits::{Block as BlockT, HashFor, Zero};
use sp_runtime::Digest;
use sp_runtime::{Digest, DigestItem};
use std::sync::Arc;

type DomainReceiptsChecker<Block, CBlock, Client, CClient, Backend, E> = ReceiptsChecker<
Expand Down Expand Up @@ -291,6 +292,10 @@ where
return Ok(None);
};

let digest = Digest {
logs: vec![DigestItem::consensus_block_info(consensus_block_hash)],
};

let domain_block_result = self
.domain_block_processor
.process_domain_block(
Expand All @@ -299,7 +304,7 @@ where
extrinsics,
invalid_bundles,
extrinsics_roots,
Digest::default(),
digest,
)
.await?;

Expand Down
3 changes: 1 addition & 2 deletions domains/client/domain-operator/src/domain_block_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,9 +341,8 @@ where
*genesis_header.state_root(),
)
} else {
crate::load_execution_receipt_by_domain_hash::<Block, CBlock, _, _>(
crate::load_execution_receipt_by_domain_hash::<Block, CBlock, _>(
&*self.client,
&self.consensus_client,
parent_hash,
parent_number,
)?
Expand Down
6 changes: 3 additions & 3 deletions domains/client/domain-operator/src/domain_bundle_producer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,12 +149,12 @@ where
global_randomness,
} = slot_info;

let best_receipt_is_written = !crate::aux_schema::consensus_block_hash_for::<
let best_receipt_is_written = crate::aux_schema::latest_consensus_block_hash_for::<
_,
_,
CBlock::Hash,
>(&*self.client, self.client.info().best_hash)?
.is_empty();
>(&*self.client, &self.client.info().best_hash)?
.is_some();

// TODO: remove once the receipt generation can be done before the domain block is
// committed to the database, in other words, only when the receipt of block N+1 has
Expand Down
10 changes: 5 additions & 5 deletions domains/client/domain-operator/src/domain_bundle_proposer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,12 +188,12 @@ where
"Collecting receipts at {parent_chain_block_hash:?}"
);

let header_block_receipt_is_written = !crate::aux_schema::consensus_block_hash_for::<
let header_block_receipt_is_written = crate::aux_schema::latest_consensus_block_hash_for::<
_,
_,
CBlock::Hash,
>(&*self.client, header_hash)?
.is_empty();
>(&*self.client, &header_hash)?
.is_some();

// TODO: remove once the receipt generation can be done before the domain block is
// committed to the database, in other words, only when the receipt of block N+1 has
Expand Down Expand Up @@ -223,15 +223,15 @@ where
));
}

// Get the domain block hash corresponding to `receipt_number` in the domain canonical chain
let domain_hash = self.client.hash(receipt_number)?.ok_or_else(|| {
sp_blockchain::Error::Backend(format!(
"Domain block hash for #{receipt_number:?} not found"
))
})?;

crate::load_execution_receipt_by_domain_hash::<Block, CBlock, _, _>(
crate::load_execution_receipt_by_domain_hash::<Block, CBlock, _>(
&*self.client,
&self.consensus_client,
domain_hash,
receipt_number,
)
Expand Down
56 changes: 21 additions & 35 deletions domains/client/domain-operator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,13 @@ use sp_api::ProvideRuntimeApi;
use sp_blockchain::HeaderBackend;
use sp_consensus::{SelectChain, SyncOracle};
use sp_consensus_slots::Slot;
use sp_domain_digests::AsPredigest;
use sp_domains::{Bundle, DomainId, ExecutionReceipt};
use sp_keystore::KeystorePtr;
use sp_runtime::traits::{
Block as BlockT, HashFor, Header as HeaderT, NumberFor, One, Saturating, Zero,
};
use sp_runtime::DigestItem;
use std::marker::PhantomData;
use std::sync::Arc;
use subspace_core_primitives::Randomness;
Expand Down Expand Up @@ -240,56 +242,40 @@ where
Ok(leaves.into_iter().rev().take(MAX_ACTIVE_LEAVES).collect())
}

pub(crate) fn load_execution_receipt_by_domain_hash<Block, CBlock, Client, CClient>(
pub(crate) fn load_execution_receipt_by_domain_hash<Block, CBlock, Client>(
domain_client: &Client,
consensus_client: &Arc<CClient>,
domain_hash: Block::Hash,
domain_number: NumberFor<Block>,
) -> Result<ExecutionReceiptFor<Block, CBlock>, sp_blockchain::Error>
where
Block: BlockT,
CBlock: BlockT,
Client: AuxStore,
CClient: HeaderBackend<CBlock>,
Client: AuxStore + HeaderBackend<Block>,
{
let not_found_error = || {
let domain_header = domain_client.header(domain_hash)?.ok_or_else(|| {
sp_blockchain::Error::Backend(format!(
"Receipt for domain block {domain_hash}#{domain_number} not found"
"Header for domain block {domain_hash}#{domain_number} not found"
))
};
})?;

// Get all the consensus blocks that mapped to `domain_hash`
let consensus_block_hashes = crate::aux_schema::consensus_block_hash_for::<_, _, CBlock::Hash>(
domain_client,
domain_hash,
)?;

// Get the consensus block that is in the current canonical consensus chain
let consensus_block_hash = match consensus_block_hashes.len() {
0 => return Err(not_found_error()),
1 => consensus_block_hashes[0],
_ => {
let mut canonical_consensus_hash = None;
for hash in consensus_block_hashes {
// Check if `hash` is in the canonical chain
let block_number = consensus_client.number(hash)?.ok_or_else(not_found_error)?;
let canonical_block_hash = consensus_client
.hash(block_number)?
.ok_or_else(not_found_error)?;

if canonical_block_hash == hash {
canonical_consensus_hash.replace(hash);
break;
}
}
canonical_consensus_hash.ok_or_else(not_found_error)?
}
};
let consensus_block_hash = domain_header
.digest()
.convert_first(DigestItem::as_consensus_block_info)
.ok_or_else(|| {
sp_blockchain::Error::Application(Box::from(
"Domain block header {domain_hash}#{domain_number} must have consensus block info predigest"
))
})?;

// Get receipt by consensus block hash
crate::aux_schema::load_execution_receipt::<_, Block, CBlock>(
domain_client,
consensus_block_hash,
)?
.ok_or_else(not_found_error)
.ok_or_else(|| {
sp_blockchain::Error::Backend(format!(
"Receipt for consensus block {consensus_block_hash} and domain block \
{domain_hash}#{domain_number} not found"
))
})
}
67 changes: 55 additions & 12 deletions domains/client/domain-operator/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,7 @@ async fn fraud_proof_verification_in_tx_pool_should_work() {

let digest = {
Digest {
logs: vec![DigestItem::primary_block_info((
logs: vec![DigestItem::consensus_block_info((
bad_receipt_number,
ferdie.client.hash(bad_receipt_number).unwrap().unwrap(),
))],
Expand Down Expand Up @@ -1819,7 +1819,7 @@ async fn test_restart_domain_operator() {
}

#[substrate_test_utils::test(flavor = "multi_thread")]
async fn test_multiple_consensus_blocks_derive_same_domain_block() {
async fn test_multiple_consensus_blocks_derive_similar_domain_block() {
let directory = TempDir::new().expect("Must be able to create temporary directory");

let mut builder = sc_cli::LoggerBuilder::new("");
Expand Down Expand Up @@ -1882,18 +1882,61 @@ async fn test_multiple_consensus_blocks_derive_same_domain_block() {
.await
.unwrap();

// The same domain block mapped to 2 different consensus blocks
let consensus_best_hashes = crate::aux_schema::consensus_block_hash_for::<
_,
_,
<CBlock as BlockT>::Hash,
>(&*alice.client, alice.client.info().best_hash)
.unwrap();
assert_ne!(consensus_block_hash_fork_a, consensus_block_hash_fork_b);

// Both consensus block from fork A and B should derive a corresponding domain block
let domain_block_hash_fork_a =
crate::aux_schema::best_domain_hash_for(&*alice.client, &consensus_block_hash_fork_a)
.unwrap()
.unwrap();
let domain_block_hash_fork_b =
crate::aux_schema::best_domain_hash_for(&*alice.client, &consensus_block_hash_fork_b)
.unwrap()
.unwrap();
assert_ne!(domain_block_hash_fork_a, domain_block_hash_fork_b);

// The domain block header should contains digest that point to the consensus block, which
// devrive the domain block
let get_header = |hash| alice.client.header(hash).unwrap().unwrap();
let get_digest_consensus_block_hash = |header: &Header| -> <CBlock as BlockT>::Hash {
header
.digest()
.convert_first(DigestItem::as_consensus_block_info)
.unwrap()
};
let domain_block_header_fork_a = get_header(domain_block_hash_fork_a);
let domain_block_header_fork_b = get_header(domain_block_hash_fork_b);
let digest_consensus_block_hash_fork_a =
get_digest_consensus_block_hash(&domain_block_header_fork_a);
assert_eq!(
digest_consensus_block_hash_fork_a,
consensus_block_hash_fork_a
);
let digest_consensus_block_hash_fork_b =
get_digest_consensus_block_hash(&domain_block_header_fork_b);
assert_eq!(
consensus_best_hashes,
vec![consensus_block_hash_fork_a, consensus_block_hash_fork_b]
digest_consensus_block_hash_fork_b,
consensus_block_hash_fork_b
);

// The domain block headers should have the same `parent_hash` and `extrinsics_root`
// but different digest and `state_root` (because digest is stored on chain)
assert_eq!(
domain_block_header_fork_a.parent_hash(),
domain_block_header_fork_b.parent_hash()
);
assert_eq!(
domain_block_header_fork_a.extrinsics_root(),
domain_block_header_fork_b.extrinsics_root()
);
assert_ne!(
domain_block_header_fork_a.digest(),
domain_block_header_fork_b.digest()
);
assert_ne!(
domain_block_header_fork_a.state_root(),
domain_block_header_fork_b.state_root()
);
assert_ne!(consensus_block_hash_fork_a, consensus_block_hash_fork_b);

// Produce one more block at fork A to make it the canonical chain and the operator
// should submit the ER of fork A
Expand Down
15 changes: 7 additions & 8 deletions domains/primitives/digests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,19 @@ const DOMAIN_REGISTRY_ENGINE_ID: ConsensusEngineId = *b"RGTR";

/// Trait to provide simpler abstractions to create predigests for runtime.
pub trait AsPredigest {
/// Return a pair of (primary_block_number, primary_block_hash).
fn as_primary_block_info<Number: Decode, Hash: Decode>(&self) -> Option<(Number, Hash)>;
/// Return `consensus_block_hash`
fn as_consensus_block_info<Hash: Decode>(&self) -> Option<Hash>;

/// Creates a new digest of primary block info for system domain.
fn primary_block_info<Number: Encode, Hash: Encode>(info: (Number, Hash)) -> Self;
/// Creates a new digest of the consensus block that derive the domain block.
fn consensus_block_info<Hash: Encode>(consensus_block_hash: Hash) -> Self;
}

impl AsPredigest for DigestItem {
/// Return a pair of (primary_block_number, primary_block_hash).
fn as_primary_block_info<Number: Decode, Hash: Decode>(&self) -> Option<(Number, Hash)> {
fn as_consensus_block_info<Hash: Decode>(&self) -> Option<Hash> {
self.pre_runtime_try_to(&DOMAIN_REGISTRY_ENGINE_ID)
}

fn primary_block_info<Number: Encode, Hash: Encode>(info: (Number, Hash)) -> Self {
DigestItem::PreRuntime(DOMAIN_REGISTRY_ENGINE_ID, info.encode())
fn consensus_block_info<Hash: Encode>(consensus_block_hash: Hash) -> Self {
DigestItem::PreRuntime(DOMAIN_REGISTRY_ENGINE_ID, consensus_block_hash.encode())
}
}