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

EVM runtime: implement GetBytecode and GetStorageAt getters. #631

Merged
merged 2 commits into from
Sep 10, 2022
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
8 changes: 4 additions & 4 deletions actors/evm/src/interpreter/instructions/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,11 +205,11 @@ pub fn call<'r, BS: Blockstore, RT: Runtime<BS>>(
// TODO this limits addressable output to 2G (31 bits full),
// but it is still probably too much and we should consistently limit further.
// See also https://github.com/filecoin-project/ref-fvm/issues/851
let output_usize = if output_size.bits() < 32 {
output_size.as_usize()
} else {
if output_size.bits() >= 32 {
return Err(StatusCode::InvalidMemoryAccess);
};
}
let output_usize = output_size.as_usize();

if output_usize > 0 {
let output_region = get_memory_region(memory, output_offset, output_size)
.map_err(|_| StatusCode::InvalidMemoryAccess)?;
Expand Down
58 changes: 58 additions & 0 deletions actors/evm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use {
crate::interpreter::{execute, Bytecode, ExecutionState, StatusCode, System, U256},
crate::state::State,
bytes::Bytes,
cid::Cid,
fil_actors_runtime::{
actor_error, cbor,
runtime::{ActorCode, Runtime},
Expand Down Expand Up @@ -32,6 +33,8 @@ const MAX_CODE_SIZE: usize = 24 << 10;
pub enum Method {
Constructor = METHOD_CONSTRUCTOR,
InvokeContract = 2,
GetBytecode = 3,
GetStorageAt = 4,
}

pub struct EvmContractActor;
Expand Down Expand Up @@ -173,6 +176,48 @@ impl EvmContractActor {
let output = RawBytes::from(exec_status.output_data.to_vec());
Ok(output)
}

pub fn bytecode<BS, RT>(rt: &mut RT) -> Result<Cid, ActorError>
where
BS: Blockstore + Clone,
RT: Runtime<BS>,
{
// Any caller can fetch the bytecode of a contract; this is now EXT* opcodes work.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this is now -> this is how

rt.validate_immediate_caller_accept_any()?;

let state: State = rt.state()?;
Ok(state.bytecode)
}

pub fn storage_at<BS, RT>(rt: &mut RT, params: GetStorageAtParams) -> Result<U256, ActorError>
where
BS: Blockstore + Clone,
RT: Runtime<BS>,
{
// This method cannot be called on-chain; other on-chain logic should not be able to
// access arbitrary storage keys from a contract.
rt.validate_immediate_caller_is([&fvm_shared::address::Address::new_id(0)])?;
Copy link
Contributor

Choose a reason for hiding this comment

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

This very funky, we have an effectively uncallable method.

Thats one extra reason to expect the client to directly read the state, or it should be a non method entry point.

Having said that, it is totally fine for now, but we should think about this general pattern.


let state: State = rt.state()?;
let blockstore = rt.store().clone();

// load the storage HAMT
let mut hamt =
Hamt::<_, _, U256>::load(&state.contract_state, blockstore).map_err(|e| {
ActorError::illegal_state(format!(
"failed to load storage HAMT on invoke: {e:?}, e"
))
})?;

let mut system = System::new(rt, &mut hamt).map_err(|e| {
ActorError::unspecified(format!("failed to create execution abstraction layer: {e:?}"))
})?;
Comment on lines +204 to +214
Copy link
Member

Choose a reason for hiding this comment

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

Looks like we need to do this a lot. Helper?

Copy link
Contributor

Choose a reason for hiding this comment

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

we should probably make one, but that's out of scope here.

I would like to do a general cleanup/beatify pass at some point.

Copy link
Contributor

Choose a reason for hiding this comment

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

btw, feel free to go wild and apply bigclaw magic in the code when there is a lull.


system
.get_storage(params.storage_key)
.map_err(|st| ActorError::unspecified(format!("failed to get storage key: {}", &st)))?
.ok_or_else(|| ActorError::not_found(String::from("storage key not found")))
}
}

impl ActorCode for EvmContractActor {
Expand All @@ -193,6 +238,14 @@ impl ActorCode for EvmContractActor {
Some(Method::InvokeContract) => {
Self::invoke_contract(rt, cbor::deserialize_params(params)?)
}
Some(Method::GetBytecode) => {
let cid = Self::bytecode(rt)?;
Ok(RawBytes::serialize(cid)?)
}
Some(Method::GetStorageAt) => {
let value = Self::storage_at(rt, cbor::deserialize_params(params)?)?;
Ok(RawBytes::serialize(value)?)
}
None => Err(actor_error!(unhandled_message; "Invalid method")),
}
}
Expand All @@ -208,3 +261,8 @@ pub struct ConstructorParams {
pub struct InvokeParams {
pub input_data: RawBytes,
}

#[derive(Serialize_tuple, Deserialize_tuple)]
pub struct GetStorageAtParams {
pub storage_key: U256,
}
124 changes: 124 additions & 0 deletions actors/evm/tests/basic.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
mod asm;

use cid::Cid;
use evm::interpreter::U256;
use fil_actor_evm as evm;
use fil_actors_runtime::test_utils::*;
use fil_actors_runtime::ActorError;
use fvm_ipld_blockstore::Blockstore;
use fvm_ipld_encoding::RawBytes;
use fvm_shared::address::Address;

Expand Down Expand Up @@ -72,3 +77,122 @@ fn basic_contract_construction_and_invocation() {
.unwrap();
assert_eq!(U256::from_big_endian(&result), U256::from(10000));
}

#[test]
fn basic_get_bytecode() {
let (init_code, verbatim_body) = {
let init = "";
let body = r#"
# get call payload size
push1 0x20
calldatasize
sub
# store payload to mem 0x00
push1 0x20
push1 0x00
calldatacopy
return
"#;

let body_bytecode = {
let mut ret = Vec::new();
let mut ingest = etk_asm::ingest::Ingest::new(&mut ret);
ingest.ingest("body", body).unwrap();
ret
};

(asm::new_contract("get_bytecode", init, body).unwrap(), body_bytecode)
};

let mut rt = MockRuntime::default();

rt.expect_validate_caller_any();
let params =
evm::ConstructorParams { bytecode: init_code.into(), input_data: RawBytes::default() };
let result = rt
.call::<evm::EvmContractActor>(
evm::Method::Constructor as u64,
&RawBytes::serialize(params).unwrap(),
)
.unwrap();
expect_empty(result);
rt.verify();
rt.reset();

rt.expect_validate_caller_any();
let returned_bytecode_cid: Cid = rt
.call::<evm::EvmContractActor>(evm::Method::GetBytecode as u64, &Default::default())
.unwrap()
.deserialize()
.unwrap();
rt.verify();

let bytecode = rt.store.get(&returned_bytecode_cid).unwrap().unwrap();

assert_eq!(bytecode.as_slice(), verbatim_body.as_slice());
}

#[test]
fn basic_get_storage_at() {
let init_code = {
// Initialize storage entry on key 0x8965 during init.
let init = r"
push2 0xfffa
push2 0x8965
sstore";
let body = r#"return"#;

asm::new_contract("get_storage_at", init, body).unwrap()
};

let mut rt = MockRuntime::default();

rt.expect_validate_caller_any();
let params =
evm::ConstructorParams { bytecode: init_code.into(), input_data: RawBytes::default() };
let result = rt
.call::<evm::EvmContractActor>(
evm::Method::Constructor as u64,
&RawBytes::serialize(params).unwrap(),
)
.unwrap();
expect_empty(result);
rt.verify();
rt.reset();

let params = evm::GetStorageAtParams { storage_key: 0x8965.into() };

let sender = Address::new_id(0); // zero address because this method is not invokable on-chain
rt.expect_validate_caller_addr(vec![sender]);
rt.caller = sender;

//
// Get the storage key that was initialized in the init code.
//
let value: U256 = rt
.call::<evm::EvmContractActor>(
evm::Method::GetStorageAt as u64,
&RawBytes::serialize(params).unwrap(),
)
.unwrap()
.deserialize()
.unwrap();
rt.verify();
rt.reset();

assert_eq!(U256::from(0xfffa), value);

//
// Get a storage key that doesn't exist.
//
let params = evm::GetStorageAtParams { storage_key: 0xaaaa.into() };

rt.expect_validate_caller_addr(vec![sender]);
let ret = rt.call::<evm::EvmContractActor>(
evm::Method::GetStorageAt as u64,
&RawBytes::serialize(params).unwrap(),
);
rt.verify();

assert_eq!(ActorError::not_found("storage key not found".to_string()), ret.err().unwrap());
}