diff --git a/prdoc/pr_3243.prdoc b/prdoc/pr_3243.prdoc
new file mode 100644
index 0000000000000000000000000000000000000000..5bad985a2672453b56ee3828483d7035b71d4b79
--- /dev/null
+++ b/prdoc/pr_3243.prdoc
@@ -0,0 +1,13 @@
+title: Don't fail fast if the weight limit of a cross contract call is too big
+
+doc:
+ - audience: Runtime Dev
+ description: |
+ Cross contracts calls will now be executed even if the supplied weight
+ limit is bigger than the reamining weight. If the **actual** weight is too low
+ they will fail in the cross contract call and roll back. This is different from the
+ old behaviour where the limit for the cross contract call must be smaller than
+ the remaining weight.
+
+crates:
+ - name: pallet-contracts
diff --git a/substrate/frame/contracts/fixtures/contracts/call_with_limit.rs b/substrate/frame/contracts/fixtures/contracts/call_with_limit.rs
index 5e98aa614e95acad471a4146c492ca12cee90da8..40cde234b44ad22a2cd2c8830d04ade2c9ff3603 100644
--- a/substrate/frame/contracts/fixtures/contracts/call_with_limit.rs
+++ b/substrate/frame/contracts/fixtures/contracts/call_with_limit.rs
@@ -31,9 +31,11 @@ pub extern "C" fn deploy() {}
#[polkavm_derive::polkavm_export]
pub extern "C" fn call() {
input!(
+ 256,
callee_addr: [u8; 32],
ref_time: u64,
proof_size: u64,
+ forwarded_input: [u8],
);
#[allow(deprecated)]
@@ -44,7 +46,7 @@ pub extern "C" fn call() {
proof_size,
None, // No deposit limit.
&0u64.to_le_bytes(), // value transferred to the contract.
- &[0u8; 0], // input data.
+ forwarded_input,
None,
)
.unwrap();
diff --git a/substrate/frame/contracts/src/exec.rs b/substrate/frame/contracts/src/exec.rs
index 2183d6b96cc5d4f84cbfa6777e0c1b0bbeec4f86..6f7131f3ea416ebe5e51655720c3bf5e2d2f4650 100644
--- a/substrate/frame/contracts/src/exec.rs
+++ b/substrate/frame/contracts/src/exec.rs
@@ -834,7 +834,7 @@ where
contract_info: CachedContract::Cached(contract_info),
account_id,
entry_point,
- nested_gas: gas_meter.nested(gas_limit)?,
+ nested_gas: gas_meter.nested(gas_limit),
nested_storage: storage_meter.nested(deposit_limit),
allows_reentry: true,
};
diff --git a/substrate/frame/contracts/src/gas.rs b/substrate/frame/contracts/src/gas.rs
index f78fb03a401db005bb11197bea605e4d615d8262..11292dc03f0323fbb0b60bc5eede8b3336f44c80 100644
--- a/substrate/frame/contracts/src/gas.rs
+++ b/substrate/frame/contracts/src/gas.rs
@@ -104,9 +104,7 @@ impl<T: Config> GasMeter<T> {
/// # Note
///
/// Passing `0` as amount is interpreted as "all remaining gas".
- pub fn nested(&mut self, amount: Weight) -> Result<Self, DispatchError> {
- // NOTE that it is ok to allocate all available gas since it still ensured
- // by `charge` that it doesn't reach zero.
+ pub fn nested(&mut self, amount: Weight) -> Self {
let amount = Weight::from_parts(
if amount.ref_time().is_zero() {
self.gas_left().ref_time()
@@ -118,33 +116,17 @@ impl<T: Config> GasMeter<T> {
} else {
amount.proof_size()
},
- );
- self.gas_left = self.gas_left.checked_sub(&amount).ok_or_else(|| <Error<T>>::OutOfGas)?;
- Ok(GasMeter::new(amount))
+ )
+ .min(self.gas_left);
+ self.gas_left -= amount;
+ GasMeter::new(amount)
}
/// Absorb the remaining gas of a nested meter after we are done using it.
pub fn absorb_nested(&mut self, nested: Self) {
- if self.gas_left.ref_time().is_zero() {
- // All of the remaining gas was inherited by the nested gas meter. When absorbing
- // we can therefore safely inherit the lowest gas that the nested gas meter experienced
- // as long as it is lower than the lowest gas that was experienced by the parent.
- // We cannot call `self.gas_left_lowest()` here because in the state that this
- // code is run the parent gas meter has `0` gas left.
- *self.gas_left_lowest.ref_time_mut() =
- nested.gas_left_lowest().ref_time().min(self.gas_left_lowest.ref_time());
- } else {
- // The nested gas meter was created with a fixed amount that did not consume all of the
- // parents (self) gas. The lowest gas that self will experience is when the nested
- // gas was pre charged with the fixed amount.
- *self.gas_left_lowest.ref_time_mut() = self.gas_left_lowest().ref_time();
- }
- if self.gas_left.proof_size().is_zero() {
- *self.gas_left_lowest.proof_size_mut() =
- nested.gas_left_lowest().proof_size().min(self.gas_left_lowest.proof_size());
- } else {
- *self.gas_left_lowest.proof_size_mut() = self.gas_left_lowest().proof_size();
- }
+ self.gas_left_lowest = (self.gas_left + nested.gas_limit)
+ .saturating_sub(nested.gas_required())
+ .min(self.gas_left_lowest);
self.gas_left += nested.gas_left;
}
diff --git a/substrate/frame/contracts/src/tests.rs b/substrate/frame/contracts/src/tests.rs
index 5711d3ccc83a527fae49f4ea6397a5a4b5fe54a9..eba959e8d465c0c72e0d699bb3e24cb849ce5f41 100644
--- a/substrate/frame/contracts/src/tests.rs
+++ b/substrate/frame/contracts/src/tests.rs
@@ -40,7 +40,7 @@ use crate::{
MigrationInProgress, Origin, Pallet, PristineCode, Schedule,
};
use assert_matches::assert_matches;
-use codec::Encode;
+use codec::{Decode, Encode};
use frame_support::{
assert_err, assert_err_ignore_postinfo, assert_err_with_weight, assert_noop, assert_ok,
derive_impl,
@@ -217,8 +217,6 @@ impl ChainExtension<Test> for TestExtension {
where
E: Ext<T = Test>,
{
- use codec::Decode;
-
let func_id = env.func_id();
let id = env.ext_id() as u32 | func_id as u32;
match func_id {
@@ -2924,12 +2922,13 @@ fn debug_message_invalid_utf8() {
}
#[test]
-fn gas_estimation_nested_call_fixed_limit() {
+fn gas_estimation_for_subcalls() {
let (caller_code, _caller_hash) = compile_module::<Test>("call_with_limit").unwrap();
- let (callee_code, _callee_hash) = compile_module::<Test>("dummy").unwrap();
+ let (call_runtime_code, _caller_hash) = compile_module::<Test>("call_runtime").unwrap();
+ let (dummy_code, _callee_hash) = compile_module::<Test>("dummy").unwrap();
ExtBuilder::default().existential_deposit(50).build().execute_with(|| {
let min_balance = Contracts::min_balance();
- let _ = <Test as Config>::Currency::set_balance(&ALICE, 1000 * min_balance);
+ let _ = <Test as Config>::Currency::set_balance(&ALICE, 2_000 * min_balance);
let addr_caller = Contracts::bare_instantiate(
ALICE,
@@ -2938,7 +2937,7 @@ fn gas_estimation_nested_call_fixed_limit() {
None,
Code::Upload(caller_code),
vec![],
- vec![0],
+ vec![],
DebugInfo::Skip,
CollectEvents::Skip,
)
@@ -2946,14 +2945,14 @@ fn gas_estimation_nested_call_fixed_limit() {
.unwrap()
.account_id;
- let addr_callee = Contracts::bare_instantiate(
+ let addr_dummy = Contracts::bare_instantiate(
ALICE,
min_balance * 100,
GAS_LIMIT,
None,
- Code::Upload(callee_code),
+ Code::Upload(dummy_code),
+ vec![],
vec![],
- vec![1],
DebugInfo::Skip,
CollectEvents::Skip,
)
@@ -2961,68 +2960,136 @@ fn gas_estimation_nested_call_fixed_limit() {
.unwrap()
.account_id;
- let input: Vec<u8> = AsRef::<[u8]>::as_ref(&addr_callee)
- .iter()
- .cloned()
- .chain((GAS_LIMIT / 5).ref_time().to_le_bytes())
- .chain((GAS_LIMIT / 5).proof_size().to_le_bytes())
- .collect();
-
- // Call in order to determine the gas that is required for this call
- let result = Contracts::bare_call(
+ let addr_call_runtime = Contracts::bare_instantiate(
ALICE,
- addr_caller.clone(),
- 0,
+ min_balance * 100,
GAS_LIMIT,
None,
- input.clone(),
+ Code::Upload(call_runtime_code),
+ vec![],
+ vec![],
DebugInfo::Skip,
CollectEvents::Skip,
- Determinism::Enforced,
- );
- assert_ok!(&result.result);
+ )
+ .result
+ .unwrap()
+ .account_id;
- // We have a subcall with a fixed gas limit. This constitutes precharging.
- assert!(result.gas_required.all_gt(result.gas_consumed));
+ // Run the test for all of those weight limits for the subcall
+ let weights = [
+ Weight::zero(),
+ GAS_LIMIT,
+ GAS_LIMIT * 2,
+ GAS_LIMIT / 5,
+ Weight::from_parts(0, GAS_LIMIT.proof_size()),
+ Weight::from_parts(GAS_LIMIT.ref_time(), 0),
+ ];
- // Make the same call using the estimated gas. Should succeed.
- assert_ok!(
- Contracts::bare_call(
- ALICE,
- addr_caller.clone(),
- 0,
- result.gas_required,
- Some(result.storage_deposit.charge_or_zero()),
- input.clone(),
- DebugInfo::Skip,
- CollectEvents::Skip,
- Determinism::Enforced,
- )
- .result
- );
+ // This call is passed to the sub call in order to create a large `required_weight`
+ let runtime_call = RuntimeCall::Dummy(pallet_dummy::Call::overestimate_pre_charge {
+ pre_charge: Weight::from_parts(10_000_000_000, 512 * 1024),
+ actual_weight: Weight::from_parts(1, 1),
+ })
+ .encode();
- // Make the same call using proof_size but less than estimated. Should fail with OutOfGas.
- let result = Contracts::bare_call(
- ALICE,
- addr_caller,
- 0,
- result.gas_required.sub_proof_size(1),
- Some(result.storage_deposit.charge_or_zero()),
- input,
- DebugInfo::Skip,
- CollectEvents::Skip,
- Determinism::Enforced,
- )
- .result;
- assert_err!(result, <Error<Test>>::OutOfGas);
+ // Encodes which contract should be sub called with which input
+ let sub_calls: [(&[u8], Vec<_>, bool); 2] = [
+ (addr_dummy.as_ref(), vec![], false),
+ (addr_call_runtime.as_ref(), runtime_call, true),
+ ];
+
+ for weight in weights {
+ for (sub_addr, sub_input, out_of_gas_in_subcall) in &sub_calls {
+ let input: Vec<u8> = sub_addr
+ .iter()
+ .cloned()
+ .chain(weight.ref_time().to_le_bytes())
+ .chain(weight.proof_size().to_le_bytes())
+ .chain(sub_input.clone())
+ .collect();
+
+ // Call in order to determine the gas that is required for this call
+ let result = Contracts::bare_call(
+ ALICE,
+ addr_caller.clone(),
+ 0,
+ GAS_LIMIT,
+ None,
+ input.clone(),
+ DebugInfo::Skip,
+ CollectEvents::Skip,
+ Determinism::Enforced,
+ );
+ assert_ok!(&result.result);
+
+ // If the out of gas happens in the subcall the caller contract
+ // will just trap. Otherwise we would need to forward an error
+ // code to signal that the sub contract ran out of gas.
+ let error: DispatchError = if *out_of_gas_in_subcall {
+ assert!(result.gas_required.all_gt(result.gas_consumed));
+ <Error<Test>>::ContractTrapped.into()
+ } else {
+ assert_eq!(result.gas_required, result.gas_consumed);
+ <Error<Test>>::OutOfGas.into()
+ };
+
+ // Make the same call using the estimated gas. Should succeed.
+ assert_ok!(
+ Contracts::bare_call(
+ ALICE,
+ addr_caller.clone(),
+ 0,
+ result.gas_required,
+ Some(result.storage_deposit.charge_or_zero()),
+ input.clone(),
+ DebugInfo::Skip,
+ CollectEvents::Skip,
+ Determinism::Enforced,
+ )
+ .result
+ );
+
+ // Check that it fails with too little ref_time
+ assert_err!(
+ Contracts::bare_call(
+ ALICE,
+ addr_caller.clone(),
+ 0,
+ result.gas_required.sub_ref_time(1),
+ Some(result.storage_deposit.charge_or_zero()),
+ input.clone(),
+ DebugInfo::Skip,
+ CollectEvents::Skip,
+ Determinism::Enforced,
+ )
+ .result,
+ error,
+ );
+
+ // Check that it fails with too little proof_size
+ assert_err!(
+ Contracts::bare_call(
+ ALICE,
+ addr_caller.clone(),
+ 0,
+ result.gas_required.sub_proof_size(1),
+ Some(result.storage_deposit.charge_or_zero()),
+ input,
+ DebugInfo::Skip,
+ CollectEvents::Skip,
+ Determinism::Enforced,
+ )
+ .result,
+ error,
+ );
+ }
+ }
});
}
#[test]
fn gas_estimation_call_runtime() {
- use codec::Decode;
let (caller_code, _caller_hash) = compile_module::<Test>("call_runtime").unwrap();
- let (callee_code, _callee_hash) = compile_module::<Test>("dummy").unwrap();
ExtBuilder::default().existential_deposit(50).build().execute_with(|| {
let min_balance = Contracts::min_balance();
let _ = <Test as Config>::Currency::set_balance(&ALICE, 1000 * min_balance);
@@ -3043,25 +3110,11 @@ fn gas_estimation_call_runtime() {
.unwrap()
.account_id;
- Contracts::bare_instantiate(
- ALICE,
- min_balance * 100,
- GAS_LIMIT,
- None,
- Code::Upload(callee_code),
- vec![],
- vec![1],
- DebugInfo::Skip,
- CollectEvents::Skip,
- )
- .result
- .unwrap();
-
// Call something trivial with a huge gas limit so that we can observe the effects
// of pre-charging. This should create a difference between consumed and required.
let call = RuntimeCall::Dummy(pallet_dummy::Call::overestimate_pre_charge {
- pre_charge: Weight::from_parts(10_000_000, 0),
- actual_weight: Weight::from_parts(100, 0),
+ pre_charge: Weight::from_parts(10_000_000, 1_000),
+ actual_weight: Weight::from_parts(100, 100),
});
let result = Contracts::bare_call(
ALICE,
@@ -3077,7 +3130,7 @@ fn gas_estimation_call_runtime() {
// contract encodes the result of the dispatch runtime
let outcome = u32::decode(&mut result.result.unwrap().data.as_ref()).unwrap();
assert_eq!(outcome, 0);
- assert!(result.gas_required.ref_time() > result.gas_consumed.ref_time());
+ assert!(result.gas_required.all_gt(result.gas_consumed));
// Make the same call using the required gas. Should succeed.
assert_ok!(