Don't cascade failure in ext_call

Instead, if callee doesn't complete successfuly, return a non-zero status code.

This will prevent cascading traps up to the top-level. Due to this some tests were altered so that they now expect successful transfer instead of a failure.

substrate/runtime/contract/src/lib.rs

@@ -31,7 +31,6 @@
 //!
 //! Failures are typically not cascading. That, for example, means that if contract A calls B and B errors
 //! somehow, then A can decide if it should proceed or error.
-//! TODO: That is not the case now, since call/create externalities traps on any error now.
 //!
 //! # Interaction with the system
 //!

substrate/runtime/contract/src/tests.rs

@@ -172,17 +172,19 @@ const CODE_TRANSFER: &str = r#"
 	;;    value_len: u32,
 	;;    input_data_ptr: u32,
 	;;    input_data_len: u32
-	;;)
-	(import "env" "ext_call" (func $ext_call (param i32 i32 i32 i32 i32 i32)))
+	;; ) -> u32
+	(import "env" "ext_call" (func $ext_call (param i32 i32 i32 i32 i32 i32) (result i32)))
 	(import "env" "memory" (memory 1 1))
 	(func (export "call")
-		(call $ext_call
-			(i32.const 4)  ;; Pointer to "callee" address.
-			(i32.const 8)  ;; Length of "callee" address.
-			(i32.const 12)  ;; 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
+		(drop
+			(call $ext_call
+				(i32.const 4)  ;; Pointer to "callee" address.
+				(i32.const 8)  ;; Length of "callee" address.
+				(i32.const 12)  ;; 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
+			)
 		)
 	)
 	;; Destination AccountId to transfer the funds.
@@ -214,10 +216,10 @@ fn contract_transfer() {
 		assert_eq!(
 			Staking::free_balance(&0),
 			// 3 - value sent with the transaction
-			// 2 * 8 - gas used by the contract (8) multiplied by gas price (2)
+			// 2 * 9 - gas used by the contract (8) multiplied by gas price (2)
 			// 2 * 135 - base gas fee for call (by transaction)
 			// 2 * 135 - base gas fee for call (by the contract)
-			100_000_000 - 3 - (2 * 8) - (2 * 135) - (2 * 135),
+			100_000_000 - 3 - (2 * 9) - (2 * 135) - (2 * 135),
 		);
 		assert_eq!(
 			Staking::free_balance(&1),
@@ -244,20 +246,20 @@ fn contract_transfer_oog() {
 		Staking::set_free_balance(&1, 11);
 		Staking::increase_total_stake_by(11);
 
-		assert_err!(
-			Contract::call(&0, 1, 3, 276, Vec::new()),
-			"vm execute returned error while call"
-		);
+		assert_ok!(Contract::call(&0, 1, 3, 135 + 135 + 7, Vec::new()));
 
 		assert_eq!(
 			Staking::free_balance(&0),
 			// 3 - value sent with the transaction
-			// 2 * 6 - gas used by the contract (6) multiplied by gas price (2)
+			// 2 * 7 - gas used by the contract (7) multiplied by gas price (2)
 			// 2 * 135 - base gas fee for call (by transaction)
 			// 2 * 135 - base gas fee for call (by contract)
-			100_000_000 - (2 * 6) - (2 * 135) - (2 * 135),
+			100_000_000 - 3 - (2 * 7) - (2 * 135) - (2 * 135),
 		);
-		assert_eq!(Staking::free_balance(&1), 11);
+
+		// Transaction level transfer should succeed.
+		assert_eq!(Staking::free_balance(&1), 14);
+		// But `ext_call` should not.
 		assert_eq!(Staking::free_balance(&CONTRACT_SHOULD_TRANSFER_TO), 0);
 	});
 }
@@ -276,20 +278,17 @@ fn contract_transfer_max_depth() {
 		Staking::set_free_balance(&CONTRACT_SHOULD_TRANSFER_TO, 11);
 		Staking::increase_total_stake_by(11);
 
-		assert_err!(
-			Contract::call(&0, CONTRACT_SHOULD_TRANSFER_TO, 3, 100_000, Vec::new()),
-			"vm execute returned error while call"
-		);
+		assert_ok!(Contract::call(&0, CONTRACT_SHOULD_TRANSFER_TO, 3, 100_000, Vec::new()));
 
 		assert_eq!(
 			Staking::free_balance(&0),
 			// 3 - value sent with the transaction
-			// 2 * 6 * 100 - gas used by the contract (6) multiplied by gas price (2)
+			// 2 * 9 * 100 - gas used by the contract (9) multiplied by gas price (2)
 			//               multiplied by max depth (100).
 			// 2 * 135 * 100 - base gas fee for call (by transaction) multiplied by max depth (100).
-			100_000_000 - (2 * 135 * 100) - (2 * 8 * 100),
+			100_000_000 - 3 - (2 * 135 * 100) - (2 * 9 * 100),
 		);
-		assert_eq!(Staking::free_balance(&CONTRACT_SHOULD_TRANSFER_TO), 11);
+		assert_eq!(Staking::free_balance(&CONTRACT_SHOULD_TRANSFER_TO), 14);
 	});
 }
 
