From 8321883cf5ae1ffdf6d4845df45f26a9c1c2409a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Wed, 24 Jun 2020 16:45:29 +0200 Subject: [PATCH 01/23] Transition getter functions to not use scratch buffer --- frame/contracts/fixtures/caller_contract.wat | 16 +- .../fixtures/check_default_rent_allowance.wat | 28 +- frame/contracts/fixtures/drain.wat | 24 +- frame/contracts/fixtures/self_destruct.wat | 31 +- .../fixtures/self_destructing_constructor.wat | 24 +- frame/contracts/src/wasm/mod.rs | 306 +++++++----------- frame/contracts/src/wasm/runtime.rs | 113 ++++--- 7 files changed, 235 insertions(+), 307 deletions(-) diff --git a/frame/contracts/fixtures/caller_contract.wat b/frame/contracts/fixtures/caller_contract.wat index 4bc122c0b1863..30566d9da881a 100644 --- a/frame/contracts/fixtures/caller_contract.wat +++ b/frame/contracts/fixtures/caller_contract.wat @@ -1,7 +1,7 @@ (module (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) - (import "env" "ext_balance" (func $ext_balance)) + (import "env" "ext_balance" (func $ext_balance (param i32 i32))) (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32) (result i32))) (import "env" "ext_instantiate" (func $ext_instantiate (param i32 i32 i64 i32 i32 i32 i32) (result i32))) (import "env" "ext_println" (func $ext_println (param i32 i32))) @@ -17,14 +17,12 @@ ) (func $current_balance (param $sp i32) (result i64) - (call $ext_balance) - (call $assert - (i32.eq (call $ext_scratch_size) (i32.const 8)) - ) - (call $ext_scratch_read + (call $ext_balance (i32.sub (get_local $sp) (i32.const 8)) - (i32.const 0) - (i32.const 8) + (i32.const 2048) + ) + (call $assert + (i32.eq (i32.load (i32.const 2048)) (i32.const 8)) ) (i64.load (i32.sub (get_local $sp) (i32.const 8))) ) @@ -272,4 +270,6 @@ (data (i32.const 0) "\00\80") ;; The value to transfer on instantiation and calls. ;; Chosen to be greater than existential deposit. (data (i32.const 8) "\00\11\22\33\44\55\66\77") ;; The input data to instantiations and calls. + + (data (i32.const 2048) "\08") ;; size of our $ext_balance output buffer ) diff --git a/frame/contracts/fixtures/check_default_rent_allowance.wat b/frame/contracts/fixtures/check_default_rent_allowance.wat index 12b3004adf7de..08e160d5d7e0c 100644 --- a/frame/contracts/fixtures/check_default_rent_allowance.wat +++ b/frame/contracts/fixtures/check_default_rent_allowance.wat @@ -1,9 +1,16 @@ (module - (import "env" "ext_rent_allowance" (func $ext_rent_allowance)) + (import "env" "ext_rent_allowance" (func $ext_rent_allowance (param i32 i32))) (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) (import "env" "memory" (memory 1 1)) + ;; [0, 8) reserved for $ext_rent_allowance output + + ;; [8, 16) length of the buffer + (data (i32.const 8) "\08") + + ;; [16, inf) zero initialized + (func $assert (param i32) (block $ok (br_if $ok @@ -16,30 +23,21 @@ (func (export "call")) (func (export "deploy") - ;; fill the scratch buffer with the rent allowance. - (call $ext_rent_allowance) + ;; fill the buffer with the rent allowance. + (call $ext_rent_allowance (i32.const 0) (i32.const 8)) - ;; assert $ext_scratch_size == 8 + ;; assert len == 8 (call $assert (i32.eq - (call $ext_scratch_size) + (i32.load (i32.const 8)) (i32.const 8) ) ) - ;; copy contents of the scratch buffer into the contract's memory. - (call $ext_scratch_read - (i32.const 8) ;; Pointer in memory to the place where to copy. - (i32.const 0) ;; Offset from the start of the scratch buffer. - (i32.const 8) ;; Count of bytes to copy. - ) - ;; assert that contents of the buffer is equal to >::max_value(). (call $assert (i64.eq - (i64.load - (i32.const 8) - ) + (i64.load (i32.const 0)) (i64.const 0xFFFFFFFFFFFFFFFF) ) ) diff --git a/frame/contracts/fixtures/drain.wat b/frame/contracts/fixtures/drain.wat index d08e1dd0d2981..77119affade63 100644 --- a/frame/contracts/fixtures/drain.wat +++ b/frame/contracts/fixtures/drain.wat @@ -1,10 +1,17 @@ (module (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) - (import "env" "ext_balance" (func $ext_balance)) + (import "env" "ext_balance" (func $ext_balance (param i32 i32))) (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32) (result i32))) (import "env" "memory" (memory 1 1)) + ;; [0, 8) reserved for $ext_balance output + + ;; [8, 16) length of the buffer + (data (i32.const 8) "\08") + + ;; [16, inf) zero initialized + (func $assert (param i32) (block $ok (br_if $ok @@ -18,31 +25,24 @@ (func (export "call") ;; Send entire remaining balance to the 0 address. - (call $ext_balance) + (call $ext_balance (i32.const 0) (i32.const 8)) ;; Balance should be encoded as a u64. (call $assert (i32.eq - (call $ext_scratch_size) + (i32.load (i32.const 8)) (i32.const 8) ) ) - ;; Read balance into memory. - (call $ext_scratch_read - (i32.const 8) ;; Pointer to write balance to - (i32.const 0) ;; Offset into scratch buffer - (i32.const 8) ;; Length of encoded balance - ) - ;; Self-destruct by sending full balance to the 0 address. (call $assert (i32.eq (call $ext_call - (i32.const 0) ;; Pointer to destination address + (i32.const 16) ;; Pointer to destination address (i32.const 8) ;; Length of destination address (i64.const 0) ;; How much gas to devote for the execution. 0 = all. - (i32.const 8) ;; Pointer to the buffer with value to transfer + (i32.const 0) ;; Pointer to the buffer with value to transfer (i32.const 8) ;; Length of the buffer with value to transfer (i32.const 0) ;; Pointer to input data buffer address (i32.const 0) ;; Length of input data buffer diff --git a/frame/contracts/fixtures/self_destruct.wat b/frame/contracts/fixtures/self_destruct.wat index 464b5c663ea4a..65ddb1cabceec 100644 --- a/frame/contracts/fixtures/self_destruct.wat +++ b/frame/contracts/fixtures/self_destruct.wat @@ -1,11 +1,21 @@ (module (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) - (import "env" "ext_address" (func $ext_address)) + (import "env" "ext_address" (func $ext_address (param i32 i32))) (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32) (result i32))) (import "env" "ext_terminate" (func $ext_terminate (param i32 i32))) (import "env" "memory" (memory 1 1)) + ;; [0, 8) reserved for $ext_address output + + ;; [8, 16) length of the buffer + (data (i32.const 8) "\08") + + ;; [16, 24) Address of django + (data (i32.const 16) "\04\00\00\00\00\00\00\00") + + ;; [24, inf) zero initialized + (func $assert (param i32) (block $ok (br_if $ok @@ -24,31 +34,24 @@ ;; well. (if (call $ext_scratch_size) (then - (call $ext_address) + (call $ext_address (i32.const 0) (i32.const 8)) ;; Expect address to be 8 bytes. (call $assert (i32.eq - (call $ext_scratch_size) + (i32.load (i32.const 8)) (i32.const 8) ) ) - ;; Read own address into memory. - (call $ext_scratch_read - (i32.const 16) ;; Pointer to write address to - (i32.const 0) ;; Offset into scratch buffer - (i32.const 8) ;; Length of encoded address - ) - ;; Recursively call self with empty input data. (call $assert (i32.eq (call $ext_call - (i32.const 16) ;; Pointer to own address + (i32.const 0) ;; Pointer to own address (i32.const 8) ;; Length of own address (i64.const 0) ;; How much gas to devote for the execution. 0 = all. - (i32.const 8) ;; Pointer to the buffer with value to transfer + (i32.const 24) ;; Pointer to the buffer with value to transfer (i32.const 8) ;; Length of the buffer with value to transfer (i32.const 0) ;; Pointer to input data buffer address (i32.const 0) ;; Length of input data buffer @@ -60,13 +63,11 @@ (else ;; Try to terminate and give balance to django. (call $ext_terminate - (i32.const 32) ;; Pointer to beneficiary address + (i32.const 16) ;; Pointer to beneficiary address (i32.const 8) ;; Length of beneficiary address ) (unreachable) ;; ext_terminate never returns ) ) ) - ;; Address of django - (data (i32.const 32) "\04\00\00\00\00\00\00\00") ) diff --git a/frame/contracts/fixtures/self_destructing_constructor.wat b/frame/contracts/fixtures/self_destructing_constructor.wat index b19d6e5b50dac..14234108dfcae 100644 --- a/frame/contracts/fixtures/self_destructing_constructor.wat +++ b/frame/contracts/fixtures/self_destructing_constructor.wat @@ -1,10 +1,17 @@ (module (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) - (import "env" "ext_balance" (func $ext_balance)) + (import "env" "ext_balance" (func $ext_balance (param i32 i32))) (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32) (result i32))) (import "env" "memory" (memory 1 1)) + ;; [0, 8) reserved for $ext_balance output + + ;; [8, 16) length of the buffer + (data (i32.const 8) "\08") + + ;; [16, inf) zero initialized + (func $assert (param i32) (block $ok (br_if $ok @@ -16,31 +23,24 @@ (func (export "deploy") ;; Send entire remaining balance to the 0 address. - (call $ext_balance) + (call $ext_balance (i32.const 0) (i32.const 8)) ;; Balance should be encoded as a u64. (call $assert (i32.eq - (call $ext_scratch_size) + (i32.load (i32.const 8)) (i32.const 8) ) ) - ;; Read balance into memory. - (call $ext_scratch_read - (i32.const 8) ;; Pointer to write balance to - (i32.const 0) ;; Offset into scratch buffer - (i32.const 8) ;; Length of encoded balance - ) - ;; Self-destruct by sending full balance to the 0 address. (call $assert (i32.eq (call $ext_call - (i32.const 0) ;; Pointer to destination address + (i32.const 16) ;; Pointer to destination address (i32.const 8) ;; Length of destination address (i64.const 0) ;; How much gas to devote for the execution. 0 = all. - (i32.const 8) ;; Pointer to the buffer with value to transfer + (i32.const 0) ;; Pointer to the buffer with value to transfer (i32.const 8) ;; Length of the buffer with value to transfer (i32.const 0) ;; Pointer to input data buffer address (i32.const 0) ;; Length of input data buffer diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index 3d2f5b154ffd4..995cdb4b958e0 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -868,11 +868,12 @@ mod tests { /// compares it with the constant 42. const CODE_CALLER: &str = r#" (module - (import "env" "ext_caller" (func $ext_caller)) - (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) - (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) + (import "env" "ext_caller" (func $ext_caller (param i32 i32))) (import "env" "memory" (memory 1 1)) + ;; size of our buffer is 32 bytes + (data (i32.const 32) "\20") + (func $assert (param i32) (block $ok (br_if $ok @@ -883,30 +884,21 @@ mod tests { ) (func (export "call") - ;; fill the scratch buffer with the caller. - (call $ext_caller) + ;; fill the buffer with the caller. + (call $ext_caller (i32.const 0) (i32.const 32)) - ;; assert $ext_scratch_size == 8 + ;; assert len == 8 (call $assert (i32.eq - (call $ext_scratch_size) + (i32.load (i32.const 32)) (i32.const 8) ) ) - ;; copy contents of the scratch buffer into the contract's memory. - (call $ext_scratch_read - (i32.const 8) ;; Pointer in memory to the place where to copy. - (i32.const 0) ;; Offset from the start of the scratch buffer. - (i32.const 8) ;; Count of bytes to copy. - ) - ;; assert that contents of the buffer is equal to the i64 value of 42. (call $assert (i64.eq - (i64.load - (i32.const 8) - ) + (i64.load (i32.const 0)) (i64.const 42) ) ) @@ -930,11 +922,12 @@ mod tests { /// compares it with the constant 69. const CODE_ADDRESS: &str = r#" (module - (import "env" "ext_address" (func $ext_address)) - (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) - (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) + (import "env" "ext_address" (func $ext_address (param i32 i32))) (import "env" "memory" (memory 1 1)) + ;; size of our buffer is 32 bytes + (data (i32.const 32) "\20") + (func $assert (param i32) (block $ok (br_if $ok @@ -945,30 +938,21 @@ mod tests { ) (func (export "call") - ;; fill the scratch buffer with the self address. - (call $ext_address) + ;; fill the buffer with the self address. + (call $ext_address (i32.const 0) (i32.const 32)) - ;; assert $ext_scratch_size == 8 + ;; assert size == 8 (call $assert (i32.eq - (call $ext_scratch_size) + (i32.load (i32.const 32)) (i32.const 8) ) ) - ;; copy contents of the scratch buffer into the contract's memory. - (call $ext_scratch_read - (i32.const 8) ;; Pointer in memory to the place where to copy. - (i32.const 0) ;; Offset from the start of the scratch buffer. - (i32.const 8) ;; Count of bytes to copy. - ) - ;; assert that contents of the buffer is equal to the i64 value of 69. (call $assert (i64.eq - (i64.load - (i32.const 8) - ) + (i64.load (i32.const 0)) (i64.const 69) ) ) @@ -990,11 +974,12 @@ mod tests { const CODE_BALANCE: &str = r#" (module - (import "env" "ext_balance" (func $ext_balance)) - (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) - (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) + (import "env" "ext_balance" (func $ext_balance (param i32 i32))) (import "env" "memory" (memory 1 1)) + ;; size of our buffer is 32 bytes + (data (i32.const 32) "\20") + (func $assert (param i32) (block $ok (br_if $ok @@ -1005,30 +990,21 @@ mod tests { ) (func (export "call") - ;; This stores the balance in the scratch buffer - (call $ext_balance) + ;; This stores the balance in the buffer + (call $ext_balance (i32.const 0) (i32.const 32)) - ;; assert $ext_scratch_size == 8 + ;; assert len == 8 (call $assert (i32.eq - (call $ext_scratch_size) + (i32.load (i32.const 32)) (i32.const 8) ) ) - ;; copy contents of the scratch buffer into the contract's memory. - (call $ext_scratch_read - (i32.const 8) ;; Pointer in memory to the place where to copy. - (i32.const 0) ;; Offset from the start of the scratch buffer. - (i32.const 8) ;; Count of bytes to copy. - ) - ;; assert that contents of the buffer is equal to the i64 value of 228. (call $assert (i64.eq - (i64.load - (i32.const 8) - ) + (i64.load (i32.const 0)) (i64.const 228) ) ) @@ -1050,11 +1026,12 @@ mod tests { const CODE_GAS_PRICE: &str = r#" (module - (import "env" "ext_gas_price" (func $ext_gas_price (param i64))) - (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) - (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) + (import "env" "ext_gas_price" (func $ext_gas_price (param i64 i32 i32))) (import "env" "memory" (memory 1 1)) + ;; size of our buffer is 32 bytes + (data (i32.const 32) "\20") + (func $assert (param i32) (block $ok (br_if $ok @@ -1065,31 +1042,22 @@ mod tests { ) (func (export "call") - ;; This stores the gas price in the scratch buffer - (call $ext_gas_price (i64.const 1)) + ;; This stores the gas price in the buffer + (call $ext_gas_price (i64.const 2) (i32.const 0) (i32.const 32)) - ;; assert $ext_scratch_size == 8 + ;; assert len == 8 (call $assert (i32.eq - (call $ext_scratch_size) + (i32.load (i32.const 32)) (i32.const 8) ) ) - ;; copy contents of the scratch buffer into the contract's memory. - (call $ext_scratch_read - (i32.const 8) ;; Pointer in memory to the place where to copy. - (i32.const 0) ;; Offset from the start of the scratch buffer. - (i32.const 8) ;; Count of bytes to copy. - ) - - ;; assert that contents of the buffer is equal to the i64 value of 1312. + ;; assert that contents of the buffer is equal to the i64 value of 2 * 1312. (call $assert (i64.eq - (i64.load - (i32.const 8) - ) - (i64.const 1312) + (i64.load (i32.const 0)) + (i64.const 2624) ) ) ) @@ -1110,12 +1078,13 @@ mod tests { const CODE_GAS_LEFT: &str = r#" (module - (import "env" "ext_gas_left" (func $ext_gas_left)) - (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) - (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) + (import "env" "ext_gas_left" (func $ext_gas_left (param i32 i32))) (import "env" "ext_return" (func $ext_return (param i32 i32))) (import "env" "memory" (memory 1 1)) + ;; size of our buffer is 32 bytes + (data (i32.const 32) "\20") + (func $assert (param i32) (block $ok (br_if $ok @@ -1126,28 +1095,19 @@ mod tests { ) (func (export "call") - ;; This stores the gas left in the scratch buffer - (call $ext_gas_left) + ;; This stores the gas left in the buffer + (call $ext_gas_left (i32.const 0) (i32.const 32)) - ;; assert $ext_scratch_size == 8 + ;; assert len == 8 (call $assert (i32.eq - (call $ext_scratch_size) + (i32.load (i32.const 32)) (i32.const 8) ) ) - ;; copy contents of the scratch buffer into the contract's memory. - (call $ext_scratch_read - (i32.const 8) ;; Pointer in memory to the place where to copy. - (i32.const 0) ;; Offset from the start of the scratch buffer. - (i32.const 8) ;; Count of bytes to copy. - ) - - (call $ext_return - (i32.const 8) - (i32.const 8) - ) + ;; return gas left + (call $ext_return (i32.const 0) (i32.const 8)) (unreachable) ) @@ -1173,11 +1133,12 @@ mod tests { const CODE_VALUE_TRANSFERRED: &str = r#" (module - (import "env" "ext_value_transferred" (func $ext_value_transferred)) - (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) - (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) + (import "env" "ext_value_transferred" (func $ext_value_transferred (param i32 i32))) (import "env" "memory" (memory 1 1)) + ;; size of our buffer is 32 bytes + (data (i32.const 32) "\20") + (func $assert (param i32) (block $ok (br_if $ok @@ -1188,30 +1149,21 @@ mod tests { ) (func (export "call") - ;; This stores the value transferred in the scratch buffer - (call $ext_value_transferred) + ;; This stores the value transferred in the buffer + (call $ext_value_transferred (i32.const 0) (i32.const 32)) - ;; assert $ext_scratch_size == 8 + ;; assert len == 8 (call $assert (i32.eq - (call $ext_scratch_size) + (i32.load (i32.const 32)) (i32.const 8) ) ) - ;; copy contents of the scratch buffer into the contract's memory. - (call $ext_scratch_read - (i32.const 8) ;; Pointer in memory to the place where to copy. - (i32.const 0) ;; Offset from the start of the scratch buffer. - (i32.const 8) ;; Count of bytes to copy. - ) - ;; assert that contents of the buffer is equal to the i64 value of 1337. (call $assert (i64.eq - (i64.load - (i32.const 8) - ) + (i64.load (i32.const 0)) (i64.const 1337) ) ) @@ -1268,11 +1220,12 @@ mod tests { const CODE_TIMESTAMP_NOW: &str = r#" (module - (import "env" "ext_now" (func $ext_now)) - (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) - (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) + (import "env" "ext_now" (func $ext_now (param i32 i32))) (import "env" "memory" (memory 1 1)) + ;; size of our buffer is 32 bytes + (data (i32.const 32) "\20") + (func $assert (param i32) (block $ok (br_if $ok @@ -1283,30 +1236,21 @@ mod tests { ) (func (export "call") - ;; This stores the block timestamp in the scratch buffer - (call $ext_now) + ;; This stores the block timestamp in the buffer + (call $ext_now (i32.const 0) (i32.const 32)) - ;; assert $ext_scratch_size == 8 + ;; assert len == 8 (call $assert (i32.eq - (call $ext_scratch_size) + (i32.load (i32.const 32)) (i32.const 8) ) ) - ;; copy contents of the scratch buffer into the contract's memory. - (call $ext_scratch_read - (i32.const 8) ;; Pointer in memory to the place where to copy. - (i32.const 0) ;; Offset from the start of the scratch buffer. - (i32.const 8) ;; Count of bytes to copy. - ) - ;; assert that contents of the buffer is equal to the i64 value of 1111. (call $assert (i64.eq - (i64.load - (i32.const 8) - ) + (i64.load (i32.const 0)) (i64.const 1111) ) ) @@ -1328,11 +1272,12 @@ mod tests { const CODE_MINIMUM_BALANCE: &str = r#" (module - (import "env" "ext_minimum_balance" (func $ext_minimum_balance)) - (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) - (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) + (import "env" "ext_minimum_balance" (func $ext_minimum_balance (param i32 i32))) (import "env" "memory" (memory 1 1)) + ;; size of our buffer is 32 bytes + (data (i32.const 32) "\20") + (func $assert (param i32) (block $ok (br_if $ok @@ -1343,29 +1288,20 @@ mod tests { ) (func (export "call") - (call $ext_minimum_balance) + (call $ext_minimum_balance (i32.const 0) (i32.const 32)) - ;; assert $ext_scratch_size == 8 + ;; assert len == 8 (call $assert (i32.eq - (call $ext_scratch_size) + (i32.load (i32.const 32)) (i32.const 8) ) ) - ;; copy contents of the scratch buffer into the contract's memory. - (call $ext_scratch_read - (i32.const 8) ;; Pointer in memory to the place where to copy. - (i32.const 0) ;; Offset from the start of the scratch buffer. - (i32.const 8) ;; Count of bytes to copy. - ) - ;; assert that contents of the buffer is equal to the i64 value of 666. (call $assert (i64.eq - (i64.load - (i32.const 8) - ) + (i64.load (i32.const 0)) (i64.const 666) ) ) @@ -1387,11 +1323,12 @@ mod tests { const CODE_TOMBSTONE_DEPOSIT: &str = r#" (module - (import "env" "ext_tombstone_deposit" (func $ext_tombstone_deposit)) - (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) - (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) + (import "env" "ext_tombstone_deposit" (func $ext_tombstone_deposit (param i32 i32))) (import "env" "memory" (memory 1 1)) + ;; size of our buffer is 32 bytes + (data (i32.const 32) "\20") + (func $assert (param i32) (block $ok (br_if $ok @@ -1402,29 +1339,20 @@ mod tests { ) (func (export "call") - (call $ext_tombstone_deposit) + (call $ext_tombstone_deposit (i32.const 0) (i32.const 32)) - ;; assert $ext_scratch_size == 8 + ;; assert len == 8 (call $assert (i32.eq - (call $ext_scratch_size) + (i32.load (i32.const 32)) (i32.const 8) ) ) - ;; copy contents of the scratch buffer into the contract's memory. - (call $ext_scratch_read - (i32.const 8) ;; Pointer in memory to the place where to copy. - (i32.const 0) ;; Offset from the start of the scratch buffer. - (i32.const 8) ;; Count of bytes to copy. - ) - ;; assert that contents of the buffer is equal to the i64 value of 16. (call $assert (i64.eq - (i64.load - (i32.const 8) - ) + (i64.load (i32.const 0)) (i64.const 16) ) ) @@ -1446,12 +1374,21 @@ mod tests { const CODE_RANDOM: &str = r#" (module - (import "env" "ext_random" (func $ext_random (param i32 i32))) - (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) - (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) + (import "env" "ext_random" (func $ext_random (param i32 i32 i32 i32))) (import "env" "ext_return" (func $ext_return (param i32 i32))) (import "env" "memory" (memory 1 1)) + ;; [0,128) is reserved for the result of PRNG. + + ;; the subject used for the PRNG. [128,160) + (data (i32.const 128) + "\00\01\02\03\04\05\06\07\08\09\0A\0B\0C\0D\0E\0F" + "\00\01\02\03\04\05\06\07\08\09\0A\0B\0C\0D\0E\0F" + ) + + ;; size of our buffer is 128 bytes + (data (i32.const 160) "\80") + (func $assert (param i32) (block $ok (br_if $ok @@ -1462,42 +1399,29 @@ mod tests { ) (func (export "call") - ;; This stores the block random seed in the scratch buffer + ;; This stores the block random seed in the buffer (call $ext_random - (i32.const 40) ;; Pointer in memory to the start of the subject buffer + (i32.const 128) ;; Pointer in memory to the start of the subject buffer (i32.const 32) ;; The subject buffer's length + (i32.const 0) ;; Pointer to the output buffer + (i32.const 160) ;; Pointer to the output buffer length ) - ;; assert $ext_scratch_size == 32 + ;; assert len == 32 (call $assert (i32.eq - (call $ext_scratch_size) + (i32.load (i32.const 160)) (i32.const 32) ) ) - ;; copy contents of the scratch buffer into the contract's memory. - (call $ext_scratch_read - (i32.const 8) ;; Pointer in memory to the place where to copy. - (i32.const 0) ;; Offset from the start of the scratch buffer. - (i32.const 32) ;; Count of bytes to copy. - ) - - ;; return the data from the contract + ;; return the random data (call $ext_return - (i32.const 8) + (i32.const 0) (i32.const 32) ) ) (func (export "deploy")) - - ;; [8,40) is reserved for the result of PRNG. - - ;; the subject used for the PRNG. [40,72) - (data (i32.const 40) - "\00\01\02\03\04\05\06\07\08\09\0A\0B\0C\0D\0E\0F" - "\00\01\02\03\04\05\06\07\08\09\0A\0B\0C\0D\0E\0F" - ) ) "#; @@ -1655,11 +1579,12 @@ mod tests { /// compares it with the constant 121. const CODE_BLOCK_NUMBER: &str = r#" (module - (import "env" "ext_block_number" (func $ext_block_number)) - (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) - (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) + (import "env" "ext_block_number" (func $ext_block_number (param i32 i32))) (import "env" "memory" (memory 1 1)) + ;; size of our buffer is 32 bytes + (data (i32.const 32) "\20") + (func $assert (param i32) (block $ok (br_if $ok @@ -1670,30 +1595,21 @@ mod tests { ) (func (export "call") - ;; This stores the block height in the scratch buffer - (call $ext_block_number) + ;; This stores the block height in the buffer + (call $ext_block_number (i32.const 0) (i32.const 32)) - ;; assert $ext_scratch_size == 8 + ;; assert len == 8 (call $assert (i32.eq - (call $ext_scratch_size) + (i32.load (i32.const 32)) (i32.const 8) ) ) - ;; copy contents of the scratch buffer into the contract's memory. - (call $ext_scratch_read - (i32.const 8) ;; Pointer in memory to the place where to copy. - (i32.const 0) ;; Offset from the start of the scratch buffer. - (i32.const 8) ;; Count of bytes to copy. - ) - ;; assert that contents of the buffer is equal to the i64 value of 121. (call $assert (i64.eq - (i64.load - (i32.const 8) - ) + (i64.load (i32.const 0)) (i64.const 121) ) ) diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index 7b64117cd2314..a18e216618f3a 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -52,6 +52,8 @@ enum SpecialTrap { Termination, /// Signals that a trap was generated because of a successful restoration. Restoration, + /// Signals that a buffer supplied by the caller is too small to fit the output. + OutputBufferTooSmall, } /// Can only be used for one call. @@ -113,6 +115,12 @@ pub(crate) fn to_execution_result( buffer: runtime.scratch_buf, }) }, + Some(SpecialTrap::OutputBufferTooSmall) => { + return Err(ExecError { + reason: "output buffer too small".into(), + buffer: runtime.scratch_buf, + }) + }, None => (), } @@ -344,6 +352,37 @@ fn write_sandbox_memory( Ok(()) } +fn write_sandbox_output( + ctx: &mut Runtime, + out_ptr: u32, + out_len_ptr: u32, + buf: &[u8], +) -> Result<(), sp_sandbox::HostError> { + if out_ptr == u32::max_value() { + return Ok(()); + } + + let buf_len = buf.len() as u32; + let len: u32 = read_sandbox_memory_as(ctx, out_len_ptr, 4)?; + + if len < buf_len { + ctx.special_trap = Some(SpecialTrap::OutputBufferTooSmall); + return Err(sp_sandbox::HostError); + } + + charge_gas( + ctx.gas_meter, + ctx.schedule, + &mut ctx.special_trap, + RuntimeToken::WriteMemory(buf_len.saturating_add(4)), + )?; + + ctx.memory.set(out_ptr, buf)?; + ctx.memory.set(out_len_ptr, &buf_len.encode())?; + + Ok(()) +} + // *********************************************************** // * AFTER MAKING A CHANGE MAKE SURE TO UPDATE COMPLEXITY.MD * // *********************************************************** @@ -679,17 +718,13 @@ define_env!(Env, , // If this is a top-level call (i.e. initiated by an extrinsic) the origin address of the // extrinsic will be returned. Otherwise, if this call is initiated by another contract then the // address of the contract will be returned. - ext_caller(ctx) => { - ctx.scratch_buf.clear(); - ctx.ext.caller().encode_to(&mut ctx.scratch_buf); - Ok(()) + ext_caller(ctx, out_ptr: u32, out_len_ptr: u32) => { + write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.caller().encode()) }, // Stores the address of the current contract into the scratch buffer. - ext_address(ctx) => { - ctx.scratch_buf.clear(); - ctx.ext.address().encode_to(&mut ctx.scratch_buf); - Ok(()) + ext_address(ctx, out_ptr: u32, out_len_ptr: u32) => { + write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.address().encode()) }, // Stores the price for the specified amount of gas in scratch buffer. @@ -697,37 +732,29 @@ define_env!(Env, , // The data is encoded as T::Balance. The current contents of the scratch buffer are overwritten. // It is recommended to avoid specifying very small values for `gas` as the prices for a single // gas can be smaller than one. - ext_gas_price(ctx, gas: u64) => { - ctx.scratch_buf.clear(); - ctx.ext.get_weight_price(gas).encode_to(&mut ctx.scratch_buf); - Ok(()) + ext_gas_price(ctx, gas: u64, out_ptr: u32, out_len_ptr: u32) => { + write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.get_weight_price(gas).encode()) }, // Stores the amount of gas left into the scratch buffer. // // The data is encoded as Gas. The current contents of the scratch buffer are overwritten. - ext_gas_left(ctx) => { - ctx.scratch_buf.clear(); - ctx.gas_meter.gas_left().encode_to(&mut ctx.scratch_buf); - Ok(()) + ext_gas_left(ctx, out_ptr: u32, out_len_ptr: u32) => { + write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.gas_meter.gas_left().encode()) }, // Stores the balance of the current account into the scratch buffer. // // The data is encoded as T::Balance. The current contents of the scratch buffer are overwritten. - ext_balance(ctx) => { - ctx.scratch_buf.clear(); - ctx.ext.balance().encode_to(&mut ctx.scratch_buf); - Ok(()) + ext_balance(ctx, out_ptr: u32, out_len_ptr: u32) => { + write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.balance().encode()) }, // Stores the value transferred along with this call or as endowment into the scratch buffer. // // The data is encoded as T::Balance. The current contents of the scratch buffer are overwritten. - ext_value_transferred(ctx) => { - ctx.scratch_buf.clear(); - ctx.ext.value_transferred().encode_to(&mut ctx.scratch_buf); - Ok(()) + ext_value_transferred(ctx, out_ptr: u32, out_len_ptr: u32) => { + write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.value_transferred().encode()) }, // Stores the random number for the current block for the given subject into the scratch @@ -735,33 +762,26 @@ define_env!(Env, , // // The data is encoded as T::Hash. The current contents of the scratch buffer are // overwritten. - ext_random(ctx, subject_ptr: u32, subject_len: u32) => { + ext_random(ctx, subject_ptr: u32, subject_len: u32, out_ptr: u32, out_len_ptr: u32) => { // The length of a subject can't exceed `max_subject_len`. if subject_len > ctx.schedule.max_subject_len { return Err(sp_sandbox::HostError); } - let subject_buf = read_sandbox_memory(ctx, subject_ptr, subject_len)?; - ctx.scratch_buf.clear(); - ctx.ext.random(&subject_buf).encode_to(&mut ctx.scratch_buf); - Ok(()) + write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.random(&subject_buf).encode()) }, // Load the latest block timestamp into the scratch buffer - ext_now(ctx) => { - ctx.scratch_buf.clear(); - ctx.ext.now().encode_to(&mut ctx.scratch_buf); - Ok(()) + ext_now(ctx, out_ptr: u32, out_len_ptr: u32) => { + write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.now().encode()) }, // Stores the minimum balance (a.k.a. existential deposit) into the scratch buffer. // // The data is encoded as T::Balance. The current contents of the scratch buffer are // overwritten. - ext_minimum_balance(ctx) => { - ctx.scratch_buf.clear(); - ctx.ext.minimum_balance().encode_to(&mut ctx.scratch_buf); - Ok(()) + ext_minimum_balance(ctx, out_ptr: u32, out_len_ptr: u32) => { + write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.minimum_balance().encode()) }, // Stores the tombstone deposit into the scratch buffer. @@ -775,10 +795,8 @@ define_env!(Env, , // a contract to leave a tombstone the balance of the contract must not go // below the sum of existential deposit and the tombstone deposit. The sum // is commonly referred as subsistence threshold in code. - ext_tombstone_deposit(ctx) => { - ctx.scratch_buf.clear(); - ctx.ext.tombstone_deposit().encode_to(&mut ctx.scratch_buf); - Ok(()) + ext_tombstone_deposit(ctx, out_ptr: u32, out_len_ptr: u32) => { + write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.tombstone_deposit().encode()) }, // Try to restore the given destination contract sacrificing the caller. @@ -952,11 +970,8 @@ define_env!(Env, , // Stores the rent allowance into the scratch buffer. // // The data is encoded as T::Balance. The current contents of the scratch buffer are overwritten. - ext_rent_allowance(ctx) => { - ctx.scratch_buf.clear(); - ctx.ext.rent_allowance().encode_to(&mut ctx.scratch_buf); - - Ok(()) + ext_rent_allowance(ctx, out_ptr: u32, out_len_ptr: u32) => { + write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.rent_allowance().encode()) }, // Prints utf8 encoded string from the data buffer. @@ -971,10 +986,8 @@ define_env!(Env, , }, // Stores the current block number of the current contract into the scratch buffer. - ext_block_number(ctx) => { - ctx.scratch_buf.clear(); - ctx.ext.block_number().encode_to(&mut ctx.scratch_buf); - Ok(()) + ext_block_number(ctx, out_ptr: u32, out_len_ptr: u32) => { + write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.block_number().encode()) }, // Computes the SHA2 256-bit hash on the given input buffer. From 92fe839a9652c673b9ebbbd8f1ba541543dec697 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Fri, 26 Jun 2020 13:33:38 +0200 Subject: [PATCH 02/23] Remove scratch buffer from ext_get_storage --- .../fixtures/destroy_and_transfer.wat | 29 ++++++++----- frame/contracts/fixtures/storage_size.wat | 17 ++++++-- frame/contracts/src/tests.rs | 2 +- frame/contracts/src/wasm/mod.rs | 41 ++++++++----------- frame/contracts/src/wasm/runtime.rs | 5 +-- 5 files changed, 53 insertions(+), 41 deletions(-) diff --git a/frame/contracts/fixtures/destroy_and_transfer.wat b/frame/contracts/fixtures/destroy_and_transfer.wat index c8cf7271d7419..451829e9ff20f 100644 --- a/frame/contracts/fixtures/destroy_and_transfer.wat +++ b/frame/contracts/fixtures/destroy_and_transfer.wat @@ -1,12 +1,26 @@ (module (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) - (import "env" "ext_get_storage" (func $ext_get_storage (param i32) (result i32))) + (import "env" "ext_get_storage" (func $ext_get_storage (param i32 i32 i32) (result i32))) (import "env" "ext_set_storage" (func $ext_set_storage (param i32 i32 i32))) (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32) (result i32))) (import "env" "ext_instantiate" (func $ext_instantiate (param i32 i32 i64 i32 i32 i32 i32) (result i32))) (import "env" "memory" (memory 1 1)) + ;; [0, 8) Endowment to send when creating contract. + (data (i32.const 0) "\00\00\01") + + ;; [8, 16) Value to send when calling contract. + + ;; [16, 48) The key to store the contract address under. + + ;; [48, 80) Buffer where to store the input to the contract + + ;; [80, 88) Buffer where to store the address of the instantiated contract + + ;; [88, 96) Size of the buffer + (data (i32.const 88) "\08") + (func $assert (param i32) (block $ok (br_if $ok @@ -75,21 +89,18 @@ (i32.eq (call $ext_get_storage (i32.const 16) ;; Pointer to the key + (i32.const 80) ;; Pointer to the value + (i32.const 88) ;; Pointer to the len of the value ) (i32.const 0) ) ) (call $assert (i32.eq - (call $ext_scratch_size) + (i32.load (i32.const 88)) (i32.const 8) ) ) - (call $ext_scratch_read - (i32.const 80) ;; The pointer where to store the contract address. - (i32.const 0) ;; Offset from the start of the scratch buffer. - (i32.const 8) ;; Count of bytes to copy. - ) ;; Calling the destination contract with non-empty input data should fail. (call $assert @@ -141,8 +152,4 @@ ) ) ) - - (data (i32.const 0) "\00\00\01") ;; Endowment to send when creating contract. - (data (i32.const 8) "") ;; Value to send when calling contract. - (data (i32.const 16) "") ;; The key to store the contract address under. ) diff --git a/frame/contracts/fixtures/storage_size.wat b/frame/contracts/fixtures/storage_size.wat index 8de9f42ee9748..20472492bacea 100644 --- a/frame/contracts/fixtures/storage_size.wat +++ b/frame/contracts/fixtures/storage_size.wat @@ -1,10 +1,20 @@ (module - (import "env" "ext_get_storage" (func $ext_get_storage (param i32) (result i32))) + (import "env" "ext_get_storage" (func $ext_get_storage (param i32 i32 i32) (result i32))) (import "env" "ext_set_storage" (func $ext_set_storage (param i32 i32 i32))) (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) (import "env" "memory" (memory 16 16)) + ;; [0, 32) storage key + (data (i32.const 0) "\01") + + ;; [32, 36) buffer where input is copied (expected size of storage item) + + ;; [36, 40) size of buffer for ext_get_storage set to max + (data (i32.const 36) "\FF\FF\FF\FF") + + ;; [40, inf) ext_get_storage_buffer + (func $assert (param i32) (block $ok (br_if $ok @@ -41,6 +51,8 @@ (i32.eq (call $ext_get_storage (i32.const 0) ;; Pointer to storage key + (i32.const 40) ;; buffer where to copy result + (i32.const 36) ;; pointer to size of buffer ) (i32.const 0) ) @@ -48,7 +60,7 @@ (call $assert (i32.eq - (call $ext_scratch_size) + (i32.load (i32.const 36)) (i32.load (i32.const 32)) ) ) @@ -56,5 +68,4 @@ (func (export "deploy")) - (data (i32.const 0) "\01") ;; Storage key (32 B) ) diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index 5303375e016ae..0e7aa2e3ae84c 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -1291,7 +1291,7 @@ fn storage_max_value_limit() { Origin::signed(ALICE), BOB, 0, - GAS_LIMIT, + GAS_LIMIT * 2, // we are copying a huge buffer Encode::encode(&self::MaxValueSize::get()), )); diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index 995cdb4b958e0..0261aee3edff9 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -781,12 +781,21 @@ mod tests { const CODE_GET_STORAGE: &str = r#" (module - (import "env" "ext_get_storage" (func $ext_get_storage (param i32) (result i32))) - (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) - (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) + (import "env" "ext_get_storage" (func $ext_get_storage (param i32 i32 i32) (result i32))) (import "env" "ext_return" (func $ext_return (param i32 i32))) (import "env" "memory" (memory 1 1)) + ;; [0, 32) key for get storage + (data (i32.const 0) + "\11\11\11\11\11\11\11\11\11\11\11\11\11\11\11\11" + "\11\11\11\11\11\11\11\11\11\11\11\11\11\11\11\11" + ) + + ;; [32, 36) buffer size = 128 bytes + (data (i32.const 32) "\80") + + ;; [36; inf) buffer where the result is copied + (func $assert (param i32) (block $ok (br_if $ok @@ -799,12 +808,13 @@ mod tests { (func (export "call") (local $buf_size i32) - ;; Load a storage value into the scratch buf. (call $assert (i32.eq (call $ext_get_storage - (i32.const 4) ;; The pointer to the storage key to fetch + (i32.const 0) ;; The pointer to the storage key to fetch + (i32.const 36) ;; Pointer to the output buffer + (i32.const 32) ;; Pointer to the size of the buffer ) ;; Return value 0 means that the value is found and there were @@ -813,19 +823,9 @@ mod tests { ) ) - ;; Find out the size of the scratch buffer + ;; Find out the size of the buffer (set_local $buf_size - (call $ext_scratch_size) - ) - - ;; Copy scratch buffer into this contract memory. - (call $ext_scratch_read - (i32.const 36) ;; The pointer where to store the scratch buffer contents, - ;; 36 = 4 + 32 - (i32.const 0) ;; Offset from the start of the scratch buffer. - (get_local ;; Count of bytes to copy. - $buf_size - ) + (i32.load (i32.const 32)) ) ;; Return the contents of the buffer @@ -839,16 +839,11 @@ mod tests { ) (func (export "deploy")) - - (data (i32.const 4) - "\11\11\11\11\11\11\11\11\11\11\11\11\11\11\11\11" - "\11\11\11\11\11\11\11\11\11\11\11\11\11\11\11\11" - ) ) "#; #[test] - fn get_storage_puts_data_into_scratch_buf() { + fn get_storage_puts_data_into_buf() { let mut mock_ext = MockExt::default(); mock_ext .storage diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index a18e216618f3a..d4bd3a5c9e6c3 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -452,14 +452,13 @@ define_env!(Env, , // // - key_ptr: pointer into the linear memory where the key // of the requested value is placed. - ext_get_storage(ctx, key_ptr: u32) -> u32 => { + ext_get_storage(ctx, key_ptr: u32, out_ptr: u32, out_len_ptr: u32) -> u32 => { let mut key: StorageKey = [0; 32]; read_sandbox_memory_into_buf(ctx, key_ptr, &mut key)?; if let Some(value) = ctx.ext.get_storage(&key) { - ctx.scratch_buf = value; + write_sandbox_output(ctx, out_ptr, out_len_ptr, &value)?; Ok(0) } else { - ctx.scratch_buf.clear(); Ok(1) } }, From e1c0e26940fc3808174bb14c8bd22fa5243017f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Mon, 29 Jun 2020 08:51:58 +0200 Subject: [PATCH 03/23] Remove scratch buffer from ext_call --- frame/contracts/fixtures/caller_contract.wat | 69 +++++++++++-------- .../fixtures/destroy_and_transfer.wat | 9 ++- frame/contracts/fixtures/drain.wat | 6 +- frame/contracts/fixtures/self_destruct.wat | 5 +- .../fixtures/self_destructing_constructor.wat | 6 +- frame/contracts/src/wasm/mod.rs | 18 +++-- frame/contracts/src/wasm/runtime.rs | 18 ++--- 7 files changed, 76 insertions(+), 55 deletions(-) diff --git a/frame/contracts/fixtures/caller_contract.wat b/frame/contracts/fixtures/caller_contract.wat index 30566d9da881a..ded8df085d04d 100644 --- a/frame/contracts/fixtures/caller_contract.wat +++ b/frame/contracts/fixtures/caller_contract.wat @@ -2,7 +2,7 @@ (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) (import "env" "ext_balance" (func $ext_balance (param i32 i32))) - (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32) (result i32))) + (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32 i32 i32) (result i32))) (import "env" "ext_instantiate" (func $ext_instantiate (param i32 i32 i64 i32 i32 i32 i32) (result i32))) (import "env" "ext_println" (func $ext_println (param i32 i32))) (import "env" "memory" (memory 1 1)) @@ -17,12 +17,16 @@ ) (func $current_balance (param $sp i32) (result i64) + (i32.store + (i32.sub (get_local $sp) (i32.const 16)) + (i32.const 8) + ) (call $ext_balance (i32.sub (get_local $sp) (i32.const 8)) - (i32.const 2048) + (i32.sub (get_local $sp) (i32.const 16)) ) (call $assert - (i32.eq (i32.load (i32.const 2048)) (i32.const 8)) + (i32.eq (i32.load (i32.sub (get_local $sp) (i32.const 16))) (i32.const 8)) ) (i64.load (i32.sub (get_local $sp) (i32.const 8))) ) @@ -149,6 +153,18 @@ (i64.eq (get_local $balance) (call $current_balance (get_local $sp))) ) + ;; Zero out destination buffer of output + (i32.store + (i32.sub (get_local $sp) (i32.const 4)) + (i32.const 0) + ) + + ;; Length of the output buffer + (i32.store + (i32.sub (get_local $sp) (i32.const 8)) + (i32.const 4) + ) + ;; Call the new contract and expect it to return failing exit code. (set_local $exit_code (call $ext_call @@ -159,6 +175,8 @@ (i32.const 8) ;; Length of the buffer with value to transfer. (i32.const 9) ;; Pointer to input data buffer address (i32.const 7) ;; Length of input data buffer + (i32.sub (get_local $sp) (i32.const 4)) ;; Ptr to output buffer + (i32.sub (get_local $sp) (i32.const 8)) ;; Ptr to output buffer len ) ) @@ -169,16 +187,7 @@ ;; Check that scratch buffer contains the expected return data. (call $assert - (i32.eq (call $ext_scratch_size) (i32.const 3)) - ) - (i32.store - (i32.sub (get_local $sp) (i32.const 4)) - (i32.const 0) - ) - (call $ext_scratch_read - (i32.sub (get_local $sp) (i32.const 4)) - (i32.const 0) - (i32.const 3) + (i32.eq (i32.load (i32.sub (get_local $sp) (i32.const 8))) (i32.const 3)) ) (call $assert (i32.eq @@ -202,6 +211,8 @@ (i32.const 8) ;; Length of the buffer with value to transfer. (i32.const 8) ;; Pointer to input data buffer address (i32.const 8) ;; Length of input data buffer + (i32.const 4294967295) ;; u32 max sentinel value: do not copy output + (i32.const 0) ;; Length is ignored in this cas ) ) @@ -210,16 +221,23 @@ (i32.eq (get_local $exit_code) (i32.const 0x0100)) ) - ;; Check that scratch buffer is empty since call trapped. - (call $assert - (i32.eq (call $ext_scratch_size) (i32.const 0)) - ) - ;; Check that balance has not changed. (call $assert (i64.eq (get_local $balance) (call $current_balance (get_local $sp))) ) + ;; Zero out destination buffer of output + (i32.store + (i32.sub (get_local $sp) (i32.const 4)) + (i32.const 0) + ) + + ;; Length of the output buffer + (i32.store + (i32.sub (get_local $sp) (i32.const 8)) + (i32.const 4) + ) + ;; Call the contract successfully. (set_local $exit_code (call $ext_call @@ -230,6 +248,8 @@ (i32.const 8) ;; Length of the buffer with value to transfer. (i32.const 8) ;; Pointer to input data buffer address (i32.const 8) ;; Length of input data buffer + (i32.sub (get_local $sp) (i32.const 4)) ;; Ptr to output buffer + (i32.sub (get_local $sp) (i32.const 8)) ;; Ptr to output buffer len ) ) @@ -240,16 +260,7 @@ ;; Check that scratch buffer contains the expected return data. (call $assert - (i32.eq (call $ext_scratch_size) (i32.const 4)) - ) - (i32.store - (i32.sub (get_local $sp) (i32.const 4)) - (i32.const 0) - ) - (call $ext_scratch_read - (i32.sub (get_local $sp) (i32.const 4)) - (i32.const 0) - (i32.const 4) + (i32.eq (i32.load (i32.sub (get_local $sp) (i32.const 8))) (i32.const 4)) ) (call $assert (i32.eq @@ -270,6 +281,4 @@ (data (i32.const 0) "\00\80") ;; The value to transfer on instantiation and calls. ;; Chosen to be greater than existential deposit. (data (i32.const 8) "\00\11\22\33\44\55\66\77") ;; The input data to instantiations and calls. - - (data (i32.const 2048) "\08") ;; size of our $ext_balance output buffer ) diff --git a/frame/contracts/fixtures/destroy_and_transfer.wat b/frame/contracts/fixtures/destroy_and_transfer.wat index 451829e9ff20f..8f08b5306dd0f 100644 --- a/frame/contracts/fixtures/destroy_and_transfer.wat +++ b/frame/contracts/fixtures/destroy_and_transfer.wat @@ -3,7 +3,7 @@ (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) (import "env" "ext_get_storage" (func $ext_get_storage (param i32 i32 i32) (result i32))) (import "env" "ext_set_storage" (func $ext_set_storage (param i32 i32 i32))) - (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32) (result i32))) + (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32 i32 i32) (result i32))) (import "env" "ext_instantiate" (func $ext_instantiate (param i32 i32 i64 i32 i32 i32 i32) (result i32))) (import "env" "memory" (memory 1 1)) @@ -113,6 +113,9 @@ (i32.const 8) ;; Length of the buffer with value to transfer (i32.const 0) ;; Pointer to input data buffer address (i32.const 1) ;; Length of input data buffer + (i32.const 4294967295) ;; u32 max sentinel value: do not copy output + (i32.const 0) ;; Length is ignored in this case + ) (i32.const 0x0100) ) @@ -129,6 +132,8 @@ (i32.const 8) ;; Length of the buffer with value to transfer (i32.const 0) ;; Pointer to input data buffer address (i32.const 0) ;; Length of input data buffer + (i32.const 4294967295) ;; u32 max sentinel value: do not copy output + (i32.const 0) ;; Length is ignored in this case ) (i32.const 0) ) @@ -147,6 +152,8 @@ (i32.const 8) ;; Length of the buffer with value to transfer (i32.const 0) ;; Pointer to input data buffer address (i32.const 1) ;; Length of input data buffer + (i32.const 4294967295) ;; u32 max sentinel value: do not copy output + (i32.const 0) ;; Length is ignored in this case ) (i32.const 0) ) diff --git a/frame/contracts/fixtures/drain.wat b/frame/contracts/fixtures/drain.wat index 77119affade63..1b3172b2a01a4 100644 --- a/frame/contracts/fixtures/drain.wat +++ b/frame/contracts/fixtures/drain.wat @@ -1,8 +1,6 @@ (module - (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) - (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) (import "env" "ext_balance" (func $ext_balance (param i32 i32))) - (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32) (result i32))) + (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32 i32 i32) (result i32))) (import "env" "memory" (memory 1 1)) ;; [0, 8) reserved for $ext_balance output @@ -46,6 +44,8 @@ (i32.const 8) ;; Length of the buffer with value to transfer (i32.const 0) ;; Pointer to input data buffer address (i32.const 0) ;; Length of input data buffer + (i32.const 4294967295) ;; u32 max sentinel value: do not copy output + (i32.const 0) ;; Length is ignored in this case ) (i32.const 0) ) diff --git a/frame/contracts/fixtures/self_destruct.wat b/frame/contracts/fixtures/self_destruct.wat index 65ddb1cabceec..52146d06e66ee 100644 --- a/frame/contracts/fixtures/self_destruct.wat +++ b/frame/contracts/fixtures/self_destruct.wat @@ -1,8 +1,7 @@ (module (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) - (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) (import "env" "ext_address" (func $ext_address (param i32 i32))) - (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32) (result i32))) + (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32 i32 i32) (result i32))) (import "env" "ext_terminate" (func $ext_terminate (param i32 i32))) (import "env" "memory" (memory 1 1)) @@ -55,6 +54,8 @@ (i32.const 8) ;; Length of the buffer with value to transfer (i32.const 0) ;; Pointer to input data buffer address (i32.const 0) ;; Length of input data buffer + (i32.const 4294967295) ;; u32 max sentinel value: do not copy output + (i32.const 0) ;; Length is ignored in this case ) (i32.const 0) ) diff --git a/frame/contracts/fixtures/self_destructing_constructor.wat b/frame/contracts/fixtures/self_destructing_constructor.wat index 14234108dfcae..3b99db001cd38 100644 --- a/frame/contracts/fixtures/self_destructing_constructor.wat +++ b/frame/contracts/fixtures/self_destructing_constructor.wat @@ -1,8 +1,6 @@ (module - (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) - (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) (import "env" "ext_balance" (func $ext_balance (param i32 i32))) - (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32) (result i32))) + (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32 i32 i32) (result i32))) (import "env" "memory" (memory 1 1)) ;; [0, 8) reserved for $ext_balance output @@ -44,6 +42,8 @@ (i32.const 8) ;; Length of the buffer with value to transfer (i32.const 0) ;; Pointer to input data buffer address (i32.const 0) ;; Length of input data buffer + (i32.const 4294967295) ;; u32 max sentinel value: do not copy output + (i32.const 0) ;; Length is ignored in this case ) (i32.const 0) ) diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index 0261aee3edff9..8f9c5d807ad0f 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -565,9 +565,11 @@ mod tests { ;; value_ptr: u32, ;; value_len: u32, ;; input_data_ptr: u32, - ;; input_data_len: u32 + ;; input_data_len: u32, + ;; result_ptr: u32, + ;; result_len_ptr: u32 ;;) -> u32 - (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32) (result i32))) + (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32 i32 i32) (result i32))) (import "env" "memory" (memory 1 1)) (func (export "call") (drop @@ -579,6 +581,8 @@ mod tests { (i32.const 8) ;; Length of the buffer with value to transfer. (i32.const 20) ;; Pointer to input data buffer address (i32.const 4) ;; Length of input data buffer + (i32.const 4294967295) ;; u32 max value is the sentinel value: do not copy output + (i32.const 0) ;; Length is ignored in this case ) ) ) @@ -611,7 +615,7 @@ mod tests { to: 9, value: 6, data: vec![1, 2, 3, 4], - gas_left: 9985500000, + gas_left: 9984500000, }] ); } @@ -728,9 +732,11 @@ mod tests { ;; value_ptr: u32, ;; value_len: u32, ;; input_data_ptr: u32, - ;; input_data_len: u32 + ;; input_data_len: u32, + ;; result_ptr: u32, + ;; result_len_ptr: u32 ;;) -> u32 - (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32) (result i32))) + (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32 i32 i32) (result i32))) (import "env" "memory" (memory 1 1)) (func (export "call") (drop @@ -742,6 +748,8 @@ mod tests { (i32.const 8) ;; Length of the buffer with value to transfer. (i32.const 20) ;; Pointer to input data buffer address (i32.const 4) ;; Length of input data buffer + (i32.const 4294967295) ;; u32 max value is the sentinel value: do not copy output + (i32.const 0) ;; Length is ignored in this cas ) ) ) diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index d4bd3a5c9e6c3..8feeb21a5ae83 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -525,16 +525,14 @@ define_env!(Env, , value_ptr: u32, value_len: u32, input_data_ptr: u32, - input_data_len: u32 + input_data_len: u32, + result_ptr: u32, + result_len_ptr: u32 ) -> u32 => { let callee: <::T as frame_system::Trait>::AccountId = read_sandbox_memory_as(ctx, callee_ptr, callee_len)?; - let value: BalanceOf<::T> = - read_sandbox_memory_as(ctx, value_ptr, value_len)?; - - // Read input data into the scratch buffer, then take ownership of it. - read_sandbox_memory_into_scratch(ctx, input_data_ptr, input_data_len)?; - let input_data = mem::replace(&mut ctx.scratch_buf, Vec::new()); + let value: BalanceOf<::T> = read_sandbox_memory_as(ctx, value_ptr, value_len)?; + let input_data = read_sandbox_memory(ctx, input_data_ptr, input_data_len)?; let nested_gas_limit = if gas == 0 { ctx.gas_meter.gas_left() @@ -560,12 +558,10 @@ define_env!(Env, , match call_outcome { Ok(output) => { - ctx.scratch_buf = output.data; + write_sandbox_output(ctx, result_ptr, result_len_ptr, &output.data)?; Ok(output.status.into()) }, - Err(buffer) => { - ctx.scratch_buf = buffer; - ctx.scratch_buf.clear(); + Err(_) => { Ok(TRAP_RETURN_CODE) }, } From 6f94941314a37fa6e0b8938e0dddc7992ece5c88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Mon, 29 Jun 2020 10:30:28 +0200 Subject: [PATCH 04/23] Remove scratch buffer from ext_instantiate --- frame/contracts/fixtures/caller_contract.wat | 42 ++++++++++--------- .../fixtures/destroy_and_transfer.wat | 15 ++++--- frame/contracts/src/wasm/mod.rs | 21 +++++++--- frame/contracts/src/wasm/runtime.rs | 33 +++++++-------- 4 files changed, 58 insertions(+), 53 deletions(-) diff --git a/frame/contracts/fixtures/caller_contract.wat b/frame/contracts/fixtures/caller_contract.wat index ded8df085d04d..5b40f7d9c4622 100644 --- a/frame/contracts/fixtures/caller_contract.wat +++ b/frame/contracts/fixtures/caller_contract.wat @@ -3,7 +3,7 @@ (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) (import "env" "ext_balance" (func $ext_balance (param i32 i32))) (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32 i32 i32) (result i32))) - (import "env" "ext_instantiate" (func $ext_instantiate (param i32 i32 i64 i32 i32 i32 i32) (result i32))) + (import "env" "ext_instantiate" (func $ext_instantiate (param i32 i32 i64 i32 i32 i32 i32 i32 i32 i32 i32) (result i32))) (import "env" "ext_println" (func $ext_println (param i32 i32))) (import "env" "memory" (memory 1 1)) @@ -69,6 +69,10 @@ (i32.const 8) ;; Length of the buffer with value to transfer. (i32.const 9) ;; Pointer to input data buffer address (i32.const 7) ;; Length of input data buffer + (i32.const 4294967295) ;; u32 max sentinel value: do not copy address + (i32.const 0) ;; Length is ignored in this case + (i32.const 4294967295) ;; u32 max sentinel value: do not copy output + (i32.const 0) ;; Length is ignored in this case ) ) @@ -77,11 +81,6 @@ (i32.eq (get_local $exit_code) (i32.const 0x11)) ) - ;; Check that scratch buffer is empty since contract instantiation failed. - (call $assert - (i32.eq (call $ext_scratch_size) (i32.const 0)) - ) - ;; Check that balance has not changed. (call $assert (i64.eq (get_local $balance) (call $current_balance (get_local $sp))) @@ -97,6 +96,10 @@ (i32.const 8) ;; Length of the buffer with value to transfer. (i32.const 8) ;; Pointer to input data buffer address (i32.const 8) ;; Length of input data buffer + (i32.const 4294967295) ;; u32 max sentinel value: do not copy address + (i32.const 0) ;; Length is ignored in this case + (i32.const 4294967295) ;; u32 max sentinel value: do not copy output + (i32.const 0) ;; Length is ignored in this case ) ) @@ -105,16 +108,17 @@ (i32.eq (get_local $exit_code) (i32.const 0x0100)) ) - ;; Check that scratch buffer is empty since contract instantiation failed. - (call $assert - (i32.eq (call $ext_scratch_size) (i32.const 0)) - ) - ;; Check that balance has not changed. (call $assert (i64.eq (get_local $balance) (call $current_balance (get_local $sp))) ) + ;; Length of the output buffer + (i32.store + (i32.sub (get_local $sp) (i32.const 4)) + (i32.const 8) + ) + ;; Deploy the contract successfully. (set_local $exit_code (call $ext_instantiate @@ -125,6 +129,11 @@ (i32.const 8) ;; Length of the buffer with value to transfer. (i32.const 8) ;; Pointer to input data buffer address (i32.const 8) ;; Length of input data buffer + (i32.const 16) ;; Pointer to the address output buffer + (i32.sub (get_local $sp) (i32.const 4)) ;; Pointer to the address buffer length + (i32.const 4294967295) ;; u32 max sentinel value: do not copy output + (i32.const 0) ;; Length is ignored in this case + ) ) @@ -133,16 +142,9 @@ (i32.eq (get_local $exit_code) (i32.const 0x00)) ) - ;; Check that scratch buffer contains the address of the new contract. + ;; Check that address has the expected length (call $assert - (i32.eq (call $ext_scratch_size) (i32.const 8)) - ) - - ;; Copy contract address from scratch buffer into this contract's memory. - (call $ext_scratch_read - (i32.const 16) ;; The pointer where to store the scratch buffer contents, - (i32.const 0) ;; Offset from the start of the scratch buffer. - (i32.const 8) ;; Count of bytes to copy. + (i32.eq (i32.load (i32.sub (get_local $sp) (i32.const 4))) (i32.const 8)) ) ;; Check that balance has been deducted. diff --git a/frame/contracts/fixtures/destroy_and_transfer.wat b/frame/contracts/fixtures/destroy_and_transfer.wat index 8f08b5306dd0f..ea88312660f29 100644 --- a/frame/contracts/fixtures/destroy_and_transfer.wat +++ b/frame/contracts/fixtures/destroy_and_transfer.wat @@ -4,7 +4,7 @@ (import "env" "ext_get_storage" (func $ext_get_storage (param i32 i32 i32) (result i32))) (import "env" "ext_set_storage" (func $ext_set_storage (param i32 i32 i32))) (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32 i32 i32) (result i32))) - (import "env" "ext_instantiate" (func $ext_instantiate (param i32 i32 i64 i32 i32 i32 i32) (result i32))) + (import "env" "ext_instantiate" (func $ext_instantiate (param i32 i32 i64 i32 i32 i32 i32 i32 i32 i32 i32) (result i32))) (import "env" "memory" (memory 1 1)) ;; [0, 8) Endowment to send when creating contract. @@ -57,23 +57,22 @@ (i32.const 8) ;; Length of the buffer with value to transfer. (i32.const 0) ;; Pointer to input data buffer address (i32.const 0) ;; Length of input data buffer + (i32.const 80) ;; Buffer where to store address of new contratc + (i32.const 88) ;; Pointer to the length of the buffer + (i32.const 4294967295) ;; u32 max sentinel value: do not copy output + (i32.const 0) ;; Length is ignored in this cas ) (i32.const 0) ) ) - ;; Read the address of the instantiated contract into memory. + ;; Check that address has expected length (call $assert (i32.eq - (call $ext_scratch_size) + (i32.load (i32.const 88)) (i32.const 8) ) ) - (call $ext_scratch_read - (i32.const 80) ;; The pointer where to store the scratch buffer contents, - (i32.const 0) ;; Offset from the start of the scratch buffer. - (i32.const 8) ;; Count of bytes to copy. - ) ;; Store the return address. (call $ext_set_storage diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index 8f9c5d807ad0f..a443c6cf75389 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -566,8 +566,8 @@ mod tests { ;; value_len: u32, ;; input_data_ptr: u32, ;; input_data_len: u32, - ;; result_ptr: u32, - ;; result_len_ptr: u32 + ;; output_ptr: u32, + ;; output_len_ptr: u32 ;;) -> u32 (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32 i32 i32) (result i32))) (import "env" "memory" (memory 1 1)) @@ -630,8 +630,13 @@ mod tests { ;; value_len: u32, ;; input_data_ptr: u32, ;; input_data_len: u32, + ;; input_data_len: u32, + ;; address_ptr: u32, + ;; address_len_ptr: u32, + ;; output_ptr: u32, + ;; output_len_ptr: u32 ;; ) -> u32 - (import "env" "ext_instantiate" (func $ext_instantiate (param i32 i32 i64 i32 i32 i32 i32) (result i32))) + (import "env" "ext_instantiate" (func $ext_instantiate (param i32 i32 i64 i32 i32 i32 i32 i32 i32 i32 i32) (result i32))) (import "env" "memory" (memory 1 1)) (func (export "call") (drop @@ -643,6 +648,10 @@ mod tests { (i32.const 8) ;; Length of the buffer with value to transfer (i32.const 12) ;; Pointer to input data buffer address (i32.const 4) ;; Length of input data buffer + (i32.const 4294967295) ;; u32 max value is the sentinel value: do not copy output + (i32.const 0) ;; Length is ignored in this case + (i32.const 4294967295) ;; u32 max value is the sentinel value: do not copy output + (i32.const 0) ;; Length is ignored in this case ) ) ) @@ -677,7 +686,7 @@ mod tests { code_hash: [0x11; 32].into(), endowment: 3, data: vec![1, 2, 3, 4], - gas_left: 9973500000, + gas_left: 9971500000, }] ); } @@ -733,8 +742,8 @@ mod tests { ;; value_len: u32, ;; input_data_ptr: u32, ;; input_data_len: u32, - ;; result_ptr: u32, - ;; result_len_ptr: u32 + ;; output_ptr: u32, + ;; output_len_ptr: u32 ;;) -> u32 (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32 i32 i32) (result i32))) (import "env" "memory" (memory 1 1)) diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index 8feeb21a5ae83..a51e6878a75ec 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -526,8 +526,8 @@ define_env!(Env, , value_len: u32, input_data_ptr: u32, input_data_len: u32, - result_ptr: u32, - result_len_ptr: u32 + output_ptr: u32, + output_len_ptr: u32 ) -> u32 => { let callee: <::T as frame_system::Trait>::AccountId = read_sandbox_memory_as(ctx, callee_ptr, callee_len)?; @@ -558,7 +558,7 @@ define_env!(Env, , match call_outcome { Ok(output) => { - write_sandbox_output(ctx, result_ptr, result_len_ptr, &output.data)?; + write_sandbox_output(ctx, output_ptr, output_len_ptr, &output.data)?; Ok(output.status.into()) }, Err(_) => { @@ -610,16 +610,16 @@ define_env!(Env, , value_ptr: u32, value_len: u32, input_data_ptr: u32, - input_data_len: u32 + input_data_len: u32, + address_ptr: u32, + address_len_ptr: u32, + output_ptr: u32, + output_len_ptr: u32 ) -> u32 => { let code_hash: CodeHash<::T> = read_sandbox_memory_as(ctx, code_hash_ptr, code_hash_len)?; - let value: BalanceOf<::T> = - read_sandbox_memory_as(ctx, value_ptr, value_len)?; - - // Read input data into the scratch buffer, then take ownership of it. - read_sandbox_memory_into_scratch(ctx, input_data_ptr, input_data_len)?; - let input_data = mem::replace(&mut ctx.scratch_buf, Vec::new()); + let value: BalanceOf<::T> = read_sandbox_memory_as(ctx, value_ptr, value_len)?; + let input_data = read_sandbox_memory(ctx, input_data_ptr, input_data_len)?; let nested_gas_limit = if gas == 0 { ctx.gas_meter.gas_left() @@ -644,18 +644,13 @@ define_env!(Env, , }); match instantiate_outcome { Ok((address, output)) => { - let is_success = output.is_success(); - ctx.scratch_buf = output.data; - ctx.scratch_buf.clear(); - if is_success { - // Write the address to the scratch buffer. - address.encode_to(&mut ctx.scratch_buf); + if output.is_success() { + write_sandbox_output(ctx, address_ptr, address_len_ptr, &address.encode())?; + write_sandbox_output(ctx, output_ptr, output_len_ptr, &output.data)?; } Ok(output.status.into()) }, - Err(buffer) => { - ctx.scratch_buf = buffer; - ctx.scratch_buf.clear(); + Err(_) => { Ok(TRAP_RETURN_CODE) }, } From 0b27908074c798dce24ec54fea5bffaee5b5af5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Mon, 29 Jun 2020 11:14:11 +0200 Subject: [PATCH 05/23] Add ext_input and remove scratch buffer --- bin/node/executor/tests/basic.rs | 29 +-- frame/contracts/fixtures/caller_contract.wat | 18 +- .../fixtures/check_default_rent_allowance.wat | 2 - frame/contracts/fixtures/crypto_hashes.wat | 22 +-- .../fixtures/destroy_and_transfer.wat | 20 +-- frame/contracts/fixtures/restoration.wat | 30 +++- .../fixtures/return_from_start_fn.wat | 3 +- frame/contracts/fixtures/return_with_data.wat | 34 ++-- frame/contracts/fixtures/self_destruct.wat | 14 +- frame/contracts/fixtures/set_rent.wat | 21 ++- frame/contracts/fixtures/storage_size.wat | 31 ++-- frame/contracts/src/exec.rs | 37 ++-- frame/contracts/src/tests.rs | 2 +- frame/contracts/src/wasm/mod.rs | 110 +++--------- frame/contracts/src/wasm/runtime.rs | 170 +++++------------- 15 files changed, 202 insertions(+), 341 deletions(-) diff --git a/bin/node/executor/tests/basic.rs b/bin/node/executor/tests/basic.rs index e4de98d90e94d..9ee7824e51982 100644 --- a/bin/node/executor/tests/basic.rs +++ b/bin/node/executor/tests/basic.rs @@ -491,32 +491,31 @@ const CODE_TRANSFER: &str = r#" ;; value_ptr: u32, ;; value_len: u32, ;; input_data_ptr: u32, -;; input_data_len: u32 +;; input_data_len: u32, +;; output_ptr: u32, +;; output_len_ptr: u32 ;; ) -> u32 -(import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32) (result i32))) -(import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) -(import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) +(import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32 i32 i32) (result i32))) +(import "env" "ext_input" (func $ext_input (param i32 i32))) (import "env" "memory" (memory 1 1)) (func (export "deploy") ) (func (export "call") (block $fail - ;; load and check the input data (which is stored in the scratch buffer). + ;; Load input data to contract memory + (call $ext_input + (i32.const 0) + (i32.const 52) + ) + ;; fail if the input size is not != 4 (br_if $fail (i32.ne (i32.const 4) - (call $ext_scratch_size) + (i32.load (i32.const 52)) ) ) - (call $ext_scratch_read - (i32.const 0) - (i32.const 0) - (i32.const 4) - ) - - (br_if $fail (i32.ne (i32.load8_u (i32.const 0)) @@ -551,6 +550,8 @@ const CODE_TRANSFER: &str = r#" (i32.const 16) ;; Length of the buffer with value to transfer. (i32.const 0) ;; Pointer to input data buffer address (i32.const 0) ;; Length of input data buffer + (i32.const 4294967295) ;; u32 max value is the sentinel value: do not copy output + (i32.const 0) ;; Length is ignored in this case ) ) @@ -571,6 +572,8 @@ const CODE_TRANSFER: &str = r#" "\06\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00" "\00\00" ) +;; Length of the input buffer +(data (i32.const 52) "\04") ) "#; diff --git a/frame/contracts/fixtures/caller_contract.wat b/frame/contracts/fixtures/caller_contract.wat index 5b40f7d9c4622..34933cdb768c0 100644 --- a/frame/contracts/fixtures/caller_contract.wat +++ b/frame/contracts/fixtures/caller_contract.wat @@ -1,6 +1,5 @@ (module - (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) - (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) + (import "env" "ext_input" (func $ext_input (param i32 i32))) (import "env" "ext_balance" (func $ext_balance (param i32 i32))) (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32 i32 i32) (result i32))) (import "env" "ext_instantiate" (func $ext_instantiate (param i32 i32 i64 i32 i32 i32 i32 i32 i32 i32 i32) (result i32))) @@ -38,21 +37,20 @@ (local $exit_code i32) (local $balance i64) + ;; Length of the buffer + (i32.store (i32.const 20) (i32.const 32)) + + ;; Copy input to this contracts memory + (call $ext_input (i32.const 24) (i32.const 20)) + ;; Input data is the code hash of the contract to be deployed. (call $assert (i32.eq - (call $ext_scratch_size) + (i32.load (i32.const 20)) (i32.const 32) ) ) - ;; Copy code hash from scratch buffer into this contract's memory. - (call $ext_scratch_read - (i32.const 24) ;; The pointer where to store the scratch buffer contents, - (i32.const 0) ;; Offset from the start of the scratch buffer. - (i32.const 32) ;; Count of bytes to copy. - ) - ;; Read current balance into local variable. (set_local $sp (i32.const 1024)) (set_local $balance diff --git a/frame/contracts/fixtures/check_default_rent_allowance.wat b/frame/contracts/fixtures/check_default_rent_allowance.wat index 08e160d5d7e0c..b3076a04325e3 100644 --- a/frame/contracts/fixtures/check_default_rent_allowance.wat +++ b/frame/contracts/fixtures/check_default_rent_allowance.wat @@ -1,7 +1,5 @@ (module (import "env" "ext_rent_allowance" (func $ext_rent_allowance (param i32 i32))) - (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) - (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) (import "env" "memory" (memory 1 1)) ;; [0, 8) reserved for $ext_rent_allowance output diff --git a/frame/contracts/fixtures/crypto_hashes.wat b/frame/contracts/fixtures/crypto_hashes.wat index 6dbca33928cb7..7e86d2b630de3 100644 --- a/frame/contracts/fixtures/crypto_hashes.wat +++ b/frame/contracts/fixtures/crypto_hashes.wat @@ -1,7 +1,6 @@ (module - (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) - (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) - (import "env" "ext_scratch_write" (func $ext_scratch_write (param i32 i32))) + (import "env" "ext_input" (func $ext_input (param i32 i32))) + (import "env" "ext_return" (func $ext_return (param i32 i32 i32))) (import "env" "ext_hash_sha2_256" (func $ext_hash_sha2_256 (param i32 i32 i32))) (import "env" "ext_hash_keccak_256" (func $ext_hash_keccak_256 (param i32 i32 i32))) @@ -50,31 +49,34 @@ ;; --------------------------------- (func (export "call") (result i32) (local $chosen_hash_fn i32) + (local $input_len_ptr i32) (local $input_ptr i32) (local $input_len i32) (local $output_ptr i32) (local $output_len i32) + (local.set $input_len_ptr (i32.const 256)) (local.set $input_ptr (i32.const 10)) - (call $ext_scratch_read (local.get $input_ptr) (i32.const 0) (call $ext_scratch_size)) + (i32.store (local.get $input_len_ptr) (i32.const 246)) + (call $ext_input (local.get $input_ptr) (local.get $input_len_ptr)) (local.set $chosen_hash_fn (i32.load8_u (local.get $input_ptr))) (if (i32.gt_u (local.get $chosen_hash_fn) (i32.const 7)) ;; We check that the chosen hash fn identifier is within bounds: [0,7] (unreachable) ) (local.set $input_ptr (i32.add (local.get $input_ptr) (i32.const 1))) - (local.set $input_len (i32.sub (call $ext_scratch_size) (i32.const 1))) - (local.set $output_ptr (i32.const 100)) + (local.set $input_len (i32.sub (i32.load (local.get $input_len_ptr)) (i32.const 1))) (local.set $output_len (i32.load8_u (local.get $chosen_hash_fn))) (call_indirect (type $hash_fn_sig) (local.get $input_ptr) (local.get $input_len) - (local.get $output_ptr) + (local.get $input_ptr) (local.get $chosen_hash_fn) ;; Which crypto hash function to execute. ) - (call $ext_scratch_write - (local.get $output_ptr) ;; Linear memory location of the output buffer. + (call $ext_return + (i32.const 0) + (local.get $input_ptr) ;; Linear memory location of the output buffer. (local.get $output_len) ;; Number of output buffer bytes. ) - (i32.const 0) + (unreachable) ) ) diff --git a/frame/contracts/fixtures/destroy_and_transfer.wat b/frame/contracts/fixtures/destroy_and_transfer.wat index ea88312660f29..71350664a3f34 100644 --- a/frame/contracts/fixtures/destroy_and_transfer.wat +++ b/frame/contracts/fixtures/destroy_and_transfer.wat @@ -1,6 +1,5 @@ (module - (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) - (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) + (import "env" "ext_input" (func $ext_input (param i32 i32))) (import "env" "ext_get_storage" (func $ext_get_storage (param i32 i32 i32) (result i32))) (import "env" "ext_set_storage" (func $ext_set_storage (param i32 i32 i32))) (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32 i32 i32) (result i32))) @@ -21,6 +20,9 @@ ;; [88, 96) Size of the buffer (data (i32.const 88) "\08") + ;; [96, 100) Size of the input buffer + (data (i32.const 96) "\20") + (func $assert (param i32) (block $ok (br_if $ok @@ -31,21 +33,15 @@ ) (func (export "deploy") - ;; Input data is the code hash of the contract to be deployed. + ;; Input data is the code hash of the contract to be deployed. + (call $ext_input (i32.const 48) (i32.const 96)) (call $assert (i32.eq - (call $ext_scratch_size) + (i32.load (i32.const 96)) (i32.const 32) ) ) - ;; Copy code hash from scratch buffer into this contract's memory. - (call $ext_scratch_read - (i32.const 48) ;; The pointer where to store the scratch buffer contents, - (i32.const 0) ;; Offset from the start of the scratch buffer. - (i32.const 32) ;; Count of bytes to copy. - ) - ;; Deploy the contract with the provided code hash. (call $assert (i32.eq @@ -57,7 +53,7 @@ (i32.const 8) ;; Length of the buffer with value to transfer. (i32.const 0) ;; Pointer to input data buffer address (i32.const 0) ;; Length of input data buffer - (i32.const 80) ;; Buffer where to store address of new contratc + (i32.const 80) ;; Buffer where to store address of new contract (i32.const 88) ;; Pointer to the length of the buffer (i32.const 4294967295) ;; u32 max sentinel value: do not copy output (i32.const 0) ;; Length is ignored in this cas diff --git a/frame/contracts/fixtures/restoration.wat b/frame/contracts/fixtures/restoration.wat index 07e11e9d38146..4107587ada7f5 100644 --- a/frame/contracts/fixtures/restoration.wat +++ b/frame/contracts/fixtures/restoration.wat @@ -1,5 +1,6 @@ (module (import "env" "ext_set_storage" (func $ext_set_storage (param i32 i32 i32))) + (import "env" "ext_input" (func $ext_input (param i32 i32))) (import "env" "ext_restore_to" (func $ext_restore_to (param i32 i32 i32 i32 i32 i32 i32 i32) @@ -7,7 +8,25 @@ ) (import "env" "memory" (memory 1 1)) + (func $assert (param i32) + (block $ok + (br_if $ok + (get_local 0) + ) + (unreachable) + ) + ) + (func (export "call") + ;; copy code hash to contract memory + (call $ext_input (i32.const 264) (i32.const 304)) + (call $assert + (i32.eq + (i32.load (i32.const 304)) + (i32.const 32) + ) + ) + (call $ext_restore_to ;; Pointer and length of the encoded dest buffer. (i32.const 256) @@ -49,12 +68,11 @@ ;; Address of bob (data (i32.const 256) "\02\00\00\00\00\00\00\00") - ;; Code hash of SET_RENT - (data (i32.const 264) - "\ab\d6\58\65\1e\83\6e\4a\18\0d\f2\6d\bc\42\ba\e9" - "\3d\64\76\e5\30\5b\33\46\bb\4d\43\99\38\21\ee\32" - ) + ;; [264, 296) Code hash of SET_RENT (copied here by ext_input) - ;; Rent allowance + ;; [296, 304) Rent allowance (data (i32.const 296) "\32\00\00\00\00\00\00\00") + + ;; [304, 308) Size of SET_RENT buffer + (data (i32.const 304) "\20") ) diff --git a/frame/contracts/fixtures/return_from_start_fn.wat b/frame/contracts/fixtures/return_from_start_fn.wat index ac898d4d944e9..ba73ef25ed39a 100644 --- a/frame/contracts/fixtures/return_from_start_fn.wat +++ b/frame/contracts/fixtures/return_from_start_fn.wat @@ -1,5 +1,5 @@ (module - (import "env" "ext_return" (func $ext_return (param i32 i32))) + (import "env" "ext_return" (func $ext_return (param i32 i32 i32))) (import "env" "ext_deposit_event" (func $ext_deposit_event (param i32 i32 i32 i32))) (import "env" "memory" (memory 1 1)) @@ -12,6 +12,7 @@ (i32.const 4) ;; The data buffer's length ) (call $ext_return + (i32.const 0) (i32.const 8) (i32.const 4) ) diff --git a/frame/contracts/fixtures/return_with_data.wat b/frame/contracts/fixtures/return_with_data.wat index 8cc84006a0b00..f4488034799cf 100644 --- a/frame/contracts/fixtures/return_with_data.wat +++ b/frame/contracts/fixtures/return_with_data.wat @@ -1,9 +1,13 @@ (module - (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) - (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) - (import "env" "ext_scratch_write" (func $ext_scratch_write (param i32 i32))) + (import "env" "ext_input" (func $ext_input (param i32 i32))) + (import "env" "ext_return" (func $ext_return (param i32 i32 i32))) (import "env" "memory" (memory 1 1)) + ;; [0, 128) buffer where input is copied + + ;; [128, 132) length of the input buffer + (data (i32.const 128) "\80") + ;; Deploy routine is the same as call. (func (export "deploy") (result i32) (call $call) @@ -11,29 +15,19 @@ ;; Call reads the first 4 bytes (LE) as the exit status and returns the rest as output data. (func $call (export "call") (result i32) - (local $buf_size i32) - (local $exit_status i32) - - ;; Find out the size of the scratch buffer - (set_local $buf_size (call $ext_scratch_size)) - - ;; Copy scratch buffer into this contract memory. - (call $ext_scratch_read - (i32.const 0) ;; The pointer where to store the scratch buffer contents, - (i32.const 0) ;; Offset from the start of the scratch buffer. - (get_local $buf_size) ;; Count of bytes to copy. - ) + ;; Copy input into this contracts memory. + (call $ext_input (i32.const 0) (i32.const 128)) ;; Copy all but the first 4 bytes of the input data as the output data. - (call $ext_scratch_write + ;; Use the first 4 bytes as exit status + (call $ext_return + (i32.load (i32.const 0)) ;; Exit status (i32.const 4) ;; Pointer to the data to return. (i32.sub ;; Count of bytes to copy. - (get_local $buf_size) + (i32.load (i32.const 128)) (i32.const 4) ) ) - - ;; Return the first 4 bytes of the input data as the exit status. - (i32.load (i32.const 0)) + (unreachable) ) ) diff --git a/frame/contracts/fixtures/self_destruct.wat b/frame/contracts/fixtures/self_destruct.wat index 52146d06e66ee..baa38e4d47be8 100644 --- a/frame/contracts/fixtures/self_destruct.wat +++ b/frame/contracts/fixtures/self_destruct.wat @@ -1,5 +1,5 @@ (module - (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) + (import "env" "ext_input" (func $ext_input (param i32 i32))) (import "env" "ext_address" (func $ext_address (param i32 i32))) (import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32 i32 i32) (result i32))) (import "env" "ext_terminate" (func $ext_terminate (param i32 i32))) @@ -13,7 +13,12 @@ ;; [16, 24) Address of django (data (i32.const 16) "\04\00\00\00\00\00\00\00") - ;; [24, inf) zero initialized + ;; [24, 32) reserved for output of $ext_input + + ;; [32, 36) length of the buffer + (data (i32.const 32) "\04") + + ;; [36, inf) zero initialized (func $assert (param i32) (block $ok @@ -31,7 +36,8 @@ ;; This should trap instead of self-destructing since a contract cannot be removed live in ;; the execution stack cannot be removed. If the recursive call traps, then trap here as ;; well. - (if (call $ext_scratch_size) + (call $ext_input (i32.const 24) (i32.const 32)) + (if (i32.load (i32.const 32)) (then (call $ext_address (i32.const 0) (i32.const 8)) @@ -50,7 +56,7 @@ (i32.const 0) ;; Pointer to own address (i32.const 8) ;; Length of own address (i64.const 0) ;; How much gas to devote for the execution. 0 = all. - (i32.const 24) ;; Pointer to the buffer with value to transfer + (i32.const 36) ;; Pointer to the buffer with value to transfer (i32.const 8) ;; Length of the buffer with value to transfer (i32.const 0) ;; Pointer to input data buffer address (i32.const 0) ;; Length of input data buffer diff --git a/frame/contracts/fixtures/set_rent.wat b/frame/contracts/fixtures/set_rent.wat index 3e6bd491bc475..ba52e9ed752ce 100644 --- a/frame/contracts/fixtures/set_rent.wat +++ b/frame/contracts/fixtures/set_rent.wat @@ -3,8 +3,7 @@ (import "env" "ext_set_storage" (func $ext_set_storage (param i32 i32 i32))) (import "env" "ext_clear_storage" (func $ext_clear_storage (param i32))) (import "env" "ext_set_rent_allowance" (func $ext_set_rent_allowance (param i32 i32))) - (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) - (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) + (import "env" "ext_input" (func $ext_input (param i32 i32))) (import "env" "memory" (memory 1 1)) ;; insert a value of 4 bytes into storage @@ -48,8 +47,10 @@ ;; Dispatch the call according to input size (func (export "call") (local $input_size i32) + (i32.store (i32.const 64) (i32.const 64)) + (call $ext_input (i32.const 1024) (i32.const 64)) (set_local $input_size - (call $ext_scratch_size) + (i32.load (i32.const 64)) ) (block $IF_ELSE (block $IF_2 @@ -75,29 +76,27 @@ ;; Set into storage a 4 bytes value ;; Set call set_rent_allowance with input (func (export "deploy") - (local $input_size i32) - (set_local $input_size - (call $ext_scratch_size) - ) (call $ext_set_storage (i32.const 0) (i32.const 0) (i32.const 4) ) - (call $ext_scratch_read + (call $ext_input (i32.const 0) - (i32.const 0) - (get_local $input_size) + (i32.const 64) ) (call $ext_set_rent_allowance (i32.const 0) - (get_local $input_size) + (i32.load (i32.const 64)) ) ) ;; Encoding of 10 in balance (data (i32.const 0) "\28") + ;; Size of the buffer at address 0 + (data (i32.const 64) "\40") + ;; encoding of Charlies's account id (data (i32.const 68) "\03") diff --git a/frame/contracts/fixtures/storage_size.wat b/frame/contracts/fixtures/storage_size.wat index 20472492bacea..579aeda3a064d 100644 --- a/frame/contracts/fixtures/storage_size.wat +++ b/frame/contracts/fixtures/storage_size.wat @@ -1,8 +1,7 @@ (module (import "env" "ext_get_storage" (func $ext_get_storage (param i32 i32 i32) (result i32))) (import "env" "ext_set_storage" (func $ext_set_storage (param i32 i32 i32))) - (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) - (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) + (import "env" "ext_input" (func $ext_input (param i32 i32))) (import "env" "memory" (memory 16 16)) ;; [0, 32) storage key @@ -10,10 +9,13 @@ ;; [32, 36) buffer where input is copied (expected size of storage item) - ;; [36, 40) size of buffer for ext_get_storage set to max - (data (i32.const 36) "\FF\FF\FF\FF") + ;; [36, 40) size of the input buffer + (data (i32.const 36) "\04") - ;; [40, inf) ext_get_storage_buffer + ;; [40, 44) size of buffer for ext_get_storage set to max + (data (i32.const 40) "\FF\FF\FF\FF") + + ;; [44, inf) ext_get_storage buffer (func $assert (param i32) (block $ok @@ -25,21 +27,16 @@ ) (func (export "call") - ;; assert $ext_scratch_size == 8 + (call $ext_input (i32.const 32) (i32.const 36)) + + ;; assert input size == 4 (call $assert (i32.eq - (call $ext_scratch_size) + (i32.load (i32.const 36)) (i32.const 4) ) ) - ;; copy contents of the scratch buffer into the contract's memory. - (call $ext_scratch_read - (i32.const 32) ;; Pointer in memory to the place where to copy. - (i32.const 0) ;; Offset from the start of the scratch buffer. - (i32.const 4) ;; Count of bytes to copy. - ) - ;; place a garbage value in storage, the size of which is specified by the call input. (call $ext_set_storage (i32.const 0) ;; Pointer to storage key @@ -51,8 +48,8 @@ (i32.eq (call $ext_get_storage (i32.const 0) ;; Pointer to storage key - (i32.const 40) ;; buffer where to copy result - (i32.const 36) ;; pointer to size of buffer + (i32.const 44) ;; buffer where to copy result + (i32.const 40) ;; pointer to size of buffer ) (i32.const 0) ) @@ -60,7 +57,7 @@ (call $assert (i32.eq - (i32.load (i32.const 36)) + (i32.load (i32.const 40)) (i32.load (i32.const 32)) ) ) diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index 4e68aac61511c..24e42c30b54fc 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -67,9 +67,6 @@ impl ExecReturnValue { #[cfg_attr(test, derive(sp_runtime::RuntimeDebug))] pub struct ExecError { pub reason: DispatchError, - /// This is an allocated buffer that may be reused. The buffer must be cleared explicitly - /// before reuse. - pub buffer: Vec, } pub type ExecResult = Result; @@ -80,11 +77,11 @@ pub type ExecResult = Result; /// ownership of buffer unless there is an error. #[macro_export] macro_rules! try_or_exec_error { - ($e:expr, $buffer:expr) => { + ($e:expr) => { match $e { Ok(val) => val, Err(reason) => return Err( - $crate::exec::ExecError { reason: reason.into(), buffer: $buffer } + $crate::exec::ExecError { reason: reason.into() } ), } } @@ -333,7 +330,6 @@ where if self.depth == self.config.max_depth as usize { return Err(ExecError { reason: "reached maximum depth, cannot make a call".into(), - buffer: input_data, }); } @@ -343,7 +339,6 @@ where { return Err(ExecError { reason: "not enough gas to pay base call fee".into(), - buffer: input_data, }); } @@ -356,7 +351,6 @@ where if let Some(ContractInfo::Tombstone(_)) = contract_info { return Err(ExecError { reason: "contract has been evicted".into(), - buffer: input_data, }); }; @@ -373,8 +367,7 @@ where &dest, value, nested, - ), - input_data + ) ); } @@ -383,8 +376,7 @@ where match storage::code_hash::(&dest) { Ok(dest_code_hash) => { let executable = try_or_exec_error!( - nested.loader.load_main(&dest_code_hash), - input_data + nested.loader.load_main(&dest_code_hash) ); let output = nested.vm .execute( @@ -410,7 +402,6 @@ where if self.depth == self.config.max_depth as usize { return Err(ExecError { reason: "reached maximum depth, cannot instantiate".into(), - buffer: input_data, }); } @@ -420,7 +411,6 @@ where { return Err(ExecError { reason: "not enough gas to pay base instantiate fee".into(), - buffer: input_data, }); } @@ -445,8 +435,7 @@ where .clone() .expect("the nested context always has to have self_trie_id"), code_hash.clone() - ), - input_data + ) ); // Send funds unconditionally here. If the `endowment` is below existential_deposit @@ -459,13 +448,11 @@ where &dest, endowment, nested, - ), - input_data + ) ); let executable = try_or_exec_error!( - nested.loader.load_init(&code_hash), - input_data + nested.loader.load_init(&code_hash) ); let output = nested.vm .execute( @@ -479,7 +466,6 @@ where if T::Currency::free_balance(&dest) < nested.config.existential_deposit { return Err(ExecError { reason: "insufficient remaining balance".into(), - buffer: output.data, }); } @@ -1230,7 +1216,6 @@ mod tests { result, Err(ExecError { reason: DispatchError::Module { message: Some("InsufficientBalance"), .. }, - buffer: _, }) ); assert_eq!(get_balance(&origin), 0); @@ -1372,7 +1357,6 @@ mod tests { r, Err(ExecError { reason: DispatchError::Other("reached maximum depth, cannot make a call"), - buffer: _, }) ); *reached_bottom = true; @@ -1627,7 +1611,7 @@ mod tests { let mut loader = MockLoader::empty(); let dummy_ch = loader.insert( - |_| Err(ExecError { reason: "It's a trap!".into(), buffer: Vec::new() }) + |_| Err(ExecError { reason: "It's a trap!".into() }) ); let instantiator_ch = loader.insert({ let dummy_ch = dummy_ch.clone(); @@ -1640,7 +1624,7 @@ mod tests { ctx.gas_meter, vec![] ), - Err(ExecError { reason: DispatchError::Other("It's a trap!"), buffer: _ }) + Err(ExecError { reason: DispatchError::Other("It's a trap!") }) ); exec_success() @@ -1693,8 +1677,7 @@ mod tests { ), Err(ExecError { reason: DispatchError::Other("insufficient remaining balance"), - buffer - }) if buffer == Vec::::new() + }) ); assert_eq!( diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index 0e7aa2e3ae84c..8ca187783cb81 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -1166,7 +1166,7 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage: DJANGO, 0, GAS_LIMIT, - vec![], + set_rent_code_hash.as_ref().to_vec(), ) }; diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index a443c6cf75389..603e09a541794 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -799,7 +799,7 @@ mod tests { const CODE_GET_STORAGE: &str = r#" (module (import "env" "ext_get_storage" (func $ext_get_storage (param i32 i32 i32) (result i32))) - (import "env" "ext_return" (func $ext_return (param i32 i32))) + (import "env" "ext_return" (func $ext_return (param i32 i32 i32))) (import "env" "memory" (memory 1 1)) ;; [0, 32) key for get storage @@ -847,6 +847,7 @@ mod tests { ;; Return the contents of the buffer (call $ext_return + (i32.const 0) (i32.const 36) (get_local $buf_size) ) @@ -1091,7 +1092,7 @@ mod tests { const CODE_GAS_LEFT: &str = r#" (module (import "env" "ext_gas_left" (func $ext_gas_left (param i32 i32))) - (import "env" "ext_return" (func $ext_return (param i32 i32))) + (import "env" "ext_return" (func $ext_return (param i32 i32 i32))) (import "env" "memory" (memory 1 1)) ;; size of our buffer is 32 bytes @@ -1119,7 +1120,7 @@ mod tests { ) ;; return gas left - (call $ext_return (i32.const 0) (i32.const 8)) + (call $ext_return (i32.const 0) (i32.const 0) (i32.const 8)) (unreachable) ) @@ -1197,12 +1198,13 @@ mod tests { const CODE_RETURN_FROM_START_FN: &str = r#" (module - (import "env" "ext_return" (func $ext_return (param i32 i32))) + (import "env" "ext_return" (func $ext_return (param i32 i32 i32))) (import "env" "memory" (memory 1 1)) (start $start) (func $start (call $ext_return + (i32.const 0) (i32.const 8) (i32.const 4) ) @@ -1387,7 +1389,7 @@ mod tests { const CODE_RANDOM: &str = r#" (module (import "env" "ext_random" (func $ext_random (param i32 i32 i32 i32))) - (import "env" "ext_return" (func $ext_return (param i32 i32))) + (import "env" "ext_return" (func $ext_return (param i32 i32 i32))) (import "env" "memory" (memory 1 1)) ;; [0,128) is reserved for the result of PRNG. @@ -1429,6 +1431,7 @@ mod tests { ;; return the random data (call $ext_return + (i32.const 0) (i32.const 0) (i32.const 32) ) @@ -1540,7 +1543,7 @@ mod tests { &mut gas_meter ), Err(ExecError { - reason: DispatchError::Other("contract trapped during execution"), buffer: _ + reason: DispatchError::Other("contract trapped during execution") }) ); } @@ -1583,7 +1586,7 @@ mod tests { MockExt::default(), &mut gas_meter ), - Err(ExecError { reason: DispatchError::Other("contract trapped during execution"), buffer: _ }) + Err(ExecError { reason: DispatchError::Other("contract trapped during execution") }) ); } @@ -1641,71 +1644,14 @@ mod tests { ).unwrap(); } - // asserts that the size of the input data is 4. - const CODE_SIMPLE_ASSERT: &str = r#" -(module - (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) - - (func $assert (param i32) - (block $ok - (br_if $ok - (get_local 0) - ) - (unreachable) - ) - ) - - (func (export "deploy")) - - (func (export "call") - (call $assert - (i32.eq - (call $ext_scratch_size) - (i32.const 4) - ) - ) - ) -) -"#; - - #[test] - fn output_buffer_capacity_preserved_on_success() { - let mut input_data = Vec::with_capacity(1_234); - input_data.extend_from_slice(&[1, 2, 3, 4][..]); - - let output = execute( - CODE_SIMPLE_ASSERT, - input_data, - MockExt::default(), - &mut GasMeter::new(GAS_LIMIT), - ).unwrap(); - - assert_eq!(output.data.len(), 0); - assert_eq!(output.data.capacity(), 1_234); - } - - #[test] - fn output_buffer_capacity_preserved_on_failure() { - let mut input_data = Vec::with_capacity(1_234); - input_data.extend_from_slice(&[1, 2, 3, 4, 5][..]); - - let error = execute( - CODE_SIMPLE_ASSERT, - input_data, - MockExt::default(), - &mut GasMeter::new(GAS_LIMIT), - ).err().unwrap(); - - assert_eq!(error.buffer.capacity(), 1_234); - } - const CODE_RETURN_WITH_DATA: &str = r#" (module - (import "env" "ext_scratch_size" (func $ext_scratch_size (result i32))) - (import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32))) - (import "env" "ext_scratch_write" (func $ext_scratch_write (param i32 i32))) + (import "env" "ext_input" (func $ext_input (param i32 i32))) + (import "env" "ext_return" (func $ext_return (param i32 i32 i32))) (import "env" "memory" (memory 1 1)) + (data (i32.const 32) "\20") + ;; Deploy routine is the same as call. (func (export "deploy") (result i32) (call $call) @@ -1716,33 +1662,25 @@ mod tests { (local $buf_size i32) (local $exit_status i32) - ;; Find out the size of the scratch buffer - (set_local $buf_size (call $ext_scratch_size)) - - ;; Copy scratch buffer into this contract memory. - (call $ext_scratch_read - (i32.const 0) ;; The pointer where to store the scratch buffer contents, - (i32.const 0) ;; Offset from the start of the scratch buffer. - (get_local $buf_size) ;; Count of bytes to copy. + ;; Copy input data this contract memory. + (call $ext_input + (i32.const 0) ;; Pointer where to store input + (i32.const 32) ;; Pointer to the length of the buffer ) ;; Copy all but the first 4 bytes of the input data as the output data. - (call $ext_scratch_write - (i32.const 4) ;; Offset from the start of the scratch buffer. - (i32.sub ;; Count of bytes to copy. - (get_local $buf_size) - (i32.const 4) - ) + (call $ext_return + (i32.load (i32.const 0)) + (i32.const 4) + (i32.sub (i32.load (i32.const 32)) (i32.const 4)) ) - - ;; Return the first 4 bytes of the input data as the exit status. - (i32.load (i32.const 0)) + (unreachable) ) ) "#; #[test] - fn return_with_success_status() { + fn ext_return_with_success_status() { let output = execute( CODE_RETURN_WITH_DATA, hex!("00112233445566778899").to_vec(), diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index a51e6878a75ec..067335a85badc 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -23,7 +23,7 @@ use crate::exec::{ use crate::gas::{Gas, GasMeter, Token, GasMeterResult}; use sp_sandbox; use frame_system; -use sp_std::{prelude::*, mem, convert::TryInto}; +use sp_std::{prelude::*, convert::TryInto}; use codec::{Decode, Encode}; use sp_runtime::traits::{Bounded, SaturatedConversion}; use sp_io::hashing::{ @@ -44,7 +44,7 @@ const TRAP_RETURN_CODE: u32 = 0x0100; /// to just terminate quickly in some cases. enum SpecialTrap { /// Signals that trap was generated in response to call `ext_return` host function. - Return(Vec), + Return(u32, Vec), /// Signals that trap was generated because the contract exhausted its gas limit. OutOfGas, /// Signals that a trap was generated in response to a succesful call to the @@ -59,7 +59,7 @@ enum SpecialTrap { /// Can only be used for one call. pub(crate) struct Runtime<'a, E: Ext + 'a> { ext: &'a mut E, - scratch_buf: Vec, + input_data: Option>, schedule: &'a Schedule, memory: sp_sandbox::Memory, gas_meter: &'a mut GasMeter, @@ -75,8 +75,7 @@ impl<'a, E: Ext + 'a> Runtime<'a, E> { ) -> Self { Runtime { ext, - // Put the input data into the scratch buffer immediately. - scratch_buf: input_data, + input_data: Some(input_data), schedule, memory, gas_meter, @@ -91,9 +90,11 @@ pub(crate) fn to_execution_result( ) -> ExecResult { match runtime.special_trap { // The trap was the result of the execution `return` host function. - Some(SpecialTrap::Return(data)) => { + Some(SpecialTrap::Return(status, data)) => { + let status = (status & 0xFF).try_into() + .expect("exit_code is masked into the range of a u8; qed"); return Ok(ExecReturnValue { - status: STATUS_SUCCESS, + status, data, }) }, @@ -112,13 +113,11 @@ pub(crate) fn to_execution_result( Some(SpecialTrap::OutOfGas) => { return Err(ExecError { reason: "ran out of gas during contract execution".into(), - buffer: runtime.scratch_buf, }) }, Some(SpecialTrap::OutputBufferTooSmall) => { return Err(ExecError { reason: "output buffer too small".into(), - buffer: runtime.scratch_buf, }) }, None => (), @@ -127,21 +126,9 @@ pub(crate) fn to_execution_result( // Check the exact type of the error. match sandbox_result { // No traps were generated. Proceed normally. - Ok(sp_sandbox::ReturnValue::Unit) => { - let mut buffer = runtime.scratch_buf; - buffer.clear(); - Ok(ExecReturnValue { status: STATUS_SUCCESS, data: buffer }) - } - Ok(sp_sandbox::ReturnValue::Value(sp_sandbox::Value::I32(exit_code))) => { - let status = (exit_code & 0xFF).try_into() - .expect("exit_code is masked into the range of a u8; qed"); - Ok(ExecReturnValue { status, data: runtime.scratch_buf }) + Ok(_) => { + Ok(ExecReturnValue { status: STATUS_SUCCESS, data: Vec::new() }) } - // This should never happen as the return type of exported functions should have been - // validated by the code preparation process. However, because panics are really - // undesirable in the runtime code, we treat this as a trap for now. Eventually, we might - // want to revisit this. - Ok(_) => Err(ExecError { reason: "return type error".into(), buffer: runtime.scratch_buf }), // `Error::Module` is returned only if instantiation or linking failed (i.e. // wasm binary tried to import a function that is not provided by the host). // This shouldn't happen because validation process ought to reject such binaries. @@ -149,10 +136,10 @@ pub(crate) fn to_execution_result( // Because panics are really undesirable in the runtime code, we treat this as // a trap for now. Eventually, we might want to revisit this. Err(sp_sandbox::Error::Module) => - Err(ExecError { reason: "validation error".into(), buffer: runtime.scratch_buf }), + Err(ExecError { reason: "validation error".into() }), // Any other kind of a trap should result in a failure. Err(sp_sandbox::Error::Execution) | Err(sp_sandbox::Error::OutOfBounds) => - Err(ExecError { reason: "contract trapped during execution".into(), buffer: runtime.scratch_buf }), + Err(ExecError { reason: "contract trapped during execution".into() }), } } @@ -258,31 +245,6 @@ fn read_sandbox_memory( Ok(buf) } -/// Read designated chunk from the sandbox memory into the scratch buffer, consuming an -/// appropriate amount of gas. Resizes the scratch buffer to the specified length on success. -/// -/// Returns `Err` if one of the following conditions occurs: -/// -/// - calculating the gas cost resulted in overflow. -/// - out of gas -/// - requested buffer is not within the bounds of the sandbox memory. -fn read_sandbox_memory_into_scratch( - ctx: &mut Runtime, - ptr: u32, - len: u32, -) -> Result<(), sp_sandbox::HostError> { - charge_gas( - ctx.gas_meter, - ctx.schedule, - &mut ctx.special_trap, - RuntimeToken::ReadMemory(len), - )?; - - ctx.scratch_buf.resize(len as usize, 0); - ctx.memory.get(ptr, ctx.scratch_buf.as_mut_slice()).map_err(|_| sp_sandbox::HostError)?; - Ok(()) -} - /// Read designated chunk from the sandbox memory into the supplied buffer, consuming /// an appropriate amount of gas. /// @@ -332,26 +294,37 @@ fn read_sandbox_memory_as( /// - calculating the gas cost resulted in overflow. /// - out of gas /// - designated area is not within the bounds of the sandbox memory. -fn write_sandbox_memory( - schedule: &Schedule, - special_trap: &mut Option, - gas_meter: &mut GasMeter, - memory: &sp_sandbox::Memory, +fn write_sandbox_memory( + ctx: &mut Runtime, ptr: u32, buf: &[u8], ) -> Result<(), sp_sandbox::HostError> { charge_gas( - gas_meter, - schedule, - special_trap, + ctx.gas_meter, + ctx.schedule, + &mut ctx.special_trap, RuntimeToken::WriteMemory(buf.len() as u32), )?; - memory.set(ptr, buf)?; + ctx.memory.set(ptr, buf)?; Ok(()) } +/// Write the given buffer and its length to the designated locations in sandbox memory. +// +/// `out_ptr` is the location in sandbox memory where `buf` should be written to. +/// `out_len_ptr` is an in-out location in sandbox memory. It is read to determine the +/// lenght of the buffer located at `out_ptr`. If that buffer is large enough the actual +/// `buf.len()` is written to this location. +/// +/// If `out_ptr` is set to the sentinel value of `u32::max_value()` the operation is +/// skipped and `Ok` is returned. This is supposed to help callers to make copying +/// output optional. For example to skip copying back the output buffer of an `ext_call` +/// when the caller is not interested in the result. +/// +/// In addition to the error conditions of `write_sandbox_memory` this functions returns +/// `Err` if the size of the buffer located at `out_ptr` is too small to fit `buf`. fn write_sandbox_output( ctx: &mut Runtime, out_ptr: u32, @@ -549,10 +522,10 @@ define_env!(Env, , nested_meter, input_data, ) - .map_err(|err| err.buffer) + .map_err(|_| ()) } // there is not enough gas to allocate for the nested call. - None => Err(input_data), + None => Err(()), } }); @@ -636,18 +609,18 @@ define_env!(Env, , nested_meter, input_data ) - .map_err(|err| err.buffer) + .map_err(|_| ()) } // there is not enough gas to allocate for the nested call. - None => Err(input_data), + None => Err(()), } }); match instantiate_outcome { Ok((address, output)) => { if output.is_success() { write_sandbox_output(ctx, address_ptr, address_len_ptr, &address.encode())?; - write_sandbox_output(ctx, output_ptr, output_len_ptr, &output.data)?; } + write_sandbox_output(ctx, output_ptr, output_len_ptr, &output.data)?; Ok(output.status.into()) }, Err(_) => { @@ -680,11 +653,19 @@ define_env!(Env, , Err(sp_sandbox::HostError) }, + ext_input(ctx, buf_ptr: u32, buf_len_ptr: u32) => { + if let Some(input) = ctx.input_data.take() { + write_sandbox_output(ctx, buf_ptr, buf_len_ptr, &input) + } else { + Err(sp_sandbox::HostError) + } + }, + // Save a data buffer as a result of the execution, terminate the execution and return a // successful result to the caller. // // This is the only way to return a data buffer to the caller. - ext_return(ctx, data_ptr: u32, data_len: u32) => { + ext_return(ctx, status_code: u32, data_ptr: u32, data_len: u32) => { charge_gas( ctx.gas_meter, ctx.schedule, @@ -692,10 +673,8 @@ define_env!(Env, , RuntimeToken::ReturnData(data_len) )?; - read_sandbox_memory_into_scratch(ctx, data_ptr, data_len)?; - let output_buf = mem::replace(&mut ctx.scratch_buf, Vec::new()); - - ctx.special_trap = Some(SpecialTrap::Return(output_buf)); + let output_data = read_sandbox_memory(ctx, data_ptr, data_len)?; + ctx.special_trap = Some(SpecialTrap::Return(status_code, output_data)); // The trap mechanism is used to immediately terminate the execution. // This trap should be handled appropriately before returning the result @@ -859,54 +838,6 @@ define_env!(Env, , Err(sp_sandbox::HostError) }, - // Returns the size of the scratch buffer. - // - // For more details on the scratch buffer see `ext_scratch_read`. - ext_scratch_size(ctx) -> u32 => { - Ok(ctx.scratch_buf.len() as u32) - }, - - // Copy data from the scratch buffer starting from `offset` with length `len` into the contract - // memory. The region at which the data should be put is specified by `dest_ptr`. - // - // In order to get size of the scratch buffer use `ext_scratch_size`. At the start of contract - // execution, the scratch buffer is filled with the input data. Whenever a contract calls - // function that uses the scratch buffer the contents of the scratch buffer are overwritten. - ext_scratch_read(ctx, dest_ptr: u32, offset: u32, len: u32) => { - let offset = offset as usize; - if offset > ctx.scratch_buf.len() { - // Offset can't be larger than scratch buffer length. - return Err(sp_sandbox::HostError); - } - - // This can't panic since `offset <= ctx.scratch_buf.len()`. - let src = &ctx.scratch_buf[offset..]; - if src.len() != len as usize { - return Err(sp_sandbox::HostError); - } - - // Finally, perform the write. - write_sandbox_memory( - ctx.schedule, - &mut ctx.special_trap, - ctx.gas_meter, - &ctx.memory, - dest_ptr, - src, - )?; - - Ok(()) - }, - - // Copy data from contract memory starting from `src_ptr` with length `len` into the scratch - // buffer. This overwrites the entire scratch buffer and resizes to `len`. Specifying a `len` - // of zero clears the scratch buffer. - // - // This should be used before exiting a call or instantiation in order to set the return data. - ext_scratch_write(ctx, src_ptr: u32, len: u32) => { - read_sandbox_memory_into_scratch(ctx, src_ptr, len) - }, - // Deposit a contract event with the data buffer and optional list of topics. There is a limit // on the maximum number of topics specified by `max_event_topics`. // @@ -1108,10 +1039,7 @@ where let hash = hash_fn(&input); // Write the resulting hash back into the sandboxed output buffer. write_sandbox_memory( - ctx.schedule, - &mut ctx.special_trap, - ctx.gas_meter, - &ctx.memory, + ctx, output_ptr, hash.as_ref(), )?; From 46191f5b38ae8b0d4a8e30c083043875ef27c2bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Wed, 1 Jul 2020 12:36:44 +0200 Subject: [PATCH 06/23] Remove the no longer used `Dispatched` event --- frame/contracts/src/lib.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 4db77a078e92d..7ae8b21e2b5f0 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -673,10 +673,6 @@ decl_event! { /// Triggered when the current schedule is updated. ScheduleUpdated(u32), - /// A call was dispatched from the given account. The bool signals whether it was - /// successful execution or not. - Dispatched(AccountId, bool), - /// An event deposited upon execution of a contract from the account. ContractExecution(AccountId, Vec), } From fa6bf8c6a33d974fe2dfced7521cdeb68cbfff89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Tue, 30 Jun 2020 17:02:46 +0200 Subject: [PATCH 07/23] Rework error handling (changes RPC exposed data) * ext_return passes a flags field instead of a return code * Flags is only for seal and not for the caller * flags: u32 replaced status_code: u8 in RPC exposed type * API functions use a unified error type (ReturnCode) * ext_transfer now traps on error to be consistent with call and instantiate --- Cargo.lock | 1 + bin/node/runtime/src/lib.rs | 2 +- frame/contracts/Cargo.toml | 1 + frame/contracts/fixtures/caller_contract.wat | 14 +-- frame/contracts/fixtures/crypto_hashes.wat | 2 +- .../fixtures/destroy_and_transfer.wat | 2 +- frame/contracts/fixtures/return_with_data.wat | 8 +- frame/contracts/fixtures/set_rent.wat | 9 +- frame/contracts/rpc/runtime-api/src/lib.rs | 5 +- frame/contracts/rpc/src/lib.rs | 10 +- frame/contracts/src/exec.rs | 44 ++++----- frame/contracts/src/lib.rs | 2 +- frame/contracts/src/tests.rs | 2 +- frame/contracts/src/wasm/mod.rs | 45 ++++----- frame/contracts/src/wasm/prepare.rs | 6 +- frame/contracts/src/wasm/runtime.rs | 94 +++++++++++++------ 16 files changed, 136 insertions(+), 111 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b5bbcd6954611..610ae3e87711c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4079,6 +4079,7 @@ name = "pallet-contracts" version = "2.0.0-rc4" dependencies = [ "assert_matches", + "bitflags", "frame-support", "frame-system", "hex-literal", diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 7bec203f8c446..96a17dfa94f57 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1013,7 +1013,7 @@ impl_runtime_apis! { Contracts::bare_call(origin, dest.into(), value, gas_limit, input_data); match exec_result { Ok(v) => ContractExecResult::Success { - status: v.status, + flags: v.flags.bits(), data: v.data, }, Err(_) => ContractExecResult::Error, diff --git a/frame/contracts/Cargo.toml b/frame/contracts/Cargo.toml index 348b8ff0e03e6..df5a47bb0e0b3 100644 --- a/frame/contracts/Cargo.toml +++ b/frame/contracts/Cargo.toml @@ -25,6 +25,7 @@ sp-sandbox = { version = "0.8.0-rc4", default-features = false, path = "../../pr frame-support = { version = "2.0.0-rc4", default-features = false, path = "../support" } frame-system = { version = "2.0.0-rc4", default-features = false, path = "../system" } pallet-contracts-primitives = { version = "2.0.0-rc4", default-features = false, path = "common" } +bitflags = "1.0" [dev-dependencies] wabt = "0.9.2" diff --git a/frame/contracts/fixtures/caller_contract.wat b/frame/contracts/fixtures/caller_contract.wat index 34933cdb768c0..459e6ce1c77ad 100644 --- a/frame/contracts/fixtures/caller_contract.wat +++ b/frame/contracts/fixtures/caller_contract.wat @@ -76,7 +76,7 @@ ;; Check non-zero exit status. (call $assert - (i32.eq (get_local $exit_code) (i32.const 0x11)) + (i32.eq (get_local $exit_code) (i32.const 2)) ;; ReturnCode::CalleeReverted ) ;; Check that balance has not changed. @@ -103,7 +103,7 @@ ;; Check for special trap exit status. (call $assert - (i32.eq (get_local $exit_code) (i32.const 0x0100)) + (i32.eq (get_local $exit_code) (i32.const 1)) ;; ReturnCode::CalleeTrapped ) ;; Check that balance has not changed. @@ -137,7 +137,7 @@ ;; Check for success exit status. (call $assert - (i32.eq (get_local $exit_code) (i32.const 0x00)) + (i32.eq (get_local $exit_code) (i32.const 0)) ;; ReturnCode::Success ) ;; Check that address has the expected length @@ -182,7 +182,7 @@ ;; Check non-zero exit status. (call $assert - (i32.eq (get_local $exit_code) (i32.const 0x11)) + (i32.eq (get_local $exit_code) (i32.const 2)) ;; ReturnCode::CalleeReverted ) ;; Check that scratch buffer contains the expected return data. @@ -218,7 +218,7 @@ ;; Check for special trap exit status. (call $assert - (i32.eq (get_local $exit_code) (i32.const 0x0100)) + (i32.eq (get_local $exit_code) (i32.const 1)) ;; ReturnCode::CalleeTrapped ) ;; Check that balance has not changed. @@ -255,7 +255,7 @@ ;; Check for success exit status. (call $assert - (i32.eq (get_local $exit_code) (i32.const 0x00)) + (i32.eq (get_local $exit_code) (i32.const 0)) ;; ReturnCode::Success ) ;; Check that scratch buffer contains the expected return data. @@ -280,5 +280,5 @@ (data (i32.const 0) "\00\80") ;; The value to transfer on instantiation and calls. ;; Chosen to be greater than existential deposit. - (data (i32.const 8) "\00\11\22\33\44\55\66\77") ;; The input data to instantiations and calls. + (data (i32.const 8) "\00\01\22\33\44\55\66\77") ;; The input data to instantiations and calls. ) diff --git a/frame/contracts/fixtures/crypto_hashes.wat b/frame/contracts/fixtures/crypto_hashes.wat index 7e86d2b630de3..0ba60e42dfd68 100644 --- a/frame/contracts/fixtures/crypto_hashes.wat +++ b/frame/contracts/fixtures/crypto_hashes.wat @@ -47,7 +47,7 @@ ;; | 2 | BLAKE2 | 256 | ;; | 3 | BLAKE2 | 128 | ;; --------------------------------- - (func (export "call") (result i32) + (func (export "call") (local $chosen_hash_fn i32) (local $input_len_ptr i32) (local $input_ptr i32) diff --git a/frame/contracts/fixtures/destroy_and_transfer.wat b/frame/contracts/fixtures/destroy_and_transfer.wat index 71350664a3f34..ee191aa019bfb 100644 --- a/frame/contracts/fixtures/destroy_and_transfer.wat +++ b/frame/contracts/fixtures/destroy_and_transfer.wat @@ -112,7 +112,7 @@ (i32.const 0) ;; Length is ignored in this case ) - (i32.const 0x0100) + (i32.const 0x1) ) ) diff --git a/frame/contracts/fixtures/return_with_data.wat b/frame/contracts/fixtures/return_with_data.wat index f4488034799cf..ad42845ae0208 100644 --- a/frame/contracts/fixtures/return_with_data.wat +++ b/frame/contracts/fixtures/return_with_data.wat @@ -9,19 +9,19 @@ (data (i32.const 128) "\80") ;; Deploy routine is the same as call. - (func (export "deploy") (result i32) + (func (export "deploy") (call $call) ) ;; Call reads the first 4 bytes (LE) as the exit status and returns the rest as output data. - (func $call (export "call") (result i32) + (func $call (export "call") ;; Copy input into this contracts memory. (call $ext_input (i32.const 0) (i32.const 128)) ;; Copy all but the first 4 bytes of the input data as the output data. - ;; Use the first 4 bytes as exit status + ;; Use the first byte as exit status (call $ext_return - (i32.load (i32.const 0)) ;; Exit status + (i32.load8_u (i32.const 0)) ;; Exit status (i32.const 4) ;; Pointer to the data to return. (i32.sub ;; Count of bytes to copy. (i32.load (i32.const 128)) diff --git a/frame/contracts/fixtures/set_rent.wat b/frame/contracts/fixtures/set_rent.wat index ba52e9ed752ce..4e6424e720104 100644 --- a/frame/contracts/fixtures/set_rent.wat +++ b/frame/contracts/fixtures/set_rent.wat @@ -1,5 +1,5 @@ (module - (import "env" "ext_transfer" (func $ext_transfer (param i32 i32 i32 i32) (result i32))) + (import "env" "ext_transfer" (func $ext_transfer (param i32 i32 i32 i32))) (import "env" "ext_set_storage" (func $ext_set_storage (param i32 i32 i32))) (import "env" "ext_clear_storage" (func $ext_clear_storage (param i32))) (import "env" "ext_set_rent_allowance" (func $ext_set_rent_allowance (param i32 i32))) @@ -24,12 +24,7 @@ ;; transfer 50 to CHARLIE (func $call_2 - (call $assert - (i32.eq - (call $ext_transfer (i32.const 68) (i32.const 8) (i32.const 76) (i32.const 8)) - (i32.const 0) - ) - ) + (call $ext_transfer (i32.const 68) (i32.const 8) (i32.const 76) (i32.const 8)) ) ;; do nothing diff --git a/frame/contracts/rpc/runtime-api/src/lib.rs b/frame/contracts/rpc/runtime-api/src/lib.rs index 84fd66826d8b3..5fb3f28bcf7ce 100644 --- a/frame/contracts/rpc/runtime-api/src/lib.rs +++ b/frame/contracts/rpc/runtime-api/src/lib.rs @@ -35,8 +35,9 @@ pub enum ContractExecResult { /// /// There is a status code and, optionally, some data returned by the contract. Success { - /// Status code returned by the contract. - status: u8, + /// Flags that the contract passed along on returning to alter its exit behaviour. + /// Described in `pallet_contracts::exec::ReturnFlags`. + flags: u32, /// Output data returned by the contract. /// /// Can be empty. diff --git a/frame/contracts/rpc/src/lib.rs b/frame/contracts/rpc/src/lib.rs index 18496c13af9fa..453764c77d7e2 100644 --- a/frame/contracts/rpc/src/lib.rs +++ b/frame/contracts/rpc/src/lib.rs @@ -92,8 +92,8 @@ pub struct CallRequest { pub enum RpcContractExecResult { /// Successful execution Success { - /// Status code - status: u8, + /// The return flags + flags: u32, /// Output data data: Bytes, }, @@ -104,8 +104,8 @@ pub enum RpcContractExecResult { impl From for RpcContractExecResult { fn from(r: ContractExecResult) -> Self { match r { - ContractExecResult::Success { status, data } => RpcContractExecResult::Success { - status, + ContractExecResult::Success { flags, data } => RpcContractExecResult::Success { + flags, data: data.into(), }, ContractExecResult::Error => RpcContractExecResult::Error(()), @@ -309,7 +309,7 @@ mod tests { let actual = serde_json::to_string(&res).unwrap(); assert_eq!(actual, expected); } - test(r#"{"success":{"status":5,"data":"0x1234"}}"#); + test(r#"{"success":{"flags":5,"data":"0x1234"}}"#); test(r#"{"error":null}"#); } } diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index 24e42c30b54fc..1f4cc6292ef46 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -19,6 +19,7 @@ use super::{CodeHash, Config, ContractAddressFor, Event, RawEvent, Trait, use crate::gas::{Gas, GasMeter, Token}; use crate::rent; use crate::storage; +use bitflags::bitflags; use sp_std::prelude::*; use sp_runtime::traits::{Bounded, Zero, Convert}; @@ -37,26 +38,27 @@ pub type StorageKey = [u8; 32]; /// A type that represents a topic of an event. At the moment a hash is used. pub type TopicOf = ::Hash; -/// A status code return to the source of a contract call or instantiation indicating success or -/// failure. A code of 0 indicates success and that changes are applied. All other codes indicate -/// failure and that changes are reverted. The particular code in the case of failure is opaque and -/// may be interpreted by the calling contract. -pub type StatusCode = u8; - -/// The status code indicating success. -pub const STATUS_SUCCESS: StatusCode = 0; +bitflags! { + /// Flags used by a contract to customize exit behaviour. + pub struct ReturnFlags: u32 { + /// If this bit is set all changes made by the contract exection are rolled back. + const REVERT = 0x0000_0001; + } +} /// Output of a contract call or instantiation which ran to completion. #[cfg_attr(test, derive(PartialEq, Eq, Debug))] pub struct ExecReturnValue { - pub status: StatusCode, + /// Flags passed along by `ext_return`. Empty when `ext_return` was never called. + pub flags: ReturnFlags, + /// Buffer passed along by `ext_return`. Empty when `ext_return` was never called. pub data: Vec, } impl ExecReturnValue { - /// Returns whether the call or instantiation exited with a successful status code. + /// We understand the absense of a revert flag as success. pub fn is_success(&self) -> bool { - self.status == STATUS_SUCCESS + !self.flags.contains(ReturnFlags::REVERT) } } @@ -387,7 +389,7 @@ where )?; Ok(output) } - Err(storage::ContractAbsentError) => Ok(ExecReturnValue { status: STATUS_SUCCESS, data: Vec::new() }), + Err(storage::ContractAbsentError) => Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: Vec::new() }), } }) } @@ -504,7 +506,7 @@ where frame_support::storage::with_transaction(|| { let output = func(&mut nested); match output { - Ok(ref rv) if rv.is_success() => Commit(output), + Ok(ref rv) if !rv.flags.contains(ReturnFlags::REVERT) => Commit(output), _ => Rollback(output), } }) @@ -853,11 +855,11 @@ fn deposit_event( mod tests { use super::{ BalanceOf, Event, ExecFeeToken, ExecResult, ExecutionContext, Ext, Loader, - RawEvent, TransferFeeKind, TransferFeeToken, Vm, + RawEvent, TransferFeeKind, TransferFeeToken, Vm, ReturnFlags, }; use crate::{ gas::GasMeter, tests::{ExtBuilder, Test, MetaEvent}, - exec::{ExecReturnValue, ExecError, STATUS_SUCCESS}, CodeHash, Config, + exec::{ExecReturnValue, ExecError}, CodeHash, Config, gas::Gas, storage, }; @@ -966,7 +968,7 @@ mod tests { } fn exec_success() -> ExecResult { - Ok(ExecReturnValue { status: STATUS_SUCCESS, data: Vec::new() }) + Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: Vec::new() }) } #[test] @@ -1082,7 +1084,7 @@ mod tests { let vm = MockVm::new(); let mut loader = MockLoader::empty(); let return_ch = loader.insert( - |_| Ok(ExecReturnValue { status: 1, data: Vec::new() }) + |_| Ok(ExecReturnValue { flags: ReturnFlags::REVERT, data: Vec::new() }) ); ExtBuilder::default().build().execute_with(|| { @@ -1233,7 +1235,7 @@ mod tests { let vm = MockVm::new(); let mut loader = MockLoader::empty(); let return_ch = loader.insert( - |_| Ok(ExecReturnValue { status: STATUS_SUCCESS, data: vec![1, 2, 3, 4] }) + |_| Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: vec![1, 2, 3, 4] }) ); ExtBuilder::default().build().execute_with(|| { @@ -1264,7 +1266,7 @@ mod tests { let vm = MockVm::new(); let mut loader = MockLoader::empty(); let return_ch = loader.insert( - |_| Ok(ExecReturnValue { status: 1, data: vec![1, 2, 3, 4] }) + |_| Ok(ExecReturnValue { flags: ReturnFlags::REVERT, data: vec![1, 2, 3, 4] }) ); ExtBuilder::default().build().execute_with(|| { @@ -1501,7 +1503,7 @@ mod tests { let mut loader = MockLoader::empty(); let dummy_ch = loader.insert( - |_| Ok(ExecReturnValue { status: STATUS_SUCCESS, data: vec![80, 65, 83, 83] }) + |_| Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: vec![80, 65, 83, 83] }) ); ExtBuilder::default().existential_deposit(15).build().execute_with(|| { @@ -1534,7 +1536,7 @@ mod tests { let mut loader = MockLoader::empty(); let dummy_ch = loader.insert( - |_| Ok(ExecReturnValue { status: 1, data: vec![70, 65, 73, 76] }) + |_| Ok(ExecReturnValue { flags: ReturnFlags::REVERT, data: vec![70, 65, 73, 76] }) ); ExtBuilder::default().existential_deposit(15).build().execute_with(|| { diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 7ae8b21e2b5f0..367d78b7e5799 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -93,7 +93,7 @@ use crate::exec::ExecutionContext; use crate::wasm::{WasmLoader, WasmVm}; pub use crate::gas::{Gas, GasMeter}; -pub use crate::exec::{ExecResult, ExecReturnValue, ExecError, StatusCode}; +pub use crate::exec::{ExecResult, ExecReturnValue, ExecError}; #[cfg(feature = "std")] use serde::{Serialize, Deserialize}; diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index 8ca187783cb81..1c868c5670d68 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -1592,7 +1592,7 @@ fn crypto_hashes() { GAS_LIMIT, params, ).unwrap(); - assert_eq!(result.status, 0); + assert!(result.is_success()); let expected = hash_fn(input.as_ref()); assert_eq!(&result.data[..*expected_size], &*expected); } diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index 603e09a541794..f38197d710d4f 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -153,7 +153,7 @@ mod tests { use std::collections::HashMap; use std::cell::RefCell; use sp_core::H256; - use crate::exec::{Ext, StorageKey, ExecError, ExecReturnValue, STATUS_SUCCESS}; + use crate::exec::{Ext, StorageKey, ExecError, ExecReturnValue, ReturnFlags}; use crate::gas::{Gas, GasMeter}; use crate::tests::{Test, Call}; use crate::wasm::prepare::prepare_contract; @@ -251,7 +251,7 @@ mod tests { Ok(( address, ExecReturnValue { - status: STATUS_SUCCESS, + flags: ReturnFlags::empty(), data: Vec::new(), }, )) @@ -285,7 +285,7 @@ mod tests { }); // Assume for now that it was just a plain transfer. // TODO: Add tests for different call outcomes. - Ok(ExecReturnValue { status: STATUS_SUCCESS, data: Vec::new() }) + Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: Vec::new() }) } fn terminate( &mut self, @@ -511,16 +511,14 @@ mod tests { ;; value_ptr: u32, ;; value_len: u32, ;;) -> u32 - (import "env" "ext_transfer" (func $ext_transfer (param i32 i32 i32 i32) (result i32))) + (import "env" "ext_transfer" (func $ext_transfer (param i32 i32 i32 i32))) (import "env" "memory" (memory 1 1)) (func (export "call") - (drop - (call $ext_transfer - (i32.const 4) ;; Pointer to "account" address. - (i32.const 8) ;; Length of "account" address. - (i32.const 12) ;; Pointer to the buffer with value to transfer - (i32.const 8) ;; Length of the buffer with value to transfer. - ) + (call $ext_transfer + (i32.const 4) ;; Pointer to "account" address. + (i32.const 8) ;; Length of "account" address. + (i32.const 12) ;; Pointer to the buffer with value to transfer + (i32.const 8) ;; Length of the buffer with value to transfer. ) ) (func (export "deploy")) @@ -551,7 +549,7 @@ mod tests { to: 7, value: 153, data: Vec::new(), - gas_left: 9989000000, + gas_left: 9989500000, }] ); } @@ -874,7 +872,7 @@ mod tests { &mut GasMeter::new(GAS_LIMIT), ).unwrap(); - assert_eq!(output, ExecReturnValue { status: STATUS_SUCCESS, data: [0x22; 32].to_vec() }); + assert_eq!(output, ExecReturnValue { flags: ReturnFlags::empty(), data: [0x22; 32].to_vec() }); } /// calls `ext_caller`, loads the address from the scratch buffer and @@ -1229,7 +1227,7 @@ mod tests { &mut GasMeter::new(GAS_LIMIT), ).unwrap(); - assert_eq!(output, ExecReturnValue { status: STATUS_SUCCESS, data: vec![1, 2, 3, 4] }); + assert_eq!(output, ExecReturnValue { flags: ReturnFlags::empty(), data: vec![1, 2, 3, 4] }); } const CODE_TIMESTAMP_NOW: &str = r#" @@ -1455,7 +1453,7 @@ mod tests { assert_eq!( output, ExecReturnValue { - status: STATUS_SUCCESS, + flags: ReturnFlags::empty(), data: hex!("000102030405060708090A0B0C0D0E0F000102030405060708090A0B0C0D0E0F").to_vec(), }, ); @@ -1653,15 +1651,12 @@ mod tests { (data (i32.const 32) "\20") ;; Deploy routine is the same as call. - (func (export "deploy") (result i32) + (func (export "deploy") (call $call) ) ;; Call reads the first 4 bytes (LE) as the exit status and returns the rest as output data. - (func $call (export "call") (result i32) - (local $buf_size i32) - (local $exit_status i32) - + (func $call (export "call") ;; Copy input data this contract memory. (call $ext_input (i32.const 0) ;; Pointer where to store input @@ -1683,25 +1678,25 @@ mod tests { fn ext_return_with_success_status() { let output = execute( CODE_RETURN_WITH_DATA, - hex!("00112233445566778899").to_vec(), + hex!("00000000445566778899").to_vec(), MockExt::default(), &mut GasMeter::new(GAS_LIMIT), ).unwrap(); - assert_eq!(output, ExecReturnValue { status: 0, data: hex!("445566778899").to_vec() }); + assert_eq!(output, ExecReturnValue { flags: ReturnFlags::empty(), data: hex!("445566778899").to_vec() }); assert!(output.is_success()); } #[test] - fn return_with_failure_status() { + fn return_with_revert_status() { let output = execute( CODE_RETURN_WITH_DATA, - hex!("112233445566778899").to_vec(), + hex!("010000005566778899").to_vec(), MockExt::default(), &mut GasMeter::new(GAS_LIMIT), ).unwrap(); - assert_eq!(output, ExecReturnValue { status: 17, data: hex!("5566778899").to_vec() }); + assert_eq!(output, ExecReturnValue { flags: ReturnFlags::REVERT, data: hex!("5566778899").to_vec() }); assert!(!output.is_success()); } } diff --git a/frame/contracts/src/wasm/prepare.rs b/frame/contracts/src/wasm/prepare.rs index ba934f353ec25..03f33f2dc627f 100644 --- a/frame/contracts/src/wasm/prepare.rs +++ b/frame/contracts/src/wasm/prepare.rs @@ -227,11 +227,7 @@ impl<'a> ContractModule<'a> { }; // Then check the signature. - // Both "call" and "deploy" has a [] -> [] or [] -> [i32] function type. - // - // The [] -> [] signature predates the [] -> [i32] signature and is supported for - // backwards compatibility. This will likely be removed once ink! is updated to - // generate modules with the new function signatures. + // Both "call" and "deploy" has a () -> () function type. let func_ty_idx = func_entries.get(fn_idx as usize) .ok_or_else(|| "export refers to non-existent function")? .type_ref(); diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index 067335a85badc..635c19be05eb3 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -18,12 +18,14 @@ use crate::{Schedule, Trait, CodeHash, BalanceOf}; use crate::exec::{ - Ext, ExecResult, ExecError, ExecReturnValue, StorageKey, TopicOf, STATUS_SUCCESS, + Ext, ExecResult, ExecError, ExecReturnValue, StorageKey, TopicOf, ReturnFlags, }; use crate::gas::{Gas, GasMeter, Token, GasMeterResult}; +use crate::wasm::env_def::ConvertibleToWasm; use sp_sandbox; +use parity_wasm::elements::ValueType; use frame_system; -use sp_std::{prelude::*, convert::TryInto}; +use sp_std::prelude::*; use codec::{Decode, Encode}; use sp_runtime::traits::{Bounded, SaturatedConversion}; use sp_io::hashing::{ @@ -33,10 +35,44 @@ use sp_io::hashing::{ sha2_256, }; -/// The value returned from ext_call and ext_instantiate contract external functions if the call or -/// instantiation traps. This value is chosen as if the execution does not trap, the return value -/// will always be an 8-bit integer, so 0x0100 is the smallest value that could not be returned. -const TRAP_RETURN_CODE: u32 = 0x0100; +/// Every error that can be returned from a runtime API call. +#[repr(u32)] +pub enum ReturnCode { + /// API call successful. + Success = 0, + /// The called function trapped and has its state changes reverted. + /// In this case no output buffer is returned. + /// Can only be returned from `ext_call` and `ext_instantiate`. + CalleeTrapped = 1, + /// The called function ran to completion but decided to revert its state. + /// An output buffer is returned when one was supplied. + /// Can only be returned from `ext_call` and `ext_instantiate`. + CalleeReverted = 2, + /// The passed key does not exist in storage. + KeyNotFound = 3, +} + +impl ConvertibleToWasm for ReturnCode { + type NativeType = Self; + const VALUE_TYPE: ValueType = ValueType::I32; + fn to_typed_value(self) -> sp_sandbox::Value { + sp_sandbox::Value::I32(self as i32) + } + fn from_typed_value(_: sp_sandbox::Value) -> Option { + // We will only ever send these values to wasm but never receive them. + unimplemented!() + } +} + +impl From for ReturnCode { + fn from(from: ExecReturnValue) -> ReturnCode { + if from.flags.contains(ReturnFlags::REVERT) { + Self::CalleeReverted + } else { + Self::Success + } + } +} /// Enumerates all possible *special* trap conditions. /// @@ -90,23 +126,24 @@ pub(crate) fn to_execution_result( ) -> ExecResult { match runtime.special_trap { // The trap was the result of the execution `return` host function. - Some(SpecialTrap::Return(status, data)) => { - let status = (status & 0xFF).try_into() - .expect("exit_code is masked into the range of a u8; qed"); + Some(SpecialTrap::Return(flags, data)) => { + let flags = ReturnFlags::from_bits(flags).ok_or_else(|| ExecError { + reason: "used reserved bit in return flags".into(), + })?; return Ok(ExecReturnValue { - status, + flags, data, }) }, Some(SpecialTrap::Termination) => { return Ok(ExecReturnValue { - status: STATUS_SUCCESS, + flags: ReturnFlags::empty(), data: Vec::new(), }) }, Some(SpecialTrap::Restoration) => { return Ok(ExecReturnValue { - status: STATUS_SUCCESS, + flags: ReturnFlags::empty(), data: Vec::new(), }) } @@ -127,7 +164,7 @@ pub(crate) fn to_execution_result( match sandbox_result { // No traps were generated. Proceed normally. Ok(_) => { - Ok(ExecReturnValue { status: STATUS_SUCCESS, data: Vec::new() }) + Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: Vec::new() }) } // `Error::Module` is returned only if instantiation or linking failed (i.e. // wasm binary tried to import a function that is not provided by the host). @@ -425,14 +462,14 @@ define_env!(Env, , // // - key_ptr: pointer into the linear memory where the key // of the requested value is placed. - ext_get_storage(ctx, key_ptr: u32, out_ptr: u32, out_len_ptr: u32) -> u32 => { + ext_get_storage(ctx, key_ptr: u32, out_ptr: u32, out_len_ptr: u32) -> ReturnCode => { let mut key: StorageKey = [0; 32]; read_sandbox_memory_into_buf(ctx, key_ptr, &mut key)?; if let Some(value) = ctx.ext.get_storage(&key) { write_sandbox_output(ctx, out_ptr, out_len_ptr, &value)?; - Ok(0) + Ok(ReturnCode::Success) } else { - Ok(1) + Ok(ReturnCode::KeyNotFound) } }, @@ -454,16 +491,13 @@ define_env!(Env, , account_len: u32, value_ptr: u32, value_len: u32 - ) -> u32 => { + ) => { let callee: <::T as frame_system::Trait>::AccountId = read_sandbox_memory_as(ctx, account_ptr, account_len)?; let value: BalanceOf<::T> = read_sandbox_memory_as(ctx, value_ptr, value_len)?; - match ctx.ext.transfer(&callee, value, ctx.gas_meter) { - Ok(_) => Ok(0), - Err(_) => Ok(1), - } + ctx.ext.transfer(&callee, value, ctx.gas_meter).map_err(|_| sp_sandbox::HostError) }, // Make a call to another contract. @@ -501,7 +535,7 @@ define_env!(Env, , input_data_len: u32, output_ptr: u32, output_len_ptr: u32 - ) -> u32 => { + ) -> ReturnCode => { let callee: <::T as frame_system::Trait>::AccountId = read_sandbox_memory_as(ctx, callee_ptr, callee_len)?; let value: BalanceOf<::T> = read_sandbox_memory_as(ctx, value_ptr, value_len)?; @@ -532,10 +566,10 @@ define_env!(Env, , match call_outcome { Ok(output) => { write_sandbox_output(ctx, output_ptr, output_len_ptr, &output.data)?; - Ok(output.status.into()) + Ok(output.into()) }, Err(_) => { - Ok(TRAP_RETURN_CODE) + Ok(ReturnCode::CalleeTrapped) }, } }, @@ -588,7 +622,7 @@ define_env!(Env, , address_len_ptr: u32, output_ptr: u32, output_len_ptr: u32 - ) -> u32 => { + ) -> ReturnCode => { let code_hash: CodeHash<::T> = read_sandbox_memory_as(ctx, code_hash_ptr, code_hash_len)?; let value: BalanceOf<::T> = read_sandbox_memory_as(ctx, value_ptr, value_len)?; @@ -617,14 +651,14 @@ define_env!(Env, , }); match instantiate_outcome { Ok((address, output)) => { - if output.is_success() { + if !output.flags.contains(ReturnFlags::REVERT) { write_sandbox_output(ctx, address_ptr, address_len_ptr, &address.encode())?; } write_sandbox_output(ctx, output_ptr, output_len_ptr, &output.data)?; - Ok(output.status.into()) + Ok(output.into()) }, Err(_) => { - Ok(TRAP_RETURN_CODE) + Ok(ReturnCode::CalleeTrapped) }, } }, @@ -665,7 +699,7 @@ define_env!(Env, , // successful result to the caller. // // This is the only way to return a data buffer to the caller. - ext_return(ctx, status_code: u32, data_ptr: u32, data_len: u32) => { + ext_return(ctx, flags: u32, data_ptr: u32, data_len: u32) => { charge_gas( ctx.gas_meter, ctx.schedule, @@ -674,7 +708,7 @@ define_env!(Env, , )?; let output_data = read_sandbox_memory(ctx, data_ptr, data_len)?; - ctx.special_trap = Some(SpecialTrap::Return(status_code, output_data)); + ctx.special_trap = Some(SpecialTrap::Return(flags, output_data)); // The trap mechanism is used to immediately terminate the execution. // This trap should be handled appropriately before returning the result From c86c2ad562c266ffaabecd66c917a21e08dd235e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Thu, 2 Jul 2020 13:21:01 +0200 Subject: [PATCH 08/23] Updated inline documentation --- frame/contracts/fixtures/caller_contract.wat | 4 +- frame/contracts/fixtures/crypto_hashes.wat | 5 +- frame/contracts/src/wasm/mod.rs | 11 +- frame/contracts/src/wasm/runtime.rs | 242 +++++++++++++------ 4 files changed, 178 insertions(+), 84 deletions(-) diff --git a/frame/contracts/fixtures/caller_contract.wat b/frame/contracts/fixtures/caller_contract.wat index 459e6ce1c77ad..369007834dc7b 100644 --- a/frame/contracts/fixtures/caller_contract.wat +++ b/frame/contracts/fixtures/caller_contract.wat @@ -185,7 +185,7 @@ (i32.eq (get_local $exit_code) (i32.const 2)) ;; ReturnCode::CalleeReverted ) - ;; Check that scratch buffer contains the expected return data. + ;; Check that output buffer contains the expected return data. (call $assert (i32.eq (i32.load (i32.sub (get_local $sp) (i32.const 8))) (i32.const 3)) ) @@ -258,7 +258,7 @@ (i32.eq (get_local $exit_code) (i32.const 0)) ;; ReturnCode::Success ) - ;; Check that scratch buffer contains the expected return data. + ;; Check that the output buffer contains the expected return data. (call $assert (i32.eq (i32.load (i32.sub (get_local $sp) (i32.const 8))) (i32.const 4)) ) diff --git a/frame/contracts/fixtures/crypto_hashes.wat b/frame/contracts/fixtures/crypto_hashes.wat index 0ba60e42dfd68..f7b244b8c1e36 100644 --- a/frame/contracts/fixtures/crypto_hashes.wat +++ b/frame/contracts/fixtures/crypto_hashes.wat @@ -24,8 +24,7 @@ ;; Called by the tests. ;; - ;; The `call` function expects data in a certain format in the scratch - ;; buffer. + ;; The `call` function expects data in a certain format in the input buffer. ;; ;; 1. The first byte encodes an identifier for the crypto hash function ;; under test. (*) @@ -33,7 +32,7 @@ ;; crypto hash function chosen in 1. ;; ;; The `deploy` function then computes the chosen crypto hash function - ;; given the input and puts the result back into the scratch buffer. + ;; given the input and puts the result into the output buffer. ;; After contract execution the test driver then asserts that the returned ;; values are equal to the expected bytes for the input and chosen hash ;; function. diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index f38197d710d4f..6b521696d9dc7 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -823,7 +823,7 @@ mod tests { (func (export "call") (local $buf_size i32) - ;; Load a storage value into the scratch buf. + ;; Load a storage value into contract memory. (call $assert (i32.eq (call $ext_get_storage @@ -875,8 +875,7 @@ mod tests { assert_eq!(output, ExecReturnValue { flags: ReturnFlags::empty(), data: [0x22; 32].to_vec() }); } - /// calls `ext_caller`, loads the address from the scratch buffer and - /// compares it with the constant 42. + /// calls `ext_caller` and compares the result with the constant 42. const CODE_CALLER: &str = r#" (module (import "env" "ext_caller" (func $ext_caller (param i32 i32))) @@ -929,8 +928,7 @@ mod tests { ).unwrap(); } - /// calls `ext_address`, loads the address from the scratch buffer and - /// compares it with the constant 69. + /// calls `ext_address` and compares the result with the constant 69. const CODE_ADDRESS: &str = r#" (module (import "env" "ext_address" (func $ext_address (param i32 i32))) @@ -1588,8 +1586,7 @@ mod tests { ); } - /// calls `ext_block_number`, loads the current block number from the scratch buffer and - /// compares it with the constant 121. + /// calls `ext_block_number` compares the result with the constant 121. const CODE_BLOCK_NUMBER: &str = r#" (module (import "env" "ext_block_number" (func $ext_block_number (param i32 i32))) diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index 635c19be05eb3..69cbe7544565c 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -456,12 +456,23 @@ define_env!(Env, , Ok(()) }, - // Retrieve the value under the given key from the storage and return 0. - // If there is no entry under the given key then this function will return 1 and - // clear the scratch buffer. + // Retrieve the value under the given key from storage. // - // - key_ptr: pointer into the linear memory where the key - // of the requested value is placed. + // # Parameters + // + // - `key_ptr`: pointer into the linear memory where the key of the requested value is placed. + // - `out_ptr`: pointer to the linear memory where the value is written to. + // - `out_len_ptr`: in-out pointer into linear memory where the buffer length + // is read from and the value length is written to. + // + // # Errors + // + // If there is no entry under the given key then this function will return + // `ReturnCode::KeyNotFound`. + // + // # Traps + // + // Traps if the supplied buffer length is smaller than the size of the stored value. ext_get_storage(ctx, key_ptr: u32, out_ptr: u32, out_len_ptr: u32) -> ReturnCode => { let mut key: StorageKey = [0; 32]; read_sandbox_memory_into_buf(ctx, key_ptr, &mut key)?; @@ -475,9 +486,7 @@ define_env!(Env, , // Transfer some value to another account. // - // If the value transfer was succesful zero is returned. Otherwise one is returned. - // The scratch buffer is not touched. The receiver can be a plain account or - // a contract. + // # Parameters // // - account_ptr: a pointer to the address of the beneficiary account // Should be decodable as an `T::AccountId`. Traps otherwise. @@ -485,6 +494,12 @@ define_env!(Env, , // - value_ptr: a pointer to the buffer with value, how much value to send. // Should be decodable as a `T::Balance`. Traps otherwise. // - value_len: length of the value buffer. + // + // # Traps + // + // Traps if the transfer wasn't succesful. This can happen when the value transfered + // brings the sender below the existential deposit. Use `ext_terminate` to remove + // the caller contract. ext_transfer( ctx, account_ptr: u32, @@ -502,18 +517,11 @@ define_env!(Env, , // Make a call to another contract. // - // If the called contract runs to completion, then this returns the status code the callee - // returns on exit in the bottom 8 bits of the return value. The top 24 bits are 0s. A status - // code of 0 indicates success, and any other code indicates a failure. On failure, any state - // changes made by the called contract are reverted. The scratch buffer is filled with the - // output data returned by the called contract, even in the case of a failure status. - // - // This call fails if it would bring the calling contract below the existential deposit. - // In order to destroy a contract `ext_terminate` must be used. + // The callees output buffer is copied to `output_ptr` and its length to `output_len_ptr`. + // The copy of the output buffer can be skipped by supplying the sentinel value + // of `u32::max_value()` to `output_ptr`. // - // If the contract traps during execution or otherwise fails to complete successfully, then - // this function clears the scratch buffer and returns 0x0100. As with a failure status, any - // state changes made by the called contract are reverted. + // # Parameters // // - callee_ptr: a pointer to the address of the callee contract. // Should be decodable as an `T::AccountId`. Traps otherwise. @@ -524,6 +532,23 @@ define_env!(Env, , // - value_len: length of the value buffer. // - input_data_ptr: a pointer to a buffer to be used as input data to the callee. // - input_data_len: length of the input data buffer. + // - output_ptr: a pointer where the output buffer is copied to. + // - output_len_ptr: in-out pointer to where the length of the buffer is read from + // and the actual length is written to. + // + // # Errors + // + // `ReturnCode::CalleeReverted`: The callee ran to completion but decided to have its + // changes reverted. The delivery of the output buffer is still possible. + // `ReturnCode::CalleeTrapped`: The callee trapped during execution. All changes are reverted + // and no output buffer is delivered. + // + // # Traps + // + // - Transfer of balance failed. This call can not bring the sender below the existential + // deposit. Use `ext_terminate` to remove the caller. + // - Callee does not exist. + // - Supplied output buffer is too small. ext_call( ctx, callee_ptr: u32, @@ -577,29 +602,14 @@ define_env!(Env, , // Instantiate a contract with the specified code hash. // // This function creates an account and executes the constructor defined in the code specified - // by the code hash. - // - // If the constructor runs to completion, then this returns the status code that the newly - // instantiated contract returns on exit in the bottom 8 bits of the return value. The top 24 - // bits are 0s. A status code of 0 indicates success, and any other code indicates a failure. - // On failure, any state changes made by the called contract are reverted and the contract is - // not instantiated. On a success status, the scratch buffer is filled with the encoded address - // of the newly instantiated contract. In the case of a failure status, the scratch buffer is - // cleared. + // by the code hash. The address of this new account is copied to `address_ptr` and its length + // to `address_len_ptr`. // - // This call fails if it would bring the calling contract below the existential deposit. - // In order to destroy a contract `ext_terminate` must be used. + // The constructors output buffer is copied to `output_ptr` and its length to `output_len_ptr`. + // The copy of the output buffer can be skipped by supplying the sentinel value + // of `u32::max_value()` to `output_ptr`. // - // If the contract traps during execution or otherwise fails to complete successfully, then - // this function clears the scratch buffer and returns 0x0100. As with a failure status, any - // state changes made by the called contract are reverted. - - // This function creates an account and executes initializer code. After the execution, - // the returned buffer is saved as the code of the created account. - // - // Returns 0 on the successful contract instantiation and puts the address of the instantiated - // contract into the scratch buffer. Otherwise, returns non-zero value and clears the scratch - // buffer. + // # Parameters // // - code_hash_ptr: a pointer to the buffer that contains the initializer code. // - code_hash_len: length of the initializer code buffer. @@ -609,6 +619,28 @@ define_env!(Env, , // - value_len: length of the value buffer. // - input_data_ptr: a pointer to a buffer to be used as input data to the initializer code. // - input_data_len: length of the input data buffer. + // - address_ptr: a pointer where the new account's address is copied to. + // - address_len_ptr: in-out pointer to where the length of the buffer is read from + // and the actual length is written to. + // - output_ptr: a pointer where the output buffer is copied to. + // - output_len_ptr: in-out pointer to where the length of the buffer is read from + // and the actual length is written to. + // + // # Errors + // + // `ReturnCode::CalleeReverted`: The callee's constructor ran to completion but decided to have + // its changes reverted. The delivery of the output buffer is still possible but the + // account was not created and no address is returned. + // `ReturnCode::CalleeTrapped`: The callee trapped during execution. All changes are reverted + // and no output buffer is delivered. The accounts was not created and no address is + // returned. + // + // # Traps + // + // - Transfer of balance failed. This call can not bring the sender below the existential + // deposit. Use `ext_terminate` to remove the caller. + // - Code hash does not exist. + // - Supplied output buffers are too small. ext_instantiate( ctx, code_hash_ptr: u32, @@ -695,10 +727,23 @@ define_env!(Env, , } }, - // Save a data buffer as a result of the execution, terminate the execution and return a - // successful result to the caller. + // Cease contract execution and save a data buffer as a result of the execution. + // + // This function never retuns as it stops execution of the caller. + // This is the only way to return a data buffer to the caller. Returning from + // execution without calling this function is equivalent to calling: + // ``` + // ext_return(0, 0, 0); + // ``` // - // This is the only way to return a data buffer to the caller. + // The flags argument is a bitfield that can be used to signal special return + // conditions to the supervisor: + // --- lsb --- + // bit 0 : REVERT - Revert all storage changes made by the caller. + // bit [1, 31]: Reserved for future use. + // --- msb --- + // + // Using a reserved bit triggers a trap. ext_return(ctx, flags: u32, data_ptr: u32, data_len: u32) => { charge_gas( ctx.gas_meter, @@ -716,55 +761,91 @@ define_env!(Env, , Err(sp_sandbox::HostError) }, - // Stores the address of the caller into the scratch buffer. + // Stores the address of the caller into the supplied buffer. + // + // The value is stored to linear memory at the address pointed to by `out_ptr`. + // `out_len_ptr` must point to a u32 value that describes the available space at + // `out_ptr`. This call overwrites it with the size of the value. If the available + // space at `out_ptr` is less than the size of the value a trap is triggered. // // If this is a top-level call (i.e. initiated by an extrinsic) the origin address of the // extrinsic will be returned. Otherwise, if this call is initiated by another contract then the - // address of the contract will be returned. + // address of the contract will be returned. The value is encoded as T::AccountId. ext_caller(ctx, out_ptr: u32, out_len_ptr: u32) => { write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.caller().encode()) }, - // Stores the address of the current contract into the scratch buffer. + // Stores the address of the current contract into the supplied buffer. + // + // The value is stored to linear memory at the address pointed to by `out_ptr`. + // `out_len_ptr` must point to a u32 value that describes the available space at + // `out_ptr`. This call overwrites it with the size of the value. If the available + // space at `out_ptr` is less than the size of the value a trap is triggered. ext_address(ctx, out_ptr: u32, out_len_ptr: u32) => { write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.address().encode()) }, - // Stores the price for the specified amount of gas in scratch buffer. + // Stores the price for the specified amount of gas into the supplied buffer. + // + // The value is stored to linear memory at the address pointed to by `out_ptr`. + // `out_len_ptr` must point to a u32 value that describes the available space at + // `out_ptr`. This call overwrites it with the size of the value. If the available + // space at `out_ptr` is less than the size of the value a trap is triggered. + // + // The data is encoded as T::Balance. + // + // # Note // - // The data is encoded as T::Balance. The current contents of the scratch buffer are overwritten. // It is recommended to avoid specifying very small values for `gas` as the prices for a single // gas can be smaller than one. ext_gas_price(ctx, gas: u64, out_ptr: u32, out_len_ptr: u32) => { write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.get_weight_price(gas).encode()) }, - // Stores the amount of gas left into the scratch buffer. + // Stores the amount of gas left into the supplied buffer. + // + // The value is stored to linear memory at the address pointed to by `out_ptr`. + // `out_len_ptr` must point to a u32 value that describes the available space at + // `out_ptr`. This call overwrites it with the size of the value. If the available + // space at `out_ptr` is less than the size of the value a trap is triggered. // - // The data is encoded as Gas. The current contents of the scratch buffer are overwritten. + // The data is encoded as Gas. ext_gas_left(ctx, out_ptr: u32, out_len_ptr: u32) => { write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.gas_meter.gas_left().encode()) }, - // Stores the balance of the current account into the scratch buffer. + // Stores the balance of the current account into the supplied buffer. // - // The data is encoded as T::Balance. The current contents of the scratch buffer are overwritten. + // The value is stored to linear memory at the address pointed to by `out_ptr`. + // `out_len_ptr` must point to a u32 value that describes the available space at + // `out_ptr`. This call overwrites it with the size of the value. If the available + // space at `out_ptr` is less than the size of the value a trap is triggered. + // + // The data is encoded as T::Balance. ext_balance(ctx, out_ptr: u32, out_len_ptr: u32) => { write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.balance().encode()) }, - // Stores the value transferred along with this call or as endowment into the scratch buffer. + // Stores the value transferred along with this call or as endowment into the supplied buffer. + // + // The value is stored to linear memory at the address pointed to by `out_ptr`. + // `out_len_ptr` must point to a u32 value that describes the available space at + // `out_ptr`. This call overwrites it with the size of the value. If the available + // space at `out_ptr` is less than the size of the value a trap is triggered. // - // The data is encoded as T::Balance. The current contents of the scratch buffer are overwritten. + // The data is encoded as T::Balance. ext_value_transferred(ctx, out_ptr: u32, out_len_ptr: u32) => { write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.value_transferred().encode()) }, - // Stores the random number for the current block for the given subject into the scratch - // buffer. + // Stores a random number for the current block and the given subject into the supplied buffer. // - // The data is encoded as T::Hash. The current contents of the scratch buffer are - // overwritten. + // The value is stored to linear memory at the address pointed to by `out_ptr`. + // `out_len_ptr` must point to a u32 value that describes the available space at + // `out_ptr`. This call overwrites it with the size of the value. If the available + // space at `out_ptr` is less than the size of the value a trap is triggered. + // + // The data is encoded as T::Hash. ext_random(ctx, subject_ptr: u32, subject_len: u32, out_ptr: u32, out_len_ptr: u32) => { // The length of a subject can't exceed `max_subject_len`. if subject_len > ctx.schedule.max_subject_len { @@ -774,23 +855,31 @@ define_env!(Env, , write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.random(&subject_buf).encode()) }, - // Load the latest block timestamp into the scratch buffer + // Load the latest block timestamp into the supplied buffer + // + // The value is stored to linear memory at the address pointed to by `out_ptr`. + // `out_len_ptr` must point to a u32 value that describes the available space at + // `out_ptr`. This call overwrites it with the size of the value. If the available + // space at `out_ptr` is less than the size of the value a trap is triggered. ext_now(ctx, out_ptr: u32, out_len_ptr: u32) => { write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.now().encode()) }, - // Stores the minimum balance (a.k.a. existential deposit) into the scratch buffer. + // Stores the minimum balance (a.k.a. existential deposit) into the supplied buffer. // - // The data is encoded as T::Balance. The current contents of the scratch buffer are - // overwritten. + // The data is encoded as T::Balance. ext_minimum_balance(ctx, out_ptr: u32, out_len_ptr: u32) => { write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.minimum_balance().encode()) }, - // Stores the tombstone deposit into the scratch buffer. + // Stores the tombstone deposit into the supplied buffer. + // + // The value is stored to linear memory at the address pointed to by `out_ptr`. + // `out_len_ptr` must point to a u32 value that describes the available space at + // `out_ptr`. This call overwrites it with the size of the value. If the available + // space at `out_ptr` is less than the size of the value a trap is triggered. // - // The data is encoded as T::Balance. The current contents of the scratch - // buffer are overwritten. + // The data is encoded as T::Balance. // // # Note // @@ -922,9 +1011,14 @@ define_env!(Env, , Ok(()) }, - // Stores the rent allowance into the scratch buffer. + // Stores the rent allowance into the supplied buffer. // - // The data is encoded as T::Balance. The current contents of the scratch buffer are overwritten. + // The value is stored to linear memory at the address pointed to by `out_ptr`. + // `out_len_ptr` must point to a u32 value that describes the available space at + // `out_ptr`. This call overwrites it with the size of the value. If the available + // space at `out_ptr` is less than the size of the value a trap is triggered. + // + // The data is encoded as T::Balance. ext_rent_allowance(ctx, out_ptr: u32, out_len_ptr: u32) => { write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.rent_allowance().encode()) }, @@ -940,7 +1034,12 @@ define_env!(Env, , Ok(()) }, - // Stores the current block number of the current contract into the scratch buffer. + // Stores the current block number of the current contract into the supplied buffer. + // + // The value is stored to linear memory at the address pointed to by `out_ptr`. + // `out_len_ptr` must point to a u32 value that describes the available space at + // `out_ptr`. This call overwrites it with the size of the value. If the available + // space at `out_ptr` is less than the size of the value a trap is triggered. ext_block_number(ctx, out_ptr: u32, out_len_ptr: u32) => { write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.block_number().encode()) }, @@ -1042,7 +1141,7 @@ define_env!(Env, , }, ); -/// Computes the given hash function on the scratch buffer. +/// Computes the given hash function on the supplied input. /// /// Reads from the sandboxed input buffer into an intermediate buffer. /// Returns the result directly to the output buffer of the sandboxed memory. @@ -1066,10 +1165,9 @@ where F: FnOnce(&[u8]) -> R, R: AsRef<[u8]>, { - // Copy the input buffer directly into the scratch buffer to avoid - // heap allocations. + // Copy input into supervisor memory. let input = read_sandbox_memory(ctx, input_ptr, input_len)?; - // Compute the hash on the scratch buffer using the given hash function. + // Compute the hash on the input buffer using the given hash function. let hash = hash_fn(&input); // Write the resulting hash back into the sandboxed output buffer. write_sandbox_memory( From ddf69ab7730d8470be8d0baa333428291e6b3c84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Fri, 3 Jul 2020 09:52:25 +0200 Subject: [PATCH 09/23] Prevent skipping of copying the output for getter API --- frame/contracts/src/wasm/mod.rs | 2 +- frame/contracts/src/wasm/runtime.rs | 59 +++++++++++++++++------------ 2 files changed, 36 insertions(+), 25 deletions(-) diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index 6b521696d9dc7..5dae7fc339648 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -646,7 +646,7 @@ mod tests { (i32.const 8) ;; Length of the buffer with value to transfer (i32.const 12) ;; Pointer to input data buffer address (i32.const 4) ;; Length of input data buffer - (i32.const 4294967295) ;; u32 max value is the sentinel value: do not copy output + (i32.const 4294967295) ;; u32 max value is the sentinel value: do not copy address (i32.const 0) ;; Length is ignored in this case (i32.const 4294967295) ;; u32 max value is the sentinel value: do not copy output (i32.const 0) ;; Length is ignored in this case diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index 69cbe7544565c..b813604143cfd 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -355,8 +355,8 @@ fn write_sandbox_memory( /// lenght of the buffer located at `out_ptr`. If that buffer is large enough the actual /// `buf.len()` is written to this location. /// -/// If `out_ptr` is set to the sentinel value of `u32::max_value()` the operation is -/// skipped and `Ok` is returned. This is supposed to help callers to make copying +/// If `out_ptr` is set to the sentinel value of `u32::max_value()` and `allow_skip` is true the +/// operation is skipped and `Ok` is returned. This is supposed to help callers to make copying /// output optional. For example to skip copying back the output buffer of an `ext_call` /// when the caller is not interested in the result. /// @@ -367,8 +367,9 @@ fn write_sandbox_output( out_ptr: u32, out_len_ptr: u32, buf: &[u8], + allow_skip: bool, ) -> Result<(), sp_sandbox::HostError> { - if out_ptr == u32::max_value() { + if allow_skip && out_ptr == u32::max_value() { return Ok(()); } @@ -477,7 +478,7 @@ define_env!(Env, , let mut key: StorageKey = [0; 32]; read_sandbox_memory_into_buf(ctx, key_ptr, &mut key)?; if let Some(value) = ctx.ext.get_storage(&key) { - write_sandbox_output(ctx, out_ptr, out_len_ptr, &value)?; + write_sandbox_output(ctx, out_ptr, out_len_ptr, &value, false)?; Ok(ReturnCode::Success) } else { Ok(ReturnCode::KeyNotFound) @@ -590,7 +591,7 @@ define_env!(Env, , match call_outcome { Ok(output) => { - write_sandbox_output(ctx, output_ptr, output_len_ptr, &output.data)?; + write_sandbox_output(ctx, output_ptr, output_len_ptr, &output.data, true)?; Ok(output.into()) }, Err(_) => { @@ -603,11 +604,11 @@ define_env!(Env, , // // This function creates an account and executes the constructor defined in the code specified // by the code hash. The address of this new account is copied to `address_ptr` and its length - // to `address_len_ptr`. + // to `address_len_ptr`. The constructors output buffer is copied to `output_ptr` and its + // length to `output_len_ptr`. // - // The constructors output buffer is copied to `output_ptr` and its length to `output_len_ptr`. - // The copy of the output buffer can be skipped by supplying the sentinel value - // of `u32::max_value()` to `output_ptr`. + // The copy of the output buffer and address can be skipped by supplying the sentinel value + // of `u32::max_value()` to `output_ptr` or `address_ptr`. // // # Parameters // @@ -684,9 +685,11 @@ define_env!(Env, , match instantiate_outcome { Ok((address, output)) => { if !output.flags.contains(ReturnFlags::REVERT) { - write_sandbox_output(ctx, address_ptr, address_len_ptr, &address.encode())?; + write_sandbox_output( + ctx, address_ptr, address_len_ptr, &address.encode(), true + )?; } - write_sandbox_output(ctx, output_ptr, output_len_ptr, &output.data)?; + write_sandbox_output(ctx, output_ptr, output_len_ptr, &output.data, true)?; Ok(output.into()) }, Err(_) => { @@ -721,7 +724,7 @@ define_env!(Env, , ext_input(ctx, buf_ptr: u32, buf_len_ptr: u32) => { if let Some(input) = ctx.input_data.take() { - write_sandbox_output(ctx, buf_ptr, buf_len_ptr, &input) + write_sandbox_output(ctx, buf_ptr, buf_len_ptr, &input, false) } else { Err(sp_sandbox::HostError) } @@ -772,7 +775,7 @@ define_env!(Env, , // extrinsic will be returned. Otherwise, if this call is initiated by another contract then the // address of the contract will be returned. The value is encoded as T::AccountId. ext_caller(ctx, out_ptr: u32, out_len_ptr: u32) => { - write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.caller().encode()) + write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.caller().encode(), false) }, // Stores the address of the current contract into the supplied buffer. @@ -782,7 +785,7 @@ define_env!(Env, , // `out_ptr`. This call overwrites it with the size of the value. If the available // space at `out_ptr` is less than the size of the value a trap is triggered. ext_address(ctx, out_ptr: u32, out_len_ptr: u32) => { - write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.address().encode()) + write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.address().encode(), false) }, // Stores the price for the specified amount of gas into the supplied buffer. @@ -799,7 +802,9 @@ define_env!(Env, , // It is recommended to avoid specifying very small values for `gas` as the prices for a single // gas can be smaller than one. ext_gas_price(ctx, gas: u64, out_ptr: u32, out_len_ptr: u32) => { - write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.get_weight_price(gas).encode()) + write_sandbox_output( + ctx, out_ptr, out_len_ptr, &ctx.ext.get_weight_price(gas).encode(), false + ) }, // Stores the amount of gas left into the supplied buffer. @@ -811,7 +816,7 @@ define_env!(Env, , // // The data is encoded as Gas. ext_gas_left(ctx, out_ptr: u32, out_len_ptr: u32) => { - write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.gas_meter.gas_left().encode()) + write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.gas_meter.gas_left().encode(), false) }, // Stores the balance of the current account into the supplied buffer. @@ -823,7 +828,7 @@ define_env!(Env, , // // The data is encoded as T::Balance. ext_balance(ctx, out_ptr: u32, out_len_ptr: u32) => { - write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.balance().encode()) + write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.balance().encode(), false) }, // Stores the value transferred along with this call or as endowment into the supplied buffer. @@ -835,7 +840,9 @@ define_env!(Env, , // // The data is encoded as T::Balance. ext_value_transferred(ctx, out_ptr: u32, out_len_ptr: u32) => { - write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.value_transferred().encode()) + write_sandbox_output( + ctx, out_ptr, out_len_ptr, &ctx.ext.value_transferred().encode(), false + ) }, // Stores a random number for the current block and the given subject into the supplied buffer. @@ -852,7 +859,9 @@ define_env!(Env, , return Err(sp_sandbox::HostError); } let subject_buf = read_sandbox_memory(ctx, subject_ptr, subject_len)?; - write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.random(&subject_buf).encode()) + write_sandbox_output( + ctx, out_ptr, out_len_ptr, &ctx.ext.random(&subject_buf).encode(), false + ) }, // Load the latest block timestamp into the supplied buffer @@ -862,14 +871,14 @@ define_env!(Env, , // `out_ptr`. This call overwrites it with the size of the value. If the available // space at `out_ptr` is less than the size of the value a trap is triggered. ext_now(ctx, out_ptr: u32, out_len_ptr: u32) => { - write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.now().encode()) + write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.now().encode(), false) }, // Stores the minimum balance (a.k.a. existential deposit) into the supplied buffer. // // The data is encoded as T::Balance. ext_minimum_balance(ctx, out_ptr: u32, out_len_ptr: u32) => { - write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.minimum_balance().encode()) + write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.minimum_balance().encode(), false) }, // Stores the tombstone deposit into the supplied buffer. @@ -888,7 +897,9 @@ define_env!(Env, , // below the sum of existential deposit and the tombstone deposit. The sum // is commonly referred as subsistence threshold in code. ext_tombstone_deposit(ctx, out_ptr: u32, out_len_ptr: u32) => { - write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.tombstone_deposit().encode()) + write_sandbox_output( + ctx, out_ptr, out_len_ptr, &ctx.ext.tombstone_deposit().encode(), false + ) }, // Try to restore the given destination contract sacrificing the caller. @@ -1020,7 +1031,7 @@ define_env!(Env, , // // The data is encoded as T::Balance. ext_rent_allowance(ctx, out_ptr: u32, out_len_ptr: u32) => { - write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.rent_allowance().encode()) + write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.rent_allowance().encode(), false) }, // Prints utf8 encoded string from the data buffer. @@ -1041,7 +1052,7 @@ define_env!(Env, , // `out_ptr`. This call overwrites it with the size of the value. If the available // space at `out_ptr` is less than the size of the value a trap is triggered. ext_block_number(ctx, out_ptr: u32, out_len_ptr: u32) => { - write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.block_number().encode()) + write_sandbox_output(ctx, out_ptr, out_len_ptr, &ctx.ext.block_number().encode(), false) }, // Computes the SHA2 256-bit hash on the given input buffer. From 868668ccb016c17fd47f0ba167dc6878e01f7b0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Fri, 3 Jul 2020 12:23:43 +0200 Subject: [PATCH 10/23] Return gas_consumed from the RPC contracts call interface --- bin/node/runtime/src/lib.rs | 3 ++- frame/contracts/rpc/runtime-api/src/lib.rs | 2 ++ frame/contracts/rpc/src/lib.rs | 11 +++++++++-- frame/contracts/src/gas.rs | 4 ++-- frame/contracts/src/lib.rs | 13 +++++++++---- frame/contracts/src/tests.rs | 2 +- 6 files changed, 25 insertions(+), 10 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 96a17dfa94f57..ceffe496dd4bb 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1009,12 +1009,13 @@ impl_runtime_apis! { gas_limit: u64, input_data: Vec, ) -> ContractExecResult { - let exec_result = + let (exec_result, gas_consumed) = Contracts::bare_call(origin, dest.into(), value, gas_limit, input_data); match exec_result { Ok(v) => ContractExecResult::Success { flags: v.flags.bits(), data: v.data, + gas_consumed: gas_consumed, }, Err(_) => ContractExecResult::Error, } diff --git a/frame/contracts/rpc/runtime-api/src/lib.rs b/frame/contracts/rpc/runtime-api/src/lib.rs index 5fb3f28bcf7ce..7d208cf7763e7 100644 --- a/frame/contracts/rpc/runtime-api/src/lib.rs +++ b/frame/contracts/rpc/runtime-api/src/lib.rs @@ -42,6 +42,8 @@ pub enum ContractExecResult { /// /// Can be empty. data: Vec, + /// How much gas was consumed by the call. + gas_consumed: u64, }, /// The contract execution either trapped or returned an error. Error, diff --git a/frame/contracts/rpc/src/lib.rs b/frame/contracts/rpc/src/lib.rs index 453764c77d7e2..d99ed1e78a652 100644 --- a/frame/contracts/rpc/src/lib.rs +++ b/frame/contracts/rpc/src/lib.rs @@ -96,6 +96,8 @@ pub enum RpcContractExecResult { flags: u32, /// Output data data: Bytes, + /// How much gas was consumed by the call. + gas_consumed: u64, }, /// Error execution Error(()), @@ -104,9 +106,14 @@ pub enum RpcContractExecResult { impl From for RpcContractExecResult { fn from(r: ContractExecResult) -> Self { match r { - ContractExecResult::Success { flags, data } => RpcContractExecResult::Success { + ContractExecResult::Success { + flags, + data, + gas_consumed + } => RpcContractExecResult::Success { flags, data: data.into(), + gas_consumed, }, ContractExecResult::Error => RpcContractExecResult::Error(()), } @@ -309,7 +316,7 @@ mod tests { let actual = serde_json::to_string(&res).unwrap(); assert_eq!(actual, expected); } - test(r#"{"success":{"flags":5,"data":"0x1234"}}"#); + test(r#"{"success":{"flags":5,"data":"0x1234","gas_consumed":5000}}"#); test(r#"{"error":null}"#); } } diff --git a/frame/contracts/src/gas.rs b/frame/contracts/src/gas.rs index 38f231c008f8b..0ae1952de0914 100644 --- a/frame/contracts/src/gas.rs +++ b/frame/contracts/src/gas.rs @@ -178,8 +178,8 @@ impl GasMeter { } } - /// Returns how much gas left from the initial budget. - fn gas_spent(&self) -> Gas { + /// Returns how much gas was used. + pub fn gas_spent(&self) -> Gas { self.gas_limit - self.gas_left } diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 367d78b7e5799..de8c9a49a973a 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -587,17 +587,22 @@ impl Module { /// /// This function is similar to `Self::call`, but doesn't perform any address lookups and better /// suitable for calling directly from Rust. + /// + /// It returns the exection result and the amount of used weight. pub fn bare_call( origin: T::AccountId, dest: T::AccountId, value: BalanceOf, gas_limit: Gas, input_data: Vec, - ) -> ExecResult { + ) -> (ExecResult, Gas) { let mut gas_meter = GasMeter::new(gas_limit); - Self::execute_wasm(origin, &mut gas_meter, |ctx, gas_meter| { - ctx.call(dest, value, gas_meter, input_data) - }) + ( + Self::execute_wasm(origin, &mut gas_meter, |ctx, gas_meter| { + ctx.call(dest, value, gas_meter, input_data) + }), + gas_meter.gas_spent(), + ) } /// Query storage of a specified contract under a specified key. diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index 1c868c5670d68..791b00fe4cdc8 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -1591,7 +1591,7 @@ fn crypto_hashes() { 0, GAS_LIMIT, params, - ).unwrap(); + ).0.unwrap(); assert!(result.is_success()); let expected = hash_fn(input.as_ref()); assert_eq!(&result.data[..*expected_size], &*expected); From 84973b043d7f2adabf781ee3d08401d6d00f9c50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Fri, 3 Jul 2020 16:43:45 +0200 Subject: [PATCH 11/23] Updated COMPLEXTITY.md --- frame/contracts/COMPLEXITY.md | 225 +++++++++++++++------------------- 1 file changed, 97 insertions(+), 128 deletions(-) diff --git a/frame/contracts/COMPLEXITY.md b/frame/contracts/COMPLEXITY.md index 7e8c2903c79b9..1fbc7390e95ba 100644 --- a/frame/contracts/COMPLEXITY.md +++ b/frame/contracts/COMPLEXITY.md @@ -4,19 +4,19 @@ This analysis is on the computing and memory complexity of specific procedures. The primary goal is to come up with decent pricing for functions that can be invoked by a user (via extrinsics) or by untrusted code that prevents DoS attacks. -# Sandboxing +## Sandboxing It makes sense to describe the sandboxing module first because the smart-contract module is built upon it. -## Memory +### Memory -### set +#### set Copies data from the supervisor's memory to the guest's memory. **complexity**: It doesn't allocate, and the computational complexity is proportional to the number of bytes to copy. -### get +#### get Copies data from the guest's memory to the supervisor's memory. @@ -78,17 +78,10 @@ The size of the arguments and the return value depends on the exact function in **complexity**: Memory and computational complexity can be considered as a constant. -# `AccountDb` +## Transactional Storage -`AccountDb` is an abstraction that supports collecting changes to accounts with the ability to efficiently reverting them. Contract -execution contexts operate on the AccountDb. All changes are flushed into underlying storage only after origin transaction succeeds. - -## Relation to the underlying storage - -At present, `AccountDb` is implemented as a cascade of overlays with the direct storage at the bottom. The direct -storage `AccountDb` leverages child tries. Each overlay is represented by a `Map`. On a commit from an overlay to an -overlay, maps are merged. On commit from an overlay to the bottommost `AccountDb` all changes are flushed to the storage -and on revert, the overlay is just discarded. +The contracts module makes use of the nested storage transactions feature offered by +the underlying storage which allows efficient roll back of changes made by contracts. > ℹ️ The underlying storage has a overlay layer implemented as a `Map`. If the runtime reads a storage location and the > respective key doesn't exist in the overlay, then the underlying storage performs a DB access, but the value won't be @@ -105,23 +98,24 @@ storage access. ## get_storage, get_code_hash, get_rent_allowance, get_balance, contract_exists -These functions check the local cache for a requested value and, if it is there, the value is returned. Otherwise, these functions will ask an underlying `AccountDb` for the value. This means that the number of lookups is proportional to the depth of the overlay cascade. If the value can't be found before reaching the bottommost `AccountDb`, then a DB read will be performed (in case `get_balance` the function `free_balance` will be invoked). - -A lookup in the local cache consists of at least one `Map` lookup, for locating the specific account. For `get_storage` there is a second lookup: because account's storage is implemented as a nested map, another lookup is required for fetching a storage value by a key. +Those query the underlying storage for the requested value. If the value was modified in the +current block they are served from the cache. Otherwise a database read is performed. -These functions return an owned value as its result, so memory usage depends on the value being returned. - -**complexity**: The memory complexity is proportional to the size of the value. The computational complexity is proportional to the depth of the overlay cascade and the size of the value; the cost is dominated by the DB read though. +**complexity**: The memory complexity is proportional to the size of the value. The computational complexity is proportional the size of the value; the cost is dominated by the DB read. ## set_storage, set_balance, set_rent_allowance -These functions only modify the local `Map`. - -A lookup in the local cache consists of at least one `Map` lookup, for locating the specific account. For `get_storage` there is a second lookup: because account's storage is implemented as a nested map, another lookup is required for fetching a storage value by a key. +These function write to the underlying storage which caches those values and does not write +them to the database immediately. -While these functions only modify the local `Map`, if changes made by them are committed to the bottommost `AccountDb`, each changed entry in the `Map` will require a DB write. Moreover, if the balance of the account is changed to be below `existential_deposit` then that account along with all its storage will be removed, which requires time proportional to the number of storage entries that account has. It should be ensured that pricing accounts for these facts. +While these functions only modify the local cache, they trigger a database write later when +all changes that were not rolled back are written to storage. Moreover, if the balance of the +account is changed to be below `existential_deposit` then that account along with all its storage +will be removed, which requires time proportional to the number of storage entries that account has. +It should be ensured that pricing accounts for these facts. -**complexity**: Each lookup has a logarithmical computing time to the number of already inserted entries. No additional memory is required. +**complexity**: Each lookup has a logarithmical computing time to the number of already inserted entries. +No additional memory is required. ## instantiate_contract @@ -131,9 +125,11 @@ Calls `contract_exists` and if it doesn't exist, do not modify the local `Map` s ## commit -In this function, all cached values will be inserted into the underlying `AccountDb` or into the storage. +In this function, all values modified in the current transactions are committed to the parent +transaction. -We are doing `N` inserts into `Map` (`O(log M)` complexity) or into the storage, where `N` is the size of the committed `Map` and `M` is the size of the map of the underlying overlay. Consider adjusting the price of modifying the `AccountDb` to account for this (since pricing for the count of entries in `commit` will make the price of commit way less predictable). No additional memory is required. +This will trigger `N` inserts into parent transaction (`O(log M)` complexity) or into the storage, where `N` is the size of the current transaction and `M` is the size of the parent transaction. Consider adjusting the price of modifying the +current transaction to account for this (since pricing for the count of entries in `commit` will make the price of commit way less predictable). No additional memory is required. Note that in case of storage modification we need to construct a key in the underlying storage. In order to do that we need: @@ -143,21 +139,21 @@ Note that in case of storage modification we need to construct a key in the unde There is also a special case to think of: if the balance of some account goes below `existential_deposit`, then all storage entries of that account will be erased, which requires time proportional to the number of storage entries that account has. -**complexity**: `N` inserts into a `Map` or eventually into the storage (if committed). Every deleted account will induce removal of all its storage which is proportional to the number of storage entries that account has. +**complexity**: `N` inserts into a transaction or eventually into the storage (if committed). Every deleted account will induce removal of all its storage which is proportional to the number of storage entries that account has. ## revert -Consists of dropping (in the Rust sense) of the `AccountDb`. +Consists of dropping (in the Rust sense) of the current transaction. **complexity**: Computing complexity is proportional to a number of changed entries in a overlay. No additional memory is required. -# Executive +## Executive -## Transfer +### Transfer This function performs the following steps: -1. Querying source and destination balances from an overlay (see `get_balance`), +1. Querying source and destination balances from the current transaction (see `get_balance`), 2. Querying `existential_deposit`. 3. Executing `ensure_account_liquid` hook. 4. Updating source and destination balance in the overlay (see `set_balance`). @@ -171,9 +167,9 @@ returns with an error. Assuming marshaled size of a balance value is of the constant size we can neglect its effect on the performance. -**complexity**: up to 2 DB reads and up to 2 DB writes (if flushed to the storage) in the standard case. If removal of the source account takes place then it will additionally perform a DB write per one storage entry that the account has. For the current `AccountDb` implementation computing complexity also depends on the depth of the `AccountDb` cascade. Memorywise it can be assumed to be constant. +**complexity**: up to 2 DB reads and up to 2 DB writes (if flushed to the storage) in the standard case. If removal of the source account takes place then it will additionally perform a DB write per one storage entry that the account has. Memorywise it can be assumed to be constant. -## Initialization +### Initialization Before a call or instantiate can be performed the execution context must be initialized. @@ -188,7 +184,7 @@ implementation they just involve a DB read. For subsequent calls and instantiations during contract execution, the initialization requires no expensive operations. -## Terminate +### Terminate This function performs the following steps: @@ -204,17 +200,17 @@ the call stack is of a fixed maximum size we consider this operation as constant we are using child trie removal which is linear in the amount of stored keys. Upcoming changes will make the account removal constant time. - -## Call +### Call This function receives input data for the contract execution. The execution consists of the following steps: 1. Initialization of the execution context. 2. Checking rent payment. 3. Loading code from the DB. -4. `transfer`-ing funds between the caller and the destination account. -5. Executing the code of the destination account. -6. Committing overlayed changed to the underlying `AccountDb`. +4. Starting a new storage transaction. +5. `transfer`-ing funds between the caller and the destination account. +6. Executing the code of the destination account. +7. Committing or rolling back the storage transaction. **Note** that the complexity of executing the contract code should be considered separately. @@ -235,22 +231,24 @@ Loading code most likely will trigger a DB read, since the code is immutable and Also, `transfer` can make up to 2 DB reads and up to 2 DB writes (if flushed to the storage) in the standard case. If removal of the source account takes place then it will additionally perform a DB write per one storage entry that the account has. -Finally, all changes are `commit`-ted into the underlying overlay. The complexity of this depends on the number of changes performed by the code. Thus, the pricing of storage modification should account for that. +Finally, the current storage transaction is closed. The complexity of this depends on the number of changes performed by the code. Thus, the pricing of storage modification should account for that. **complexity**: + - Only for the first invocation of the contract: up to 5 DB reads and one DB write as well as logic executed by `ensure_can_withdraw`, `withdraw`, `make_free_balance_be`. - On top of that for every invocation: Up to 5 DB reads. DB read of the code is of dynamic size. There can also be up to 2 DB writes (if flushed to the storage). Additionally, if the source account removal takes place a DB write will be performed per one storage entry that the account has. -## Instantiate +### Instantiate This function takes the code of the constructor and input data. Instantiation of a contract consists of the following steps: 1. Initialization of the execution context. 2. Calling `DetermineContractAddress` hook to determine an address for the contract, -3. `transfer`-ing funds between self and the newly instantiated contract. -4. Executing the constructor code. This will yield the final code of the code. -5. Storing the code for the newly instantiated contract in the overlay. -6. Committing overlayed changed to the underlying `AccountDb`. +3. Starting a new storage transaction. +4. `transfer`-ing funds between self and the newly instantiated contract. +5. Executing the constructor code. This will yield the final code of the code. +6. Storing the code for the newly instantiated contract in the overlay. +7. Committing or rolling back the storage transaction. **Note** that the complexity of executing the constructor code should be considered separately. @@ -262,19 +260,40 @@ Also, `transfer` can make up to 2 DB reads and up to 2 DB writes (if flushed to Storing the code in the overlay may induce another DB write (if flushed to the storage) with the size proportional to the size of the constructor code. -Finally, all changes are `commit`-ted into the underlying overlay. The complexity of this depends on the number of changes performed by the constructor code. Thus, the pricing of storage modification should account for that. +Finally, the current storage transaction is closed.. The complexity of this depends on the number of changes performed by the constructor code. Thus, the pricing of storage modification should account for that. **complexity**: Up to 2 DB reads and induces up to 3 DB writes (if flushed to the storage), one of which is dependent on the size of the code. Additionally, if the source account removal takes place a DB write will be performed per one storage entry that the account has. -# Externalities +## Contracts API + +Each API function invoked from a contract can involve some overhead. + +## Getter functions + +Those are simple getter functions which copy a requested value to contract memory. They +all have the following two arguments: + +- `output_ptr`: Pointer into contract memory where to copy the value. +- `output_len_ptr`: Pointer into contract memory where the size of the buffer is stored. The size of the copied value is also stored there. -Each external function invoked from a contract can involve some overhead. +**complexity**: The size of the returned value is constant for a given runtime. Therefore we +consider its complexity constant even though some of them might involve at most one DB read. -## ext_gas +This is the list of getters: -**complexity**: This is of constant complexity. +- ext_caller +- ext_address +- ext_gas_price +- ext_gas_left +- ext_balance +- ext_value_transferred +- ext_now +- ext_minimum_balance +- ext_tombstone_deposit +- ext_rent_allowance +- ext_block_number -## ext_set_storage +### ext_set_storage This function receives a `key` and `value` as arguments. It consists of the following steps: @@ -283,7 +302,7 @@ This function receives a `key` and `value` as arguments. It consists of the foll **complexity**: Complexity is proportional to the size of the `value`. This function induces a DB write of size proportional to the `value` size (if flushed to the storage), so should be priced accordingly. -## ext_clear_storage +### ext_clear_storage This function receives a `key` as argument. It consists of the following steps: @@ -293,23 +312,22 @@ This function receives a `key` as argument. It consists of the following steps: **complexity**: Complexity is constant. This function induces a DB write to clear the storage entry (upon being flushed to the storage) and should be priced accordingly. -## ext_get_storage +### ext_get_storage This function receives a `key` as an argument. It consists of the following steps: 1. Reading the sandbox memory for `key` (see sandboxing memory get). 2. Reading the storage with the given key (see `get_storage`). It receives back the owned result buffer. -3. Replacing the scratch buffer. +3. Writing the storage value to contract memory. Key is of a constant size. Therefore, the sandbox memory load can be considered to be of constant complexity. Unless the value is cached, a DB read will be performed. The size of the value is not known until the read is performed. Moreover, the DB read has to be synchronous and no progress can be made until the value is fetched. -**complexity**: The memory and computing complexity is proportional to the size of the fetched value. This function performs a -DB read. +**complexity**: The memory and computing complexity is proportional to the size of the fetched value. This function performs a DB read. -## ext_transfer +### ext_transfer This function receives the following arguments: @@ -320,18 +338,19 @@ It consists of the following steps: 1. Loading `account` buffer from the sandbox memory (see sandboxing memory get) and then decoding it. 2. Loading `value` buffer from the sandbox memory and then decoding it. -4. Invoking the executive function `transfer`. +3. Invoking the executive function `transfer`. Loading of `account` and `value` buffers should be charged. This is because the sizes of buffers are specified by the calling code, even though marshaled representations are, essentially, of constant size. This can be fixed by assigning an upper bound for sizes of `AccountId` and `Balance`. -## ext_call +### ext_call This function receives the following arguments: - `callee` buffer of a marshaled `AccountId`, - `gas` limit which is plain u64, - `value` buffer of a marshaled `Balance`, -- `input_data`. An arbitrarily sized byte vector. +- `input_data` an arbitrarily sized byte vector. +- `output_ptr` pointer to contract memory. It consists of the following steps: @@ -339,14 +358,15 @@ It consists of the following steps: 2. Loading `value` buffer from the sandbox memory and then decoding it. 3. Loading `input_data` buffer from the sandbox memory. 4. Invoking the executive function `call`. +5. Writing output buffer to contract memory. Loading of `callee` and `value` buffers should be charged. This is because the sizes of buffers are specified by the calling code, even though marshaled representations are, essentially, of constant size. This can be fixed by assigning an upper bound for sizes of `AccountId` and `Balance`. Loading `input_data` should be charged in any case. -**complexity**: All complexity comes from loading buffers and executing `call` executive function. The former component is proportional to the sizes of `callee`, `value` and `input_data` buffers. The latter component completely depends on the complexity of `call` executive function, and also dominated by it. +**complexity**: All complexity comes from loading and writing buffers and executing `call` executive function. The former component is proportional to the sizes of `callee`, `value`, `input_data` and `output_ptr` buffers. The latter component completely depends on the complexity of `call` executive function, and also dominated by it. -## ext_instantiate +### ext_instantiate This function receives the following arguments: @@ -368,7 +388,7 @@ Loading `init_code` and `input_data` should be charged in any case. **complexity**: All complexity comes from loading buffers and executing `instantiate` executive function. The former component is proportional to the sizes of `init_code`, `value` and `input_data` buffers. The latter component completely depends on the complexity of `instantiate` executive function and also dominated by it. -## ext_terminate +### ext_terminate This function receives the following arguments: @@ -382,16 +402,23 @@ Loading of the `beneficiary` buffer should be charged. This is because the sizes **complexity**: All complexity comes from loading buffers and executing `terminate` executive function. The former component is proportional to the size of the `beneficiary` buffer. The latter component completely depends on the complexity of `terminate` executive function and also dominated by it. -## ext_return +### ext_input -This function receives a `data` buffer as an argument. Execution of the function consists of the following steps: +This function receives a pointer to contract memory. It copies the input to the contract call to this location. -1. Loading `data` buffer from the sandbox memory (see sandboxing memory get), -2. Trapping +**complexity**: The complextity is proportional to the size of the input buffer. + +### ext_return + +This function receives a `data` buffer and `flags` asarguments. Execution of the function consists of the following steps: + +1. Loading `data` buffer from the sandbox memory (see sandboxing memory get). +2. Storing the `u32` flags value. +3. Trapping **complexity**: The complexity of this function is proportional to the size of the `data` buffer. -## ext_deposit_event +### ext_deposit_event This function receives a `data` buffer as an argument. Execution of the function consists of the following steps: @@ -402,49 +429,7 @@ This function receives a `data` buffer as an argument. Execution of the function **complexity**: The complexity of this function is proportional to the size of the `data` buffer. -## ext_caller - -This function serializes the address of the caller into the scratch buffer. - -**complexity**: Assuming that the address is of constant size, this function has constant complexity. - -## ext_random - -This function serializes a random number generated by the given subject into the scratch buffer. -The complexity of this function highly depends on the complexity of `System::random`. `max_subject_len` -limits the size of the subject buffer. - -**complexity**: The complexity of this function depends on the implementation of `System::random`. - -## ext_now - -This function serializes the current block's timestamp into the scratch buffer. - -**complexity**: Assuming that the timestamp is of constant size, this function has constant complexity. - -## ext_scratch_size - -This function returns the size of the scratch buffer. - -**complexity**: This function is of constant complexity. - -## ext_scratch_read - -This function copies slice of data from the scratch buffer to the sandbox memory. The calling code specifies the slice length. Execution of the function consists of the following steps: - -1. Storing a specified slice of the scratch buffer into the sandbox memory (see sandboxing memory set) - -**complexity**: The computing complexity of this function is proportional to the length of the slice. No additional memory is required. - -## ext_scratch_write - -This function copies slice of data from the sandbox memory to the scratch buffer. The calling code specifies the slice length. Execution of the function consists of the following steps: - -1. Loading a slice from the sandbox memory into the (see sandboxing memory get) - -**complexity**: Complexity is proportional to the length of the slice. - -## ext_set_rent_allowance +### ext_set_rent_allowance This function receives the following argument: @@ -457,22 +442,6 @@ It consists of the following steps: **complexity**: Complexity is proportional to the size of the `value`. This function induces a DB write of size proportional to the `value` size (if flushed to the storage), so should be priced accordingly. -## ext_rent_allowance - -It consists of the following steps: - -1. Invoking `get_rent_allowance` AccountDB function. -2. Serializing the rent allowance of the current contract into the scratch buffer. - -**complexity**: Assuming that the rent allowance is of constant size, this function has constant complexity. This -function performs a DB read. - -## ext_block_number - -This function serializes the current block's number into the scratch buffer. - -**complexity**: Assuming that the block number is of constant size, this function has constant complexity. - ## Built-in hashing functions This paragraph concerns the following supported built-in hash functions: From d205f60de3061c6b56047406617d11792a087c25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Fri, 3 Jul 2020 19:00:04 +0200 Subject: [PATCH 12/23] Rename ext_gas_price to ext_weight_to_fee --- frame/contracts/COMPLEXITY.md | 2 +- frame/contracts/src/wasm/mod.rs | 4 ++-- frame/contracts/src/wasm/runtime.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/frame/contracts/COMPLEXITY.md b/frame/contracts/COMPLEXITY.md index 1fbc7390e95ba..89b8e9d507a13 100644 --- a/frame/contracts/COMPLEXITY.md +++ b/frame/contracts/COMPLEXITY.md @@ -283,7 +283,7 @@ This is the list of getters: - ext_caller - ext_address -- ext_gas_price +- ext_weight_to_fee - ext_gas_left - ext_balance - ext_value_transferred diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index 5dae7fc339648..a3f6f06b7dfa1 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -1035,7 +1035,7 @@ mod tests { const CODE_GAS_PRICE: &str = r#" (module - (import "env" "ext_gas_price" (func $ext_gas_price (param i64 i32 i32))) + (import "env" "ext_weight_to_fee" (func $ext_weight_to_fee (param i64 i32 i32))) (import "env" "memory" (memory 1 1)) ;; size of our buffer is 32 bytes @@ -1052,7 +1052,7 @@ mod tests { (func (export "call") ;; This stores the gas price in the buffer - (call $ext_gas_price (i64.const 2) (i32.const 0) (i32.const 32)) + (call $ext_weight_to_fee (i64.const 2) (i32.const 0) (i32.const 32)) ;; assert len == 8 (call $assert diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index b813604143cfd..d8e7fc62ec596 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -801,7 +801,7 @@ define_env!(Env, , // // It is recommended to avoid specifying very small values for `gas` as the prices for a single // gas can be smaller than one. - ext_gas_price(ctx, gas: u64, out_ptr: u32, out_len_ptr: u32) => { + ext_weight_to_fee(ctx, gas: u64, out_ptr: u32, out_len_ptr: u32) => { write_sandbox_output( ctx, out_ptr, out_len_ptr, &ctx.ext.get_weight_price(gas).encode(), false ) From e09694509938eaafba579b0b185640a97d7e02b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Tue, 7 Jul 2020 11:16:27 +0200 Subject: [PATCH 13/23] Align comments with spaces --- frame/contracts/src/wasm/runtime.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index d8e7fc62ec596..f3520c8b589a4 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -464,7 +464,7 @@ define_env!(Env, , // - `key_ptr`: pointer into the linear memory where the key of the requested value is placed. // - `out_ptr`: pointer to the linear memory where the value is written to. // - `out_len_ptr`: in-out pointer into linear memory where the buffer length - // is read from and the value length is written to. + // is read from and the value length is written to. // // # Errors // @@ -535,7 +535,7 @@ define_env!(Env, , // - input_data_len: length of the input data buffer. // - output_ptr: a pointer where the output buffer is copied to. // - output_len_ptr: in-out pointer to where the length of the buffer is read from - // and the actual length is written to. + // and the actual length is written to. // // # Errors // @@ -625,7 +625,7 @@ define_env!(Env, , // and the actual length is written to. // - output_ptr: a pointer where the output buffer is copied to. // - output_len_ptr: in-out pointer to where the length of the buffer is read from - // and the actual length is written to. + // and the actual length is written to. // // # Errors // From 6b0c1f0746fa7ffd9a9980586821f98725ab24eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Tue, 7 Jul 2020 11:58:46 +0200 Subject: [PATCH 14/23] Removed no longer used `ExecError` --- frame/contracts/src/exec.rs | 139 +++++++++------------------- frame/contracts/src/lib.rs | 6 +- frame/contracts/src/wasm/mod.rs | 12 +-- frame/contracts/src/wasm/runtime.rs | 20 ++-- 4 files changed, 59 insertions(+), 118 deletions(-) diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index 1f4cc6292ef46..78959c4402dc3 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -62,32 +62,7 @@ impl ExecReturnValue { } } -/// An error indicating some failure to execute a contract call or instantiation. This can include -/// VM-specific errors during execution (eg. division by 0, OOB access, failure to satisfy some -/// precondition of a system call, etc.) or errors with the orchestration (eg. out-of-gas errors, a -/// non-existent destination contract, etc.). -#[cfg_attr(test, derive(sp_runtime::RuntimeDebug))] -pub struct ExecError { - pub reason: DispatchError, -} - -pub type ExecResult = Result; - -/// Evaluate an expression of type Result<_, &'static str> and either resolve to the value if Ok or -/// wrap the error string into an ExecutionError with the provided buffer and return from the -/// enclosing function. This macro is used instead of .map_err(..)? in order to avoid taking -/// ownership of buffer unless there is an error. -#[macro_export] -macro_rules! try_or_exec_error { - ($e:expr) => { - match $e { - Ok(val) => val, - Err(reason) => return Err( - $crate::exec::ExecError { reason: reason.into() } - ), - } - } -} +pub type ExecResult = Result; /// An interface that provides access to the external environment in which the /// smart-contract is executed. @@ -117,7 +92,7 @@ pub trait Ext { value: BalanceOf, gas_meter: &mut GasMeter, input_data: Vec, - ) -> Result<(AccountIdOf, ExecReturnValue), ExecError>; + ) -> Result<(AccountIdOf, ExecReturnValue), DispatchError>; /// Transfer some amount of funds into the specified account. fn transfer( @@ -330,18 +305,14 @@ where input_data: Vec, ) -> ExecResult { if self.depth == self.config.max_depth as usize { - return Err(ExecError { - reason: "reached maximum depth, cannot make a call".into(), - }); + Err("reached maximum depth, cannot make a call")? } if gas_meter .charge(self.config, ExecFeeToken::Call) .is_out_of_gas() { - return Err(ExecError { - reason: "not enough gas to pay base call fee".into(), - }); + Err("not enough gas to pay base call fee")? } // Assumption: `collect_rent` doesn't collide with overlay because @@ -351,9 +322,7 @@ where // Calls to dead contracts always fail. if let Some(ContractInfo::Tombstone(_)) = contract_info { - return Err(ExecError { - reason: "contract has been evicted".into(), - }); + Err("contract has been evicted")? }; let caller = self.self_account.clone(); @@ -361,25 +330,21 @@ where self.with_nested_context(dest.clone(), dest_trie_id, |nested| { if value > BalanceOf::::zero() { - try_or_exec_error!( - transfer( - gas_meter, - TransferCause::Call, - &caller, - &dest, - value, - nested, - ) - ); + transfer( + gas_meter, + TransferCause::Call, + &caller, + &dest, + value, + nested, + )? } // If code_hash is not none, then the destination account is a live contract, otherwise // it is a regular account since tombstone accounts have already been rejected. match storage::code_hash::(&dest) { Ok(dest_code_hash) => { - let executable = try_or_exec_error!( - nested.loader.load_main(&dest_code_hash) - ); + let executable = nested.loader.load_main(&dest_code_hash)?; let output = nested.vm .execute( &executable, @@ -400,20 +365,16 @@ where gas_meter: &mut GasMeter, code_hash: &CodeHash, input_data: Vec, - ) -> Result<(T::AccountId, ExecReturnValue), ExecError> { + ) -> Result<(T::AccountId, ExecReturnValue), DispatchError> { if self.depth == self.config.max_depth as usize { - return Err(ExecError { - reason: "reached maximum depth, cannot instantiate".into(), - }); + Err("reached maximum depth, cannot instantiate")? } if gas_meter .charge(self.config, ExecFeeToken::Instantiate) .is_out_of_gas() { - return Err(ExecError { - reason: "not enough gas to pay base instantiate fee".into(), - }); + Err("not enough gas to pay base instantiate fee")? } let caller = self.self_account.clone(); @@ -429,33 +390,27 @@ where let dest_trie_id = ::TrieIdGenerator::trie_id(&dest); let output = self.with_nested_context(dest.clone(), Some(dest_trie_id), |nested| { - try_or_exec_error!( - storage::place_contract::( - &dest, - nested - .self_trie_id - .clone() - .expect("the nested context always has to have self_trie_id"), - code_hash.clone() - ) - ); + storage::place_contract::( + &dest, + nested + .self_trie_id + .clone() + .expect("the nested context always has to have self_trie_id"), + code_hash.clone() + )?; // Send funds unconditionally here. If the `endowment` is below existential_deposit // then error will be returned here. - try_or_exec_error!( - transfer( - gas_meter, - TransferCause::Instantiate, - &caller, - &dest, - endowment, - nested, - ) - ); - - let executable = try_or_exec_error!( - nested.loader.load_init(&code_hash) - ); + transfer( + gas_meter, + TransferCause::Instantiate, + &caller, + &dest, + endowment, + nested, + )?; + + let executable = nested.loader.load_init(&code_hash)?; let output = nested.vm .execute( &executable, @@ -466,9 +421,7 @@ where // Error out if insufficient remaining balance. if T::Currency::free_balance(&dest) < nested.config.existential_deposit { - return Err(ExecError { - reason: "insufficient remaining balance".into(), - }); + Err("insufficient remaining balance")? } // Deposit an instantiation event. @@ -669,7 +622,7 @@ where endowment: BalanceOf, gas_meter: &mut GasMeter, input_data: Vec, - ) -> Result<(AccountIdOf, ExecReturnValue), ExecError> { + ) -> Result<(AccountIdOf, ExecReturnValue), DispatchError> { self.ctx.instantiate(endowment, gas_meter, code_hash, input_data) } @@ -859,7 +812,7 @@ mod tests { }; use crate::{ gas::GasMeter, tests::{ExtBuilder, Test, MetaEvent}, - exec::{ExecReturnValue, ExecError}, CodeHash, Config, + exec::ExecReturnValue, CodeHash, Config, gas::Gas, storage, }; @@ -1216,9 +1169,7 @@ mod tests { assert_matches!( result, - Err(ExecError { - reason: DispatchError::Module { message: Some("InsufficientBalance"), .. }, - }) + Err(DispatchError::Module { message: Some("InsufficientBalance"), .. }) ); assert_eq!(get_balance(&origin), 0); assert_eq!(get_balance(&dest), 0); @@ -1357,9 +1308,7 @@ mod tests { // Verify that we've got proper error and set `reached_bottom`. assert_matches!( r, - Err(ExecError { - reason: DispatchError::Other("reached maximum depth, cannot make a call"), - }) + Err(DispatchError::Other("reached maximum depth, cannot make a call")) ); *reached_bottom = true; } else { @@ -1613,7 +1562,7 @@ mod tests { let mut loader = MockLoader::empty(); let dummy_ch = loader.insert( - |_| Err(ExecError { reason: "It's a trap!".into() }) + |_| Err("It's a trap!".into()) ); let instantiator_ch = loader.insert({ let dummy_ch = dummy_ch.clone(); @@ -1626,7 +1575,7 @@ mod tests { ctx.gas_meter, vec![] ), - Err(ExecError { reason: DispatchError::Other("It's a trap!") }) + Err(DispatchError::Other("It's a trap!")) ); exec_success() @@ -1677,9 +1626,7 @@ mod tests { &terminate_ch, vec![], ), - Err(ExecError { - reason: DispatchError::Other("insufficient remaining balance"), - }) + Err(DispatchError::Other("insufficient remaining balance")) ); assert_eq!( diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index de8c9a49a973a..cd40d9955e133 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -93,7 +93,7 @@ use crate::exec::ExecutionContext; use crate::wasm::{WasmLoader, WasmVm}; pub use crate::gas::{Gas, GasMeter}; -pub use crate::exec::{ExecResult, ExecReturnValue, ExecError}; +pub use crate::exec::{ExecResult, ExecReturnValue}; #[cfg(feature = "std")] use serde::{Serialize, Deserialize}; @@ -515,7 +515,7 @@ decl_module! { let result = Self::execute_wasm(origin, &mut gas_meter, |ctx, gas_meter| { ctx.call(dest, value, gas_meter, data) }); - gas_meter.into_dispatch_result(result.map_err(|e| e.reason)) + gas_meter.into_dispatch_result(result) } /// Instantiates a new contract from the `codehash` generated by `put_code`, optionally transferring some balance. @@ -543,7 +543,7 @@ decl_module! { ctx.instantiate(endowment, gas_meter, &code_hash, data) .map(|(_address, output)| output) }); - gas_meter.into_dispatch_result(result.map_err(|e| e.reason)) + gas_meter.into_dispatch_result(result) } /// Allows block producers to claim a small reward for evicting a contract. If a block producer diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index a3f6f06b7dfa1..9063bd9363270 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -153,7 +153,7 @@ mod tests { use std::collections::HashMap; use std::cell::RefCell; use sp_core::H256; - use crate::exec::{Ext, StorageKey, ExecError, ExecReturnValue, ReturnFlags}; + use crate::exec::{Ext, StorageKey, ExecReturnValue, ReturnFlags}; use crate::gas::{Gas, GasMeter}; use crate::tests::{Test, Call}; use crate::wasm::prepare::prepare_contract; @@ -238,7 +238,7 @@ mod tests { endowment: u64, gas_meter: &mut GasMeter, data: Vec, - ) -> Result<(u64, ExecReturnValue), ExecError> { + ) -> Result<(u64, ExecReturnValue), DispatchError> { self.instantiates.push(InstantiateEntry { code_hash: code_hash.clone(), endowment, @@ -390,7 +390,7 @@ mod tests { value: u64, gas_meter: &mut GasMeter, input_data: Vec, - ) -> Result<(u64, ExecReturnValue), ExecError> { + ) -> Result<(u64, ExecReturnValue), DispatchError> { (**self).instantiate(code, value, gas_meter, input_data) } fn transfer( @@ -1538,9 +1538,7 @@ mod tests { MockExt::default(), &mut gas_meter ), - Err(ExecError { - reason: DispatchError::Other("contract trapped during execution") - }) + Err(DispatchError::Other("contract trapped during execution")) ); } @@ -1582,7 +1580,7 @@ mod tests { MockExt::default(), &mut gas_meter ), - Err(ExecError { reason: DispatchError::Other("contract trapped during execution") }) + Err(DispatchError::Other("contract trapped during execution")) ); } diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index f3520c8b589a4..3c7bba38f2a1c 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -18,7 +18,7 @@ use crate::{Schedule, Trait, CodeHash, BalanceOf}; use crate::exec::{ - Ext, ExecResult, ExecError, ExecReturnValue, StorageKey, TopicOf, ReturnFlags, + Ext, ExecResult, ExecReturnValue, StorageKey, TopicOf, ReturnFlags, }; use crate::gas::{Gas, GasMeter, Token, GasMeterResult}; use crate::wasm::env_def::ConvertibleToWasm; @@ -127,9 +127,9 @@ pub(crate) fn to_execution_result( match runtime.special_trap { // The trap was the result of the execution `return` host function. Some(SpecialTrap::Return(flags, data)) => { - let flags = ReturnFlags::from_bits(flags).ok_or_else(|| ExecError { - reason: "used reserved bit in return flags".into(), - })?; + let flags = ReturnFlags::from_bits(flags).ok_or_else(|| + "used reserved bit in return flags" + )?; return Ok(ExecReturnValue { flags, data, @@ -148,14 +148,10 @@ pub(crate) fn to_execution_result( }) } Some(SpecialTrap::OutOfGas) => { - return Err(ExecError { - reason: "ran out of gas during contract execution".into(), - }) + Err("ran out of gas during contract execution")? }, Some(SpecialTrap::OutputBufferTooSmall) => { - return Err(ExecError { - reason: "output buffer too small".into(), - }) + Err("output buffer too small")? }, None => (), } @@ -173,10 +169,10 @@ pub(crate) fn to_execution_result( // Because panics are really undesirable in the runtime code, we treat this as // a trap for now. Eventually, we might want to revisit this. Err(sp_sandbox::Error::Module) => - Err(ExecError { reason: "validation error".into() }), + Err("validation error")?, // Any other kind of a trap should result in a failure. Err(sp_sandbox::Error::Execution) | Err(sp_sandbox::Error::OutOfBounds) => - Err(ExecError { reason: "contract trapped during execution".into() }), + Err("contract trapped during execution")?, } } From 8e5170c43df61475e5e36d0fd28e3f42bab1d45d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Tue, 7 Jul 2020 12:01:06 +0200 Subject: [PATCH 15/23] Remove possible panic in `from_typed_value` --- frame/contracts/src/wasm/runtime.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index 3c7bba38f2a1c..566d8bca9ca7a 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -59,8 +59,8 @@ impl ConvertibleToWasm for ReturnCode { sp_sandbox::Value::I32(self as i32) } fn from_typed_value(_: sp_sandbox::Value) -> Option { - // We will only ever send these values to wasm but never receive them. - unimplemented!() + debug_assert!(false, "We will never receive a ReturnCode but only send it to wasm."); + None } } From 08edc84b386c894d0d35fa743c3f268cd5ecb51c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Tue, 7 Jul 2020 12:12:59 +0200 Subject: [PATCH 16/23] Use a struct as associated data for SpecialTrap::Return --- frame/contracts/src/wasm/runtime.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index 566d8bca9ca7a..fa6e1063d45e1 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -74,13 +74,22 @@ impl From for ReturnCode { } } +/// The data passed through when a contract uses `ext_return`. +struct ReturnData { + /// The flags as passed through by the contract. They are still unchecked and + /// will later be parsed into a `ReturnFlags` bitflags struct. + flags: u32, + /// The output buffer passed by the contract as return data. + data: Vec, +} + /// Enumerates all possible *special* trap conditions. /// /// In this runtime traps used not only for signaling about errors but also /// to just terminate quickly in some cases. enum SpecialTrap { /// Signals that trap was generated in response to call `ext_return` host function. - Return(u32, Vec), + Return(ReturnData), /// Signals that trap was generated because the contract exhausted its gas limit. OutOfGas, /// Signals that a trap was generated in response to a succesful call to the @@ -126,7 +135,7 @@ pub(crate) fn to_execution_result( ) -> ExecResult { match runtime.special_trap { // The trap was the result of the execution `return` host function. - Some(SpecialTrap::Return(flags, data)) => { + Some(SpecialTrap::Return(ReturnData{ flags, data })) => { let flags = ReturnFlags::from_bits(flags).ok_or_else(|| "used reserved bit in return flags" )?; @@ -751,8 +760,10 @@ define_env!(Env, , RuntimeToken::ReturnData(data_len) )?; - let output_data = read_sandbox_memory(ctx, data_ptr, data_len)?; - ctx.special_trap = Some(SpecialTrap::Return(flags, output_data)); + ctx.special_trap = Some(SpecialTrap::Return(ReturnData { + flags, + data: read_sandbox_memory(ctx, data_ptr, data_len)?, + })); // The trap mechanism is used to immediately terminate the execution. // This trap should be handled appropriately before returning the result From 792c942d7386de86e56fb7d4acb4fc9e0b1dd0de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Tue, 7 Jul 2020 12:15:59 +0200 Subject: [PATCH 17/23] Fix nits in COMPLEXITY.md --- frame/contracts/COMPLEXITY.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/frame/contracts/COMPLEXITY.md b/frame/contracts/COMPLEXITY.md index 89b8e9d507a13..dbb1a5c5cdef2 100644 --- a/frame/contracts/COMPLEXITY.md +++ b/frame/contracts/COMPLEXITY.md @@ -277,7 +277,10 @@ all have the following two arguments: - `output_len_ptr`: Pointer into contract memory where the size of the buffer is stored. The size of the copied value is also stored there. **complexity**: The size of the returned value is constant for a given runtime. Therefore we -consider its complexity constant even though some of them might involve at most one DB read. +consider its complexity constant even though some of them might involve at most one DB read. Some of those +functions call into other pallets of the runtime. The assumption here is that those functions are also +linear in regard to the size of the data that is returned and therefore considered constant for a +given runtime. This is the list of getters: @@ -410,7 +413,7 @@ This function receives a pointer to contract memory. It copies the input to the ### ext_return -This function receives a `data` buffer and `flags` asarguments. Execution of the function consists of the following steps: +This function receives a `data` buffer and `flags` arguments. Execution of the function consists of the following steps: 1. Loading `data` buffer from the sandbox memory (see sandboxing memory get). 2. Storing the `u32` flags value. From 41f1d552fea6c1da7e8daae10238b9efd7644689 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Tue, 7 Jul 2020 12:38:31 +0200 Subject: [PATCH 18/23] Renamed SpecialTrap to TrapReason --- frame/contracts/src/lib.rs | 6 +++- frame/contracts/src/wasm/runtime.rs | 49 ++++++++++++++--------------- 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index cd40d9955e133..2195bc09fceb9 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -410,7 +410,11 @@ decl_error! { /// Tombstones don't match. InvalidTombstone, /// An origin TrieId written in the current block. - InvalidContractOrigin + InvalidContractOrigin, + /// The executed contract exhausted its gas limit. + OutOfGas, + /// The output buffer supplied to a contract API call was too small. + OutputBufferTooSmall, } } diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index fa6e1063d45e1..c2e2f1acfafcf 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -16,7 +16,7 @@ //! Environment definition of the wasm smart-contract runtime. -use crate::{Schedule, Trait, CodeHash, BalanceOf}; +use crate::{Schedule, Trait, CodeHash, BalanceOf, Error}; use crate::exec::{ Ext, ExecResult, ExecReturnValue, StorageKey, TopicOf, ReturnFlags, }; @@ -25,6 +25,7 @@ use crate::wasm::env_def::ConvertibleToWasm; use sp_sandbox; use parity_wasm::elements::ValueType; use frame_system; +use frame_support::dispatch::DispatchError; use sp_std::prelude::*; use codec::{Decode, Encode}; use sp_runtime::traits::{Bounded, SaturatedConversion}; @@ -83,22 +84,23 @@ struct ReturnData { data: Vec, } -/// Enumerates all possible *special* trap conditions. +/// Enumerates all possible reasons why a trap was generated. /// -/// In this runtime traps used not only for signaling about errors but also -/// to just terminate quickly in some cases. -enum SpecialTrap { +/// This is either used to supply the caller with more information about why an error +/// occurred (the SupervisorError variant). +/// The other case is where the trap does not constitute an error but rather was invoked +/// as a quick way to terminate the application (all other variants). +enum TrapReason { + /// The supervisor trapped the contract because of an error condition occurred during + /// execution in privileged code. + SupervisorError(DispatchError), /// Signals that trap was generated in response to call `ext_return` host function. Return(ReturnData), - /// Signals that trap was generated because the contract exhausted its gas limit. - OutOfGas, /// Signals that a trap was generated in response to a succesful call to the /// `ext_terminate` host function. Termination, /// Signals that a trap was generated because of a successful restoration. Restoration, - /// Signals that a buffer supplied by the caller is too small to fit the output. - OutputBufferTooSmall, } /// Can only be used for one call. @@ -108,7 +110,7 @@ pub(crate) struct Runtime<'a, E: Ext + 'a> { schedule: &'a Schedule, memory: sp_sandbox::Memory, gas_meter: &'a mut GasMeter, - special_trap: Option, + special_trap: Option, } impl<'a, E: Ext + 'a> Runtime<'a, E> { pub(crate) fn new( @@ -135,7 +137,7 @@ pub(crate) fn to_execution_result( ) -> ExecResult { match runtime.special_trap { // The trap was the result of the execution `return` host function. - Some(SpecialTrap::Return(ReturnData{ flags, data })) => { + Some(TrapReason::Return(ReturnData{ flags, data })) => { let flags = ReturnFlags::from_bits(flags).ok_or_else(|| "used reserved bit in return flags" )?; @@ -144,24 +146,19 @@ pub(crate) fn to_execution_result( data, }) }, - Some(SpecialTrap::Termination) => { + Some(TrapReason::Termination) => { return Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: Vec::new(), }) }, - Some(SpecialTrap::Restoration) => { + Some(TrapReason::Restoration) => { return Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: Vec::new(), }) } - Some(SpecialTrap::OutOfGas) => { - Err("ran out of gas during contract execution")? - }, - Some(SpecialTrap::OutputBufferTooSmall) => { - Err("output buffer too small")? - }, + Some(TrapReason::SupervisorError(error)) => Err(error)?, None => (), } @@ -250,13 +247,13 @@ impl Token for RuntimeToken { fn charge_gas>( gas_meter: &mut GasMeter, metadata: &Tok::Metadata, - special_trap: &mut Option, + special_trap: &mut Option, token: Tok, ) -> Result<(), sp_sandbox::HostError> { match gas_meter.charge(metadata, token) { GasMeterResult::Proceed => Ok(()), GasMeterResult::OutOfGas => { - *special_trap = Some(SpecialTrap::OutOfGas); + *special_trap = Some(TrapReason::SupervisorError(Error::::OutOfGas.into())); Err(sp_sandbox::HostError) }, } @@ -382,7 +379,9 @@ fn write_sandbox_output( let len: u32 = read_sandbox_memory_as(ctx, out_len_ptr, 4)?; if len < buf_len { - ctx.special_trap = Some(SpecialTrap::OutputBufferTooSmall); + ctx.special_trap = Some(TrapReason::SupervisorError( + Error::::OutputBufferTooSmall.into() + )); return Err(sp_sandbox::HostError); } @@ -722,7 +721,7 @@ define_env!(Env, , read_sandbox_memory_as(ctx, beneficiary_ptr, beneficiary_len)?; if let Ok(_) = ctx.ext.terminate(&beneficiary, ctx.gas_meter) { - ctx.special_trap = Some(SpecialTrap::Termination); + ctx.special_trap = Some(TrapReason::Termination); } Err(sp_sandbox::HostError) }, @@ -760,7 +759,7 @@ define_env!(Env, , RuntimeToken::ReturnData(data_len) )?; - ctx.special_trap = Some(SpecialTrap::Return(ReturnData { + ctx.special_trap = Some(TrapReason::Return(ReturnData { flags, data: read_sandbox_memory(ctx, data_ptr, data_len)?, })); @@ -974,7 +973,7 @@ define_env!(Env, , rent_allowance, delta, ) { - ctx.special_trap = Some(SpecialTrap::Restoration); + ctx.special_trap = Some(TrapReason::Restoration); } Err(sp_sandbox::HostError) }, From 8373842afed075c36fb05b40f273f722251d21a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Tue, 7 Jul 2020 16:42:32 +0200 Subject: [PATCH 19/23] Fix test --- frame/contracts/src/tests.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index 791b00fe4cdc8..878f7a64ba71d 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -17,6 +17,7 @@ use crate::{ BalanceOf, ContractAddressFor, ContractInfo, ContractInfoOf, GenesisConfig, Module, RawAliveContractInfo, RawEvent, Trait, TrieId, Schedule, TrieIdGenerator, gas::Gas, + Error, }; use assert_matches::assert_matches; use hex_literal::*; @@ -475,7 +476,7 @@ fn run_out_of_gas() { 67_500_000, vec![], ), - "ran out of gas during contract execution" + Error::::OutOfGas, ); }); } From 9021d20b61a23ef228f16a768d1ea1264c81e6bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Thu, 9 Jul 2020 10:14:41 +0200 Subject: [PATCH 20/23] Finish renaming special_trap -> trap_reason --- frame/contracts/src/wasm/runtime.rs | 32 ++++++++++++++--------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index c2e2f1acfafcf..be874ef424275 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -110,7 +110,7 @@ pub(crate) struct Runtime<'a, E: Ext + 'a> { schedule: &'a Schedule, memory: sp_sandbox::Memory, gas_meter: &'a mut GasMeter, - special_trap: Option, + trap_reason: Option, } impl<'a, E: Ext + 'a> Runtime<'a, E> { pub(crate) fn new( @@ -126,7 +126,7 @@ impl<'a, E: Ext + 'a> Runtime<'a, E> { schedule, memory, gas_meter, - special_trap: None, + trap_reason: None, } } } @@ -135,7 +135,7 @@ pub(crate) fn to_execution_result( runtime: Runtime, sandbox_result: Result, ) -> ExecResult { - match runtime.special_trap { + match runtime.trap_reason { // The trap was the result of the execution `return` host function. Some(TrapReason::Return(ReturnData{ flags, data })) => { let flags = ReturnFlags::from_bits(flags).ok_or_else(|| @@ -247,13 +247,13 @@ impl Token for RuntimeToken { fn charge_gas>( gas_meter: &mut GasMeter, metadata: &Tok::Metadata, - special_trap: &mut Option, + trap_reason: &mut Option, token: Tok, ) -> Result<(), sp_sandbox::HostError> { match gas_meter.charge(metadata, token) { GasMeterResult::Proceed => Ok(()), GasMeterResult::OutOfGas => { - *special_trap = Some(TrapReason::SupervisorError(Error::::OutOfGas.into())); + *trap_reason = Some(TrapReason::SupervisorError(Error::::OutOfGas.into())); Err(sp_sandbox::HostError) }, } @@ -275,7 +275,7 @@ fn read_sandbox_memory( charge_gas( ctx.gas_meter, ctx.schedule, - &mut ctx.special_trap, + &mut ctx.trap_reason, RuntimeToken::ReadMemory(len), )?; @@ -300,7 +300,7 @@ fn read_sandbox_memory_into_buf( charge_gas( ctx.gas_meter, ctx.schedule, - &mut ctx.special_trap, + &mut ctx.trap_reason, RuntimeToken::ReadMemory(buf.len() as u32), )?; @@ -341,7 +341,7 @@ fn write_sandbox_memory( charge_gas( ctx.gas_meter, ctx.schedule, - &mut ctx.special_trap, + &mut ctx.trap_reason, RuntimeToken::WriteMemory(buf.len() as u32), )?; @@ -379,7 +379,7 @@ fn write_sandbox_output( let len: u32 = read_sandbox_memory_as(ctx, out_len_ptr, 4)?; if len < buf_len { - ctx.special_trap = Some(TrapReason::SupervisorError( + ctx.trap_reason = Some(TrapReason::SupervisorError( Error::::OutputBufferTooSmall.into() )); return Err(sp_sandbox::HostError); @@ -388,7 +388,7 @@ fn write_sandbox_output( charge_gas( ctx.gas_meter, ctx.schedule, - &mut ctx.special_trap, + &mut ctx.trap_reason, RuntimeToken::WriteMemory(buf_len.saturating_add(4)), )?; @@ -416,7 +416,7 @@ define_env!(Env, , charge_gas( &mut ctx.gas_meter, ctx.schedule, - &mut ctx.special_trap, + &mut ctx.trap_reason, RuntimeToken::Explicit(amount) )?; Ok(()) @@ -721,7 +721,7 @@ define_env!(Env, , read_sandbox_memory_as(ctx, beneficiary_ptr, beneficiary_len)?; if let Ok(_) = ctx.ext.terminate(&beneficiary, ctx.gas_meter) { - ctx.special_trap = Some(TrapReason::Termination); + ctx.trap_reason = Some(TrapReason::Termination); } Err(sp_sandbox::HostError) }, @@ -755,11 +755,11 @@ define_env!(Env, , charge_gas( ctx.gas_meter, ctx.schedule, - &mut ctx.special_trap, + &mut ctx.trap_reason, RuntimeToken::ReturnData(data_len) )?; - ctx.special_trap = Some(TrapReason::Return(ReturnData { + ctx.trap_reason = Some(TrapReason::Return(ReturnData { flags, data: read_sandbox_memory(ctx, data_ptr, data_len)?, })); @@ -973,7 +973,7 @@ define_env!(Env, , rent_allowance, delta, ) { - ctx.special_trap = Some(TrapReason::Restoration); + ctx.trap_reason = Some(TrapReason::Restoration); } Err(sp_sandbox::HostError) }, @@ -1007,7 +1007,7 @@ define_env!(Env, , charge_gas( ctx.gas_meter, ctx.schedule, - &mut ctx.special_trap, + &mut ctx.trap_reason, RuntimeToken::DepositEvent(topics.len() as u32, data_len) )?; ctx.ext.deposit_event(topics, event_data); From 9ef80e10a7eb2505d4238a554a8458a0ac086d4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Thu, 9 Jul 2020 10:19:46 +0200 Subject: [PATCH 21/23] Remove no longer used get_runtime_storage --- frame/contracts/src/exec.rs | 9 --------- frame/contracts/src/wasm/mod.rs | 15 --------------- 2 files changed, 24 deletions(-) diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index 78959c4402dc3..473cf74176443 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -182,11 +182,6 @@ pub trait Ext { /// Returns the maximum allowed size of a storage item. fn max_value_size(&self) -> u32; - /// Returns the value of runtime under the given key. - /// - /// Returns `None` if the value doesn't exist. - fn get_runtime_storage(&self, key: &[u8]) -> Option>; - /// Returns the price for the specified amount of weight. fn get_weight_price(&self, weight: Weight) -> BalanceOf; } @@ -780,10 +775,6 @@ where self.ctx.config.max_value_size } - fn get_runtime_storage(&self, key: &[u8]) -> Option> { - unhashed::get_raw(&key) - } - fn get_weight_price(&self, weight: Weight) -> BalanceOf { T::WeightPrice::convert(weight) } diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index 9063bd9363270..8c5d5811faf09 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -358,18 +358,6 @@ mod tests { fn max_value_size(&self) -> u32 { 16_384 } - fn get_runtime_storage(&self, key: &[u8]) -> Option> { - let opt_value = self.runtime_storage_keys - .borrow_mut() - .remove(key); - opt_value.unwrap_or_else(|| - panic!( - "{:?} doesn't exist. values that do exist {:?}", - key, - self.runtime_storage_keys - ) - ) - } fn get_weight_price(&self, weight: Weight) -> BalanceOf { BalanceOf::::from(1312_u32).saturating_mul(weight.into()) } @@ -470,9 +458,6 @@ mod tests { fn max_value_size(&self) -> u32 { (**self).max_value_size() } - fn get_runtime_storage(&self, key: &[u8]) -> Option> { - (**self).get_runtime_storage(key) - } fn get_weight_price(&self, weight: Weight) -> BalanceOf { (**self).get_weight_price(weight) } From fe110edeb1be5871ce0f65ab31c5dbfaa4f1f79a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Thu, 9 Jul 2020 13:15:55 +0200 Subject: [PATCH 22/23] fixup! Remove no longer used get_runtime_storage --- frame/contracts/src/exec.rs | 2 +- frame/contracts/src/wasm/mod.rs | 12 ------------ 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index 473cf74176443..27b843c5e163f 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -24,7 +24,7 @@ use bitflags::bitflags; use sp_std::prelude::*; use sp_runtime::traits::{Bounded, Zero, Convert}; use frame_support::{ - storage::unhashed, dispatch::DispatchError, + dispatch::DispatchError, traits::{ExistenceRequirement, Currency, Time, Randomness}, weights::Weight, }; diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index 8c5d5811faf09..500c0f4dcc520 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -151,7 +151,6 @@ impl<'a, T: Trait> crate::exec::Vm for WasmVm<'a> { mod tests { use super::*; use std::collections::HashMap; - use std::cell::RefCell; use sp_core::H256; use crate::exec::{Ext, StorageKey, ExecReturnValue, ReturnFlags}; use crate::gas::{Gas, GasMeter}; @@ -210,17 +209,6 @@ mod tests { // (topics, data) events: Vec<(Vec, Vec)>, next_account_id: u64, - - /// Runtime storage keys works the following way. - /// - /// - If the test code requests a value and it doesn't exist in this storage map then a - /// panic happens. - /// - If the value does exist it is returned and then removed from the map. So a panic - /// happens if the same value is requested for the second time. - /// - /// This behavior is used to prevent mixing up an access to unexpected location and empty - /// cell. - runtime_storage_keys: RefCell, Option>>>, } impl Ext for MockExt { From 9deebd39fd4435cb96351f824dfa34753e51782d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Thu, 9 Jul 2020 14:33:36 +0200 Subject: [PATCH 23/23] Removed tabs for comment aligment --- frame/contracts/src/wasm/runtime.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index be874ef424275..6d272ce929f93 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -544,14 +544,14 @@ define_env!(Env, , // # Errors // // `ReturnCode::CalleeReverted`: The callee ran to completion but decided to have its - // changes reverted. The delivery of the output buffer is still possible. + // changes reverted. The delivery of the output buffer is still possible. // `ReturnCode::CalleeTrapped`: The callee trapped during execution. All changes are reverted - // and no output buffer is delivered. + // and no output buffer is delivered. // // # Traps // // - Transfer of balance failed. This call can not bring the sender below the existential - // deposit. Use `ext_terminate` to remove the caller. + // deposit. Use `ext_terminate` to remove the caller. // - Callee does not exist. // - Supplied output buffer is too small. ext_call( @@ -643,7 +643,7 @@ define_env!(Env, , // # Traps // // - Transfer of balance failed. This call can not bring the sender below the existential - // deposit. Use `ext_terminate` to remove the caller. + // deposit. Use `ext_terminate` to remove the caller. // - Code hash does not exist. // - Supplied output buffers are too small. ext_instantiate(