Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

contracts: Add new seal_call that offers new features #8909

Merged
12 commits merged into from
Jun 7, 2021
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions frame/contracts/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ In other words: Upgrading this pallet will not break pre-existing contracts.

### Added

- Add new **unstable** version of `seal_call` that offers more features.

- New **unstable** `seal_rent_params` and `seal_rent_status` contract callable function.
[#8231](https://github.com/paritytech/substrate/pull/8231)
[#8780](https://github.com/paritytech/substrate/pull/8780)
Expand Down
1 change: 1 addition & 0 deletions frame/contracts/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ readme = "README.md"
targets = ["x86_64-unknown-linux-gnu"]

[dependencies]
bitflags = "1.0"
codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false, features = ["derive"] }
log = { version = "0.4", default-features = false }
parity-wasm = { version = "0.42", default-features = false }
Expand Down
162 changes: 136 additions & 26 deletions frame/contracts/src/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ pub trait Ext: sealing::Sealed {
to: AccountIdOf<Self::T>,
value: BalanceOf<Self::T>,
input_data: Vec<u8>,
allows_reentry: bool,
) -> Result<(ExecReturnValue, u32), (ExecError, u32)>;

/// Instantiate a contract from the given code.
Expand Down Expand Up @@ -457,6 +458,8 @@ pub struct Frame<T: Config> {
entry_point: ExportedFunction,
/// The gas meter capped to the supplied gas limit.
nested_meter: GasMeter<T>,
/// If `false` the contract enabled its defense against reentrance attacks.
allows_reentry: bool,
}

/// Parameter passed in when creating a new `Frame`.
Expand Down Expand Up @@ -731,6 +734,7 @@ where
entry_point,
nested_meter: gas_meter.nested(gas_limit)
.map_err(|e| (e.into(), executable.code_len()))?,
allows_reentry: true,
};

Ok((frame, executable))
Expand Down Expand Up @@ -1014,6 +1018,11 @@ where
self.frames().skip(1).any(|f| &f.account_id == account_id)
}

/// Returns whether the specified contract allows to be reentered right now.
fn allows_reentry(&self, id: &AccountIdOf<T>) -> bool {
!self.frames().any(|f| &f.account_id == id && !f.allows_reentry)
}

