Skip to content

Commit

Permalink
contracts: Don't rely on reserved balances keeping an account alive (p…
Browse files Browse the repository at this point in the history
…aritytech#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 <bigcyrill@hotmail.com>
Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>

* Update stale docs

* Use 16 bytes prefix for address derivation

* Update frame/contracts/src/address.rs

Co-authored-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

* Fix merge

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

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

Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>

---------

Co-authored-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>
Co-authored-by: command-bot <>
  • Loading branch information
3 people authored and ukint-vs committed Apr 10, 2023
1 parent ff75c59 commit d18d2ad
Show file tree
Hide file tree
Showing 11 changed files with 2,098 additions and 2,173 deletions.
55 changes: 0 additions & 55 deletions frame/contracts/fixtures/caller_contract.wat
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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
Expand All @@ -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))
Expand Down Expand Up @@ -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.
Expand Down
5 changes: 2 additions & 3 deletions frame/contracts/fixtures/drain.wat
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
)
)
)
Expand Down
16 changes: 9 additions & 7 deletions frame/contracts/primitives/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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),
}

Expand Down
81 changes: 81 additions & 0 deletions frame/contracts/src/address.rs
Original file line number Diff line number Diff line change
@@ -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")
}
}
Loading

0 comments on commit d18d2ad

Please sign in to comment.