From d280b581257041ba36c8bfd7395abdcde308dab4 Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Tue, 9 Apr 2019 15:10:55 +0100 Subject: [PATCH 1/6] Implement `ext_println` in contract runtime --- srml/contract/src/wasm/runtime.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/srml/contract/src/wasm/runtime.rs b/srml/contract/src/wasm/runtime.rs index 37ab9fa88708f..f02d8a3090a9b 100644 --- a/srml/contract/src/wasm/runtime.rs +++ b/srml/contract/src/wasm/runtime.rs @@ -628,4 +628,16 @@ define_env!(Env, , Ok(()) }, + + // Prints utf8 encoded string from the data buffer. + // MUST only be called from contracts deployed to `--dev` chains, NOT for use in production chains. + // This function may be removed in the future and superseded by "Diagnostic Runtimes": + // https://github.com/paritytech/substrate/issues/2082 + ext_println(ctx, str_ptr: u32, str_len: u32) => { + let data = read_sandbox_memory(ctx, str_ptr, str_len)?; + if let Ok(utf8) = core::str::from_utf8(&data) { + runtime_io::print(utf8); + } + Ok(()) + }, ); From 98279821abd82b03cde70c9cacd71f508404e9a6 Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Tue, 9 Apr 2019 17:01:36 +0100 Subject: [PATCH 2/6] Only allow contracts to import `ext_println` on dev chains --- srml/contract/src/lib.rs | 5 ++++ srml/contract/src/wasm/prepare.rs | 38 +++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/srml/contract/src/lib.rs b/srml/contract/src/lib.rs index c919439d54650..0145de37c383d 100644 --- a/srml/contract/src/lib.rs +++ b/srml/contract/src/lib.rs @@ -527,6 +527,10 @@ pub struct Schedule { /// What is the maximal memory pages amount is allowed to have for /// a contract. pub max_memory_pages: u32, + + /// Whether debug features are enabled for contracts. + /// MUST only be enabled for `dev` chains, NOT for production chains + pub enable_debug: bool, } impl> Default for Schedule { @@ -543,6 +547,7 @@ impl> Default for Schedule { sandbox_data_write_cost: Gas::sa(1), max_stack_height: 64 * 1024, max_memory_pages: 16, + enable_debug: false, } } } diff --git a/srml/contract/src/wasm/prepare.rs b/srml/contract/src/wasm/prepare.rs index 50edff32971a9..d0f7cb2f523ea 100644 --- a/srml/contract/src/wasm/prepare.rs +++ b/srml/contract/src/wasm/prepare.rs @@ -239,6 +239,12 @@ impl<'a, Gas: 'a + As + Clone> ContractModule<'a, Gas> { .get(*type_idx as usize) .ok_or_else(|| "validation: import entry points to a non-existent type")?; + // We disallow importing `ext_println` unless debug features are enabled, + // which should only be allowed on a dev chain + if !self.schedule.enable_debug && import.field().as_bytes() == b"ext_println" { + return Err("module imports `ext_println` but debug features disabled"); + } + // We disallow importing `gas` function here since it is treated as implementation detail. if import.field().as_bytes() == b"gas" || !C::can_satisfy(import.field().as_bytes(), func_ty) @@ -347,6 +353,8 @@ mod tests { gas(_ctx, _amount: u32) => { unreachable!(); }, nop(_ctx, _unused: u64) => { unreachable!(); }, + + ext_println(_ctx, _ptr: u32, _len: u32) => { unreachable!(); }, ); macro_rules! prepare_test { @@ -572,6 +580,36 @@ mod tests { "#, Err("module imports a non-existent function") ); + + prepare_test!(ext_println_debug_disabled, + r#" + (module + (import "env" "ext_println" (func $ext_println (param i32 i32))) + + (func (export "call")) + (func (export "deploy")) + ) + "#, + Err("module imports `ext_println` but debug features disabled") + ); + + #[test] + fn ext_println_debug_enabled() { + let wasm = wabt::Wat2Wasm::new().validate(false).convert( + r#" + (module + (import "env" "ext_println" (func $ext_println (param i32 i32))) + + (func (export "call")) + (func (export "deploy")) + ) + "# + ).unwrap(); + let mut schedule = Schedule::::default(); + schedule.enable_debug = true; + let r = prepare_contract::(wasm.as_ref(), &schedule); + assert_matches!(r, Ok(_)); + } } mod entrypoints { From c08bdd0223e833894a90825e538f051aeb161c03 Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Tue, 9 Apr 2019 17:35:23 +0100 Subject: [PATCH 3/6] Configure dev chain to allow contracts with `ext_println` --- node/cli/src/chain_spec.rs | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/node/cli/src/chain_spec.rs b/node/cli/src/chain_spec.rs index a5ddd5ab81e54..a7f0a6fcc58ef 100644 --- a/node/cli/src/chain_spec.rs +++ b/node/cli/src/chain_spec.rs @@ -216,6 +216,7 @@ pub fn testnet_genesis( initial_authorities: Vec<(AccountId, AccountId, AuthorityId)>, root_key: AccountId, endowed_accounts: Option>, + enable_contract_debug: bool, ) -> GenesisConfig { let endowed_accounts: Vec = endowed_accounts.unwrap_or_else(|| { vec![ @@ -237,6 +238,22 @@ pub fn testnet_genesis( const STASH: u128 = 1 << 20; const ENDOWMENT: u128 = 1 << 20; + let mut contract_config = ContractConfig { + transaction_base_fee: 1, + transaction_byte_fee: 0, + transfer_fee: 0, + creation_fee: 0, + contract_fee: 21, + call_base_fee: 135, + create_base_fee: 175, + gas_price: 1, + max_depth: 1024, + block_gas_limit: 10_000_000, + current_schedule: Default::default(), + }; + // this should only be enabled on development chains + contract_config.current_schedule.enable_debug = enable_contract_debug; + GenesisConfig { consensus: Some(ConsensusConfig { code: include_bytes!("../../runtime/wasm/target/wasm32-unknown-unknown/release/node_runtime.compact.wasm").to_vec(), @@ -308,19 +325,7 @@ pub fn testnet_genesis( spend_period: 12 * 60 * 24, burn: Permill::from_percent(50), }), - contract: Some(ContractConfig { - transaction_base_fee: 1, - transaction_byte_fee: 0, - transfer_fee: 0, - creation_fee: 0, - contract_fee: 21, - call_base_fee: 135, - create_base_fee: 175, - gas_price: 1, - max_depth: 1024, - block_gas_limit: 10_000_000, - current_schedule: Default::default(), - }), + contract: Some(contract_config), sudo: Some(SudoConfig { key: root_key, }), @@ -337,6 +342,7 @@ fn development_config_genesis() -> GenesisConfig { ], get_account_id_from_seed("Alice"), None, + true, ) } @@ -353,6 +359,7 @@ fn local_testnet_genesis() -> GenesisConfig { ], get_account_id_from_seed("Alice"), None, + false, ) } From 24461cea2599c48b11c34c310f8080098d41a77e Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Wed, 10 Apr 2019 09:03:37 +0100 Subject: [PATCH 4/6] Increment spec version --- node/runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 32ba893c669f8..e9764ef1c0bdc 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -59,7 +59,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("node"), impl_name: create_runtime_str!("substrate-node"), authoring_version: 10, - spec_version: 58, + spec_version: 59, impl_version: 59, apis: RUNTIME_API_VERSIONS, }; From 3215116addf28c63eb13af59d9e433686009f977 Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Thu, 11 Apr 2019 10:37:36 +0100 Subject: [PATCH 5/6] Docs --- srml/contract/src/wasm/runtime.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/srml/contract/src/wasm/runtime.rs b/srml/contract/src/wasm/runtime.rs index f02d8a3090a9b..1c48d248d2973 100644 --- a/srml/contract/src/wasm/runtime.rs +++ b/srml/contract/src/wasm/runtime.rs @@ -630,9 +630,8 @@ define_env!(Env, , }, // Prints utf8 encoded string from the data buffer. - // MUST only be called from contracts deployed to `--dev` chains, NOT for use in production chains. - // This function may be removed in the future and superseded by "Diagnostic Runtimes": - // https://github.com/paritytech/substrate/issues/2082 + // Only available on `--dev` chains. + // This function may be removed at any time, superseded by a more general contract debugging feature. ext_println(ctx, str_ptr: u32, str_len: u32) => { let data = read_sandbox_memory(ctx, str_ptr, str_len)?; if let Ok(utf8) = core::str::from_utf8(&data) { From b5c889fbceaea5a45446c196ae8079d7fbd167a4 Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Thu, 11 Apr 2019 10:42:43 +0100 Subject: [PATCH 6/6] Rename config to the more specific enable_println --- node/cli/src/chain_spec.rs | 4 ++-- srml/contract/src/lib.rs | 6 +++--- srml/contract/src/wasm/prepare.rs | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/node/cli/src/chain_spec.rs b/node/cli/src/chain_spec.rs index a7f0a6fcc58ef..217d3b1312898 100644 --- a/node/cli/src/chain_spec.rs +++ b/node/cli/src/chain_spec.rs @@ -216,7 +216,7 @@ pub fn testnet_genesis( initial_authorities: Vec<(AccountId, AccountId, AuthorityId)>, root_key: AccountId, endowed_accounts: Option>, - enable_contract_debug: bool, + enable_println: bool, ) -> GenesisConfig { let endowed_accounts: Vec = endowed_accounts.unwrap_or_else(|| { vec![ @@ -252,7 +252,7 @@ pub fn testnet_genesis( current_schedule: Default::default(), }; // this should only be enabled on development chains - contract_config.current_schedule.enable_debug = enable_contract_debug; + contract_config.current_schedule.enable_println = enable_println; GenesisConfig { consensus: Some(ConsensusConfig { diff --git a/srml/contract/src/lib.rs b/srml/contract/src/lib.rs index 0145de37c383d..c471451253158 100644 --- a/srml/contract/src/lib.rs +++ b/srml/contract/src/lib.rs @@ -528,9 +528,9 @@ pub struct Schedule { /// a contract. pub max_memory_pages: u32, - /// Whether debug features are enabled for contracts. + /// Whether the `ext_println` function is allowed to be used contracts. /// MUST only be enabled for `dev` chains, NOT for production chains - pub enable_debug: bool, + pub enable_println: bool, } impl> Default for Schedule { @@ -547,7 +547,7 @@ impl> Default for Schedule { sandbox_data_write_cost: Gas::sa(1), max_stack_height: 64 * 1024, max_memory_pages: 16, - enable_debug: false, + enable_println: false, } } } diff --git a/srml/contract/src/wasm/prepare.rs b/srml/contract/src/wasm/prepare.rs index d0f7cb2f523ea..15922c3d99959 100644 --- a/srml/contract/src/wasm/prepare.rs +++ b/srml/contract/src/wasm/prepare.rs @@ -241,7 +241,7 @@ impl<'a, Gas: 'a + As + Clone> ContractModule<'a, Gas> { // We disallow importing `ext_println` unless debug features are enabled, // which should only be allowed on a dev chain - if !self.schedule.enable_debug && import.field().as_bytes() == b"ext_println" { + if !self.schedule.enable_println && import.field().as_bytes() == b"ext_println" { return Err("module imports `ext_println` but debug features disabled"); } @@ -606,7 +606,7 @@ mod tests { "# ).unwrap(); let mut schedule = Schedule::::default(); - schedule.enable_debug = true; + schedule.enable_println = true; let r = prepare_contract::(wasm.as_ref(), &schedule); assert_matches!(r, Ok(_)); }