Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[pallet-revive] Update delegate_call to accept address and weight #6111

Merged
merged 24 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
acde6b4
update delegate_call to take address and weight
ermalkaleci Oct 17, 2024
02dace6
fix fixtures
ermalkaleci Oct 17, 2024
23b9319
Merge branch 'master' of github.com:ermalkaleci/polkadot-sdk into fea…
ermalkaleci Oct 23, 2024
697422b
fix test
ermalkaleci Oct 23, 2024
7f374c0
add prdoc
ermalkaleci Oct 23, 2024
4a29f0a
fmt
ermalkaleci Oct 23, 2024
78d39c2
fmt
ermalkaleci Oct 23, 2024
171167a
update prdoc
ermalkaleci Oct 28, 2024
67174f0
Merge branch 'master' of github.com:ermalkaleci/polkadot-sdk into fea…
ermalkaleci Oct 28, 2024
f899e9e
add option to limit storage deposit
ermalkaleci Oct 28, 2024
a5f2f1c
clippy
ermalkaleci Oct 28, 2024
c05bd51
fix immutable read on delegate call
ermalkaleci Oct 31, 2024
ea118a0
Merge branch 'master' of github.com:ermalkaleci/polkadot-sdk into fea…
ermalkaleci Oct 31, 2024
46af335
refactor
ermalkaleci Oct 31, 2024
ad9008e
Merge branch 'master' of github.com:ermalkaleci/polkadot-sdk into fea…
ermalkaleci Oct 31, 2024
63844ee
fmt
ermalkaleci Oct 31, 2024
bb79cda
Revert "refactor"
ermalkaleci Oct 31, 2024
fc6a762
merge caller and callee into delegate info
ermalkaleci Nov 1, 2024
7073ee3
apply suggestion
ermalkaleci Nov 6, 2024
9f06a03
add deposit_limit test
ermalkaleci Nov 6, 2024
95d1ffd
fmt
ermalkaleci Nov 6, 2024
aee61a6
Merge branch 'master' into feat/update-delegate-call
athei Nov 18, 2024
20767b6
Merge branch 'master' into feat/update-delegate-call
athei Nov 18, 2024
5c1f1ed
fix test
ermalkaleci Nov 19, 2024
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
17 changes: 17 additions & 0 deletions prdoc/pr_6111.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
title: "[pallet-revive] Update delegate_call to accept address and weight"

doc:
- audience: Runtime Dev
description: |
Enhance the `delegate_call` function to accept an `address` target parameter instead of a `code_hash`.
This allows direct identification of the target contract using the provided address.
Additionally, introduce parameters for specifying a customizable `ref_time` limit and `proof_size` limit,
thereby improving flexibility and control during contract interactions.

