contracts: Don't rely on reserved balances keeping an account alive (#13369)

* Move storage deposits to their own account

* Take ed for contract's account from origin

* Apply suggestions from code review

Co-authored-by: Cyrill Leutwiler <[email protected]>
Co-authored-by: Sasha Gryaznov <[email protected]>

* Update stale docs

* Use 16 bytes prefix for address derivation

* Update frame/contracts/src/address.rs

Co-authored-by: Cyrill Leutwiler <[email protected]>

* Fix merge

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

* Update frame/contracts/primitives/src/lib.rs

Co-authored-by: Sasha Gryaznov <[email protected]>

---------

Co-authored-by: Cyrill Leutwiler <[email protected]>
Co-authored-by: Sasha Gryaznov <[email protected]>
Co-authored-by: command-bot <>

Author: 3 people

frame/contracts/fixtures/caller_contract.wat

@@ -16,27 +16,11 @@
 		)
 	)
 
-	(func $current_balance (param $sp i32) (result i64)
-		(i32.store
-			(i32.sub (get_local $sp) (i32.const 16))
-			(i32.const 8)
-		)
-		(call $seal_balance
-			(i32.sub (get_local $sp) (i32.const 8))
-			(i32.sub (get_local $sp) (i32.const 16))
-		)
-		(call $assert
-			(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)))
-	)
-
 	(func (export "deploy"))
 
 	(func (export "call")
 		(local $sp i32)
 		(local $exit_code i32)
-		(local $balance i64)
 
 		;; Length of the buffer
 		(i32.store (i32.const 20) (i32.const 32))
@@ -54,9 +38,6 @@
 
 		;; Read current balance into local variable.
 		(set_local $sp (i32.const 1024))
-		(set_local $balance
-			(call $current_balance (get_local $sp))
-		)
 
 		;; Fail to deploy the contract since it returns a non-zero exit status.
 		(set_local $exit_code
@@ -82,11 +63,6 @@
 			(i32.eq (get_local $exit_code) (i32.const 2)) ;; ReturnCode::CalleeReverted
 		)
 
-		;; Check that balance has not changed.
-		(call $assert
-			(i64.eq (get_local $balance) (call $current_balance (get_local $sp)))
-		)
-
 		;; Fail to deploy the contract due to insufficient gas.
 		(set_local $exit_code
 			(call $seal_instantiate
@@ -112,11 +88,6 @@
 			(i32.eq (get_local $exit_code) (i32.const 1)) ;; ReturnCode::CalleeTrapped
 		)
 
-		;; 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))
@@ -153,14 +124,6 @@
 			(i32.eq (i32.load (i32.sub (get_local $sp) (i32.const 4))) (i32.const 32))
 		)
 
-		;; Check that balance has been deducted.
-		(set_local $balance
-			(i64.sub (get_local $balance) (i64.load (i32.const 0)))
-		)
-		(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))
@@ -204,11 +167,6 @@
 			)
 		)
 
-		;; Check that balance has not changed.
-		(call $assert
-			(i64.eq (get_local $balance) (call $current_balance (get_local $sp)))
-		)
-
 		;; Fail to call the contract due to insufficient gas.
 		(set_local $exit_code
 			(call $seal_call
@@ -229,11 +187,6 @@
 			(i32.eq (get_local $exit_code) (i32.const 1)) ;; ReturnCode::CalleeTrapped
 		)
 
-		;; 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))
@@ -276,14 +229,6 @@
 				(i32.const 0x77665544)
 			)
 		)
-
-		;; Check that balance has been deducted.
-		(set_local $balance
-			(i64.sub (get_local $balance) (i64.load (i32.const 0)))
-		)
-		(call $assert
-			(i64.eq (get_local $balance) (call $current_balance (get_local $sp)))
-		)
 	)
 
 	(data (i32.const 0) "\00\80")		;; The value to transfer on instantiation and calls.

frame/contracts/fixtures/drain.wat

@@ -34,8 +34,7 @@
 		)
 
 		;; Try to self-destruct by sending full balance to the 0 address.
-		;; All the *free* balance will be send away, which is a valid thing to do
-		;; because the storage deposits will keep the account alive.
+		;; The call will fail because a contract transfer has a keep alive requirement
 		(call $assert
 			(i32.eq
 				(call $seal_transfer
@@ -44,7 +43,7 @@
 					(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) ;; ReturnCode::Success
+				(i32.const 5) ;; ReturnCode::TransferFailed
 			)
 		)
 	)

frame/contracts/primitives/src/lib.rs