@@ -388,12 +387,12 @@ fn contract_create() {
 		);
 
 		// 11 - value sent with the transaction
-		// 2 * 132 - gas spent by the deployer contract (130) multiplied by gas price (2)
+		// 2 * 134 - gas spent by the deployer contract (134) multiplied by gas price (2)
 		// 2 * 135 - base gas fee for call (top level)
 		// 2 * 175 - base gas fee for create (by contract)
 		// ((21 / 2) * 2) - price per account creation
 		let expected_gas_after_create =
-			100_000_000 - 11 - (2 * 132) - (2 * 135) - (2 * 175) - ((21 / 2) * 2);
+			100_000_000 - 11 - (2 * 134) - (2 * 135) - (2 * 175) - ((21 / 2) * 2);
 		assert_eq!(Staking::free_balance(&0), expected_gas_after_create);
 		assert_eq!(Staking::free_balance(&1), 8);
 		assert_eq!(Staking::free_balance(&derived_address), 3);
@@ -404,10 +403,10 @@ fn contract_create() {
 		assert_eq!(
 			Staking::free_balance(&0),
 			// 22 - value sent with the transaction
-			// (2 * 8) - gas used by the contract
+			// (2 * 9) - gas used by the contract
 			// (2 * 135) - base gas fee for call (top level)
 			// (2 * 135) - base gas fee for call (by transfer contract)
-			expected_gas_after_create - 22 - (2 * 8) - (2 * 135) - (2 * 135),
+			expected_gas_after_create - 22 - (2 * 9) - (2 * 135) - (2 * 135),
 		);
 		assert_eq!(Staking::free_balance(&derived_address), 22 - 3);
 		assert_eq!(Staking::free_balance(&9), 36);
@@ -439,12 +438,12 @@ fn top_level_create() {
 		));
 
 		// 11 - value sent with the transaction
-		// (3 * 124) - gas spent by the ctor
+		// (3 * 126) - gas spent by the ctor
 		// (3 * 175) - base gas fee for create (175) (top level) multipled by gas price (3)
 		// ((21 / 3) * 3) - price for contract creation
 		assert_eq!(
 			Staking::free_balance(&0),
-			100_000_000 - 11 - (3 * 124) - (3 * 175) - ((21 / 3) * 3)
+			100_000_000 - 11 - (3 * 126) - (3 * 175) - ((21 / 3) * 3)
 		);
 		assert_eq!(Staking::free_balance(&derived_address), 30 + 11);
 

substrate/runtime/contract/src/vm/env_def/mod.rs

@@ -183,7 +183,7 @@ define_env!(init_env, <E: Ext>,
 	},
 
 	// ext_call(transfer_to_ptr: u32, transfer_to_len: u32, value_ptr: u32, value_len: u32, input_data_ptr: u32, input_data_len: u32)
-	ext_call(ctx, callee_ptr: u32, callee_len: u32, value_ptr: u32, value_len: u32, input_data_ptr: u32, input_data_len: u32) => {
+	ext_call(ctx, callee_ptr: u32, callee_len: u32, value_ptr: u32, value_len: u32, input_data_ptr: u32, input_data_len: u32) -> u32 => {
 		let mut callee = Vec::new();
 		callee.resize(callee_len as usize, 0);
 		ctx.memory().get(callee_ptr, &mut callee)?;
@@ -214,9 +214,8 @@ define_env!(init_env, <E: Ext>,
 
 		match call_outcome {
 			// TODO: Find a way how to pass return_data back to the this sandbox.
-			Ok(CallReceipt { .. }) => Ok(()),
-			// TODO: Return a status code value that can be handled by the caller instead of a trap.
-			Err(_) => Err(sandbox::HostError),
+			Ok(CallReceipt { .. }) => Ok(0),
+			Err(_) => Ok(1),
 		}
 	},
 

substrate/runtime/contract/src/vm/mod.rs

@@ -345,17 +345,19 @@ mod tests {
 	;;    value_len: u32,
 	;;    input_data_ptr: u32,
 	;;    input_data_len: u32
-	;;)
-	(import "env" "ext_call" (func $ext_call (param i32 i32 i32 i32 i32 i32)))
+	;;) -> u32
+	(import "env" "ext_call" (func $ext_call (param i32 i32 i32 i32 i32 i32) (result i32)))
 	(import "env" "memory" (memory 1 1))
 	(func (export "call")
-		(call $ext_call
-			(i32.const 4)   ;; Pointer to "callee" address.
-			(i32.const 8)   ;; Length of "callee" address.
-			(i32.const 12)  ;; Pointer to the buffer with value to transfer
-			(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
+		(drop
+			(call $ext_call
+				(i32.const 4)  ;; Pointer to "callee" address.
+				(i32.const 8)  ;; Length of "callee" address.
+				(i32.const 12) ;; Pointer to the buffer with value to transfer
+				(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
+			)
 		)
 	)
 	;; Destination AccountId to transfer the funds.