diff --git a/prdoc/pr_9744.prdoc b/prdoc/pr_9744.prdoc new file mode 100644 index 0000000000000..38f212af1510c --- /dev/null +++ b/prdoc/pr_9744.prdoc @@ -0,0 +1,9 @@ +title: '[pallet-revive] fix CodeInfo owner' +doc: +- audience: Runtime Dev + description: Fix CodeInfo owner, it should always be set to the origin of the transaction +crates: +- name: pallet-revive-fixtures + bump: patch +- name: pallet-revive + bump: patch diff --git a/substrate/frame/revive/fixtures/contracts/CallerWithConstructor.sol b/substrate/frame/revive/fixtures/contracts/CallerWithConstructor.sol new file mode 100644 index 0000000000000..7804076dabfa6 --- /dev/null +++ b/substrate/frame/revive/fixtures/contracts/CallerWithConstructor.sol @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: MIT +pragma solidity >=0.8.0; + +contract CallerWithConstructor { + CallerWithConstructorCallee callee; + + constructor() { + callee = new CallerWithConstructorCallee(); + } + + function callBar() public view returns (uint) { + return callee.bar(); + } +} + +contract CallerWithConstructorCallee { + function bar() public pure returns (uint) { + return 42; + } +} + diff --git a/substrate/frame/revive/fixtures/src/lib.rs b/substrate/frame/revive/fixtures/src/lib.rs index 022dcb9b43f09..68710449264ec 100644 --- a/substrate/frame/revive/fixtures/src/lib.rs +++ b/substrate/frame/revive/fixtures/src/lib.rs @@ -50,9 +50,11 @@ pub fn compile_module_with_type( fixture_name: &str, fixture_type: FixtureType, ) -> anyhow::Result<(Vec, sp_core::H256)> { + use anyhow::Context; let out_dir: std::path::PathBuf = FIXTURE_DIR.into(); let fixture_path = out_dir.join(format!("{fixture_name}{}", fixture_type.file_extension())); - let binary = std::fs::read(fixture_path)?; + let binary = std::fs::read(&fixture_path) + .with_context(|| format!("Failed to load fixture {fixture_path:?}"))?; let code_hash = sp_io::hashing::keccak_256(&binary); Ok((binary, sp_core::H256(code_hash))) } diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index 84467b23d5b67..a74f8ecc695f1 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -1257,11 +1257,11 @@ where return Ok(output); } - let frame = self.top_frame_mut(); - // The deposit we charge for a contract depends on the size of the immutable data. // Hence we need to delay charging the base deposit after execution. - if entry_point == ExportedFunction::Constructor { + let frame = if entry_point == ExportedFunction::Constructor { + let origin = self.origin.account_id()?.clone(); + let frame = self.top_frame_mut(); let contract_info = frame.contract_info(); // if we are dealing with EVM bytecode // We upload the new runtime code, and update the code @@ -1273,10 +1273,7 @@ where output.data.clone() }; - let mut module = crate::ContractBlob::::from_evm_runtime_code( - data, - caller.account_id()?.clone(), - )?; + let mut module = crate::ContractBlob::::from_evm_runtime_code(data, origin)?; module.store_code(skip_transfer)?; code_deposit = module.code_info().deposit(); contract_info.code_hash = *module.code_hash(); @@ -1288,7 +1285,10 @@ where frame .nested_storage .charge_deposit(frame.account_id.clone(), StorageDeposit::Charge(deposit)); - } + frame + } else { + self.top_frame_mut() + }; // The storage deposit is only charged at the end of every call stack. // To make sure that no sub call uses more than it is allowed to, diff --git a/substrate/frame/revive/src/test_utils/builder.rs b/substrate/frame/revive/src/test_utils/builder.rs index 2769484c68558..a1aeea6cdfa67 100644 --- a/substrate/frame/revive/src/test_utils/builder.rs +++ b/substrate/frame/revive/src/test_utils/builder.rs @@ -141,6 +141,14 @@ builder!( bump_nonce: BumpNonce, ) -> ContractResult>; + pub fn concat_evm_data(mut self, more_data: &[u8]) -> Self { + let Code::Upload(code) = &mut self.code else { + panic!("concat_evm_data should only be used with Code::Upload"); + }; + code.extend_from_slice(more_data); + self + } + /// Set the call's evm_value using a native_value amount. pub fn native_value(mut self, value: BalanceOf) -> Self { self.evm_value = Pallet::::convert_native_to_evm(value); diff --git a/substrate/frame/revive/src/tests/sol/contract.rs b/substrate/frame/revive/src/tests/sol/contract.rs index 701aa21fd86bd..bfcfbcfb93b33 100644 --- a/substrate/frame/revive/src/tests/sol/contract.rs +++ b/substrate/frame/revive/src/tests/sol/contract.rs @@ -24,7 +24,7 @@ use crate::{ }; use alloy_core::{ primitives::{Bytes, FixedBytes, U256}, - sol_types::SolCall, + sol_types::{SolCall, SolInterface}, }; use frame_support::{assert_err, traits::fungible::Mutate}; use pallet_revive_fixtures::{compile_module_with_type, Callee, Caller, FixtureType}; @@ -453,3 +453,23 @@ fn create2_works() { assert_eq!(magic_number, echo_output, "Callee.echo must return 42"); }); } + +#[test] +fn instantiate_from_constructor_works() { + use pallet_revive_fixtures::CallerWithConstructor::*; + + let (caller_code, _) = + compile_module_with_type("CallerWithConstructor", FixtureType::Solc).unwrap(); + + ExtBuilder::default().build().execute_with(|| { + let _ = ::Currency::set_balance(&ALICE, 100_000_000_000); + + let Contract { addr, .. } = + builder::bare_instantiate(Code::Upload(caller_code)).build_and_unwrap_contract(); + + let data = CallerWithConstructorCalls::callBar(callBarCall {}).abi_encode(); + let result = builder::bare_call(addr).data(data).build_and_unwrap_result(); + let result = callBarCall::abi_decode_returns(&result.data).unwrap(); + assert_eq!(result, U256::from(42)); + }); +}