Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

contracts: Passing zero as deposit_limit to call_v2 correctly limits the deposit to nothing #3392

Merged
merged 1 commit into from
Feb 19, 2024
Merged

contracts: Passing zero as deposit_limit to call_v2 correctly limits the deposit to nothing #3392

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 26 additions & 65 deletions substrate/frame/contracts/src/exec.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ pub trait Ext: sealing::Sealed {
fn call(
&mut self,
gas_limit: Weight,
deposit_limit: BalanceOf<Self::T>,
deposit_limit: Option<BalanceOf<Self::T>>,
to: AccountIdOf<Self::T>,
value: BalanceOf<Self::T>,
input_data: Vec<u8>,
Expand All @@ -168,7 +168,7 @@ pub trait Ext: sealing::Sealed {
fn instantiate(
&mut self,
gas_limit: Weight,
deposit_limit: BalanceOf<Self::T>,
deposit_limit: Option<BalanceOf<Self::T>>,
code: CodeHash<Self::T>,
value: BalanceOf<Self::T>,
input_data: Vec<u8>,
Expand Down Expand Up @@ -747,7 +747,7 @@ where
gas_meter,
Weight::zero(),
storage_meter,
BalanceOf::<T>::zero(),
None,
determinism,
)?;

Expand Down Expand Up @@ -779,7 +779,7 @@ where
gas_meter: &mut GasMeter<T>,
gas_limit: Weight,
storage_meter: &mut storage::meter::GenericMeter<T, S>,
deposit_limit: BalanceOf<T>,
deposit_limit: Option<BalanceOf<T>>,
determinism: Determinism,
) -> Result<(Frame<T>, E, Option<u64>), ExecError> {
let (account_id, contract_info, executable, delegate_caller, entry_point, nonce) =
Expand Down Expand Up @@ -848,7 +848,7 @@ where
frame_args: FrameArgs<T, E>,
value_transferred: BalanceOf<T>,
gas_limit: Weight,
deposit_limit: BalanceOf<T>,
deposit_limit: Option<BalanceOf<T>>,
) -> Result<E, ExecError> {
if self.frames.len() == T::CallStack::size() {
return Err(Error::<T>::MaxCallDepthReached.into())
Expand Down Expand Up @@ -1185,7 +1185,7 @@ where
fn call(
&mut self,
gas_limit: Weight,
deposit_limit: BalanceOf<T>,
deposit_limit: Option<BalanceOf<T>>,
to: T::AccountId,
value: BalanceOf<T>,
input_data: Vec<u8>,
Expand Down Expand Up @@ -1246,15 +1246,15 @@ where
},
value,
Weight::zero(),
BalanceOf::<T>::zero(),
None,
)?;
self.run(executable, input_data)
}

fn instantiate(
&mut self,
gas_limit: Weight,
deposit_limit: BalanceOf<Self::T>,
deposit_limit: Option<BalanceOf<Self::T>>,
code_hash: CodeHash<T>,
value: BalanceOf<T>,
input_data: Vec<u8>,
Expand Down Expand Up @@ -2091,7 +2091,7 @@ mod tests {
let value = Default::default();
let recurse_ch = MockLoader::insert(Call, |ctx, _| {
// Try to call into yourself.
let r = ctx.ext.call(Weight::zero(), BalanceOf::<Test>::zero(), BOB, 0, vec![], true);
let r = ctx.ext.call(Weight::zero(), None, BOB, 0, vec![], true);

ReachedBottom::mutate(|reached_bottom| {
if !*reached_bottom {
Expand Down Expand Up @@ -2149,11 +2149,7 @@ mod tests {
});

// Call into CHARLIE contract.
assert_matches!(
ctx.ext
.call(Weight::zero(), BalanceOf::<Test>::zero(), CHARLIE, 0, vec![], true),
Ok(_)
);
assert_matches!(ctx.ext.call(Weight::zero(), None, CHARLIE, 0, vec![], true), Ok(_));
exec_success()
});
let charlie_ch = MockLoader::insert(Call, |ctx, _| {
Expand Down Expand Up @@ -2297,8 +2293,7 @@ mod tests {
// ALICE is the origin of the call stack
assert!(ctx.ext.caller_is_origin());
// BOB calls CHARLIE
ctx.ext
.call(Weight::zero(), BalanceOf::<Test>::zero(), CHARLIE, 0, vec![], true)
ctx.ext.call(Weight::zero(), None, CHARLIE, 0, vec![], true)
});

ExtBuilder::default().build().execute_with(|| {
Expand Down Expand Up @@ -2396,8 +2391,7 @@ mod tests {
// root is the origin of the call stack.
assert!(ctx.ext.caller_is_root());
// BOB calls CHARLIE.
ctx.ext
.call(Weight::zero(), BalanceOf::<Test>::zero(), CHARLIE, 0, vec![], true)
ctx.ext.call(Weight::zero(), None, CHARLIE, 0, vec![], true)
});

ExtBuilder::default().build().execute_with(|| {
Expand Down Expand Up @@ -2430,11 +2424,7 @@ mod tests {
assert_eq!(*ctx.ext.address(), BOB);

// Call into charlie contract.
assert_matches!(
ctx.ext
.call(Weight::zero(), BalanceOf::<Test>::zero(), CHARLIE, 0, vec![], true),
Ok(_)
);
assert_matches!(ctx.ext.call(Weight::zero(), None, CHARLIE, 0, vec![], true), Ok(_));
exec_success()
});
let charlie_ch = MockLoader::insert(Call, |ctx, _| {
Expand Down Expand Up @@ -2609,7 +2599,7 @@ mod tests {
.ext
.instantiate(
Weight::zero(),
BalanceOf::<Test>::zero(),
None,
dummy_ch,
<Test as Config>::Currency::minimum_balance(),
vec![],
Expand Down Expand Up @@ -2685,7 +2675,7 @@ mod tests {
assert_matches!(
ctx.ext.instantiate(
Weight::zero(),
BalanceOf::<Test>::zero(),
None,
dummy_ch,
<Test as Config>::Currency::minimum_balance(),
vec![],
Expand Down Expand Up @@ -2794,25 +2784,15 @@ mod tests {
assert_eq!(info.storage_byte_deposit, 0);
info.storage_byte_deposit = 42;
assert_eq!(
ctx.ext.call(
Weight::zero(),
BalanceOf::<Test>::zero(),
CHARLIE,
0,
vec![],
true
),
ctx.ext.call(Weight::zero(), None, CHARLIE, 0, vec![], true),
exec_trapped()
);
assert_eq!(ctx.ext.contract_info().storage_byte_deposit, 42);
}
exec_success()
});
let code_charlie = MockLoader::insert(Call, |ctx, _| {
assert!(ctx
.ext
.call(Weight::zero(), BalanceOf::<Test>::zero(), BOB, 0, vec![99], true)
.is_ok());
assert!(ctx.ext.call(Weight::zero(), None, BOB, 0, vec![99], true).is_ok());
exec_trapped()
});

Expand Down Expand Up @@ -2844,7 +2824,7 @@ mod tests {
fn recursive_call_during_constructor_fails() {
let code = MockLoader::insert(Constructor, |ctx, _| {
assert_matches!(
ctx.ext.call(Weight::zero(), BalanceOf::<Test>::zero(), ctx.ext.address().clone(), 0, vec![], true),
ctx.ext.call(Weight::zero(), None, ctx.ext.address().clone(), 0, vec![], true),
Err(ExecError{error, ..}) if error == <Error<Test>>::ContractNotFound.into()
);
exec_success()
Expand Down Expand Up @@ -2994,7 +2974,7 @@ mod tests {
// 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(Weight::zero(), BalanceOf::<Test>::zero(), dest, 0, vec![], false)
ctx.ext.call(Weight::zero(), None, dest, 0, vec![], false)
});

let code_charlie = MockLoader::insert(Call, |_, _| exec_success());
Expand Down Expand Up @@ -3043,16 +3023,15 @@ mod tests {
fn call_deny_reentry() {
let code_bob = MockLoader::insert(Call, |ctx, _| {
if ctx.input_data[0] == 0 {
ctx.ext
.call(Weight::zero(), BalanceOf::<Test>::zero(), CHARLIE, 0, vec![], false)
ctx.ext.call(Weight::zero(), None, CHARLIE, 0, vec![], false)
} else {
exec_success()
}
});

// call BOB with input set to '1'
let code_charlie = MockLoader::insert(Call, |ctx, _| {
ctx.ext.call(Weight::zero(), BalanceOf::<Test>::zero(), BOB, 0, vec![1], true)
ctx.ext.call(Weight::zero(), None, BOB, 0, vec![1], true)
});

ExtBuilder::default().build().execute_with(|| {
Expand Down Expand Up @@ -3248,7 +3227,7 @@ mod tests {
ctx.ext
.instantiate(
Weight::zero(),
BalanceOf::<Test>::zero(),
None,
fail_code,
ctx.ext.minimum_balance() * 100,
vec![],
Expand All @@ -3262,7 +3241,7 @@ mod tests {
.ext
.instantiate(
Weight::zero(),
BalanceOf::<Test>::zero(),
None,
success_code,
ctx.ext.minimum_balance() * 100,
vec![],
Expand All @@ -3271,9 +3250,7 @@ mod tests {
.unwrap();

// a plain call should not influence the account counter
ctx.ext
.call(Weight::zero(), BalanceOf::<Test>::zero(), account_id, 0, vec![], false)
.unwrap();
ctx.ext.call(Weight::zero(), None, account_id, 0, vec![], false).unwrap();

exec_success()
});
Expand Down Expand Up @@ -3791,31 +3768,15 @@ mod tests {
assert_eq!(ctx.ext.nonce(), 1);
// Should not change with a failed instantiation
assert_err!(
ctx.ext.instantiate(
Weight::zero(),
BalanceOf::<Test>::zero(),
fail_code,
0,
vec![],
&[],
),
ctx.ext.instantiate(Weight::zero(), None, fail_code, 0, vec![], &[],),
ExecError {
error: <Error<Test>>::ContractTrapped.into(),
origin: ErrorOrigin::Callee
}
);
assert_eq!(ctx.ext.nonce(), 1);
// Successful instantiation increments
ctx.ext
.instantiate(
Weight::zero(),
BalanceOf::<Test>::zero(),
success_code,
0,
vec![],
&[],
)
.unwrap();
ctx.ext.instantiate(Weight::zero(), None, success_code, 0, vec![], &[]).unwrap();
assert_eq!(ctx.ext.nonce(), 2);
exec_success()
});
Expand Down
30 changes: 15 additions & 15 deletions substrate/frame/contracts/src/storage/meter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,21 +283,21 @@ where
E: Ext<T>,
S: State + Default + Debug,
{
/// Create a new child that has its `limit`.
/// Passing `0` as the limit is interpreted as to take whatever is remaining from its parent.
/// Create a new child with an optional `limit` on how much deposit it can consume.
///
/// This is called whenever a new subcall is initiated in order to track the storage
/// usage for this sub call separately. This is necessary because we want to exchange balance
/// with the current contract we are interacting with.
pub fn nested(&self, limit: BalanceOf<T>) -> RawMeter<T, E, Nested> {
pub fn nested(&self, limit: Option<BalanceOf<T>>) -> RawMeter<T, E, Nested> {
debug_assert!(matches!(self.contract_state(), ContractState::Alive));
// If a special limit is specified higher than it is available,
// we want to enforce the lesser limit to the nested meter, to fail in the sub-call.
let limit = self.available().min(limit);
if limit.is_zero() {
RawMeter { limit: self.available(), ..Default::default() }
if let Some(limit) = limit {
RawMeter {
limit: self.available().min(limit),
nested: Nested::OwnLimit,
..Default::default()
}
} else {
RawMeter { limit, nested: Nested::OwnLimit, ..Default::default() }
RawMeter { limit: self.available(), ..Default::default() }
}
}

Expand Down Expand Up @@ -750,7 +750,7 @@ mod tests {
assert_eq!(meter.available(), 1_000);

// an empty charge does not create a `Charge` entry
let mut nested0 = meter.nested(BalanceOf::<Test>::zero());
let mut nested0 = meter.nested(None);
nested0.charge(&Default::default());
meter.absorb(nested0, &BOB, None);

Expand Down Expand Up @@ -812,7 +812,7 @@ mod tests {
bytes_deposit: 100,
items_deposit: 10,
});
let mut nested0 = meter.nested(BalanceOf::<Test>::zero());
let mut nested0 = meter.nested(None);
nested0.charge(&Diff {
bytes_added: 108,
bytes_removed: 5,
Expand All @@ -827,7 +827,7 @@ mod tests {
bytes_deposit: 100,
items_deposit: 20,
});
let mut nested1 = nested0.nested(BalanceOf::<Test>::zero());
let mut nested1 = nested0.nested(None);
nested1.charge(&Diff { items_removed: 5, ..Default::default() });
nested0.absorb(nested1, &CHARLIE, Some(&mut nested1_info));

Expand All @@ -837,7 +837,7 @@ mod tests {
bytes_deposit: 100,
items_deposit: 20,
});
let mut nested2 = nested0.nested(BalanceOf::<Test>::zero());
let mut nested2 = nested0.nested(None);
nested2.charge(&Diff { items_removed: 7, ..Default::default() });
nested0.absorb(nested2, &CHARLIE, Some(&mut nested2_info));

Expand Down Expand Up @@ -891,7 +891,7 @@ mod tests {
let mut meter = TestMeter::new(&test_case.origin, Some(1_000), 0).unwrap();
assert_eq!(meter.available(), 1_000);

let mut nested0 = meter.nested(BalanceOf::<Test>::zero());
let mut nested0 = meter.nested(None);
nested0.charge(&Diff {
bytes_added: 5,
bytes_removed: 1,
Expand All @@ -906,7 +906,7 @@ mod tests {
bytes_deposit: 100,
items_deposit: 20,
});
let mut nested1 = nested0.nested(BalanceOf::<Test>::zero());
let mut nested1 = nested0.nested(None);
nested1.charge(&Diff { items_removed: 5, ..Default::default() });
nested1.charge(&Diff { bytes_added: 20, ..Default::default() });
nested1.terminate(&nested1_info, CHARLIE);
Expand Down
4 changes: 2 additions & 2 deletions substrate/frame/contracts/src/wasm/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,7 @@ mod tests {
fn call(
&mut self,
_gas_limit: Weight,
_deposit_limit: BalanceOf<Self::T>,
_deposit_limit: Option<BalanceOf<Self::T>>,
to: AccountIdOf<Self::T>,
value: u64,
data: Vec<u8>,
Expand All @@ -569,7 +569,7 @@ mod tests {
fn instantiate(
&mut self,
gas_limit: Weight,
_deposit_limit: BalanceOf<Self::T>,
_deposit_limit: Option<BalanceOf<Self::T>>,
code_hash: CodeHash<Test>,
value: u64,
data: Vec<u8>,
Expand Down
Loading
Loading