diff --git a/substrate/core/client/src/call_executor.rs b/substrate/core/client/src/call_executor.rs index f956f27b50..c107e6f2bb 100644 --- a/substrate/core/client/src/call_executor.rs +++ b/substrate/core/client/src/call_executor.rs @@ -110,7 +110,7 @@ where manager: ExecutionManager, native_call: Option, side_effects_handler: Option<&mut O>, - ) -> Result<(NativeOrEncoded, S::Transaction, Option>), error::Error>; + ) -> Result<(NativeOrEncoded, (S::Transaction, H::Out), Option>), error::Error>; /// Execute a call to a contract on top of given state, gathering execution proof. /// @@ -314,7 +314,11 @@ where manager: ExecutionManager, native_call: Option, side_effects_handler: Option<&mut O>, - ) -> error::Result<(NativeOrEncoded, S::Transaction, Option>)> { + ) -> error::Result<( + NativeOrEncoded, + (S::Transaction, ::Out), + Option>, + )> { state_machine::new( state, self.backend.changes_trie_storage(), diff --git a/substrate/core/client/src/client.rs b/substrate/core/client/src/client.rs index 3f3f1563b8..0e87976b8e 100644 --- a/substrate/core/client/src/client.rs +++ b/substrate/core/client/src/client.rs @@ -1027,7 +1027,11 @@ impl Client where let (top, children) = overlay.into_committed(); let children = children.map(|(sk, it)| (sk, it.collect())).collect(); - Ok((Some(storage_update), Some(changes_update), Some((top.collect(), children)))) + if import_headers.post().state_root() != &storage_update.1 { + return Err(error::Error::InvalidStateRoot); + } + + Ok((Some(storage_update.0), Some(changes_update), Some((top.collect(), children)))) }, None => Ok((None, None, None)) } diff --git a/substrate/core/client/src/error.rs b/substrate/core/client/src/error.rs index b807d5e11c..2de5a42781 100644 --- a/substrate/core/client/src/error.rs +++ b/substrate/core/client/src/error.rs @@ -94,6 +94,9 @@ pub enum Error { /// Hash that is required for building CHT is missing. #[display(fmt = "Failed to get hash of block for building CHT")] MissingHashRequiredForCHT, + /// Invalid calculated state root on block import. + #[display(fmt = "Calculated state root does not match.")] + InvalidStateRoot, /// A convenience variant for String #[display(fmt = "{}", _0)] Msg(String), diff --git a/substrate/core/client/src/light/call_executor.rs b/substrate/core/client/src/light/call_executor.rs index 4dba803921..3d77492860 100644 --- a/substrate/core/client/src/light/call_executor.rs +++ b/substrate/core/client/src/light/call_executor.rs @@ -170,7 +170,11 @@ where _m: ExecutionManager, _native_call: Option, _side_effects_handler: Option<&mut O>, - ) -> ClientResult<(NativeOrEncoded, S::Transaction, Option>)> { + ) -> ClientResult<( + NativeOrEncoded, + (S::Transaction, ::Out), + Option>, + )> { Err(ClientError::NotAvailableOnLightClient.into()) } @@ -343,7 +347,11 @@ impl CallExecutor for _manager: ExecutionManager, native_call: Option, side_effects_handler: Option<&mut O>, - ) -> ClientResult<(NativeOrEncoded, S::Transaction, Option>)> { + ) -> ClientResult<( + NativeOrEncoded, + (S::Transaction, ::Out), + Option>, + )> { // there's no actual way/need to specify native/wasm execution strategy on light node // => we can safely ignore passed values diff --git a/substrate/core/state-machine/src/ext.rs b/substrate/core/state-machine/src/ext.rs index 5a0daeb348..dcc63cb196 100644 --- a/substrate/core/state-machine/src/ext.rs +++ b/substrate/core/state-machine/src/ext.rs @@ -115,7 +115,7 @@ where } /// Get the transaction necessary to update the backend. - pub fn transaction(mut self) -> (B::Transaction, Option>) { + pub fn transaction(mut self) -> ((B::Transaction, H::Out), Option>) { let _ = self.storage_root(); let (storage_transaction, changes_trie_transaction) = ( @@ -126,7 +126,7 @@ where ); ( - storage_transaction.0, + storage_transaction, changes_trie_transaction, ) } diff --git a/substrate/core/state-machine/src/lib.rs b/substrate/core/state-machine/src/lib.rs index 2c98cc8a30..954ff38979 100644 --- a/substrate/core/state-machine/src/lib.rs +++ b/substrate/core/state-machine/src/lib.rs @@ -502,7 +502,7 @@ impl<'a, H, N, B, T, O, Exec> StateMachine<'a, H, N, B, T, O, Exec> where pub fn execute( &mut self, strategy: ExecutionStrategy, - ) -> Result<(Vec, B::Transaction, Option>), Box> { + ) -> Result<(Vec, (B::Transaction, H::Out), Option>), Box> { // We are not giving a native call and thus we are sure that the result can never be a native // value. self.execute_using_consensus_failure_handler::<_, NeverNativeValue, fn() -> _>( @@ -522,7 +522,12 @@ impl<'a, H, N, B, T, O, Exec> StateMachine<'a, H, N, B, T, O, Exec> where compute_tx: bool, use_native: bool, native_call: Option, - ) -> (CallResult, bool, Option, Option>) where + ) -> ( + CallResult, + bool, + Option<(B::Transaction, H::Out)>, + Option>, + ) where R: Decode + Encode + PartialEq, NC: FnOnce() -> result::Result + UnwindSafe, { @@ -554,7 +559,7 @@ impl<'a, H, N, B, T, O, Exec> StateMachine<'a, H, N, B, T, O, Exec> where mut native_call: Option, orig_prospective: OverlayedChangeSet, on_consensus_failure: Handler, - ) -> (CallResult, Option, Option>) where + ) -> (CallResult, Option<(B::Transaction, H::Out)>, Option>) where R: Decode + Encode + PartialEq, NC: FnOnce() -> result::Result + UnwindSafe, Handler: FnOnce( @@ -585,7 +590,7 @@ impl<'a, H, N, B, T, O, Exec> StateMachine<'a, H, N, B, T, O, Exec> where compute_tx: bool, mut native_call: Option, orig_prospective: OverlayedChangeSet, - ) -> (CallResult, Option, Option>) where + ) -> (CallResult, Option<(B::Transaction, H::Out)>, Option>) where R: Decode + Encode + PartialEq, NC: FnOnce() -> result::Result + UnwindSafe, { @@ -613,7 +618,11 @@ impl<'a, H, N, B, T, O, Exec> StateMachine<'a, H, N, B, T, O, Exec> where manager: ExecutionManager, compute_tx: bool, mut native_call: Option, - ) -> Result<(NativeOrEncoded, Option, Option>), Box> where + ) -> Result<( + NativeOrEncoded, + Option<(B::Transaction, H::Out)>, + Option> + ), Box> where R: Decode + Encode + PartialEq, NC: FnOnce() -> result::Result + UnwindSafe, Handler: FnOnce( diff --git a/substrate/core/test-runtime/src/system.rs b/substrate/core/test-runtime/src/system.rs index 267d322e87..536db066ae 100644 --- a/substrate/core/test-runtime/src/system.rs +++ b/substrate/core/test-runtime/src/system.rs @@ -110,6 +110,8 @@ fn execute_block_with_state_root_handler( storage::unhashed::kill(well_known_keys::EXTRINSIC_INDEX); }); + let o_new_authorities = ::take(); + if let Mode::Overwrite = mode { header.state_root = storage_root().into(); } else { @@ -124,7 +126,7 @@ fn execute_block_with_state_root_handler( if let Some(storage_changes_root) = storage_changes_root(header.parent_hash.into()) { digest.push(generic::DigestItem::ChangesTrieRoot(storage_changes_root.into())); } - if let Some(new_authorities) = ::take() { + if let Some(new_authorities) = o_new_authorities { digest.push(generic::DigestItem::Consensus(*b"aura", new_authorities.encode())); digest.push(generic::DigestItem::Consensus(*b"babe", new_authorities.encode())); } @@ -203,6 +205,7 @@ pub fn finalize_block() -> Header { let parent_hash = ::take(); let mut digest = ::take().expect("StorageDigest is set by `initialize_block`"); + let o_new_authorities = ::take(); // This MUST come after all changes to storage are done. Otherwise we will fail the // “Storage root does not match that calculated” assertion. let storage_root = BlakeTwo256::storage_root(); @@ -211,7 +214,8 @@ pub fn finalize_block() -> Header { if let Some(storage_changes_root) = storage_changes_root { digest.push(generic::DigestItem::ChangesTrieRoot(storage_changes_root)); } - if let Some(new_authorities) = ::take() { + + if let Some(new_authorities) = o_new_authorities { digest.push(generic::DigestItem::Consensus(*b"aura", new_authorities.encode())); digest.push(generic::DigestItem::Consensus(*b"babe", new_authorities.encode())); }