Skip to content

Commit c29e72a

Browse files
[pallet-revive] allow delegate calls to non-contract accounts (#7729)
This PR changes the behavior of delegate calls when the callee is not a contract account: Instead of returning a `CodeNotFound` error, this is allowed and the caller observes a successful call with empty output. The change makes for example the following contract behave the same as on EVM: ```Solidity contract DelegateCall { function delegateToLibrary() external returns (bool) { address testAddress = 0x0000000000000000000000000000000000000000; (bool success, ) = testAddress.delegatecall( abi.encodeWithSignature("test()") ); return success; } } ``` Closes paritytech/revive#235 --------- Signed-off-by: xermicus <[email protected]> Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 parent 10f7f41 commit c29e72a

File tree

3 files changed

+45
-28
lines changed

3 files changed

+45
-28
lines changed

prdoc/pr_7729.prdoc

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
title: '[pallet-revive] allow delegate calls to non-contract accounts'
2+
doc:
3+
- audience: Runtime Dev
4+
description: |-
5+
This PR changes the behavior of delegate calls when the callee is not a contract account: Instead of returning a `CodeNotFound` error, this is allowed and the caller observes a successful call with empty output.
6+
7+
The change makes for example the following contract behave the same as on EVM:
8+
9+
```Solidity
10+
contract DelegateCall {
11+
function delegateToLibrary() external returns (bool) {
12+
address testAddress = 0x0000000000000000000000000000000000000000;
13+
(bool success, ) = testAddress.delegatecall(
14+
abi.encodeWithSignature("test()")
15+
);
16+
return success;
17+
}
18+
}
19+
```
20+
21+
Closes https://github.com/paritytech/revive/issues/235
22+
crates:
23+
- name: pallet-revive
24+
bump: major

substrate/frame/revive/src/exec.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1575,10 +1575,9 @@ where
15751575
// This is for example the case for unknown code hashes or creating the frame fails.
15761576
*self.last_frame_output_mut() = Default::default();
15771577

1578-
let code_hash = ContractInfoOf::<T>::get(&address)
1579-
.ok_or(Error::<T>::CodeNotFound)
1580-
.map(|c| c.code_hash)?;
1581-
let executable = E::from_storage(code_hash, self.gas_meter_mut())?;
1578+
// Delegate-calls to non-contract accounts are considered success.
1579+
let Some(info) = ContractInfoOf::<T>::get(&address) else { return Ok(()) };
1580+
let executable = E::from_storage(info.code_hash, self.gas_meter_mut())?;
15821581
let top_frame = self.top_frame_mut();
15831582
let contract_info = top_frame.contract_info().clone();
15841583
let account_id = top_frame.account_id.clone();

substrate/frame/revive/src/exec/tests.rs

Lines changed: 18 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ use crate::{
3333
AddressMapper, Error,
3434
};
3535
use assert_matches::assert_matches;
36-
use frame_support::{assert_err, assert_noop, assert_ok, parameter_types};
36+
use frame_support::{assert_err, assert_ok, parameter_types};
3737
use frame_system::{AccountInfo, EventRecord, Phase};
3838
use pallet_revive_uapi::ReturnFlags;
3939
use pretty_assertions::assert_eq;
@@ -376,19 +376,16 @@ fn delegate_call_missing_contract() {
376376
let origin = Origin::from_account_id(ALICE);
377377
let mut storage_meter = storage::meter::Meter::new(&origin, 0, 55).unwrap();
378378

379-
// contract code missing
380-
assert_noop!(
381-
MockStack::run_call(
382-
origin.clone(),
383-
BOB_ADDR,
384-
&mut GasMeter::<Test>::new(GAS_LIMIT),
385-
&mut storage_meter,
386-
U256::zero(),
387-
vec![],
388-
false,
389-
),
390-
ExecError { error: Error::<Test>::CodeNotFound.into(), origin: ErrorOrigin::Callee }
391-
);
379+
// contract code missing should still succeed to mimic EVM behavior.
380+
assert_ok!(MockStack::run_call(
381+
origin.clone(),
382+
BOB_ADDR,
383+
&mut GasMeter::<Test>::new(GAS_LIMIT),
384+
&mut storage_meter,
385+
U256::zero(),
386+
vec![],
387+
false,
388+
));
392389

393390
// add missing contract code
394391
place_contract(&CHARLIE, missing_ch);
@@ -2631,17 +2628,14 @@ fn last_frame_output_is_always_reset() {
26312628
);
26322629
assert_eq!(ctx.ext.last_frame_output(), &Default::default());
26332630

2634-
// An unknown code hash to fail the delegate_call on the first condition.
2631+
// An unknown code hash should succeed but clear the output.
26352632
*ctx.ext.last_frame_output_mut() = output_revert();
2636-
assert_eq!(
2637-
ctx.ext.delegate_call(
2638-
Weight::zero(),
2639-
U256::zero(),
2640-
H160([0xff; 20]),
2641-
Default::default()
2642-
),
2643-
Err(Error::<Test>::CodeNotFound.into())
2644-
);
2633+
assert_ok!(ctx.ext.delegate_call(
2634+
Weight::zero(),
2635+
U256::zero(),
2636+
H160([0xff; 20]),
2637+
Default::default()
2638+
));
26452639
assert_eq!(ctx.ext.last_frame_output(), &Default::default());
26462640

26472641
// An unknown code hash to fail instantiation on the first condition.

0 commit comments

Comments
 (0)