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.

Sign up for GitHub

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

Jump to bottom

Domain block production for domains v2 #1567

Merged
merged 9 commits into from
Jul 5, 2023
Merged

Domain block production for domains v2 #1567

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
52c49b3
Add domain_number field to ExecutionReceipt
NingLin-P Jun 20, 2023
c6535e4
Skip building domain block if there are no bundle contains in the pri…
NingLin-P Jun 21, 2023
9a09c21
Introduce auxiliary storage for primary/domain block hash to handle c…
NingLin-P Jun 21, 2023
2f3c921
Reset the domain best block according to the new best primary block
NingLin-P Jun 21, 2023
ff5023b
Clarify the usage of primary_number and domain_number in the executor…
NingLin-P Jun 21, 2023
0929f44
Add test test_domain_block_production
NingLin-P Jun 21, 2023
33f7b7c
Apply review suggestions
NingLin-P Jul 3, 2023
36fc466
Apply review suggestions
NingLin-P Jul 5, 2023
1c835c1
Merge branch 'main' into domain-block
NingLin-P Jul 5, 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
1 change: 1 addition & 0 deletions crates/pallet-domains/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ fn create_dummy_receipt(
ExecutionReceipt {
primary_number,
primary_hash,
domain_number: primary_number,
domain_hash: H256::random(),
trace: if primary_number == 0 {
Vec::new()
Expand Down
16 changes: 15 additions & 1 deletion crates/sp-domains/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,8 @@ pub struct ExecutionReceipt<Number, Hash, DomainHash> {
pub primary_number: Number,
/// Hash of the origin primary block this receipt corresponds to.
pub primary_hash: Hash,
/// Domain block number.
pub domain_number: Number,
/// Hash of the domain block this receipt points to.
pub domain_hash: DomainHash,
/// List of storage roots collected during the domain block execution.
Expand All @@ -414,7 +416,7 @@ impl<Number: Encode, Hash: Encode, DomainHash: Encode> ExecutionReceipt<Number,
}
}

impl<Number: Zero, Hash, DomainHash: Default> ExecutionReceipt<Number, Hash, DomainHash> {
impl<Number: Copy + Zero, Hash, DomainHash: Default> ExecutionReceipt<Number, Hash, DomainHash> {
#[cfg(any(feature = "std", feature = "runtime-benchmarks"))]
pub fn dummy(
primary_number: Number,
Expand All @@ -428,11 +430,23 @@ impl<Number: Zero, Hash, DomainHash: Default> ExecutionReceipt<Number, Hash, Dom
ExecutionReceipt {
primary_number,
primary_hash,
domain_number: primary_number,
domain_hash: Default::default(),
trace,
trace_root: Default::default(),
}
}

pub fn genesis(primary_genesis_hash: Hash) -> ExecutionReceipt<Number, Hash, DomainHash> {
ExecutionReceipt {
primary_number: Zero::zero(),
primary_hash: primary_genesis_hash,
domain_number: Zero::zero(),
domain_hash: Default::default(),
trace: Default::default(),
trace_root: Default::default(),
}
}
}

/// List of [`OpaqueBundle`].
Expand Down
17 changes: 10 additions & 7 deletions domains/client/block-preprocessor/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,18 +273,17 @@ where
) -> sp_blockchain::Result<Vec<Vec<u8>>> {
// `domain_hash` is unused in `preprocess_primary_block` when using stateless runtime api.
let domain_hash = Default::default();
Ok(self
.preprocess_primary_block(primary_hash, domain_hash)?
.into_iter()
.map(|ext| ext.encode())
.collect())
match self.preprocess_primary_block(primary_hash, domain_hash)? {
Some(extrinsics) => Ok(extrinsics.into_iter().map(|ext| ext.encode()).collect()),
None => Ok(Vec::new()),
}
}

pub fn preprocess_primary_block(
&self,
primary_hash: PBlock::Hash,
domain_hash: Block::Hash,
) -> sp_blockchain::Result<Vec<Block::Extrinsic>> {
) -> sp_blockchain::Result<Option<Vec<Block::Extrinsic>>> {
let (primary_extrinsics, shuffling_seed, maybe_new_runtime) =
prepare_domain_block_elements::<Block, PBlock, _>(
self.domain_id,
Expand All @@ -297,6 +296,10 @@ where
.runtime_api()
.extract_successful_bundles(primary_hash, primary_extrinsics)?;

if bundles.is_empty() && maybe_new_runtime.is_none() {
return Ok(None);
}

let extrinsics = compile_own_domain_bundles::<Block, PBlock>(bundles);

let extrinsics_in_bundle = deduplicate_and_shuffle_extrinsics(
Expand Down Expand Up @@ -330,7 +333,7 @@ where
extrinsics.push(set_code_extrinsic);
}

Ok(extrinsics)
Ok(Some(extrinsics))
}

fn filter_invalid_xdm_extrinsics(
Expand Down
105 changes: 97 additions & 8 deletions domains/client/domain-executor/src/aux_schema.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,32 @@ const BAD_RECEIPT_MISMATCH_INFO: &[u8] = b"bad_receipt_mismatch_info";
const BAD_RECEIPT_NUMBERS: &[u8] = b"bad_receipt_numbers";

/// domain_block_hash => primary_block_hash
///
/// Updated only when there is a new domain block produced
const PRIMARY_HASH: &[u8] = b"primary_hash";

/// domain_block_hash => latest_primary_block_hash
///
/// It's important to note that a primary block could possibly contain no bundles for a specific domain,
/// leading to the situation where multiple primary blocks could correspond to the same domain block.
///
/// PrimaryBlock10 --> DomainBlock5
/// PrimaryBlock11 --> DomainBlock5
/// PrimaryBlock12 --> DomainBlock5
///
/// This mapping is designed to track the most recent primary block that derives the domain block
/// identified by `domain_block_hash`, e.g., Hash(DomainBlock5) => Hash(PrimaryBlock12).
const LATEST_PRIMARY_HASH: &[u8] = b"latest_primary_hash";

/// primary_block_hash => best_domain_block_hash
///
/// This mapping tracks the mapping of a primary block and the corresponding domain block derived
/// until this primary block:
/// - Hash(PrimaryBlock10) => Hash(DomainBlock5)
/// - Hash(PrimaryBlock11) => Hash(DomainBlock5)
/// - Hash(PrimaryBlock12) => Hash(DomainBlock5)
const BEST_DOMAIN_HASH: &[u8] = b"best_domain_hash";

/// Prune the execution receipts when they reach this number.
const PRUNING_DEPTH: BlockNumber = 1000;

Expand Down Expand Up @@ -66,6 +90,7 @@ where
{
let block_number = execution_receipt.primary_number;
let primary_hash = execution_receipt.primary_hash;
let domain_hash = execution_receipt.domain_hash;

let block_number_key = (EXECUTION_RECEIPT_BLOCK_NUMBER, block_number).encode();
let mut hashes_at_block_number =
Expand Down Expand Up @@ -109,6 +134,11 @@ where
execution_receipt_key(primary_hash).as_slice(),
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to change. ER should be stored against domain block hash and processed primary block hash should map domain block hash.

Copy link
Member Author

Choose a reason for hiding this comment

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

We use the domain block hash as the key before but there is some issue, PTAL #1254

Copy link
Contributor

Choose a reason for hiding this comment

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

The advantage of using primary block hash as the key has seemingly been diminished by v2 as now each primary block hash is mapped to an optional ER as well, let's reassess these two options. @NingLin-P Perhaps you could make a brief doc for them on the use cases so that we don't have to look into the code? I can do it as well if you prefer.

Copy link
Member Author

Choose a reason for hiding this comment

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

Perhaps you could make a brief doc for them on the use cases so that we don't have to look into the code?

I'm willing to do that, but I think there is not really a trade-off we need to make, using domain_hash will just bring back the issue #1254 because:

When processing the consensus block, the operator checks the ER contains inside is the same as the one it produces locally, if not then use the local ER to construct and submit a fraud proof.

When using domain_hash as key we are unable to find and load the local ER if the incoming ER is different from the local one because the incoming_ER.domain_hash is also different, thus load_local_ER(incoming_ER.domain_hash) will always result in None then we are unable to construct the fraud proof properly.

Copy link
Contributor

Choose a reason for hiding this comment

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

The same problem is back, but there might be another solution given the context has been changed a lot, I'm not entirely sure, hence want to make it assessed again under the v2 design.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have revisited the v2 fraud proof, which only mentioned the operator should "detecting any discrepancies" but didn't give more detail.

But IMO, the requirement is still the same under v2 design, namely get the local ER that driving from the same consensus block as the incoming ER thus seems inevitable to use the consensus block hash as the key for ER.

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant to have a brief intro for the scenarios here so that we can come to a conclusion naturally and even help the readers who are not familiar with the context.

So, the problem here is how to track the receipt on the operator's side so that it can be retrieved later.

Use cases

  1. Retrieve the receipt for bundle production

    • Given the tip receipt on the consensus chain, include the next receipt or confirm the tip receipt in the bundle.
    • Given domain_block_number ⇒ domain_block_hash ⇒ load the corresponding local receipt

  2. Retrieve the receipt of a specific domain block in order to compare the local trace and the other trace for fraud proof.

    • Given an external receipt ⇒ external_receipt.consensus_block_hash ⇒ load the corresponding local receipt.
    • There is no domain_block_hash in the v2 ExecutionReceipt structure.
      Actually, even if we include domain_block_hash in the receipt, it’s unusable in case the bad receipt, because it’s never verified and any trace mismatch will cause a different domain block hash since the state must be different, bad_receipt.domain_block_hash never exists on the honest operator’s end.

Options

  • Use domain_block_hash as the key (domain_block_hash ⇒ ER)
    • Works for use case 1 directly, but has no way to work for use case 2.
  • Use consensus_block_hash as the key (consensus_block_hash ⇒ ER)
    • Works for use case 2 directly, but we have to additionally track a mapping of domain_block_hash ⇒ consensus_block_hash for use case 1.

To conclude, the receipt needs to be keyed by consensus_block_hash.

execution_receipt.encode().as_slice(),
),
// TODO: prune the stale mappings.
(
(PRIMARY_HASH, domain_hash).encode().as_slice(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this storage? Primary hash should be part of the domain block and can be fetch from the runtime no?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is mainly for load_execution_receipt_by_domain_hash.

Primary hash should be part of the domain block and can be fetch from the runtime no?

Is it? do you mean the ER in the consensus chain runtime?

primary_hash.encode().as_slice(),
),
(
block_number_key.as_slice(),
hashes_at_block_number.encode().as_slice(),
Expand All @@ -125,6 +155,26 @@ where
)
}

/// Load the execution receipt for given domain block hash.
pub(super) fn load_execution_receipt_by_domain_hash<Backend, Hash, Number, PHash>(
backend: &Backend,
domain_hash: Hash,
) -> ClientResult<Option<ExecutionReceipt<Number, PHash, Hash>>>
where
Backend: AuxStore,
Hash: Encode + Decode,
Number: Decode,
PHash: Encode + Decode,
{
match primary_hash_for::<Backend, Hash, PHash>(backend, domain_hash)? {
Some(primary_block_hash) => load_decode(
Copy link
Contributor

Choose a reason for hiding this comment

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

This redirection is unnecessary here if the domain_hash is mapped to ER

backend,
execution_receipt_key(primary_block_hash).as_slice(),
),
None => Ok(None),
}
}

/// Load the execution receipt for given primary block hash.
pub(super) fn load_execution_receipt<Backend, Hash, Number, PHash>(
backend: &Backend,
Expand All @@ -142,27 +192,65 @@ where
)
}

pub(super) fn track_domain_hash_to_primary_hash<Backend, Hash, PHash>(
pub(super) fn track_domain_hash_and_primary_hash<Backend, Hash, PHash>(
backend: &Backend,
domain_hash: Hash,
primary_hash: PHash,
best_domain_hash: Hash,
latest_primary_hash: PHash,
) -> ClientResult<()>
where
Backend: AuxStore,
Hash: Encode,
Hash: Clone + Encode,
PHash: Encode,
{
// TODO: prune the stale mappings.

backend.insert_aux(
&[(
(PRIMARY_HASH, domain_hash).encode().as_slice(),
primary_hash.encode().as_slice(),
)],
&[
(
(LATEST_PRIMARY_HASH, best_domain_hash.clone())
.encode()
.as_slice(),
latest_primary_hash.encode().as_slice(),
),
(
(BEST_DOMAIN_HASH, latest_primary_hash).encode().as_slice(),
best_domain_hash.encode().as_slice(),
),
],
vec![],
)
}

pub(super) fn best_domain_hash_for<Backend, Hash, PHash>(
backend: &Backend,
primary_hash: &PHash,
) -> ClientResult<Option<Hash>>
where
Backend: AuxStore,
Hash: Decode,
PHash: Encode,
{
load_decode(
backend,
(BEST_DOMAIN_HASH, primary_hash).encode().as_slice(),
)
}

pub(super) fn latest_primary_hash_for<Backend, Hash, PHash>(
backend: &Backend,
domain_hash: &Hash,
) -> ClientResult<Option<PHash>>
where
Backend: AuxStore,
Hash: Encode,
PHash: Decode,
{
load_decode(
backend,
(LATEST_PRIMARY_HASH, domain_hash).encode().as_slice(),
)
}

pub(super) fn primary_hash_for<Backend, Hash, PHash>(
backend: &Backend,
domain_hash: Hash,
Expand Down Expand Up @@ -411,6 +499,7 @@ mod tests {
ExecutionReceipt {
primary_number,
primary_hash: H256::random(),
domain_number: primary_number,
domain_hash: H256::random(),
trace: Default::default(),
trace_root: Default::default(),
Expand Down
Loading