From f2b7aaa1299cb123d980a780e48ee75c24441bd8 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Wed, 23 Oct 2019 16:32:30 +0200 Subject: [PATCH 1/6] Make StorageProofChecker happy --- Cargo.lock | 1 - srml/bridge/Cargo.toml | 2 +- srml/bridge/src/lib.rs | 57 +++++++++++++++++++++++++++++++----------- 3 files changed, 44 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e8b18d94a793d..054e70f7f2581 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4085,7 +4085,6 @@ dependencies = [ "srml-session 2.0.0", "srml-support 2.0.0", "srml-system 2.0.0", - "substrate-externalities 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..5085acc60ced6 100644 --- a/srml/bridge/Cargo.toml +++ b/srml/bridge/Cargo.toml @@ -9,6 +9,7 @@ edition = "2018" [dependencies] codec = { package = "parity-scale-codec", version = "1.0.0", default-features = false } 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 +18,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/lib.rs b/srml/bridge/src/lib.rs index f437c0445dd5b..5b7dfced16c5b 100644 --- a/srml/bridge/src/lib.rs +++ b/srml/bridge/src/lib.rs @@ -35,7 +35,9 @@ mod error; mod storage_proof; +use crate::storage_proof::StorageProofChecker; use codec::{Encode, Decode}; +use primitives::{Blake2Hasher}; use sr_primitives::traits::{Header, Member}; use support::{ decl_error, decl_module, decl_storage, @@ -90,32 +92,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_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); } @@ -139,8 +136,17 @@ impl Module { Ok(()) // Otherwise, Error::InvalidStorageProof } - fn check_validator_set_proof(_proof: Vec) -> std::result::Result<(), Error> { - // ... Do some math ... + fn check_validator_set_proof( + state_root: &T::Hash, + proof: Vec>, + _validator_set: &Vec, + ) -> std::result::Result<(), Error> { + let _checker = ::Hasher>>::new( + *state_root, + proof.clone() + ) + .unwrap(); + Ok(()) // Otherwise, Error::InvalidValidatorSetProof } } @@ -214,6 +220,31 @@ mod tests { }); } + #[test] + fn it_can_validate_validator_sets() { + use state_machine::{prove_read, backend::{Backend, InMemory}}; + + let validators = vec![0, 5, 10]; + + // construct storage proof + let backend = >::from(vec![ + (None, b"validator1".to_vec(), Some(b"alice".to_vec())), + (None, b"validator2".to_vec(), Some(b"bob".to_vec())) + ]); + let root = backend.storage_root(std::iter::empty()).0; + + // Generates a storage read proof + let proof = prove_read(backend, &[&b"validator1"[..], &b"validator2"[..]]).unwrap(); + + // check proof in runtime + let checker = >::new(root, proof.clone()).unwrap(); + assert_eq!(checker.read_value(b"validator1"), Ok(Some(b"alice".to_vec()))); + assert_eq!(checker.read_value(b"validator2"), Ok(Some(b"bob".to_vec()))); + + // with_externalities(&mut new_test_ext(), || { + // }); + } + #[test] fn it_creates_a_new_bridge() { let dummy_state_root = H256::default(); @@ -228,8 +259,6 @@ 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( From 8f1846a3636e27cafc9e8814bacef18a4a18fc97 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Wed, 23 Oct 2019 17:34:48 +0200 Subject: [PATCH 2/6] Update some tests --- srml/bridge/src/lib.rs | 43 ++++++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/srml/bridge/src/lib.rs b/srml/bridge/src/lib.rs index 5b7dfced16c5b..68a3b9d953e33 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,6 +29,7 @@ //! (__)\ )\/\ //! ||----w | //! || || +//!``` // Ensure we're `no_std` when compiling for Wasm. #![cfg_attr(not(feature = "std"), no_std)] @@ -37,7 +39,6 @@ mod storage_proof; use crate::storage_proof::StorageProofChecker; use codec::{Encode, Decode}; -use primitives::{Blake2Hasher}; use sr_primitives::traits::{Header, Member}; use support::{ decl_error, decl_module, decl_storage, @@ -131,16 +132,12 @@ decl_error! { } impl Module { - fn check_storage_proof(_proof: Vec) -> std::result::Result<(), Error> { - // ... Do some math ... - Ok(()) // Otherwise, Error::InvalidStorageProof - } - fn check_validator_set_proof( state_root: &T::Hash, proof: Vec>, _validator_set: &Vec, ) -> std::result::Result<(), Error> { + let _checker = ::Hasher>>::new( *state_root, proof.clone() @@ -155,7 +152,7 @@ impl Module { mod tests { use super::*; - use primitives::H256; + use primitives::{Blake2Hasher, H256}; use sr_primitives::{ Perbill, traits::{Header as HeaderT, IdentityLookup}, testing::Header, generic::Digest, }; @@ -169,7 +166,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 @@ -220,12 +217,9 @@ mod tests { }); } - #[test] - fn it_can_validate_validator_sets() { + fn create_dummy_validator_proof() -> (H256, Vec>) { use state_machine::{prove_read, backend::{Backend, InMemory}}; - let validators = vec![0, 5, 10]; - // construct storage proof let backend = >::from(vec![ (None, b"validator1".to_vec(), Some(b"alice".to_vec())), @@ -236,22 +230,26 @@ mod tests { // Generates a storage read proof let proof = prove_read(backend, &[&b"validator1"[..], &b"validator2"[..]]).unwrap(); - // check proof in runtime - let checker = >::new(root, proof.clone()).unwrap(); - assert_eq!(checker.read_value(b"validator1"), Ok(Some(b"alice".to_vec()))); - assert_eq!(checker.read_value(b"validator2"), Ok(Some(b"bob".to_vec()))); + (root, proof) + } + + #[test] + fn it_can_validate_validator_sets() { - // with_externalities(&mut new_test_ext(), || { - // }); + let validators = vec![0, 5, 10]; + let (root, proof) = create_dummy_validator_proof(); + + assert_ok!(MockBridge::check_validator_set_proof(&root, proof, &validators)); } #[test] fn it_creates_a_new_bridge() { - let dummy_state_root = H256::default(); + let (root, proof) = create_dummy_validator_proof(); + 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(), }; @@ -265,8 +263,7 @@ mod tests { Origin::signed(1), test_header, vec![1, 2, 3], - vec![], - vec![], + proof, )); assert_eq!( @@ -274,7 +271,7 @@ mod tests { Some(BridgeInfo { last_finalized_block_number: 42, last_finalized_block_hash: test_hash, - last_finalized_state_root: dummy_state_root, + last_finalized_state_root: root, current_validator_set: vec![1, 2, 3], })); From c621fa544311b5faffcdbcbfecebf28b8f60b2de Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 25 Oct 2019 12:19:50 +0200 Subject: [PATCH 3/6] Check given validator set against set found in storage --- srml/bridge/src/lib.rs | 63 +++++++++++++++++++++++++++++------------- 1 file changed, 44 insertions(+), 19 deletions(-) diff --git a/srml/bridge/src/lib.rs b/srml/bridge/src/lib.rs index 68a3b9d953e33..d1ee2edde4941 100644 --- a/srml/bridge/src/lib.rs +++ b/srml/bridge/src/lib.rs @@ -52,7 +52,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<(T::ValidatorId, ValidatorWeight)>, } impl BridgeInfo { @@ -60,7 +60,7 @@ impl BridgeInfo { block_number: &T::BlockNumber, block_hash: &T::Hash, state_root: &T::Hash, - validator_set: Vec, + validator_set: Vec<(T::ValidatorId, ValidatorWeight)>, ) -> Self { // I don't like how this is done, should come back to... @@ -74,6 +74,7 @@ impl BridgeInfo { } type BridgeId = u64; +type ValidatorWeight = u64; pub trait Trait: system::Trait { /// A stable ID for a validator. @@ -97,7 +98,7 @@ decl_module! { fn initialize_bridge( origin, block_header: T::Header, - validator_set: Vec, + validator_set: Vec<(T::ValidatorId, ValidatorWeight)>, validator_set_proof: Vec>, ) { // NOTE: Will want to make this a governance issued call @@ -128,6 +129,7 @@ decl_error! { pub enum Error { InvalidStorageProof, InvalidValidatorSetProof, + ValidatorSetMismatch, } } @@ -135,16 +137,26 @@ impl Module { fn check_validator_set_proof( state_root: &T::Hash, proof: Vec>, - _validator_set: &Vec, + validator_set: &Vec<(T::ValidatorId, ValidatorWeight)>, ) -> std::result::Result<(), Error> { - let _checker = ::Hasher>>::new( + // pub const GRANDPA_AUTHORITIES_KEY: &'static [u8] = b":grandpa_authorities"; + // pub type AuthorityList = Vec<(AuthorityId, AuthorityWeight)>; + let checker = ::Hasher>>::new( *state_root, proof.clone() - ) - .unwrap(); + ).unwrap(); - Ok(()) // Otherwise, Error::InvalidValidatorSetProof + // 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").unwrap().unwrap(); + + if encoded_validator_set == actual_validator_set { + Ok(()) + } else { + Err(Error::ValidatorSetMismatch) + } } } @@ -156,7 +168,7 @@ mod tests { 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! { @@ -217,34 +229,47 @@ mod tests { }); } - fn create_dummy_validator_proof() -> (H256, Vec>) { + fn create_dummy_validator_proof(validator_set: Vec<(DummyValidatorId, ValidatorWeight)>) -> (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"validator1".to_vec(), Some(b"alice".to_vec())), - (None, b"validator2".to_vec(), Some(b"bob".to_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"validator1"[..], &b"validator2"[..]]).unwrap(); + let proof = prove_read(backend, &[&b":grandpa_authorities"[..]]).unwrap(); (root, proof) } #[test] fn it_can_validate_validator_sets() { - - let validators = vec![0, 5, 10]; - let (root, proof) = create_dummy_validator_proof(); + let validators = vec![(1, 1), (2, 1), (3, 1)]; + let (root, proof) = create_dummy_validator_proof(validators.clone()); assert_ok!(MockBridge::check_validator_set_proof(&root, proof, &validators)); } + #[test] + fn it_rejects_invalid_validator_sets() { + let validators = vec![(1, 1), (2, 1), (3, 1)]; + let (root, proof) = create_dummy_validator_proof(validators.clone()); + + let invalid_validators = vec![(3, 1), (2, 1), (1, 1)]; + assert_err!( + MockBridge::check_validator_set_proof(&root, proof, &invalid_validators), + Error::ValidatorSetMismatch + ); + } + #[test] fn it_creates_a_new_bridge() { - let (root, proof) = create_dummy_validator_proof(); + let validators = vec![(1, 1), (2, 1), (3, 1)]; + let (root, proof) = create_dummy_validator_proof(validators.clone()); let test_header = Header { parent_hash: H256::default(), @@ -262,7 +287,7 @@ mod tests { MockBridge::initialize_bridge( Origin::signed(1), test_header, - vec![1, 2, 3], + validators.clone(), proof, )); @@ -272,7 +297,7 @@ mod tests { last_finalized_block_number: 42, last_finalized_block_hash: test_hash, last_finalized_state_root: root, - current_validator_set: vec![1, 2, 3], + current_validator_set: validators.clone(), })); assert_eq!(MockBridge::num_bridges(), 1); From 4b707e602bbea717d0df7930559b6031f572864b Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 25 Oct 2019 18:14:16 +0200 Subject: [PATCH 4/6] Use Finality Grandpa's Authority Id and Weight --- Cargo.lock | 1 + srml/bridge/Cargo.toml | 1 + srml/bridge/src/lib.rs | 66 ++++++++++++++++++++++-------------------- 3 files changed, 36 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 054e70f7f2581..c6a45f97c790e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4085,6 +4085,7 @@ dependencies = [ "srml-session 2.0.0", "srml-support 2.0.0", "srml-system 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 5085acc60ced6..6e63d0d6f443c 100644 --- a/srml/bridge/Cargo.toml +++ b/srml/bridge/Cargo.toml @@ -8,6 +8,7 @@ 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"] } diff --git a/srml/bridge/src/lib.rs b/srml/bridge/src/lib.rs index d1ee2edde4941..526a9511a9838 100644 --- a/srml/bridge/src/lib.rs +++ b/srml/bridge/src/lib.rs @@ -39,10 +39,10 @@ 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}; @@ -52,7 +52,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<(T::ValidatorId, ValidatorWeight)>, + current_validator_set: Vec<(AuthorityId, AuthorityWeight)>, } impl BridgeInfo { @@ -60,7 +60,7 @@ impl BridgeInfo { block_number: &T::BlockNumber, block_hash: &T::Hash, state_root: &T::Hash, - validator_set: Vec<(T::ValidatorId, ValidatorWeight)>, + validator_set: Vec<(AuthorityId, AuthorityWeight)>, ) -> Self { // I don't like how this is done, should come back to... @@ -74,12 +74,8 @@ impl BridgeInfo { } type BridgeId = u64; -type ValidatorWeight = 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 { @@ -98,7 +94,7 @@ decl_module! { fn initialize_bridge( origin, block_header: T::Header, - validator_set: Vec<(T::ValidatorId, ValidatorWeight)>, + validator_set: Vec<(AuthorityId, AuthorityWeight)>, validator_set_proof: Vec>, ) { // NOTE: Will want to make this a governance issued call @@ -137,11 +133,9 @@ impl Module { fn check_validator_set_proof( state_root: &T::Hash, proof: Vec>, - validator_set: &Vec<(T::ValidatorId, ValidatorWeight)>, + validator_set: &Vec<(AuthorityId, AuthorityWeight)>, ) -> std::result::Result<(), Error> { - // pub const GRANDPA_AUTHORITIES_KEY: &'static [u8] = b":grandpa_authorities"; - // pub type AuthorityList = Vec<(AuthorityId, AuthorityWeight)>; let checker = ::Hasher>>::new( *state_root, proof.clone() @@ -164,13 +158,12 @@ impl Module { mod tests { use super::*; - use primitives::{Blake2Hasher, H256}; + use primitives::{Blake2Hasher, H256, Public}; use sr_primitives::{ Perbill, traits::{Header as HeaderT, IdentityLookup}, testing::Header, generic::Digest, }; use support::{assert_ok, assert_err, impl_outer_origin, parameter_types}; - // NOTE: What's this for? impl_outer_origin! { pub enum Origin for Test {} } @@ -190,7 +183,7 @@ mod tests { pub const AvailableBlockRatio: Perbill = Perbill::one(); } - type DummyValidatorId = u64; + type DummyAuthorityId = u64; impl system::Trait for Test { type Origin = Origin; @@ -199,7 +192,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 = (); @@ -210,9 +203,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(); @@ -229,7 +220,15 @@ mod tests { }); } - fn create_dummy_validator_proof(validator_set: Vec<(DummyValidatorId, ValidatorWeight)>) -> (H256, Vec>) { + 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(); @@ -248,28 +247,31 @@ mod tests { #[test] fn it_can_validate_validator_sets() { - let validators = vec![(1, 1), (2, 1), (3, 1)]; - let (root, proof) = create_dummy_validator_proof(validators.clone()); + let authorities = get_dummy_authorities(); + let (root, proof) = create_dummy_validator_proof(authorities.clone()); - assert_ok!(MockBridge::check_validator_set_proof(&root, proof, &validators)); + assert_ok!(MockBridge::check_validator_set_proof(&root, proof, &authorities)); } #[test] fn it_rejects_invalid_validator_sets() { - let validators = vec![(1, 1), (2, 1), (3, 1)]; - let (root, proof) = create_dummy_validator_proof(validators.clone()); + 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; - let invalid_validators = vec![(3, 1), (2, 1), (1, 1)]; assert_err!( - MockBridge::check_validator_set_proof(&root, proof, &invalid_validators), + MockBridge::check_validator_set_proof(&root, proof, &invalid_authorities), Error::ValidatorSetMismatch ); } #[test] fn it_creates_a_new_bridge() { - let validators = vec![(1, 1), (2, 1), (3, 1)]; - let (root, proof) = create_dummy_validator_proof(validators.clone()); + let authorities = get_dummy_authorities(); + let (root, proof) = create_dummy_validator_proof(authorities.clone()); let test_header = Header { parent_hash: H256::default(), @@ -287,7 +289,7 @@ mod tests { MockBridge::initialize_bridge( Origin::signed(1), test_header, - validators.clone(), + authorities.clone(), proof, )); @@ -297,7 +299,7 @@ mod tests { last_finalized_block_number: 42, last_finalized_block_hash: test_hash, last_finalized_state_root: root, - current_validator_set: validators.clone(), + current_validator_set: authorities.clone(), })); assert_eq!(MockBridge::num_bridges(), 1); From 1c3923dd089237f81f7f9ab30c7930e1bb0fa854 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Wed, 30 Oct 2019 14:45:18 +0100 Subject: [PATCH 5/6] Add better error handling --- srml/bridge/src/error.rs | 11 +++++++++++ srml/bridge/src/lib.rs | 7 +++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/srml/bridge/src/error.rs b/srml/bridge/src/error.rs index 182b0883d1e62..a5e28fdba8e41 100644 --- a/srml/bridge/src/error.rs +++ b/srml/bridge/src/error.rs @@ -16,6 +16,8 @@ //! Common error types for the srml-bridge crate. +use crate::Error as BridgeError; + #[derive(PartialEq)] #[cfg_attr(feature = "std", derive(Debug))] pub enum Error { @@ -23,3 +25,12 @@ pub enum Error { StorageValueUnavailable, } +impl From for BridgeError { + fn from(e: Error) -> BridgeError { + match e { + Error::StorageRootMismatch => BridgeError::InvalidStorageProof, + Error::StorageValueUnavailable => BridgeError::StorageValueUnavailable, + } + } +} + diff --git a/srml/bridge/src/lib.rs b/srml/bridge/src/lib.rs index 526a9511a9838..77801b688e688 100644 --- a/srml/bridge/src/lib.rs +++ b/srml/bridge/src/lib.rs @@ -124,6 +124,7 @@ decl_error! { // Error for the Bridge module pub enum Error { InvalidStorageProof, + StorageValueUnavailable, InvalidValidatorSetProof, ValidatorSetMismatch, } @@ -139,12 +140,14 @@ impl Module { let checker = ::Hasher>>::new( *state_root, proof.clone() - ).unwrap(); + )?; // 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").unwrap().unwrap(); + let actual_validator_set = checker + .read_value(b":grandpa_authorities")? + .ok_or(Error::StorageValueUnavailable)?; if encoded_validator_set == actual_validator_set { Ok(()) From 7c200ccc668501a4c89cb8756126bc38c20907d3 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Wed, 30 Oct 2019 18:00:21 +0100 Subject: [PATCH 6/6] Use error type from decl_error! macro --- srml/bridge/src/error.rs | 36 -------------------------------- srml/bridge/src/lib.rs | 2 +- srml/bridge/src/storage_proof.rs | 2 +- 3 files changed, 2 insertions(+), 38 deletions(-) delete mode 100644 srml/bridge/src/error.rs diff --git a/srml/bridge/src/error.rs b/srml/bridge/src/error.rs deleted file mode 100644 index a5e28fdba8e41..0000000000000 --- a/srml/bridge/src/error.rs +++ /dev/null @@ -1,36 +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. - -use crate::Error as BridgeError; - -#[derive(PartialEq)] -#[cfg_attr(feature = "std", derive(Debug))] -pub enum Error { - StorageRootMismatch, - StorageValueUnavailable, -} - -impl From for BridgeError { - fn from(e: Error) -> BridgeError { - match e { - Error::StorageRootMismatch => BridgeError::InvalidStorageProof, - Error::StorageValueUnavailable => BridgeError::StorageValueUnavailable, - } - } -} - diff --git a/srml/bridge/src/lib.rs b/srml/bridge/src/lib.rs index 77801b688e688..b217b15ffe8ae 100644 --- a/srml/bridge/src/lib.rs +++ b/srml/bridge/src/lib.rs @@ -34,7 +34,6 @@ // 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; @@ -124,6 +123,7 @@ decl_error! { // Error for the Bridge module pub enum Error { InvalidStorageProof, + StorageRootMismatch, StorageValueUnavailable, InvalidValidatorSetProof, ValidatorSetMismatch, 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