crates:
- name: pallet-revive
bump: major
- name: pallet-revive-fixtures
bump: patch
- name: pallet-revive-uapi
bump: major
8 changes: 6 additions & 2 deletions substrate/frame/revive/fixtures/contracts/delegate_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,11 @@ pub extern "C" fn deploy() {}
#[no_mangle]
#[polkavm_derive::polkavm_export]
pub extern "C" fn call() {
input!(code_hash: &[u8; 32],);
input!(
address: &[u8; 20],
ref_time: u64,
proof_size: u64,
);

let mut key = [0u8; 32];
key[0] = 1u8;
Expand All @@ -42,7 +46,7 @@ pub extern "C" fn call() {
assert!(value[0] == 2u8);

let input = [0u8; 0];
api::delegate_call(uapi::CallFlags::empty(), code_hash, &input, None).unwrap();
api::delegate_call(uapi::CallFlags::empty(), address, ref_time, proof_size, None, &input, None).unwrap();

api::get_storage(StorageFlags::empty(), &key, value).unwrap();
assert!(value[0] == 1u8);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ pub extern "C" fn deploy() {}
#[no_mangle]
#[polkavm_derive::polkavm_export]
pub extern "C" fn call() {
input!(code_hash: &[u8; 32],);
input!(address: &[u8; 20],);

// Delegate call into passed code hash.
// Delegate call into passed address.
let input = [0u8; 0];
api::delegate_call(uapi::CallFlags::empty(), code_hash, &input, None).unwrap();
api::delegate_call(uapi::CallFlags::empty(), address, 0, 0, None, &input, None).unwrap();
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const ALICE_FALLBACK: [u8; 20] = [1u8; 20];
fn load_input(delegate_call: bool) {
input!(
action: u32,
address: &[u8; 20],
code_hash: &[u8; 32],
);

Expand All @@ -51,7 +52,7 @@ fn load_input(delegate_call: bool) {
}

if delegate_call {
api::delegate_call(uapi::CallFlags::empty(), code_hash, &[], None).unwrap();
api::delegate_call(uapi::CallFlags::empty(), address, 0, 0, None, &[], None).unwrap();
}
}

Expand Down
25 changes: 18 additions & 7 deletions substrate/frame/revive/src/benchmarking/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1522,25 +1522,36 @@ mod benchmarks {

#[benchmark(pov_mode = Measured)]
fn seal_delegate_call() -> Result<(), BenchmarkError> {
let hash = Contract::<T>::with_index(1, WasmModule::dummy(), vec![])?.info()?.code_hash;
let Contract { account_id: address, .. } =
Contract::<T>::with_index(1, WasmModule::dummy(), vec![]).unwrap();

let address_bytes = address.encode();
let address_len = address_bytes.len() as u32;

let deposit: BalanceOf<T> = (u32::MAX - 100).into();
let deposit_bytes = Into::<U256>::into(deposit).encode();

let mut setup = CallSetup::<T>::default();
setup.set_storage_deposit_limit(deposit);
setup.set_origin(Origin::from_account_id(setup.contract().account_id.clone()));

let (mut ext, _) = setup.ext();
let mut runtime = crate::wasm::Runtime::<_, [u8]>::new(&mut ext, vec![]);
let mut memory = memory!(hash.encode(),);
let mut memory = memory!(address_bytes, deposit_bytes,);

let result;
#[block]
{
result = runtime.bench_delegate_call(
memory.as_mut_slice(),
0, // flags
0, // code_hash_ptr
0, // input_data_ptr
0, // input_data_len
SENTINEL, // output_ptr
0, // flags
0, // address_ptr
0, // ref_time_limit
0, // proof_size_limit
address_len, // deposit_ptr
0, // input_data_ptr
0, // input_data_len
SENTINEL, // output_ptr
0,
);
}
Expand Down
99 changes: 83 additions & 16 deletions substrate/frame/revive/src/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,13 @@ pub trait Ext: sealing::Sealed {
/// Execute code in the current frame.
///
/// Returns the code size of the called contract.
fn delegate_call(&mut self, code: H256, input_data: Vec<u8>) -> Result<(), ExecError>;
fn delegate_call(
&mut self,
gas_limit: Weight,
ermalkaleci marked this conversation as resolved.
Show resolved Hide resolved
deposit_limit: U256,
address: H160,
input_data: Vec<u8>,
) -> Result<(), ExecError>;

/// Instantiate a contract from the given code.
///
Expand Down Expand Up @@ -1394,12 +1400,19 @@ where
result
}

fn delegate_call(&mut self, code_hash: H256, input_data: Vec<u8>) -> Result<(), ExecError> {
fn delegate_call(
&mut self,
gas_limit: Weight,
deposit_limit: U256,
address: H160,
input_data: Vec<u8>,
) -> Result<(), ExecError> {
// We reset the return data now, so it is cleared out even if no new frame was executed.
// This is for example the case for unknown code hashes or creating the frame fails.
*self.last_frame_output_mut() = Default::default();

let executable = E::from_storage(code_hash, self.gas_meter_mut())?;
let contract_info = ContractInfoOf::<T>::get(&address).ok_or(Error::<T>::CodeNotFound)?;
let executable = E::from_storage(contract_info.code_hash, self.gas_meter_mut())?;
let top_frame = self.top_frame_mut();
let contract_info = top_frame.contract_info().clone();
let account_id = top_frame.account_id.clone();
Expand All @@ -1411,8 +1424,8 @@ where
delegated_call: Some(DelegatedCall { executable, caller: self.caller().clone() }),
},
value,
Weight::zero(),
BalanceOf::<T>::zero(),
gas_limit,
deposit_limit.try_into().map_err(|_| Error::<T>::BalanceConversionFailed)?,
self.is_read_only(),
)?;
self.run(executable.expect(FRAME_ALWAYS_EXISTS_ON_INSTANTIATE), input_data)
Expand Down Expand Up @@ -1838,7 +1851,7 @@ mod tests {
AddressMapper, Error,
};
use assert_matches::assert_matches;
use frame_support::{assert_err, assert_ok, parameter_types};
use frame_support::{assert_err, assert_noop, assert_ok, parameter_types};
use frame_system::{AccountInfo, EventRecord, Phase};
use pallet_revive_uapi::ReturnFlags;
use pretty_assertions::assert_eq;
Expand Down Expand Up @@ -2061,33 +2074,84 @@ mod tests {

let delegate_ch = MockLoader::insert(Call, move |ctx, _| {
assert_eq!(ctx.ext.value_transferred(), U256::from(value));
let _ = ctx.ext.delegate_call(success_ch, Vec::new())?;
let _ =
ctx.ext.delegate_call(Weight::zero(), U256::zero(), CHARLIE_ADDR, Vec::new())?;
Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: Vec::new() })
});

ExtBuilder::default().build().execute_with(|| {
place_contract(&BOB, delegate_ch);
place_contract(&CHARLIE, success_ch);
set_balance(&ALICE, 100);
let balance = get_balance(&BOB_FALLBACK);
let origin = Origin::from_account_id(ALICE);
let mut storage_meter = storage::meter::Meter::new(&origin, 0, 55).unwrap();

let _ = MockStack::run_call(
assert_ok!(MockStack::run_call(
origin,
BOB_ADDR,
&mut GasMeter::<Test>::new(GAS_LIMIT),
&mut storage_meter,
value,
vec![],
None,
)
.unwrap();
));

assert_eq!(get_balance(&ALICE), 100 - value);
assert_eq!(get_balance(&BOB_FALLBACK), balance + value);
});
}

#[test]
fn delegate_call_missing_contract() {
let missing_ch = MockLoader::insert(Call, move |_ctx, _| {
Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: Vec::new() })
});

let delegate_ch = MockLoader::insert(Call, move |ctx, _| {
let _ =
ctx.ext.delegate_call(Weight::zero(), U256::zero(), CHARLIE_ADDR, Vec::new())?;
Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: Vec::new() })
});

