diff --git a/Cargo.lock b/Cargo.lock index e8b18d94a793d..c6a45f97c790e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4085,7 +4085,7 @@ dependencies = [ "srml-session 2.0.0", "srml-support 2.0.0", "srml-system 2.0.0", - "substrate-externalities 2.0.0", + "substrate-finality-grandpa-primitives 2.0.0", "substrate-primitives 2.0.0", "substrate-state-machine 2.0.0", "substrate-trie 2.0.0", diff --git a/srml/bridge/Cargo.toml b/srml/bridge/Cargo.toml index 92c93954bf186..6e63d0d6f443c 100644 --- a/srml/bridge/Cargo.toml +++ b/srml/bridge/Cargo.toml @@ -8,7 +8,9 @@ edition = "2018" [dependencies] codec = { package = "parity-scale-codec", version = "1.0.0", default-features = false } +fg_primitives = { package = "substrate-finality-grandpa-primitives", path = "../../core/finality-grandpa/primitives" } hash-db = { version = "0.15.2", default-features = false } +primitives = { package = "substrate-primitives", path = "../../core/primitives" } session = { package = "srml-session", path = "../session", default-features = false, features = ["historical"] } serde = { version = "1.0", optional = true } sr-primitives = { path = "../../core/sr-primitives", default-features = false } @@ -17,7 +19,6 @@ system = { package = "srml-system", path = "../system", default-features = false trie = { package = "substrate-trie", path = "../../core/trie", default-features = false } [dev-dependencies] -primitives = { package = "substrate-primitives", path = "../../core/primitives" } runtime-io = { package = "sr-io", path = "../../core/sr-io", default-features = false } state-machine = { package = "substrate-state-machine", path = "../../core/state-machine" } diff --git a/srml/bridge/src/error.rs b/srml/bridge/src/error.rs deleted file mode 100644 index 182b0883d1e62..0000000000000 --- a/srml/bridge/src/error.rs +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright 2017-2019 Parity Technologies (UK) Ltd. -// This file is part of Substrate. - -// Substrate is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// Substrate is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with Substrate. If not, see . - -//! Common error types for the srml-bridge crate. - -#[derive(PartialEq)] -#[cfg_attr(feature = "std", derive(Debug))] -pub enum Error { - StorageRootMismatch, - StorageValueUnavailable, -} - diff --git a/srml/bridge/src/lib.rs b/srml/bridge/src/lib.rs index f437c0445dd5b..b217b15ffe8ae 100644 --- a/srml/bridge/src/lib.rs +++ b/srml/bridge/src/lib.rs @@ -19,6 +19,7 @@ //! This will eventually have some useful documentation. //! For now though, enjoy this cow's wisdom. //! +//!```ignore //!________________________________________ //! / You are only young once, but you can \ //! \ stay immature indefinitely. / @@ -28,18 +29,19 @@ //! (__)\ )\/\ //! ||----w | //! || || +//!``` // Ensure we're `no_std` when compiling for Wasm. #![cfg_attr(not(feature = "std"), no_std)] -mod error; mod storage_proof; +use crate::storage_proof::StorageProofChecker; use codec::{Encode, Decode}; -use sr_primitives::traits::{Header, Member}; +use fg_primitives::{AuthorityId, AuthorityWeight}; +use sr_primitives::traits::Header; use support::{ decl_error, decl_module, decl_storage, - Parameter, }; use system::{ensure_signed}; @@ -49,7 +51,7 @@ pub struct BridgeInfo { last_finalized_block_number: T::BlockNumber, last_finalized_block_hash: T::Hash, last_finalized_state_root: T::Hash, - current_validator_set: Vec, + current_validator_set: Vec<(AuthorityId, AuthorityWeight)>, } impl BridgeInfo { @@ -57,7 +59,7 @@ impl BridgeInfo { block_number: &T::BlockNumber, block_hash: &T::Hash, state_root: &T::Hash, - validator_set: Vec, + validator_set: Vec<(AuthorityId, AuthorityWeight)>, ) -> Self { // I don't like how this is done, should come back to... @@ -72,10 +74,7 @@ impl BridgeInfo { type BridgeId = u64; -pub trait Trait: system::Trait { - /// A stable ID for a validator. - type ValidatorId: Member + Parameter; -} +pub trait Trait: system::Trait {} decl_storage! { trait Store for Module as Bridge { @@ -90,32 +89,27 @@ decl_storage! { decl_module! { pub struct Module for enum Call where origin: T::Origin { - // TODO: Figure out the proper type for these proofs + // TODO: Change proof type to StorageProof once #3834 is merged fn initialize_bridge( origin, block_header: T::Header, - validator_set: Vec, - validator_set_proof: Vec, - storage_proof: Vec, + validator_set: Vec<(AuthorityId, AuthorityWeight)>, + validator_set_proof: Vec>, ) { // NOTE: Will want to make this a governance issued call let _sender = ensure_signed(origin)?; - Self::check_storage_proof(storage_proof)?; - Self::check_validator_set_proof(validator_set_proof)?; - let block_number = block_header.number(); let block_hash = block_header.hash(); let state_root = block_header.state_root(); + + Self::check_validator_set_proof(state_root, validator_set_proof, &validator_set)?; + let bridge_info = BridgeInfo::new(block_number, &block_hash, state_root, validator_set); let new_bridge_id = NumBridges::get() + 1; - - // Hmm, can a call to `insert` fail? - // If this is an Option, why do we not need to wrap it in Some(T)? >::insert(new_bridge_id, bridge_info); - // Only increase the number of bridges if the insert call succeeds NumBridges::put(new_bridge_id); } @@ -129,19 +123,37 @@ decl_error! { // Error for the Bridge module pub enum Error { InvalidStorageProof, + StorageRootMismatch, + StorageValueUnavailable, InvalidValidatorSetProof, + ValidatorSetMismatch, } } impl Module { - fn check_storage_proof(_proof: Vec) -> std::result::Result<(), Error> { - // ... Do some math ... - Ok(()) // Otherwise, Error::InvalidStorageProof - } - - fn check_validator_set_proof(_proof: Vec) -> std::result::Result<(), Error> { - // ... Do some math ... - Ok(()) // Otherwise, Error::InvalidValidatorSetProof + fn check_validator_set_proof( + state_root: &T::Hash, + proof: Vec>, + validator_set: &Vec<(AuthorityId, AuthorityWeight)>, + ) -> std::result::Result<(), Error> { + + let checker = ::Hasher>>::new( + *state_root, + proof.clone() + )?; + + // By encoding the given set we should have an easy way to compare + // with the stuff we get out of storage via `read_value` + let encoded_validator_set = validator_set.encode(); + let actual_validator_set = checker + .read_value(b":grandpa_authorities")? + .ok_or(Error::StorageValueUnavailable)?; + + if encoded_validator_set == actual_validator_set { + Ok(()) + } else { + Err(Error::ValidatorSetMismatch) + } } } @@ -149,13 +161,12 @@ impl Module { mod tests { use super::*; - use primitives::H256; + use primitives::{Blake2Hasher, H256, Public}; use sr_primitives::{ Perbill, traits::{Header as HeaderT, IdentityLookup}, testing::Header, generic::Digest, }; - use support::{assert_ok, impl_outer_origin, parameter_types}; + use support::{assert_ok, assert_err, impl_outer_origin, parameter_types}; - // NOTE: What's this for? impl_outer_origin! { pub enum Origin for Test {} } @@ -163,7 +174,7 @@ mod tests { #[derive(Clone, PartialEq, Eq, Debug)] pub struct Test; - type System = system::Module; + type _System = system::Module; type MockBridge = Module; // TODO: Figure out what I actually need from here @@ -175,7 +186,7 @@ mod tests { pub const AvailableBlockRatio: Perbill = Perbill::one(); } - type DummyValidatorId = u64; + type DummyAuthorityId = u64; impl system::Trait for Test { type Origin = Origin; @@ -184,7 +195,7 @@ mod tests { type Call = (); type Hash = H256; type Hashing = sr_primitives::traits::BlakeTwo256; - type AccountId = DummyValidatorId; + type AccountId = DummyAuthorityId; type Lookup = IdentityLookup; type Header = Header; type Event = (); @@ -195,9 +206,7 @@ mod tests { type Version = (); } - impl Trait for Test { - type ValidatorId = DummyValidatorId; - } + impl Trait for Test {} fn new_test_ext() -> runtime_io::TestExternalities { let mut t = system::GenesisConfig::default().build_storage::().unwrap(); @@ -214,13 +223,63 @@ mod tests { }); } + fn get_dummy_authorities() -> Vec<(AuthorityId, AuthorityWeight)> { + let authority1 = (AuthorityId::from_slice(&[1; 32]), 1); + let authority2 = (AuthorityId::from_slice(&[2; 32]), 1); + let authority3 = (AuthorityId::from_slice(&[3; 32]), 1); + + vec![authority1, authority2, authority3] + } + + fn create_dummy_validator_proof(validator_set: Vec<(AuthorityId, AuthorityWeight)>) -> (H256, Vec>) { + use state_machine::{prove_read, backend::{Backend, InMemory}}; + + let encoded_set = validator_set.encode(); + + // construct storage proof + let backend = >::from(vec![ + (None, b":grandpa_authorities".to_vec(), Some(encoded_set)), + ]); + let root = backend.storage_root(std::iter::empty()).0; + + // Generates a storage read proof + let proof = prove_read(backend, &[&b":grandpa_authorities"[..]]).unwrap(); + + (root, proof) + } + + #[test] + fn it_can_validate_validator_sets() { + let authorities = get_dummy_authorities(); + let (root, proof) = create_dummy_validator_proof(authorities.clone()); + + assert_ok!(MockBridge::check_validator_set_proof(&root, proof, &authorities)); + } + + #[test] + fn it_rejects_invalid_validator_sets() { + let mut authorities = get_dummy_authorities(); + let (root, proof) = create_dummy_validator_proof(authorities.clone()); + + // Do something to make the authority set invalid + authorities.reverse(); + let invalid_authorities = authorities; + + assert_err!( + MockBridge::check_validator_set_proof(&root, proof, &invalid_authorities), + Error::ValidatorSetMismatch + ); + } + #[test] fn it_creates_a_new_bridge() { - let dummy_state_root = H256::default(); + let authorities = get_dummy_authorities(); + let (root, proof) = create_dummy_validator_proof(authorities.clone()); + let test_header = Header { parent_hash: H256::default(), number: 42, - state_root: dummy_state_root, + state_root: root, extrinsics_root: H256::default(), digest: Digest::default(), }; @@ -228,16 +287,13 @@ mod tests { new_test_ext().execute_with(|| { assert_eq!(MockBridge::num_bridges(), 0); - assert_eq!(MockBridge::tracked_bridges(0), None); - dbg!(&test_header); assert_ok!( MockBridge::initialize_bridge( Origin::signed(1), test_header, - vec![1, 2, 3], - vec![], - vec![], + authorities.clone(), + proof, )); assert_eq!( @@ -245,8 +301,8 @@ mod tests { Some(BridgeInfo { last_finalized_block_number: 42, last_finalized_block_hash: test_hash, - last_finalized_state_root: dummy_state_root, - current_validator_set: vec![1, 2, 3], + last_finalized_state_root: root, + current_validator_set: authorities.clone(), })); assert_eq!(MockBridge::num_bridges(), 1); diff --git a/srml/bridge/src/storage_proof.rs b/srml/bridge/src/storage_proof.rs index 8217bb3b32e2f..b88192d56f2c4 100644 --- a/srml/bridge/src/storage_proof.rs +++ b/srml/bridge/src/storage_proof.rs @@ -19,7 +19,7 @@ use hash_db::{Hasher, HashDB, EMPTY_PREFIX}; use trie::{MemoryDB, Trie, trie_types::TrieDB}; -use crate::error::Error; +use crate::Error; /// This struct is used to read storage values from a subset of a Merklized database. The "proof" /// is a subset of the nodes in the Merkle structure of the database, so that it provides