From 9c66742cd2fca36dbcabad992ec45727cab988aa Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Mon, 26 Jun 2023 14:17:37 +0100 Subject: [PATCH] FM-99: Ethereum API methods (Part 4) (#128) * FM-99: eth_call * FM-99: eth_estimateGas * FM-99: Trying to deploy a contract * FM-99: Initialize the burnt funds actor * FM-99: Initialize a placeholder reward actor * FM-99: Fix calldata encoding * FM-99: Move helper structs * FM-99: Test eth signature * FM-99: Fixes so we can deploy * FM-99: Set gas on the call --- Cargo.lock | 1 + fendermint/eth/api/examples/ethers.rs | 140 +++++++++++++++--- fendermint/eth/api/src/apis/eth.rs | 76 +++++++--- fendermint/eth/api/src/apis/mod.rs | 4 +- fendermint/eth/api/src/conv/from_eth.rs | 17 ++- fendermint/eth/api/src/state.rs | 11 ++ fendermint/rpc/src/response.rs | 8 + fendermint/testing/smoke-test/scripts/init.sh | 2 +- .../vm/actor_interface/src/burntfunds.rs | 6 + fendermint/vm/actor_interface/src/lib.rs | 12 +- fendermint/vm/actor_interface/src/reward.rs | 8 + fendermint/vm/interpreter/src/fvm/genesis.rs | 49 ++++-- fendermint/vm/interpreter/src/fvm/query.rs | 6 + fendermint/vm/interpreter/src/signed.rs | 8 +- fendermint/vm/message/Cargo.toml | 4 +- fendermint/vm/message/src/conv/from_eth.rs | 8 +- fendermint/vm/message/src/conv/from_fvm.rs | 122 ++++++++------- fendermint/vm/message/src/conv/mod.rs | 76 ++++++++++ fendermint/vm/message/src/query.rs | 2 + fendermint/vm/message/src/signed.rs | 46 ++---- 20 files changed, 455 insertions(+), 151 deletions(-) create mode 100644 fendermint/vm/actor_interface/src/burntfunds.rs create mode 100644 fendermint/vm/actor_interface/src/reward.rs diff --git a/Cargo.lock b/Cargo.lock index f7dff8f9..e97aa9f3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2422,6 +2422,7 @@ dependencies = [ "arbitrary", "blake2b_simd", "cid", + "ethers", "ethers-core", "fendermint_testing", "fendermint_vm_actor_interface", diff --git a/fendermint/eth/api/examples/ethers.rs b/fendermint/eth/api/examples/ethers.rs index da7a841f..467d3b05 100644 --- a/fendermint/eth/api/examples/ethers.rs +++ b/fendermint/eth/api/examples/ethers.rs @@ -28,21 +28,22 @@ use std::{ fmt::Debug, path::{Path, PathBuf}, + sync::Arc, time::Duration, }; -use anyhow::{anyhow, Context}; +use anyhow::{anyhow, bail, Context}; use clap::Parser; use ethers::{ - prelude::SignerMiddleware, + prelude::{abigen, ContractCall, ContractFactory, SignerMiddleware}, providers::{Http, Middleware, Provider, ProviderError}, signers::{Signer, Wallet}, }; use ethers_core::{ k256::ecdsa::SigningKey, types::{ - Address, BlockId, BlockNumber, Eip1559TransactionRequest, TransactionReceipt, H160, H256, - U256, U64, + transaction::eip2718::TypedTransaction, Address, BlockId, BlockNumber, Bytes, + Eip1559TransactionRequest, TransactionReceipt, H160, H256, U256, U64, }, }; use fendermint_rpc::message::MessageFactory; @@ -51,6 +52,25 @@ use libsecp256k1::SecretKey; use tracing::Level; type TestMiddleware = SignerMiddleware, Wallet>; +type TestContractCall = ContractCall; + +// This assumes that https://github.com/filecoin-project/builtin-actors is checked out next to this project, +// which the Makefile in the root takes care of with `make actor-bundle`, a dependency of creating docker images. +const SIMPLECOIN_HEX: &'static str = + include_str!("../../../../../builtin-actors/actors/evm/tests/contracts/SimpleCoin.bin"); + +const SIMPLECOIN_ABI: &'static str = + include_str!("../../../../../builtin-actors/actors/evm/tests/contracts/SimpleCoin.abi"); + +/// Gas limit to set for transactions. +const ENOUGH_GAS: u64 = 10_000_000_000u64; + +// Generate a statically typed interface for the contract. +// An example of what it looks like is at https://github.com/filecoin-project/ref-fvm/blob/evm-integration-tests/testing/integration/tests/evm/src/simple_coin/simple_coin.rs +abigen!( + SimpleCoin, + "../../../../builtin-actors/actors/evm/tests/contracts/SimpleCoin.abi" +); #[derive(Parser, Debug)] pub struct Options { @@ -180,6 +200,8 @@ impl TestAccount { // - eth_getTransactionReceipt // - eth_feeHistory // - eth_sendRawTransaction +// - eth_call +// - eth_estimateGas // // DOING: // @@ -199,8 +221,6 @@ impl TestAccount { // - eth_mining // - eth_subscribe // - eth_unsubscribe -// - eth_call -// - eth_estimateGas // - eth_getStorageAt // - eth_getCode // @@ -224,12 +244,16 @@ async fn run(provider: Provider, opts: Options) -> anyhow::Result<()> { bn.as_u64() > 0 })?; + // Go back one block, so we can be sure there are results. + let bn = bn - 1; + let chain_id = request("eth_chainId", provider.get_chainid().await, |id| { !id.is_zero() })?; let mw = make_middleware(provider.clone(), chain_id.as_u64(), &from) .context("failed to create middleware")?; + let mw = Arc::new(mw); request( "eth_getBalance", @@ -300,8 +324,16 @@ async fn run(provider: Provider, opts: Options) -> anyhow::Result<()> { !id.is_zero() })?; - // Send the transaction and wait for receipt - let receipt = example_transfer(mw, to).await.context("transfer failed")?; + tracing::info!("sending example transfer"); + + let transfer = make_transfer(&mw, to) + .await + .context("failed to make a transfer")?; + + let receipt = send_transaction(&mw, transfer.clone()) + .await + .context("failed to send transfer")?; + let tx_hash = receipt.transaction_hash; let bn = receipt.block_number.unwrap(); let bh = receipt.block_hash.unwrap(); @@ -353,29 +385,101 @@ async fn run(provider: Provider, opts: Options) -> anyhow::Result<()> { |tx| tx.is_some(), )?; + // Calling with 0 nonce so the node figures out the latest value. + let mut probe_tx = transfer.clone(); + probe_tx.set_nonce(0); + let probe_height = BlockId::Number(BlockNumber::Number(bn)); + + request( + "eth_call", + provider.call(&probe_tx, Some(probe_height)).await, + |_| true, + )?; + + request( + "eth_estimateGas", + provider.estimate_gas(&probe_tx, Some(probe_height)).await, + |gas: &U256| !gas.is_zero(), + )?; + + tracing::info!("deploying SimpleCoin"); + + let bytecode = + Bytes::from(hex::decode(SIMPLECOIN_HEX).context("failed to decode contract hex")?); + let abi = serde_json::from_str::(SIMPLECOIN_ABI)?; + + let factory = ContractFactory::new(abi, bytecode, mw.clone()); + let mut deployer = factory.deploy(())?; + + // Fill the fields so we can debug any difference between this and the node. + // Using `Some` block ID because with `None` the eth_estimateGas call would receive invalid parameters. + mw.fill_transaction(&mut deployer.tx, Some(BlockId::Number(BlockNumber::Latest))) + .await?; + tracing::info!(sighash = ?deployer.tx.sighash(), "deployment tx"); + + // NOTE: This will call eth_estimateGas to figure out how much gas to use, because we don't set it, + // unlike in the case of the example transfer. What the [Provider::fill_transaction] will _also_ do + // is estimate the fees using eth_feeHistory, here: + // https://github.com/gakonst/ethers-rs/blob/df165b84229cdc1c65e8522e0c1aeead3746d9a8/ethers-providers/src/rpc/provider.rs#LL300C30-L300C51 + // These were set to zero in the earlier example transfer, ie. it was basically paid for by the miner (which is not at the moment charged), + // so the test passed. Here, however, there will be a non-zero cost to pay by the deployer, and therefore those balances + // have to be much higher than the defaults used earlier, e.g. the deployment cost 30 FIL, and we used to give 1 FIL. + let (contract, receipt) = deployer + .send_with_receipt() + .await + .context("failed to send deployment")?; + + tracing::info!(addr = ?contract.address(), "SimpleCoin deployed"); + + let contract = SimpleCoin::new(contract.address(), contract.client()); + + let _tx_hash = receipt.transaction_hash; + let _bn = receipt.block_number.unwrap(); + let _bh = receipt.block_hash.unwrap(); + + let mut coin_call: TestContractCall = contract.get_balance(from.eth_addr); + mw.fill_transaction( + &mut coin_call.tx, + Some(BlockId::Number(BlockNumber::Latest)), + ) + .await?; + + let coin_balance: U256 = coin_call.call().await.context("coin balance call failed")?; + + if coin_balance != U256::from(10000) { + bail!("unexpected coin balance: {coin_balance}"); + } + Ok(()) } -/// Make an example transfer. -async fn example_transfer( - mw: TestMiddleware, - to: TestAccount, -) -> anyhow::Result { +async fn make_transfer(mw: &TestMiddleware, to: TestAccount) -> anyhow::Result { // Create a transaction to transfer 1000 atto. let tx = Eip1559TransactionRequest::new().to(to.eth_addr).value(1000); - // Set the gas based on the testkit so it doesn't trigger estimation (which isn't implemented yet). - let tx = tx - .gas(10_000_000_000u64) + // Set the gas based on the testkit so it doesn't trigger estimation. + let mut tx = tx + .gas(ENOUGH_GAS) .max_fee_per_gas(0) - .max_priority_fee_per_gas(0); + .max_priority_fee_per_gas(0) + .into(); + // Fill in the missing fields like `from` and `nonce` (which involves querying the API). + mw.fill_transaction(&mut tx, None).await?; + + Ok(tx) +} + +async fn send_transaction( + mw: &TestMiddleware, + tx: TypedTransaction, +) -> anyhow::Result { // `send_transaction` will fill in the missing fields like `from` and `nonce` (which involves querying the API). let receipt = mw .send_transaction(tx, None) .await .context("failed to send transaction")? - .log_msg("Pending transfer") + .log_msg("Pending transaction") .retries(5) .await? .context("Missing receipt")?; diff --git a/fendermint/eth/api/src/apis/eth.rs b/fendermint/eth/api/src/apis/eth.rs index a9b2b35e..a6071567 100644 --- a/fendermint/eth/api/src/apis/eth.rs +++ b/fendermint/eth/api/src/apis/eth.rs @@ -7,11 +7,12 @@ // * https://github.com/filecoin-project/lotus/blob/v1.23.1-rc2/node/impl/full/eth.go use anyhow::Context; +use ethers_core::types as et; use ethers_core::types::transaction::eip2718::TypedTransaction; -use ethers_core::types::{self as et, BlockId}; use ethers_core::utils::rlp; use fendermint_rpc::message::MessageFactory; use fendermint_rpc::query::QueryClient; +use fendermint_rpc::response::decode_fevm_invoke; use fendermint_vm_message::chain::ChainMessage; use fendermint_vm_message::signed::SignedMessage; use fvm_shared::crypto::signature::Signature; @@ -190,10 +191,7 @@ pub async fn get_balance( where C: Client + Sync + Send + Send, { - let header = match block_id { - BlockId::Number(n) => data.header_by_height(n).await?, - BlockId::Hash(h) => data.header_by_hash(h).await?, - }; + let header = data.header_by_id(block_id).await?; let height = header.height; let addr = to_fvm_address(addr); let res = data.client.actor_state(&addr, Some(height)).await?; @@ -331,10 +329,7 @@ pub async fn get_transaction_count( where C: Client + Sync + Send + Send, { - let header = match block_id { - BlockId::Number(n) => data.header_by_height(n).await?, - BlockId::Hash(h) => data.header_by_hash(h).await?, - }; + let header = data.header_by_id(block_id).await?; let height = header.height; let addr = to_fvm_address(addr); let res = data.client.actor_state(&addr, Some(height)).await?; @@ -432,18 +427,10 @@ where C: Client + Sync + Send + Send, { let rlp = rlp::Rlp::new(tx.as_ref()); - let (tx, sig) = et::transaction::eip2718::TypedTransaction::decode_signed(&rlp) + let (tx, sig) = TypedTransaction::decode_signed(&rlp) .context("failed to decode RLP as signed TypedTransaction")?; - let msg = match tx { - TypedTransaction::Eip1559(tx) => to_fvm_message(&tx)?, - TypedTransaction::Legacy(_) | TypedTransaction::Eip2930(_) => { - return error( - ExitCode::USR_ILLEGAL_ARGUMENT, - "unexpected transaction type", - ) - } - }; + let msg = to_fvm_message(tx)?; let msg = SignedMessage { message: msg, signature: Signature::new_secp256k1(sig.to_vec()), @@ -460,3 +447,54 @@ where ) } } + +/// Executes a new message call immediately without creating a transaction on the block chain. +pub async fn call( + data: JsonRpcData, + Params((tx, block_id)): Params<(TypedTransaction, et::BlockId)>, +) -> JsonRpcResult +where + C: Client + Sync + Send + Send, +{ + let msg = to_fvm_message(tx)?; + let header = data.header_by_id(block_id).await?; + let response = data.client.call(msg, Some(header.height)).await?; + let deliver_tx = response.value; + + // Based on Lotus, we should return the data from the receipt. + if deliver_tx.code.is_err() { + error(ExitCode::new(deliver_tx.code.value()), deliver_tx.info) + } else { + let return_data = decode_fevm_invoke(&deliver_tx) + .context("error decoding data from deliver_tx in query")?; + + Ok(et::Bytes::from(return_data)) + } +} + +/// Generates and returns an estimate of how much gas is necessary to allow the transaction to complete. +/// The transaction will not be added to the blockchain. +/// Note that the estimate may be significantly more than the amount of gas actually used by the transaction, f +/// or a variety of reasons including EVM mechanics and node performance. +pub async fn estimate_gas( + data: JsonRpcData, + Params((tx, block_id)): Params<(TypedTransaction, et::BlockId)>, +) -> JsonRpcResult +where + C: Client + Sync + Send + Send, +{ + let msg = to_fvm_message(tx)?; + let header = data.header_by_id(block_id).await?; + let response = data.client.estimate_gas(msg, Some(header.height)).await?; + let estimate = response.value; + + // Based on Lotus, we should return the data from the receipt. + if !estimate.exit_code.is_success() { + error( + estimate.exit_code, + format!("failed to estimate gas: {}", estimate.info), + ) + } else { + Ok(estimate.gas_limit.into()) + } +} diff --git a/fendermint/eth/api/src/apis/mod.rs b/fendermint/eth/api/src/apis/mod.rs index 53f643dc..c3cb61ca 100644 --- a/fendermint/eth/api/src/apis/mod.rs +++ b/fendermint/eth/api/src/apis/mod.rs @@ -29,13 +29,13 @@ pub fn register_methods(server: ServerBuilder) -> ServerBuilder anyhow::Result { tendermint::Hash::try_from(value.as_bytes().to_vec()) .context("failed to convert to Tendermint Hash") } + +pub fn to_fvm_message(tx: TypedTransaction) -> JsonRpcResult { + match tx { + TypedTransaction::Eip1559(tx) => { + Ok(fendermint_vm_message::conv::from_eth::to_fvm_message(&tx)?) + } + TypedTransaction::Legacy(_) | TypedTransaction::Eip2930(_) => error( + ExitCode::USR_ILLEGAL_ARGUMENT, + "unexpected transaction type", + ), + } +} diff --git a/fendermint/eth/api/src/state.rs b/fendermint/eth/api/src/state.rs index c2c1be9d..c1733da9 100644 --- a/fendermint/eth/api/src/state.rs +++ b/fendermint/eth/api/src/state.rs @@ -85,6 +85,17 @@ where Ok(header) } + /// Get the Tendermint header at a specificed height or hash. + pub async fn header_by_id( + &self, + block_id: et::BlockId, + ) -> JsonRpcResult { + match block_id { + et::BlockId::Number(n) => self.header_by_height(n).await, + et::BlockId::Hash(h) => self.header_by_hash(h).await, + } + } + /// Get a Tendermint block by hash, if it exists. pub async fn block_by_hash_opt( &self, diff --git a/fendermint/rpc/src/response.rs b/fendermint/rpc/src/response.rs index d5171f4b..0d53a738 100644 --- a/fendermint/rpc/src/response.rs +++ b/fendermint/rpc/src/response.rs @@ -42,6 +42,14 @@ pub fn decode_fevm_create(deliver_tx: &DeliverTx) -> anyhow::Result anyhow::Result> { let data = decode_data(&deliver_tx.data)?; + + // Some calls like transfers between Ethereum accounts don't return any data. + if data.is_empty() { + return Ok(data); + } + + // This is the data return by the FEVM itself, not something wrapping another piece, + // that is, it's as if it was returning `CreateReturn`, it's returning `RawBytes` encoded as IPLD. fvm_ipld_encoding::from_slice::(&data) .map(|bz| bz.0) .map_err(|e| anyhow!("failed to deserialize bytes returned by FEVM: {e}")) diff --git a/fendermint/testing/smoke-test/scripts/init.sh b/fendermint/testing/smoke-test/scripts/init.sh index b6a63798..459964d6 100755 --- a/fendermint/testing/smoke-test/scripts/init.sh +++ b/fendermint/testing/smoke-test/scripts/init.sh @@ -40,7 +40,7 @@ for NAME in emily eric; do fendermint \ genesis --genesis-file $GENESIS_FILE \ add-account --public-key $KEYS_DIR/$NAME.pk \ - --balance 1000000000000000000 \ + --balance 1000000000000000000000 \ --kind ethereum done diff --git a/fendermint/vm/actor_interface/src/burntfunds.rs b/fendermint/vm/actor_interface/src/burntfunds.rs new file mode 100644 index 00000000..797cb327 --- /dev/null +++ b/fendermint/vm/actor_interface/src/burntfunds.rs @@ -0,0 +1,6 @@ +// Copyright 2022-2023 Protocol Labs +// SPDX-License-Identifier: Apache-2.0, MIT + +// The burnt funds actor is just an Account actor. + +define_id!(BURNT_FUNDS { id: 99 }); diff --git a/fendermint/vm/actor_interface/src/lib.rs b/fendermint/vm/actor_interface/src/lib.rs index c305ba88..96ff9271 100644 --- a/fendermint/vm/actor_interface/src/lib.rs +++ b/fendermint/vm/actor_interface/src/lib.rs @@ -26,17 +26,24 @@ macro_rules! define_code { }; } -macro_rules! define_singleton { - ($name:ident { id: $id:literal, code_id: $code_id:literal }) => { +macro_rules! define_id { + ($name:ident { id: $id:literal }) => { paste::paste! { pub const [<$name _ACTOR_ID>]: fvm_shared::ActorID = $id; pub const [<$name _ACTOR_ADDR>]: fvm_shared::address::Address = fvm_shared::address::Address::new_id([<$name _ACTOR_ID>]); } + }; +} + +macro_rules! define_singleton { + ($name:ident { id: $id:literal, code_id: $code_id:literal }) => { + define_id!($name { id: $id }); define_code!($name { code_id: $code_id }); }; } pub mod account; +pub mod burntfunds; pub mod cron; pub mod eam; pub mod ethaccount; @@ -44,4 +51,5 @@ pub mod evm; pub mod init; pub mod multisig; pub mod placeholder; +pub mod reward; pub mod system; diff --git a/fendermint/vm/actor_interface/src/reward.rs b/fendermint/vm/actor_interface/src/reward.rs new file mode 100644 index 00000000..7accc1ae --- /dev/null +++ b/fendermint/vm/actor_interface/src/reward.rs @@ -0,0 +1,8 @@ +// Copyright 2022-2023 Protocol Labs +// SPDX-License-Identifier: Apache-2.0, MIT + +// The reward actor is a singleton, but for now let's just use a +// simple account, instead of the one in the built-in actors library, +// because that has too many Filecoin mainnet specific things. + +define_id!(REWARD { id: 2 }); diff --git a/fendermint/vm/interpreter/src/fvm/genesis.rs b/fendermint/vm/interpreter/src/fvm/genesis.rs index 9274d225..38f031d4 100644 --- a/fendermint/vm/interpreter/src/fvm/genesis.rs +++ b/fendermint/vm/interpreter/src/fvm/genesis.rs @@ -2,7 +2,9 @@ // SPDX-License-Identifier: Apache-2.0, MIT use async_trait::async_trait; -use fendermint_vm_actor_interface::{cron, eam, init, system, EMPTY_ARR}; +use fendermint_vm_actor_interface::{ + account, burntfunds, cron, eam, init, reward, system, EMPTY_ARR, +}; use fendermint_vm_core::{chainid, Timestamp}; use fendermint_vm_genesis::{ActorMeta, Genesis, Validator}; use fvm_ipld_blockstore::Blockstore; @@ -48,10 +50,13 @@ where /// TODO: /// * burnt funds? /// * faucet? + /// * rewards? /// * IPC /// - /// See [Lotus](https://github.com/filecoin-project/lotus/blob/v1.20.4/chain/gen/genesis/genesis.go) for reference - /// and the [ref-fvm tester](https://github.com/filecoin-project/ref-fvm/blob/fvm%40v3.1.0/testing/integration/src/tester.rs#L99-L103). + /// See genesis initialization in: + /// * [Lotus](https://github.com/filecoin-project/lotus/blob/v1.20.4/chain/gen/genesis/genesis.go) + /// * [ref-fvm tester](https://github.com/filecoin-project/ref-fvm/blob/fvm%40v3.1.0/testing/integration/src/tester.rs#L99-L103) + /// * [fvm-workbench](https://github.com/anorth/fvm-workbench/blob/67219b3fd0b5654d54f722ab5acea6ec0abb2edc/builtin/src/genesis.rs) async fn init( &self, mut state: Self::State, @@ -76,13 +81,12 @@ where }; // System actor - let system_state = system::State { - builtin_actors: state.manifest_data_cid, - }; state.create_actor( system::SYSTEM_ACTOR_CODE_ID, system::SYSTEM_ACTOR_ID, - &system_state, + &system::State { + builtin_actors: state.manifest_data_cid, + }, TokenAmount::zero(), None, )?; @@ -100,13 +104,12 @@ where )?; // Cron actor - let cron_state = cron::State { - entries: vec![], // TODO: Maybe with the IPC. - }; state.create_actor( cron::CRON_ACTOR_CODE_ID, cron::CRON_ACTOR_ID, - &cron_state, + &cron::State { + entries: vec![], // TODO: Maybe with the IPC. + }, TokenAmount::zero(), None, )?; @@ -120,6 +123,30 @@ where None, )?; + // Burnt funds actor (it's just an account). + state.create_actor( + account::ACCOUNT_ACTOR_CODE_ID, + burntfunds::BURNT_FUNDS_ACTOR_ID, + &account::State { + address: burntfunds::BURNT_FUNDS_ACTOR_ADDR, + }, + TokenAmount::zero(), + None, + )?; + + // A placeholder for the reward actor, beause I don't think + // using the one in the builtin actors library would be appropriate. + // This effectively burns the miner rewards. Better than panicking. + state.create_actor( + account::ACCOUNT_ACTOR_CODE_ID, + reward::REWARD_ACTOR_ID, + &account::State { + address: reward::REWARD_ACTOR_ADDR, + }, + TokenAmount::zero(), + None, + )?; + // Create accounts let mut next_id = init::FIRST_NON_SINGLETON_ADDR + addr_to_id.len() as u64; for a in genesis.accounts { diff --git a/fendermint/vm/interpreter/src/fvm/query.rs b/fendermint/vm/interpreter/src/fvm/query.rs index c29fcf5a..00b75d08 100644 --- a/fendermint/vm/interpreter/src/fvm/query.rs +++ b/fendermint/vm/interpreter/src/fvm/query.rs @@ -70,10 +70,16 @@ where // XXX: This value is for Filecoin, and it's not even used by the FVM, but at least it should not have a problem with it. msg.gas_limit = BLOCK_GAS_LIMIT; + // TODO: This actually fails if the caller doesn't have enough gas. + // Should we modify the state tree up front to give it more? let apply_ret = state.call(*msg)?; let est = GasEstimate { exit_code: apply_ret.msg_receipt.exit_code, + info: apply_ret + .failure_info + .map(|x| x.to_string()) + .unwrap_or_default(), gas_limit: apply_ret.msg_receipt.gas_used, }; diff --git a/fendermint/vm/interpreter/src/signed.rs b/fendermint/vm/interpreter/src/signed.rs index 45568312..aa69978d 100644 --- a/fendermint/vm/interpreter/src/signed.rs +++ b/fendermint/vm/interpreter/src/signed.rs @@ -53,7 +53,9 @@ where match msg.verify(chain_id) { Err(SignedMessageError::Ipld(e)) => Err(anyhow!(e)), - Err(SignedMessageError::Ethereum(e)) => Err(e), + Err(SignedMessageError::Ethereum(e)) => { + Ok((state, Err(InvalidSignature(e.to_string())))) + } Err(SignedMessageError::InvalidSignature(s)) => { // TODO: We can penalize the validator for including an invalid signature. Ok((state, Err(InvalidSignature(s)))) @@ -98,7 +100,9 @@ where match verify_result { Err(SignedMessageError::Ipld(e)) => Err(anyhow!(e)), - Err(SignedMessageError::Ethereum(e)) => Err(e), + Err(SignedMessageError::Ethereum(e)) => { + Ok((state, Err(InvalidSignature(e.to_string())))) + } Err(SignedMessageError::InvalidSignature(s)) => { // There is nobody we can punish for this, we can just tell Tendermint to discard this message, // and potentially block the source IP address. diff --git a/fendermint/vm/message/Cargo.toml b/fendermint/vm/message/Cargo.toml index 874f3d7b..bc5c9489 100644 --- a/fendermint/vm/message/Cargo.toml +++ b/fendermint/vm/message/Cargo.toml @@ -32,9 +32,11 @@ fendermint_vm_actor_interface = { path = "../actor_interface" } fendermint_testing = { path = "../../testing", optional = true } [dev-dependencies] +ethers = { workspace = true } +hex = { workspace = true } quickcheck = { workspace = true } quickcheck_macros = { workspace = true } -hex = { workspace = true } + # Enable arb on self for tests. # Ideally we could do this with `#[cfg(any(test, feature = "arb"))]`, diff --git a/fendermint/vm/message/src/conv/from_eth.rs b/fendermint/vm/message/src/conv/from_eth.rs index f45822aa..bbcf3ca3 100644 --- a/fendermint/vm/message/src/conv/from_eth.rs +++ b/fendermint/vm/message/src/conv/from_eth.rs @@ -8,7 +8,7 @@ use fendermint_vm_actor_interface::{ eam::{self, EthAddress}, evm, }; -use fvm_ipld_encoding::RawBytes; +use fvm_ipld_encoding::{BytesSer, RawBytes}; use fvm_shared::{ address::Address, bigint::{BigInt, Sign}, @@ -39,6 +39,10 @@ pub fn to_fvm_message(tx: &Eip1559TransactionRequest) -> anyhow::Result // this should be usable as a delegated address. let from = to_fvm_address(tx.from.unwrap_or_default()); + // Wrap calldata in IPLD byte format. + let calldata = tx.data.clone().unwrap_or_default().to_vec(); + let params = RawBytes::serialize(BytesSer(&calldata))?; + let msg = Message { version: 0, from, @@ -46,7 +50,7 @@ pub fn to_fvm_message(tx: &Eip1559TransactionRequest) -> anyhow::Result sequence: tx.nonce.unwrap_or_default().as_u64(), value: to_fvm_tokens(&tx.value.unwrap_or_default()), method_num, - params: RawBytes::new(tx.data.clone().unwrap_or_default().to_vec()), + params, gas_limit: tx .gas .map(|gas| gas.min(U256::from(u64::MAX)).as_u64()) diff --git a/fendermint/vm/message/src/conv/from_fvm.rs b/fendermint/vm/message/src/conv/from_fvm.rs index 557f43ba..fe7f7e91 100644 --- a/fendermint/vm/message/src/conv/from_fvm.rs +++ b/fendermint/vm/message/src/conv/from_fvm.rs @@ -10,6 +10,7 @@ use ethers_core::types as et; use ethers_core::types::transaction::eip2718::TypedTransaction; use fendermint_vm_actor_interface::eam::EthAddress; use fendermint_vm_actor_interface::eam::EAM_ACTOR_ID; +use fvm_ipld_encoding::BytesDe; use fvm_shared::address::Address; use fvm_shared::bigint::BigInt; use fvm_shared::chainid::ChainID; @@ -39,6 +40,8 @@ pub fn to_eth_address(addr: &Address) -> Option { Payload::Delegated(d) if d.namespace() == EAM_ACTOR_ID && d.subaddress().len() == 20 => { Some(et::H160::from_slice(d.subaddress())) } + // Deployments should be sent with an empty `to`. + Payload::ID(EAM_ACTOR_ID) => None, // It should be possible to send to an ethereum account by ID. Payload::ID(id) => Some(et::H160::from_slice(&EthAddress::from_id(*id).0)), // XXX: The following fit into the type but are not valid ethereum addresses. @@ -99,88 +102,51 @@ pub fn to_eth_transaction(msg: &Message, chain_id: &ChainID) -> anyhow::Result(params).map(|bz| bz.0)?; + let mut tx = et::Eip1559TransactionRequest::new() .chain_id(chain_id) .from(to_eth_address(from).unwrap_or_default()) .nonce(*sequence) - .value(to_eth_tokens(value)?) .gas(*gas_limit) .max_fee_per_gas(to_eth_tokens(gas_fee_cap)?) .max_priority_fee_per_gas(to_eth_tokens(gas_premium)?) - .data(et::Bytes::from(params.to_vec())); + .data(et::Bytes::from(data)); tx.to = to_eth_address(to).map(et::NameOrAddress::Address); + // NOTE: It's impossible to tell if the original Ethereum transaction sent None or Some(0). + // The ethers deployer sends None, so let's assume that's the useful behavour to match. + // Luckily the RLP encoding at some point seems to resolve them to the same thing. + if !value.is_zero() { + tx.value = Some(to_eth_tokens(value)?); + } + Ok(tx.into()) } #[cfg(test)] pub mod tests { - use std::{array, str::FromStr}; + use std::str::FromStr; - use fendermint_testing::arb::{ArbMessage, ArbTokenAmount}; - use fendermint_vm_actor_interface::{ - eam::{EthAddress, EAM_ACTOR_ID}, - evm, - }; + use ethers::signers::{Signer, Wallet}; + use ethers_core::utils::rlp; + use ethers_core::{k256::ecdsa::SigningKey, types::transaction::eip2718::TypedTransaction}; + use fendermint_testing::arb::ArbTokenAmount; use fendermint_vm_message::signed::SignedMessage; - use fvm_shared::{ - address::Address, - bigint::{BigInt, Integer}, - chainid::ChainID, - econ::TokenAmount, - message::Message, - }; + use fvm_shared::crypto::signature::Signature; + use fvm_shared::{bigint::BigInt, chainid::ChainID, econ::TokenAmount}; use libsecp256k1::SecretKey; use quickcheck_macros::quickcheck; use rand::{rngs::StdRng, SeedableRng}; - use crate::conv::from_eth::to_fvm_message; - - use super::{to_eth_signature, to_eth_tokens, to_eth_transaction, MAX_U256}; - - #[derive(Clone, Debug)] - struct EthDelegatedAddress(Address); - - impl quickcheck::Arbitrary for EthDelegatedAddress { - fn arbitrary(g: &mut quickcheck::Gen) -> Self { - let mut subaddr: [u8; 20] = array::from_fn(|_| u8::arbitrary(g)); - while EthAddress(subaddr).is_masked_id() { - subaddr[0] = u8::arbitrary(g); - } - Self(Address::new_delegated(EAM_ACTOR_ID, &subaddr).unwrap()) - } - } - - #[derive(Clone, Debug)] - struct EthTokenAmount(TokenAmount); - - impl quickcheck::Arbitrary for EthTokenAmount { - fn arbitrary(g: &mut quickcheck::Gen) -> Self { - let t = ArbTokenAmount::arbitrary(g).0; - let (_, t) = t.atto().div_mod_floor(&MAX_U256); - Self(TokenAmount::from_atto(t)) - } - } + use crate::conv::{ + from_eth::to_fvm_message, + tests::{EthMessage, KeyPair}, + }; - /// Message that only contains data which can survive a roundtrip. - #[derive(Clone, Debug)] - pub struct EthMessage(pub Message); - - impl quickcheck::Arbitrary for EthMessage { - fn arbitrary(g: &mut quickcheck::Gen) -> Self { - let mut m = ArbMessage::arbitrary(g).0; - m.version = 0; - m.method_num = evm::Method::InvokeContract as u64; - m.from = EthDelegatedAddress::arbitrary(g).0; - m.to = EthDelegatedAddress::arbitrary(g).0; - m.value = EthTokenAmount::arbitrary(g).0; - m.gas_fee_cap = EthTokenAmount::arbitrary(g).0; - m.gas_premium = EthTokenAmount::arbitrary(g).0; - Self(m) - } - } + use super::{to_eth_signature, to_eth_tokens, to_eth_transaction}; #[quickcheck] fn prop_to_eth_tokens(tokens: ArbTokenAmount) -> bool { @@ -232,10 +198,42 @@ pub mod tests { fn prop_to_and_from_eth_transaction(msg: EthMessage, chain_id: u64) { let chain_id = ChainID::from(chain_id); let msg0 = msg.0; - let tx = to_eth_transaction(&msg0, &chain_id).unwrap(); - let tx = tx.as_eip1559_ref().unwrap(); - let msg1 = to_fvm_message(tx).unwrap(); + let tx = to_eth_transaction(&msg0, &chain_id).expect("to_eth_transaction failed"); + let tx = tx.as_eip1559_ref().expect("not an eip1559 transaction"); + let msg1 = to_fvm_message(tx).expect("to_fvm_message failed"); assert_eq!(msg1, msg0) } + + #[quickcheck] + fn prop_eth_signature(msg: EthMessage, chain_id: u64, key_pair: KeyPair) { + // ethers has `to_eip155_v` which would fail with u64 overflow if the chain ID is too big. + let chain_id = chain_id / 3; + + let chain_id = ChainID::from(chain_id); + let msg0 = msg.0; + let tx = to_eth_transaction(&msg0, &chain_id).expect("to_eth_transaction failed"); + + let wallet: Wallet = Wallet::from_bytes(&key_pair.sk.serialize()) + .expect("failed to create wallet") + .with_chain_id(chain_id); + + let sig = wallet.sign_transaction_sync(&tx).expect("failed to sign"); + + let bz = tx.rlp_signed(&sig); + let rlp = rlp::Rlp::new(bz.as_ref()); + + let (tx1, sig) = TypedTransaction::decode_signed(&rlp) + .expect("failed to decode RLP as signed TypedTransaction"); + + let tx1 = tx1.as_eip1559_ref().expect("not an eip1559 transaction"); + let msg1 = to_fvm_message(tx1).expect("to_fvm_message failed"); + + let signed = SignedMessage { + message: msg1, + signature: Signature::new_secp256k1(sig.to_vec()), + }; + + signed.verify(&chain_id).expect("signature should be valid") + } } diff --git a/fendermint/vm/message/src/conv/mod.rs b/fendermint/vm/message/src/conv/mod.rs index 43d11b6d..90f12967 100644 --- a/fendermint/vm/message/src/conv/mod.rs +++ b/fendermint/vm/message/src/conv/mod.rs @@ -3,3 +3,79 @@ pub mod from_eth; pub mod from_fvm; + +#[cfg(test)] +pub mod tests { + use fendermint_testing::arb::{ArbMessage, ArbTokenAmount}; + use fendermint_vm_actor_interface::{ + eam::{self, EthAddress}, + evm, + }; + use fvm_ipld_encoding::{BytesSer, RawBytes}; + use fvm_shared::{address::Address, bigint::Integer, econ::TokenAmount, message::Message}; + use rand::{rngs::StdRng, SeedableRng}; + + use super::from_fvm::MAX_U256; + + #[derive(Clone, Debug)] + struct EthDelegatedAddress(Address); + + impl quickcheck::Arbitrary for EthDelegatedAddress { + fn arbitrary(g: &mut quickcheck::Gen) -> Self { + let mut subaddr: [u8; 20] = std::array::from_fn(|_| u8::arbitrary(g)); + while EthAddress(subaddr).is_masked_id() { + subaddr[0] = u8::arbitrary(g); + } + Self(Address::new_delegated(eam::EAM_ACTOR_ID, &subaddr).unwrap()) + } + } + + #[derive(Clone, Debug)] + struct EthTokenAmount(TokenAmount); + + impl quickcheck::Arbitrary for EthTokenAmount { + fn arbitrary(g: &mut quickcheck::Gen) -> Self { + let t = ArbTokenAmount::arbitrary(g).0; + let (_, t) = t.atto().div_mod_floor(&MAX_U256); + Self(TokenAmount::from_atto(t)) + } + } + + /// Message that only contains data which can survive a roundtrip. + #[derive(Clone, Debug)] + pub struct EthMessage(pub Message); + + impl quickcheck::Arbitrary for EthMessage { + fn arbitrary(g: &mut quickcheck::Gen) -> Self { + let mut m = ArbMessage::arbitrary(g).0; + m.version = 0; + m.method_num = evm::Method::InvokeContract as u64; + m.from = EthDelegatedAddress::arbitrary(g).0; + m.to = EthDelegatedAddress::arbitrary(g).0; + m.value = EthTokenAmount::arbitrary(g).0; + m.gas_fee_cap = EthTokenAmount::arbitrary(g).0; + m.gas_premium = EthTokenAmount::arbitrary(g).0; + // The random bytes will fail to deserialize. + // With the EVM we expect them to be IPLD serialized bytes. + m.params = + RawBytes::serialize(BytesSer(m.params.bytes())).expect("failedto serialize params"); + Self(m) + } + } + + #[derive(Debug, Clone)] + pub struct KeyPair { + pub sk: libsecp256k1::SecretKey, + pub pk: libsecp256k1::PublicKey, + } + + impl quickcheck::Arbitrary for KeyPair { + fn arbitrary(g: &mut quickcheck::Gen) -> Self { + let seed = u64::arbitrary(g); + let mut rng = StdRng::seed_from_u64(seed); + let sk = libsecp256k1::SecretKey::random(&mut rng); + let pk = libsecp256k1::PublicKey::from_secret_key(&sk); + Self { sk, pk } + } + } +} diff --git a/fendermint/vm/message/src/query.rs b/fendermint/vm/message/src/query.rs index 87899098..2e0dd9f7 100644 --- a/fendermint/vm/message/src/query.rs +++ b/fendermint/vm/message/src/query.rs @@ -71,6 +71,8 @@ pub struct ActorState { pub struct GasEstimate { /// Exit code, potentially signalling out-of-gas errors, or that the actor was not found. pub exit_code: ExitCode, + /// Any information about failed estimations. + pub info: String, /// Gas used during the probing. /// /// Potentially contains an over-estimate, but it should be within the account balance limit. diff --git a/fendermint/vm/message/src/signed.rs b/fendermint/vm/message/src/signed.rs index 614b2ecc..6c414568 100644 --- a/fendermint/vm/message/src/signed.rs +++ b/fendermint/vm/message/src/signed.rs @@ -98,6 +98,7 @@ impl SignedMessage { Some(addr) => { let tx = from_fvm::to_eth_transaction(message, chain_id) .map_err(SignedMessageError::Ethereum)?; + Ok(Signable::Ethereum((tx.sighash(), addr))) } None => { @@ -128,7 +129,7 @@ impl SignedMessage { if rec == from { verify_eth_method(message) } else { - Err(SignedMessageError::InvalidSignature("the Ethereum delegated address did not match the one recovered from the signature".into())) + Err(SignedMessageError::InvalidSignature(format!("the Ethereum delegated address did not match the one recovered from the signature (sighash = {:?})", hash))) } } Signable::Regular(data) => { @@ -223,17 +224,21 @@ fn maybe_eth_address(addr: &Address) -> Option { /// The method ID is not part of the signature, so someone could modify it, which is /// why we have to check explicitly that there is nothing untowards going on. fn verify_eth_method(msg: &Message) -> Result<(), SignedMessageError> { - if msg.to == eam::EAM_ACTOR_ADDR && msg.method_num != eam::Method::CreateExternal as u64 { - Err(SignedMessageError::Ethereum(anyhow!( - "The EAM actor can only be called with CreateExternal" - ))) + if msg.to == eam::EAM_ACTOR_ADDR { + if msg.method_num != eam::Method::CreateExternal as u64 { + return Err(SignedMessageError::Ethereum(anyhow!( + "The EAM actor can only be called with CreateExternal; got {}", + msg.method_num + ))); + } } else if msg.method_num != evm::Method::InvokeContract as u64 { - Err(SignedMessageError::Ethereum(anyhow!( - "An EVM actor can only be called with InvokeContract" - ))) - } else { - Ok(()) + return Err(SignedMessageError::Ethereum(anyhow!( + "An EVM actor can only be called with InvokeContract; got {} - {}", + msg.to, + msg.method_num + ))); } + Ok(()) } /// Signed message with an invalid random signature. @@ -260,28 +265,11 @@ mod tests { use fendermint_vm_actor_interface::eam::EthAddress; use fvm_shared::{address::Address, chainid::ChainID}; use quickcheck_macros::quickcheck; - use rand::{rngs::StdRng, SeedableRng}; - use crate::conv::from_fvm::tests::EthMessage; + use crate::conv::tests::{EthMessage, KeyPair}; use super::SignedMessage; - #[derive(Debug, Clone)] - struct KeyPair { - sk: libsecp256k1::SecretKey, - pk: libsecp256k1::PublicKey, - } - - impl quickcheck::Arbitrary for KeyPair { - fn arbitrary(g: &mut quickcheck::Gen) -> Self { - let seed = u64::arbitrary(g); - let mut rng = StdRng::seed_from_u64(seed); - let sk = libsecp256k1::SecretKey::random(&mut rng); - let pk = libsecp256k1::PublicKey::from_secret_key(&sk); - Self { sk, pk } - } - } - #[quickcheck] fn chain_id_in_signature( msg: SignedMessage, @@ -320,8 +308,6 @@ mod tests { let mut msg = msg.0; msg.from = Address::from(ea); - eprintln!("from = {:?}", msg.from); - let signed = SignedMessage::new_secp256k1(msg, &sk, &chain_id).map_err(|e| e.to_string())?;