Skip to content

Commit

Permalink
Merge pull request #2216 from subspace/fix-incompatible
Browse files Browse the repository at this point in the history
Fix operator incompatible with gemini-3g
  • Loading branch information
NingLin-P authored Nov 9, 2023
2 parents 01345d4 + b062949 commit 4900d85
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 8 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions crates/sp-domains-fraud-proof/src/verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,9 @@ where
return Err(InvalidBundleEquivocationError::MismatchedOperatorAndDomain);
}

// TODO: `consensus_block_hash` is an default/empty value because this field is skipped
// during encode/decode due to incompatible with Gemini-3g, thus the verificaion will fail.
// This should be fix on new network when the field is not skipped.
let consensus_block_hash = header_1.header.proof_of_election.consensus_block_hash;
let domain_id = header_1.header.proof_of_election.domain_id;
let operator_id = header_1.header.proof_of_election.operator_id;
Expand Down
3 changes: 3 additions & 0 deletions crates/sp-domains/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,9 @@ pub struct ProofOfElection<CHash> {
/// Operator index in the OperatorRegistry.
pub operator_id: OperatorId,
/// Consensus block hash at which proof of election was derived.
// TODO: skipping encode/decode this field becasue it is mismatch with the struct
// on the Gemini-3g runtime, remove `#[codec(skip)]` before new network
#[codec(skip)]
pub consensus_block_hash: CHash,
}

Expand Down
1 change: 0 additions & 1 deletion domains/client/block-preprocessor/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ sp-messenger = { version = "0.1.0", path = "../../primitives/messenger" }
sp-runtime = { version = "24.0.0", git = "https://github.com/subspace/polkadot-sdk", rev = "892bf8e938c6bd2b893d3827d1093cd81baa59a1" }
sp-state-machine = { version = "0.28.0", git = "https://github.com/subspace/polkadot-sdk", rev = "892bf8e938c6bd2b893d3827d1093cd81baa59a1" }
sp-timestamp = { version = "4.0.0-dev", git = "https://github.com/subspace/polkadot-sdk", rev = "892bf8e938c6bd2b893d3827d1093cd81baa59a1" }
sp-trie = { version = "22.0.0", git = "https://github.com/subspace/polkadot-sdk", rev = "892bf8e938c6bd2b893d3827d1093cd81baa59a1" }
subspace-core-primitives = { version = "0.1.0", path = "../../../crates/subspace-core-primitives" }
subspace-runtime-primitives = { version = "0.1.0", path = "../../../crates/subspace-runtime-primitives" }
tracing = "0.1.37"
Expand Down
18 changes: 12 additions & 6 deletions domains/client/block-preprocessor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,11 @@ use sp_blockchain::HeaderBackend;
use sp_core::H256;
use sp_domains::extrinsics::deduplicate_and_shuffle_extrinsics;
use sp_domains::{
DomainId, DomainsApi, ExecutionReceipt, ExtrinsicDigest, HeaderHashingFor, InboxedBundle,
InvalidBundleType, OpaqueBundle, OpaqueBundles, ReceiptValidity,
DomainId, DomainsApi, ExecutionReceipt, HeaderHashingFor, InboxedBundle, InvalidBundleType,
OpaqueBundle, OpaqueBundles, ReceiptValidity,
};
use sp_messenger::MessengerApi;
use sp_runtime::traits::{Block as BlockT, NumberFor};
use sp_trie::LayoutV1;
use std::collections::VecDeque;
use std::marker::PhantomData;
use std::sync::Arc;
Expand Down Expand Up @@ -231,9 +230,16 @@ where
.map(|(signer, tx)| {
(
signer.clone(),
ExtrinsicDigest::new::<LayoutV1<HeaderHashingFor<Block::Header>>>(
tx.encode(),
),
HeaderHashingFor::<Block::Header>::hash_of(tx),
// TODO: enable before the new network
// NOTE: we should using the `ExtrinsicDigest` to calculate the `bundle_digest`
// to keep consistency with the `domain_block_extrinsic_root` fraud proof, but
// this is incompatible with Gemini-3g network because operators who have this
// change will produce a different ER than those who don't have, cause the ER
// derivation become non-deterministic.
// ExtrinsicDigest::new::<LayoutV1<HeaderHashingFor<Block::Header>>>(
// tx.encode(),
// ),
)
})
.collect();
Expand Down
9 changes: 9 additions & 0 deletions domains/client/domain-operator/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1471,7 +1471,11 @@ async fn test_invalid_domain_block_hash_proof_creation() {
.is_none());
}

// Disable because the `bundle_digest` value used in the ER is inconsistent with the value
// used in the fraud proof, the fix is not deploy due to incompatible with the Gemini-3g
// TODO: enable once the fix is deployed
#[tokio::test(flavor = "multi_thread")]
#[ignore]
async fn test_invalid_domain_extrinsics_root_proof_creation() {
let directory = TempDir::new().expect("Must be able to create temporary directory");

Expand Down Expand Up @@ -1604,7 +1608,12 @@ async fn test_invalid_domain_extrinsics_root_proof_creation() {
.is_none());
}

// Disable because the `ProofOfElection::consensus_block_hash` is skipped during encode/decode
// due to incompatible with Gemini-3g, thus it is the empty value in the fraud proof and will
// failed when used to get the required runtime state during verification.
// TODO: enable once the field is not skipped.
#[tokio::test(flavor = "multi_thread")]
#[ignore]
async fn test_bundle_equivocation_fraud_proof() {
let directory = TempDir::new().expect("Must be able to create temporary directory");

Expand Down

0 comments on commit 4900d85

Please sign in to comment.