diff --git a/crates/subspace-fraud-proof/src/tests.rs b/crates/subspace-fraud-proof/src/tests.rs index f85f92ca18..e9b40fd2de 100644 --- a/crates/subspace-fraud-proof/src/tests.rs +++ b/crates/subspace-fraud-proof/src/tests.rs @@ -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, ))], @@ -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, ))], @@ -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(), ))], diff --git a/domains/client/domain-operator/src/aux_schema.rs b/domains/client/domain-operator/src/aux_schema.rs index c744a866f8..c2ca1b2c88 100644 --- a/domains/client/domain-operator/src/aux_schema.rs +++ b/domains/client/domain-operator/src/aux_schema.rs @@ -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 -/// -/// 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, @@ -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 = @@ -131,24 +122,12 @@ where } } - let consensus_hashes = { - let mut hashes = - consensus_block_hash_for::(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(), @@ -242,21 +221,6 @@ where ) } -pub(super) fn consensus_block_hash_for( - backend: &Backend, - domain_hash: Hash, -) -> ClientResult> -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( diff --git a/domains/client/domain-operator/src/bundle_processor.rs b/domains/client/domain-operator/src/bundle_processor.rs index fd2501841d..f831382757 100644 --- a/domains/client/domain-operator/src/bundle_processor.rs +++ b/domains/client/domain-operator/src/bundle_processor.rs @@ -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 = ReceiptsChecker< @@ -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( @@ -299,7 +304,7 @@ where extrinsics, invalid_bundles, extrinsics_roots, - Digest::default(), + digest, ) .await?; diff --git a/domains/client/domain-operator/src/domain_block_processor.rs b/domains/client/domain-operator/src/domain_block_processor.rs index 9ddab279a1..ef8a3615f8 100644 --- a/domains/client/domain-operator/src/domain_block_processor.rs +++ b/domains/client/domain-operator/src/domain_block_processor.rs @@ -341,9 +341,8 @@ where *genesis_header.state_root(), ) } else { - crate::load_execution_receipt_by_domain_hash::( + crate::load_execution_receipt_by_domain_hash::( &*self.client, - &self.consensus_client, parent_hash, parent_number, )? diff --git a/domains/client/domain-operator/src/domain_bundle_producer.rs b/domains/client/domain-operator/src/domain_bundle_producer.rs index 85c1e53e1c..943a8ca7fb 100644 --- a/domains/client/domain-operator/src/domain_bundle_producer.rs +++ b/domains/client/domain-operator/src/domain_bundle_producer.rs @@ -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 diff --git a/domains/client/domain-operator/src/domain_bundle_proposer.rs b/domains/client/domain-operator/src/domain_bundle_proposer.rs index b16284c635..dae12241de 100644 --- a/domains/client/domain-operator/src/domain_bundle_proposer.rs +++ b/domains/client/domain-operator/src/domain_bundle_proposer.rs @@ -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 @@ -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::( + crate::load_execution_receipt_by_domain_hash::( &*self.client, - &self.consensus_client, domain_hash, receipt_number, ) diff --git a/domains/client/domain-operator/src/lib.rs b/domains/client/domain-operator/src/lib.rs index f66ec5eae5..42032858f6 100644 --- a/domains/client/domain-operator/src/lib.rs +++ b/domains/client/domain-operator/src/lib.rs @@ -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; @@ -240,56 +242,40 @@ where Ok(leaves.into_iter().rev().take(MAX_ACTIVE_LEAVES).collect()) } -pub(crate) fn load_execution_receipt_by_domain_hash( +pub(crate) fn load_execution_receipt_by_domain_hash( domain_client: &Client, - consensus_client: &Arc, domain_hash: Block::Hash, domain_number: NumberFor, ) -> Result, sp_blockchain::Error> where Block: BlockT, CBlock: BlockT, - Client: AuxStore, - CClient: HeaderBackend, + Client: AuxStore + HeaderBackend, { - 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" + )) + }) } diff --git a/domains/client/domain-operator/src/tests.rs b/domains/client/domain-operator/src/tests.rs index a4bbfce93f..8e44787d22 100644 --- a/domains/client/domain-operator/src/tests.rs +++ b/domains/client/domain-operator/src/tests.rs @@ -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(), ))], @@ -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(""); @@ -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::< - _, - _, - ::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| -> ::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 diff --git a/domains/primitives/digests/src/lib.rs b/domains/primitives/digests/src/lib.rs index 037c28cd1a..482cb37631 100644 --- a/domains/primitives/digests/src/lib.rs +++ b/domains/primitives/digests/src/lib.rs @@ -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(&self) -> Option<(Number, Hash)>; + /// Return `consensus_block_hash` + fn as_consensus_block_info(&self) -> Option; - /// Creates a new digest of primary block info for system domain. - fn primary_block_info(info: (Number, Hash)) -> Self; + /// Creates a new digest of the consensus block that derive the domain block. + fn consensus_block_info(consensus_block_hash: Hash) -> Self; } impl AsPredigest for DigestItem { - /// Return a pair of (primary_block_number, primary_block_hash). - fn as_primary_block_info(&self) -> Option<(Number, Hash)> { + fn as_consensus_block_info(&self) -> Option { self.pre_runtime_try_to(&DOMAIN_REGISTRY_ENGINE_ID) } - fn primary_block_info(info: (Number, Hash)) -> Self { - DigestItem::PreRuntime(DOMAIN_REGISTRY_ENGINE_ID, info.encode()) + fn consensus_block_info(consensus_block_hash: Hash) -> Self { + DigestItem::PreRuntime(DOMAIN_REGISTRY_ENGINE_ID, consensus_block_hash.encode()) } }