@@ -47,10 +47,12 @@ pub struct ContractResult<R, Balance> {
 	/// Additionally, any `seal_call` or `seal_instantiate` makes use of pre-charging
 	/// when a non-zero `gas_limit` argument is supplied.
 	pub gas_required: Weight,
-	/// How much balance was deposited and reserved during execution in order to pay for storage.
+	/// How much balance was paid by the origin into the contract's deposit account in order to
+	/// pay for storage.
 	///
-	/// The storage deposit is never actually charged from the caller in case of [`Self::result`]
-	/// is `Err`. This is because on error all storage changes are rolled back.
+	/// The storage deposit is never actually charged from the origin in case of [`Self::result`]
+	/// is `Err`. This is because on error all storage changes are rolled back including the
+	/// payment of the deposit.
 	pub storage_deposit: StorageDeposit<Balance>,
 	/// An optional debug message. This message is only filled when explicitly requested
 	/// by the code that calls into the contract. Otherwise it is empty.
@@ -159,12 +161,12 @@ pub enum StorageDeposit<Balance> {
 	/// The transaction reduced storage consumption.
 	///
 	/// This means that the specified amount of balance was transferred from the involved
-	/// contracts to the call origin.
+	/// deposit accounts to the origin.
 	Refund(Balance),
-	/// The transaction increased overall storage usage.
+	/// The transaction increased storage consumption.
 	///
-	/// This means that the specified amount of balance was transferred from the call origin
-	/// to the contracts involved.
+	/// This means that the specified amount of balance was transferred from the origin
+	/// to the involved deposit accounts.
 	Charge(Balance),
 }
 

frame/contracts/src/address.rs

@@ -0,0 +1,81 @@
+// This file is part of Substrate.
+
+// Copyright (C) 2018-2022 Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//! Functions that deal with address derivation.
+
+use crate::{CodeHash, Config};
+use codec::{Decode, Encode};
+use sp_runtime::traits::{Hash, TrailingZeroInput};
+
+/// Provides the contract address generation method.
+///
+/// See [`DefaultAddressGenerator`] for the default implementation.
+///
+/// # Note for implementors
+///
+/// 1. Make sure that there are no collisions, different inputs never lead to the same output.
+/// 2. Make sure that the same inputs lead to the same output.
+pub trait AddressGenerator<T: Config> {
+	/// The address of a contract based on the given instantiate parameters.
+	///
+	/// Changing the formular for an already deployed chain is fine as long as no collisons
+	/// with the old formular. Changes only affect existing contracts.
+	fn contract_address(
+		deploying_address: &T::AccountId,
+		code_hash: &CodeHash<T>,
+		input_data: &[u8],
+		salt: &[u8],
+	) -> T::AccountId;
+
+	/// The address of the deposit account of `contract_addr`.
+	///
+	/// The address is generated once on instantiation and then stored in the contracts
+	/// metadata. Hence changes do only affect newly created contracts.
+	fn deposit_address(contract_addr: &T::AccountId) -> T::AccountId;
+}
+
+/// Default address generator.
+///
+/// This is the default address generator used by contract instantiation. Its result
+/// is only dependent on its inputs. It can therefore be used to reliably predict the
+/// address of a contract. This is akin to the formula of eth's CREATE2 opcode. There
+/// is no CREATE equivalent because CREATE2 is strictly more powerful.
+/// Formula:
+/// `hash("contract_addr_v1" ++ deploying_address ++ code_hash ++ input_data ++ salt)`
+pub struct DefaultAddressGenerator;
+
+impl<T: Config> AddressGenerator<T> for DefaultAddressGenerator {
+	/// Formula: `hash("contract_addr_v1" ++ deploying_address ++ code_hash ++ input_data ++ salt)`
+	fn contract_address(
+		deploying_address: &T::AccountId,
+		code_hash: &CodeHash<T>,
+		input_data: &[u8],
+		salt: &[u8],
+	) -> T::AccountId {
+		let entropy = (b"contract_addr_v1", deploying_address, code_hash, input_data, salt)
+			.using_encoded(T::Hashing::hash);
+		Decode::decode(&mut TrailingZeroInput::new(entropy.as_ref()))
+			.expect("infinite length input; no invalid inputs for type; qed")
+	}
+
+	/// Formula: `hash("contract_depo_v1" ++ contract_addr)`
+	fn deposit_address(contract_addr: &T::AccountId) -> T::AccountId {
+		let entropy = (b"contract_depo_v1", contract_addr).using_encoded(T::Hashing::hash);
+		Decode::decode(&mut TrailingZeroInput::new(entropy.as_ref()))
+			.expect("infinite length input; no invalid inputs for type; qed")
+	}
+}