ExtBuilder::default().build().execute_with(|| {
place_contract(&BOB, delegate_ch);
set_balance(&ALICE, 100);

let origin = Origin::from_account_id(ALICE);
let mut storage_meter = storage::meter::Meter::new(&origin, 0, 55).unwrap();

// contract code missing
assert_noop!(
MockStack::run_call(
origin.clone(),
BOB_ADDR,
&mut GasMeter::<Test>::new(GAS_LIMIT),
&mut storage_meter,
0,
vec![],
None,
),
ExecError {
error: Error::<Test>::CodeNotFound.into(),
origin: ErrorOrigin::Callee,
}
);

// add missing contract code
place_contract(&CHARLIE, missing_ch);
assert_ok!(MockStack::run_call(
origin,
BOB_ADDR,
&mut GasMeter::<Test>::new(GAS_LIMIT),
&mut storage_meter,
0,
vec![],
None,
));
});
}

#[test]
fn changes_are_reverted_on_failing_call() {
// This test verifies that changes are reverted on a call which fails (or equally, returns
Expand Down Expand Up @@ -4419,7 +4483,12 @@ mod tests {
// An unknown code hash to fail the delegate_call on the first condition.
*ctx.ext.last_frame_output_mut() = output_revert();
assert_eq!(
ctx.ext.delegate_call(invalid_code_hash, Default::default()),
ctx.ext.delegate_call(
Weight::zero(),
U256::zero(),
H160([0xff; 20]),
Default::default()
),
Err(Error::<Test>::CodeNotFound.into())
);
assert_eq!(ctx.ext.last_frame_output(), &Default::default());
Expand Down Expand Up @@ -4538,11 +4607,9 @@ mod tests {

// In a delegate call, we should witness the caller immutable data
assert_eq!(
ctx.ext.delegate_call(charlie_ch, Vec::new()).map(|_| ctx
.ext
.last_frame_output()
.data
.clone()),
ctx.ext
.delegate_call(Weight::zero(), U256::zero(), CHARLIE_ADDR, Vec::new())
.map(|_| ctx.ext.last_frame_output().data.clone()),
Ok(vec![1])
);

Expand Down
Loading
Loading