Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Extrinsics root is calculated as part of block-building #120

Merged
merged 11 commits into from
Apr 12, 2018
9 changes: 6 additions & 3 deletions Cargo.lock

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

Binary file not shown.
Binary file not shown.
1 change: 1 addition & 0 deletions polkadot/api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ authors = ["Parity Technologies <[email protected]>"]

[dependencies]
error-chain = "0.11"
log = "0.3"
polkadot-executor = { path = "../executor" }
polkadot-runtime = { path = "../runtime" }
polkadot-primitives = { path = "../primitives" }
Expand Down
11 changes: 5 additions & 6 deletions polkadot/api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ extern crate substrate_state_machine as state_machine;
#[macro_use]
extern crate error_chain;

#[macro_use]
extern crate log;

#[cfg(test)]
extern crate substrate_keyring as keyring;

Expand Down Expand Up @@ -174,7 +177,7 @@ impl<B: Backend> PolkadotApi for Client<B, NativeExecutor<LocalDispatch>>
fn check_id(&self, id: BlockId) -> Result<CheckedId> {
// bail if the code is not the same as the natively linked.
if self.code_at(&id)? != LocalDispatch::native_equivalent() {
bail!(ErrorKind::UnknownRuntime);
warn!("This node is out of date. Block authoring may not work correctly.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dangerous proposition. I think authorities would prefer to get slashed for being temporarily offline than risk accidentally losing their whole bond.

also means that when we update the runtime, if >2/3 of the authorities have this behavior, they will continue growing the chain with the wrong rules.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I get why this change is in this PR -- the genesis.wasm is a nuisance to work around, but we should just stop checking this stuff in and have an environment variable for when we want to use a specific runtime wasm!)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be honest we're going to have to introduce semantic versioning here anyway. it's impractical to ensure that a node is updated, restarted and where necessary migrated at exactly the block boundary when the runtime changes, so there will need to be changeover periods. bailing here is unreasonable.

Copy link
Contributor

@rphmeier rphmeier Apr 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, but there are different approaches to that, and given that the API can change between runtimes, it's likely we might do something based on linking multiple versions of the polkadot-api crate. Network, consensus, and other client-level code might change between runtimes as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'd rather just have semantic versioning of the API and be done with it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we agree to classify that as beyond the scope of this PR and introduce that later?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure

}

Ok(CheckedId(id))
Expand Down Expand Up @@ -324,19 +327,15 @@ impl<S: state_machine::Backend> BlockBuilder for ClientBlockBuilder<S>
}