/// Increments the cached account id and returns the value to be used for the trie_id.
fn next_trie_seed(&mut self) -> u64 {
let next = if let Some(current) = self.account_counter {
Expand Down Expand Up @@ -1045,25 +1054,44 @@ where
to: T::AccountId,
value: BalanceOf<T>,
input_data: Vec<u8>,
allows_reentry: bool,
) -> Result<(ExecReturnValue, u32), (ExecError, u32)> {
// We ignore instantiate frames in our search for a cached contract.
// Otherwise it would be possible to recursively call a contract from its own
// constructor: We disallow calling not fully constructed contracts.
let cached_info = self
.frames()
.find(|f| f.entry_point == ExportedFunction::Call && f.account_id == to)
.and_then(|f| {
match &f.contract_info {
CachedContract::Cached(contract) => Some(contract.clone()),
_ => None,
}
});
let executable = self.push_frame(
FrameArgs::Call{dest: to, cached_info},
value,
gas_limit
)?;
self.run(executable, input_data)
// Before pushing the new frame: Protect the caller contract against reentrancy attacks.
// It is important to do this before calling `allows_reentry` so that a direct recursion
// is caught by it.
self.top_frame_mut().allows_reentry = allows_reentry;

let try_call = || {
if !self.allows_reentry(&to) {
return Err((<Error<T>>::ReentranceDenied.into(), 0));
}
// We ignore instantiate frames in our search for a cached contract.
// Otherwise it would be possible to recursively call a contract from its own
// constructor: We disallow calling not fully constructed contracts.
let cached_info = self
.frames()
.find(|f| f.entry_point == ExportedFunction::Call && f.account_id == to)
.and_then(|f| {
match &f.contract_info {
CachedContract::Cached(contract) => Some(contract.clone()),
_ => None,
}
});
let executable = self.push_frame(
FrameArgs::Call{dest: to, cached_info},
value,
gas_limit
)?;
self.run(executable, input_data)
};

// We need to make sure to reset `allows_reentry` even on failure.
let result = try_call();

// Protection is on a per call basis.
self.top_frame_mut().allows_reentry = true;

result
}

fn instantiate(
Expand Down Expand Up @@ -1097,7 +1125,7 @@ where
beneficiary: &AccountIdOf<Self::T>,
) -> Result<u32, (DispatchError, u32)> {
if self.is_recursive() {
return Err((Error::<T>::ReentranceDenied.into(), 0));
return Err((Error::<T>::TerminatedWhileReentrant.into(), 0));
}
let frame = self.top_frame_mut();
let info = frame.terminate();
Expand Down Expand Up @@ -1125,7 +1153,7 @@ where
delta: Vec<StorageKey>,
) -> Result<(u32, u32), (DispatchError, u32, u32)> {
if self.is_recursive() {
return Err((Error::<T>::ReentranceDenied.into(), 0, 0));
return Err((Error::<T>::TerminatedWhileReentrant.into(), 0, 0));
}
let origin_contract = self.top_frame_mut().contract_info().clone();
let result = Rent::<T, E>::restore_to(
Expand Down Expand Up @@ -1308,12 +1336,14 @@ mod tests {
exec::ExportedFunction::*,
Error, Weight,
};
use codec::{Encode, Decode};
use sp_core::Bytes;
use sp_runtime::DispatchError;
use assert_matches::assert_matches;
use std::{cell::RefCell, collections::HashMap, rc::Rc};
use pretty_assertions::{assert_eq, assert_ne};
use pallet_contracts_primitives::ReturnFlags;
use frame_support::{assert_ok, assert_err};

type MockStack<'a> = Stack<'a, Test, MockExecutable>;

Expand Down Expand Up @@ -1731,7 +1761,7 @@ mod tests {
let value = Default::default();
let recurse_ch = MockLoader::insert(Call, |ctx, _| {
// Try to call into yourself.
let r = ctx.ext.call(0, BOB, 0, vec![]);
let r = ctx.ext.call(0, BOB, 0, vec![], true);

REACHED_BOTTOM.with(|reached_bottom| {
let mut reached_bottom = reached_bottom.borrow_mut();
Expand Down Expand Up @@ -1789,7 +1819,7 @@ mod tests {

// Call into CHARLIE contract.
assert_matches!(
ctx.ext.call(0, CHARLIE, 0, vec![]),
ctx.ext.call(0, CHARLIE, 0, vec![], true),
Ok(_)
);
exec_success()
Expand Down Expand Up @@ -1832,7 +1862,7 @@ mod tests {

// Call into charlie contract.
assert_matches!(
ctx.ext.call(0, CHARLIE, 0, vec![]),
ctx.ext.call(0, CHARLIE, 0, vec![], true),
Ok(_)
);
exec_success()
Expand Down Expand Up @@ -2263,7 +2293,7 @@ mod tests {
assert_ne!(original_allowance, changed_allowance);
ctx.ext.set_rent_allowance(changed_allowance);
assert_eq!(
ctx.ext.call(0, CHARLIE, 0, vec![]).map(|v| v.0).map_err(|e| e.0),
ctx.ext.call(0, CHARLIE, 0, vec![], true).map(|v| v.0).map_err(|e| e.0),
exec_trapped()
);
assert_eq!(ctx.ext.rent_allowance(), changed_allowance);
Expand All @@ -2272,7 +2302,7 @@ mod tests {
exec_success()
});
let code_charlie = MockLoader::insert(Call, |ctx, _| {
assert!(ctx.ext.call(0, BOB, 0, vec![99]).is_ok());
assert!(ctx.ext.call(0, BOB, 0, vec![99], true).is_ok());
exec_trapped()
});

Expand All @@ -2299,7 +2329,7 @@ mod tests {
fn recursive_call_during_constructor_fails() {
let code = MockLoader::insert(Constructor, |ctx, _| {
assert_matches!(
ctx.ext.call(0, ctx.ext.address().clone(), 0, vec![]),
ctx.ext.call(0, ctx.ext.address().clone(), 0, vec![], true),
Err((ExecError{error, ..}, _)) if error == <Error<Test>>::ContractNotFound.into()
);
exec_success()
Expand Down Expand Up @@ -2390,4 +2420,84 @@ mod tests {

assert_eq!(&String::from_utf8(debug_buffer).unwrap(), "This is a testMore text");
}

#[test]
fn call_reentry_direct_recursion() {
// call the contract passed as input with disabled reentry
let code_bob = MockLoader::insert(Call, |ctx, _| {
let dest = Decode::decode(&mut ctx.input_data.as_ref()).unwrap();
ctx.ext.call(0, dest, 0, vec![], false).map(|v| v.0).map_err(|e| e.0)
});

let code_charlie = MockLoader::insert(Call, |_, _| {
exec_success()
});

ExtBuilder::default().build().execute_with(|| {
let schedule = <Test as Config>::Schedule::get();
place_contract(&BOB, code_bob);
place_contract(&CHARLIE, code_charlie);

// Calling another contract should succeed
assert_ok!(MockStack::run_call(
ALICE,
BOB,
&mut GasMeter::<Test>::new(GAS_LIMIT),
&schedule,
0,
CHARLIE.encode(),
None,
));

// Calling into oneself fails
assert_err!(
MockStack::run_call(
ALICE,
BOB,
&mut GasMeter::<Test>::new(GAS_LIMIT),
&schedule,
0,
BOB.encode(),
None,
).map_err(|e| e.0.error),
<Error<Test>>::ReentranceDenied,
);
});
}

#[test]
fn call_deny_reentry() {
let code_bob = MockLoader::insert(Call, |ctx, _| {
if ctx.input_data[0] == 0 {
ctx.ext.call(0, CHARLIE, 0, vec![], false).map(|v| v.0).map_err(|e| e.0)
} else {
exec_success()
}
});

// call BOB with input set to '1'
let code_charlie = MockLoader::insert(Call, |ctx, _| {
ctx.ext.call(0, BOB, 0, vec![1], true).map(|v| v.0).map_err(|e| e.0)
});

ExtBuilder::default().build().execute_with(|| {
let schedule = <Test as Config>::Schedule::get();
place_contract(&BOB, code_bob);
place_contract(&CHARLIE, code_charlie);

// Calling another contract should succeed
athei marked this conversation as resolved.
Show resolved Hide resolved
assert_err!(
MockStack::run_call(
ALICE,
BOB,
&mut GasMeter::<Test>::new(GAS_LIMIT),
&schedule,
0,
vec![0],
None,
).map_err(|e| e.0.error),
<Error<Test>>::ReentranceDenied,
);
});
}
}
13 changes: 7 additions & 6 deletions frame/contracts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,12 +562,11 @@ pub mod pallet {
ContractTrapped,
/// The size defined in `T::MaxValueSize` was exceeded.
ValueTooLarge,
/// The action performed is not allowed while the contract performing it is already
/// on the call stack. Those actions are contract self destruction and restoration
/// of a tombstone.
ReentranceDenied,
/// `seal_input` was called twice from the same contract execution context.
InputAlreadyRead,
/// Termination of a conteact is not allowed while the contract is already
athei marked this conversation as resolved.
Show resolved Hide resolved
/// on the call stack. Can be triggered by `seal_terminate` or `seal_restore_to.
TerminatedWhileReentrant,
/// `seal_call` forwarded this contracts input. It therefore no longer available.
athei marked this conversation as resolved.
Show resolved Hide resolved
InputForwarded,
/// The subject passed to `seal_random` exceeds the limit.
RandomSubjectTooLong,
/// The amount of topics passed to `seal_deposit_events` exceeds the limit.
Expand Down Expand Up @@ -602,6 +601,8 @@ pub mod pallet {
TerminatedInConstructor,
/// The debug message specified to `seal_debug_message` does contain invalid UTF-8.
DebugMessageInvalidUTF8,
/// A call tried to invoke a contract that is flagged as non-reentrant.
ReentranceDenied,
}

/// A mapping from an original code hash to the original code, untouched by instrumentation.
Expand Down
Loading