Skip to content

Commit

Permalink
Merge pull request #1903 from subspace/domain-digest-gemini-3f
Browse files Browse the repository at this point in the history
Cherry-pick: Add consensus_block_info digest to domain block header and use it to load ER
  • Loading branch information
NingLin-P authored Aug 29, 2023
2 parents 502e597 + 8719fa0 commit a56afdc
Show file tree
Hide file tree
Showing 9 changed files with 102 additions and 106 deletions.
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())
}
}

0 comments on commit a56afdc

Please sign in to comment.