fn bake(mut self) -> Block {
use substrate_runtime_executive::extrinsics_root;

let mut ext = state_machine::Ext {
overlay: &mut self.changes,
backend: &self.state,
};

let mut final_header = ::substrate_executor::with_native_environment(
let final_header = ::substrate_executor::with_native_environment(
&mut ext,
move || runtime::Executive::finalise_block()
).expect("all inherent extrinsics pushed; all other extrinsics executed correctly; qed");

final_header.extrinsics_root = extrinsics_root::<runtime_io::BlakeTwo256, _>(&self.extrinsics);
Block {
header: final_header,
extrinsics: self.extrinsics,
Expand Down
2 changes: 1 addition & 1 deletion polkadot/consensus/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ parking_lot = "0.4"
tokio-core = "0.1.12"
ed25519 = { path = "../../substrate/ed25519" }
error-chain = "0.11"
log = "0.4"
log = "0.3"
polkadot-api = { path = "../api" }
polkadot-collator = { path = "../collator" }
polkadot-primitives = { path = "../primitives" }
Expand Down
Binary file modified polkadot/runtime/wasm/genesis.wasm
Binary file not shown.
Binary file not shown.
Binary file not shown.
2 changes: 1 addition & 1 deletion polkadot/service/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ futures = "0.1.17"
parking_lot = "0.4"
tokio-timer = "0.1.2"
error-chain = "0.11"
log = "0.4"
log = "0.3"
tokio-core = "0.1.12"
ed25519 = { path = "../../substrate/ed25519" }
polkadot-primitives = { path = "../primitives" }
Expand Down
1 change: 1 addition & 0 deletions polkadot/transaction-pool/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ version = "0.1.0"
authors = ["Parity Technologies <[email protected]>"]

[dependencies]
log = "0.4.0"
transaction-pool = "1.9.0"
error-chain = "0.11"
polkadot-api = { path = "../api" }
Expand Down
24 changes: 15 additions & 9 deletions polkadot/transaction-pool/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ extern crate transaction_pool;
#[macro_use]
extern crate error_chain;

#[macro_use]
extern crate log;

use std::collections::HashMap;
use std::cmp::Ordering;
use std::sync::Arc;
Expand Down Expand Up @@ -296,12 +299,10 @@ impl<'a, T: 'a + PolkadotApi> transaction_pool::Ready<VerifiedTransaction> for R
let next_index = self.known_indices.entry(sender)
.or_insert_with(|| api_handle.index(at_block, sender).ok().unwrap_or_else(u64::max_value));

*next_index += 1;

match xt.inner.index.cmp(&next_index) {
Ordering::Greater => Readiness::Future,
Ordering::Equal => Readiness::Ready,
Ordering::Less => Readiness::Stalled,
Ordering::Less => Readiness::Stalled, // TODO: this is not "stalled" but rather stale and can be discarded.
}
}
}
Expand Down Expand Up @@ -330,12 +331,14 @@ impl TransactionPool {

let verified = VerifiedTransaction::create(xt, insertion_index)?;

info!("Extrinsic verified {:?}. Importing...", verified);

// TODO: just use a foreign link when the error type is made public.
let hash = verified.hash.clone();
self.inner.import(verified)
.map_err(|e|
match e {
// TODO: make error types public in transaction_pool. For now just treat all errors as AlradyImported
// TODO: make error types public in transaction_pool. For now just treat all errors as AlreadyImported
_ => ErrorKind::AlreadyImported(hash),
// transaction_pool::error::AlreadyImported(h) => ErrorKind::AlreadyImported(h),
// e => ErrorKind::Import(Box::new(e)),
Expand Down Expand Up @@ -376,11 +379,14 @@ impl TransactionPool {

impl substrate_rpc::author::AsyncAuthorApi for TransactionPool {
fn submit_extrinsic(&mut self, xt: Extrinsic) -> substrate_rpc::author::error::Result<()> {
self.import(xt
.using_encoded(|ref mut s| UncheckedExtrinsic::decode(s))
.ok_or(substrate_rpc::author::error::ErrorKind::InvalidFormat)?)
.map(|_| ())
.map_err(|_| substrate_rpc::author::error::ErrorKind::PoolError.into())
use substrate_primitives::hexdisplay::HexDisplay;
info!("Extrinsic submitted: {}", HexDisplay::from(&xt.0));
let xt = xt.using_encoded(|ref mut s| UncheckedExtrinsic::decode(s))
.ok_or(substrate_rpc::author::error::ErrorKind::InvalidFormat)?;
info!("Correctly formatted: {:?}", xt);
self.import(xt)
.map(|_| ())
.map_err(|_| substrate_rpc::author::error::ErrorKind::PoolError.into())
}
}

Expand Down
2 changes: 1 addition & 1 deletion substrate/bft/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ ed25519 = { path = "../ed25519" }
tokio-timer = "0.1.2"
parking_lot = "0.4"
error-chain = "0.11"
log = "0.4"
log = "0.3"

[dev-dependencies]
substrate-keyring = { path = "../keyring" }
Expand Down
16 changes: 8 additions & 8 deletions substrate/executor/src/wasm_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,19 +179,19 @@ impl_function_executor!(this: FunctionExecutor<'e, E>,
let key = this.memory.get(key_data, key_len as usize).map_err(|_| DummyUserError)?;
let value = this.memory.get(value_data, value_len as usize).map_err(|_| DummyUserError)?;
if let Some(preimage) = this.hash_lookup.get(&key) {
info!(target: "wasm-trace", "*** Setting storage: %{} -> {} [k={}]", ascii_format(&preimage), HexDisplay::from(&value), HexDisplay::from(&key));
trace!(target: "wasm-trace", "*** Setting storage: %{} -> {} [k={}]", ascii_format(&preimage), HexDisplay::from(&value), HexDisplay::from(&key));
} else {
info!(target: "wasm-trace", "*** Setting storage: {} -> {} [k={}]", ascii_format(&key), HexDisplay::from(&value), HexDisplay::from(&key));
trace!(target: "wasm-trace", "*** Setting storage: {} -> {} [k={}]", ascii_format(&key), HexDisplay::from(&value), HexDisplay::from(&key));
}
this.ext.set_storage(key, value);
Ok(())
},
ext_clear_storage(key_data: *const u8, key_len: u32) => {
let key = this.memory.get(key_data, key_len as usize).map_err(|_| DummyUserError)?;
if let Some(preimage) = this.hash_lookup.get(&key) {
info!(target: "wasm-trace", "*** Clearing storage: %{} [k={}]", ascii_format(&preimage), HexDisplay::from(&key));
trace!(target: "wasm-trace", "*** Clearing storage: %{} [k={}]", ascii_format(&preimage), HexDisplay::from(&key));
} else {
info!(target: "wasm-trace", "*** Clearing storage: {} [k={}]", ascii_format(&key), HexDisplay::from(&key));
trace!(target: "wasm-trace", "*** Clearing storage: {} [k={}]", ascii_format(&key), HexDisplay::from(&key));
}
this.ext.clear_storage(&key);
Ok(())
Expand All @@ -202,9 +202,9 @@ impl_function_executor!(this: FunctionExecutor<'e, E>,
let maybe_value = this.ext.storage(&key);

if let Some(preimage) = this.hash_lookup.get(&key) {
info!(target: "wasm-trace", " Getting storage: %{} == {} [k={}]", ascii_format(&preimage), if let Some(ref b) = maybe_value { format!("{}", HexDisplay::from(b)) } else { "<empty>".to_owned() }, HexDisplay::from(&key));
trace!(target: "wasm-trace", " Getting storage: %{} == {} [k={}]", ascii_format(&preimage), if let Some(ref b) = maybe_value { format!("{}", HexDisplay::from(b)) } else { "<empty>".to_owned() }, HexDisplay::from(&key));
} else {
info!(target: "wasm-trace", " Getting storage: {} == {} [k={}]", ascii_format(&key), if let Some(ref b) = maybe_value { format!("{}", HexDisplay::from(b)) } else { "<empty>".to_owned() }, HexDisplay::from(&key));
trace!(target: "wasm-trace", " Getting storage: {} == {} [k={}]", ascii_format(&key), if let Some(ref b) = maybe_value { format!("{}", HexDisplay::from(b)) } else { "<empty>".to_owned() }, HexDisplay::from(&key));
}

if let Some(value) = maybe_value {
Expand All @@ -222,9 +222,9 @@ impl_function_executor!(this: FunctionExecutor<'e, E>,
let key = this.memory.get(key_data, key_len as usize).map_err(|_| DummyUserError)?;
let maybe_value = this.ext.storage(&key);
if let Some(preimage) = this.hash_lookup.get(&key) {
info!(target: "wasm-trace", " Getting storage: %{} == {} [k={}]", ascii_format(&preimage), if let Some(ref b) = maybe_value { format!("{}", HexDisplay::from(b)) } else { "<empty>".to_owned() }, HexDisplay::from(&key));
trace!(target: "wasm-trace", " Getting storage: %{} == {} [k={}]", ascii_format(&preimage), if let Some(ref b) = maybe_value { format!("{}", HexDisplay::from(b)) } else { "<empty>".to_owned() }, HexDisplay::from(&key));
} else {
info!(target: "wasm-trace", " Getting storage: {} == {} [k={}]", ascii_format(&key), if let Some(ref b) = maybe_value { format!("{}", HexDisplay::from(b)) } else { "<empty>".to_owned() }, HexDisplay::from(&key));
trace!(target: "wasm-trace", " Getting storage: {} == {} [k={}]", ascii_format(&key), if let Some(ref b) = maybe_value { format!("{}", HexDisplay::from(b)) } else { "<empty>".to_owned() }, HexDisplay::from(&key));
}
if let Some(value) = maybe_value {
let value = &value[value_offset as usize..];
Expand Down
Binary file not shown.
Binary file not shown.
1 change: 1 addition & 0 deletions substrate/rpc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ authors = ["Parity Technologies <[email protected]>"]

[dependencies]
parking_lot = "0.4"
log = "0.3"
error-chain = "0.11"
jsonrpc-core = { git="https://github.com/paritytech/jsonrpc.git" }
jsonrpc-macros = { git="https://github.com/paritytech/jsonrpc.git" }
Expand Down
2 changes: 2 additions & 0 deletions substrate/rpc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ extern crate substrate_state_machine as state_machine;
extern crate error_chain;
#[macro_use]
extern crate jsonrpc_macros;
#[macro_use]
extern crate log;

#[cfg(test)]
extern crate substrate_executor;
Expand Down
27 changes: 23 additions & 4 deletions substrate/rpc/src/state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,21 @@ use self::error::Result;
build_rpc_trait! {
/// Polkadot state API
pub trait StateApi {
/// Returns a storage entry.
#[rpc(name = "state_getStorageAt")]
fn storage_at(&self, StorageKey, block::HeaderHash) -> Result<StorageData>;

/// Call a contract.
#[rpc(name = "state_callAt")]
fn call_at(&self, String, Vec<u8>, block::HeaderHash) -> Result<Vec<u8>>;

/// Returns a storage entry.
#[rpc(name = "state_getStorage")]
fn storage(&self, StorageKey, block::HeaderHash) -> Result<StorageData>;
fn storage(&self, StorageKey) -> Result<StorageData>;

/// Call a contract.
#[rpc(name = "state_call")]
fn call(&self, String, Vec<u8>, block::HeaderHash) -> Result<Vec<u8>>;
fn call(&self, String, Vec<u8>) -> Result<Vec<u8>>;
}
}

Expand All @@ -47,11 +55,22 @@ impl<B, E> StateApi for Arc<Client<B, E>> where
E: state_machine::CodeExecutor + Send + Sync + 'static,
client::error::Error: From<<<B as client::backend::Backend>::State as state_machine::backend::Backend>::Error>,
{
fn storage(&self, key: StorageKey, block: block::HeaderHash) -> Result<StorageData> {
fn storage_at(&self, key: StorageKey, block: block::HeaderHash) -> Result<StorageData> {
Ok(self.as_ref().storage(&block::Id::Hash(block), &key)?)
}

fn call(&self, method: String, data: Vec<u8>, block: block::HeaderHash) -> Result<Vec<u8>> {
fn call_at(&self, method: String, data: Vec<u8>, block: block::HeaderHash) -> Result<Vec<u8>> {
Ok(self.as_ref().call(&block::Id::Hash(block), &method, &data)?.return_data)
}

fn storage(&self, key: StorageKey) -> Result<StorageData> {
let at = block::Id::Hash(self.as_ref().info()?.chain.best_hash);
use primitives::hexdisplay::HexDisplay;
info!("Querying storage at {:?} for key {}", at, HexDisplay::from(&key.0));
Ok(self.as_ref().storage(&at, &key)?)
}

fn call(&self, method: String, data: Vec<u8>) -> Result<Vec<u8>> {
Ok(self.as_ref().call(&block::Id::Hash(self.as_ref().info()?.chain.best_hash), &method, &data)?.return_data)
}
}
4 changes: 2 additions & 2 deletions substrate/rpc/src/state/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ fn should_return_storage() {
let genesis_hash = test_genesis_block.blake2_256().into();

assert_matches!(
StateApi::storage(&client, StorageKey(vec![10]), genesis_hash),
StateApi::storage_at(&client, StorageKey(vec![10]), genesis_hash),
Err(Error(ErrorKind::Client(client::error::ErrorKind::NoValueForKey(ref k)), _)) if *k == vec![10]
)
}
Expand All @@ -55,7 +55,7 @@ fn should_call_contract() {
let genesis_hash = test_genesis_block.blake2_256().into();

assert_matches!(
StateApi::call(&client, "balanceOf".into(), vec![1,2,3], genesis_hash),
StateApi::call_at(&client, "balanceOf".into(), vec![1,2,3], genesis_hash),
Err(Error(ErrorKind::Client(client::error::ErrorKind::Execution(_)), _))
)
}
23 changes: 14 additions & 9 deletions substrate/runtime/executive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,7 @@ use runtime_io::Hashing;
use runtime_support::StorageValue;
use primitives::traits::{self, Header, Zero, One, Checkable, Applyable, CheckEqual, Executable, MakePayment};
use codec::Slicable;

/// Compute the extrinsics root of a list of extrinsics.
pub fn extrinsics_root<H: Hashing, E: Slicable>(extrinsics: &[E]) -> H::Output {
let xts = extrinsics.iter().map(Slicable::encode).collect::<Vec<_>>();
let xts = xts.iter().map(Vec::as_slice).collect::<Vec<_>>();
H::enumerated_trie_root(&xts)
}
use system::extrinsics_root;

pub struct Executive<
System,
Expand Down Expand Up @@ -103,7 +97,7 @@ impl<

// execute transactions
let (header, extrinsics) = block.deconstruct();
extrinsics.into_iter().for_each(Self::apply_extrinsic);
extrinsics.into_iter().for_each(Self::apply_extrinsic_inner);

// post-transactional book-keeping.
Finalisation::execute();
Expand All @@ -120,15 +114,26 @@ impl<
pub fn finalise_block() -> System::Header {
Finalisation::execute();

// setup extrinsics
<system::Module<System>>::derive_extrinsics();

let header = <system::Module<System>>::finalise();
Self::post_finalise(&header);

header
}

/// Apply outside of the block execution function.
/// This doesn't attempt to validate anything regarding the block.
/// This doesn't attempt to validate anything regarding the block, but it builds a list of uxt
/// hashes.
pub fn apply_extrinsic(uxt: Block::Extrinsic) {
<system::Module<System>>::note_extrinsic(uxt.encode());
Self::apply_extrinsic_inner(uxt);
}

/// Apply outside of the block execution function.
/// This doesn't attempt to validate anything regarding the block.
fn apply_extrinsic_inner(uxt: Block::Extrinsic) {
// Verify the signature is good.
let xt = match uxt.check() {
Ok(xt) => xt,
Expand Down
Loading