Revamp invalid state transition proof #2072

NingLin-P · 2023-10-06T20:11:04Z

~~The first 2 commits are picked from #1983, depends on which PRs merge first, the latter one should remove these commits.~~

This PR revamps the invalid state transition proof based on the v2 design, notable changes compared to v1:

Many fields of InvalidStateTransitionProof are replaced by the field bad_receipt_hash, the bad receipt is retrieved from the consensus chain and its data is used instead during the verification
~~Add the runtime_code_with_proof field to the fraud proof which contains the domain runtime code along with the storage proof~~
The call data of the execution proof are provided by the fraud proof directly and verified against the bad receipt
Introduce the DomainBlock config item to pallet-domains, which is used to construct the domain block header during the invalid state transition fraud proof
Add host functions for getting the domain runtime code and checking the execution proof

Code contributor checklist:

I have read, understood and followed contributing guide

Signed-off-by: linning <linningde25@gmail.com>

It is used to construct the domain block header during the verification of the invalid state transtion in the upcoming commits, this commit also removed the DomainNumber config item since it can be derived from DomainBlock, but the DomainHash is still needed for converting DomainHash to/from H256. Signed-off-by: linning <linningde25@gmail.com>

… execution proof They are used in the verification of the invalid state transition fraud proof Signed-off-by: linning <linningde25@gmail.com>

Mainly remove fields that already exist in the bad receipt Signed-off-by: linning <linningde25@gmail.com>

…nsus runtime Signed-off-by: linning <linningde25@gmail.com>

Since we don't need to verify this fraud proof in the client side anymore Signed-off-by: linning <linningde25@gmail.com>

Signed-off-by: linning <linningde25@gmail.com>

NingLin-P · 2023-10-16T13:56:53Z

This PR is updated according to the host function approach, PTAL thanks!

vedhavyas

Make sense. Left some questions and nits.

crates/pallet-domains/src/lib.rs

crates/pallet-domains/src/block_tree.rs

crates/pallet-domains/src/lib.rs

crates/sp-domains-fraud-proof/src/host_functions.rs

crates/sp-domains/src/lib.rs

crates/sp-domains/src/fraud_proof.rs

…id-state-proof

Signed-off-by: linning <linningde25@gmail.com>

vedhavyas

Make sense to me. Left some question on Storage Proof generation and Verification

crates/pallet-domains/src/lib.rs

vedhavyas · 2023-10-18T16:05:45Z

crates/sp-domains/src/valued_trie_root.rs

