Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions prdoc/pr_9744.prdoc
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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;
}
}

4 changes: 3 additions & 1 deletion substrate/frame/revive/fixtures/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,11 @@ pub fn compile_module_with_type(
fixture_name: &str,
fixture_type: FixtureType,
) -> anyhow::Result<(Vec<u8>, 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)))
}
Expand Down
16 changes: 8 additions & 8 deletions substrate/frame/revive/src/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -1273,10 +1273,7 @@ where
output.data.clone()
};

let mut module = crate::ContractBlob::<T>::from_evm_runtime_code(
data,
caller.account_id()?.clone(),
)?;
let mut module = crate::ContractBlob::<T>::from_evm_runtime_code(data, origin)?;
module.store_code(skip_transfer)?;
code_deposit = module.code_info().deposit();
contract_info.code_hash = *module.code_hash();
Expand All @@ -1288,7 +1285,10 @@ where
frame
.nested_storage
.charge_deposit(frame.account_id.clone(), StorageDeposit::Charge(deposit));
}
frame
} else {
self.top_frame_mut()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pgherveou What is the point of reassigning frame here and adding the else branch? Doesn't it end up to be just self.top_frame_mut() for both branches?

};

// 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,
Expand Down
8 changes: 8 additions & 0 deletions substrate/frame/revive/src/test_utils/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,14 @@ builder!(
bump_nonce: BumpNonce,
) -> ContractResult<InstantiateReturnValue, BalanceOf<T>>;

pub fn concat_evm_data(mut self, more_data: &[u8]) -> Self {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simplified the fixture, so not used here, but useful for later

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<T>) -> Self {
self.evm_value = Pallet::<T>::convert_native_to_evm(value);
Expand Down
22 changes: 21 additions & 1 deletion substrate/frame/revive/src/tests/sol/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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 _ = <Test as Config>::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));
});
}
Loading