+/// Gnenerate a proof-of-inclusion of the value at the given `input[index]`
+///
+/// Return `None` if the given `index` out of range or fail to generate the proof
+pub fn generate_proof<Layout: TrieLayout>(input: &[Vec<u8>], index: u32) -> Option<Vec<Vec<u8>>> {


I missed this earlier. Shouldn't this return a Storage Proof type instead. It would be nice to have that instead of returning some bare vectors.

Vec<Vec<u8>> is what returned by trie_db::proof::generate_proof, we can't use StorageProof unless they can be verified in the same way.

vedhavyas · 2023-10-18T16:06:39Z

crates/sp-domains/src/valued_trie_root.rs

+}
+
+/// Verify the proof-of-inclusion of the `data` at the `index` of the merkle root `root`
+pub fn verify_proof<Layout>(


There is already a storage proof verification in the verification module. We should use that instead. Ideally, we should call it Storage Proof Provider that can generate proofs and verify proofs

I have explored this direction a bit but verify_and_get_value doesn't work directly, while both approaches use trie-db under the hook, there must be some differences between generating StorageProof and trie_db::proof::generate_proof (the later is used to generating proof-of-inclusion specifically).

I will leave a TODO for now, let me if you have a better idea.

I think it should be generated something similart to - https://github.com/subspace/subspace/blob/main/crates/pallet-domains/src/tests.rs#L829

Anyway leave a TODO and we can revisit this

vedhavyas · 2023-10-18T16:35:09Z

domains/client/domain-operator/src/fraud_proof.rs

-        .map_err(|_| FraudProofError::InvalidStateRootType)?;
-
-        let digest = Digest::default();
+        let digest = self.client.runtime_api().block_digest(block_hash)?;


I'm sorry to confuse you on the digests. I have a question here. The digest you are fetching here is the post block digest. So it includes all the digest items that were added during the state transition and on_finalization. So the digests might not the same digests that were passed during the new block initialization then, correct ? So we may be adding the digest logs that will added in the later execution in the intialization itself. Thoughts @NingLin-P

Okay, I took a closer look. I think the first version of this PR is fine, namely only including the consensus_block_info digest which is the only inherent digest and doesn't need the digest storage proof at all.

For ApplyExtrinsic and FinalizeBlock, the necessary extrinsic will be executed before generating the execution proof (see prepare_storage_changes_before and prepare_storage_changes_before_finalize_block) so the digests emitted during block execution/finalization will be included in the proof properly.

One last question about "there are digests from frontier we should consider here that are missing", do you mean the digest emitted during block execution/finalization or the inherent digest?

…nd some cleanup Signed-off-by: linning <linningde25@gmail.com>

vedhavyas · 2023-10-19T06:19:21Z

crates/sp-domains/src/valued_trie_root.rs

+}
+
+/// Verify the proof-of-inclusion of the `data` at the `index` of the merkle root `root`
+pub fn verify_proof<Layout>(


I think it should be generated something similart to - https://github.com/subspace/subspace/blob/main/crates/pallet-domains/src/tests.rs#L829

Anyway leave a TODO and we can revisit this

NingLin-P requested review from ParthDesai and dariolina October 6, 2023 20:11

NingLin-P requested review from vedhavyas, nazar-pc and rg3l3dr as code owners October 6, 2023 20:11

NingLin-P force-pushed the revamp-invalid-state-proof branch from 6c1be67 to 2f92af4 Compare October 7, 2023 14:35

NingLin-P requested a review from shamil-gadelshin as a code owner October 7, 2023 14:35

NingLin-P force-pushed the revamp-invalid-state-proof branch 2 times, most recently from 8b93038 to c500f7d Compare October 9, 2023 07:51

NingLin-P marked this pull request as draft October 12, 2023 15:13

NingLin-P added 7 commits October 16, 2023 21:40

Add merkle proof based on the valued_trie_root

bda969b

Signed-off-by: linning <linningde25@gmail.com>

Introduce host functions for getting domain runtime code and checking…

1575ff3

… execution proof They are used in the verification of the invalid state transition fraud proof Signed-off-by: linning <linningde25@gmail.com>

Revamp the InvalidStateTransitionProof struct and generation

4988b02

Mainly remove fields that already exist in the bad receipt Signed-off-by: linning <linningde25@gmail.com>

Revamp the invalid state transition verification and move it to conse…

f0771c9

…nsus runtime Signed-off-by: linning <linningde25@gmail.com>

Remove InvalidStateTransitionProofVerifier

d6efd7f

Since we don't need to verify this fraud proof in the client side anymore Signed-off-by: linning <linningde25@gmail.com>

Revamp bad_receipts_to_delete

cb77a6f

Signed-off-by: linning <linningde25@gmail.com>

NingLin-P force-pushed the revamp-invalid-state-proof branch from c500f7d to cb77a6f Compare October 16, 2023 13:54

NingLin-P marked this pull request as ready for review October 16, 2023 13:55

vedhavyas reviewed Oct 17, 2023

View reviewed changes

NingLin-P added 2 commits October 18, 2023 23:03

Merge branch 'main' of github.com:subspace/subspace into revamp-inval…

a3e256d

…id-state-proof

Apply review suggestions

bc81164

Signed-off-by: linning <linningde25@gmail.com>

NingLin-P force-pushed the revamp-invalid-state-proof branch from bd1398b to bc81164 Compare October 18, 2023 15:53

vedhavyas reviewed Oct 18, 2023

View reviewed changes

Remove digest_storage_proof from the invalid state transition proof a…

1063a3c

…nd some cleanup Signed-off-by: linning <linningde25@gmail.com>

vedhavyas approved these changes Oct 19, 2023

View reviewed changes

NingLin-P added this pull request to the merge queue Oct 19, 2023

Merged via the queue into main with commit 5d06efe Oct 19, 2023
10 checks passed

NingLin-P deleted the revamp-invalid-state-proof branch October 19, 2023 09:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revamp invalid state transition proof #2072

Revamp invalid state transition proof #2072

NingLin-P commented Oct 6, 2023 •

edited

Loading

NingLin-P commented Oct 16, 2023

vedhavyas left a comment

vedhavyas left a comment

vedhavyas Oct 18, 2023

NingLin-P Oct 18, 2023

vedhavyas Oct 18, 2023 •

edited

Loading

NingLin-P Oct 18, 2023

vedhavyas Oct 19, 2023

vedhavyas Oct 18, 2023

NingLin-P Oct 18, 2023

vedhavyas Oct 19, 2023

Revamp invalid state transition proof #2072

Revamp invalid state transition proof #2072

Conversation

NingLin-P commented Oct 6, 2023 • edited Loading

Code contributor checklist:

NingLin-P commented Oct 16, 2023

vedhavyas left a comment

Choose a reason for hiding this comment

vedhavyas left a comment

Choose a reason for hiding this comment

vedhavyas Oct 18, 2023

Choose a reason for hiding this comment

NingLin-P Oct 18, 2023

Choose a reason for hiding this comment

vedhavyas Oct 18, 2023 • edited Loading

Choose a reason for hiding this comment

NingLin-P Oct 18, 2023

Choose a reason for hiding this comment

vedhavyas Oct 19, 2023

Choose a reason for hiding this comment

vedhavyas Oct 18, 2023

Choose a reason for hiding this comment

NingLin-P Oct 18, 2023

Choose a reason for hiding this comment

vedhavyas Oct 19, 2023

Choose a reason for hiding this comment

NingLin-P commented Oct 6, 2023 •

edited

Loading

vedhavyas Oct 18, 2023 •

edited

Loading