Skip to content
This repository has been archived by the owner on Jan 11, 2024. It is now read-only.

FIX: to_eth_transaction set wrong input bytes #467

Merged
merged 4 commits into from
Dec 16, 2023
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
16 changes: 16 additions & 0 deletions fendermint/eth/api/examples/ethers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,13 @@ where
// Not using `prepare_call` here because `send_transaction` will fill the missing fields.
let coin_send_value = U256::from(100);
let coin_send: TestContractCall<_, bool> = contract.send_coin(to.eth_addr, coin_send_value);

// Take note of the inputs to ascertain it's the same we get back.
let tx_input = match coin_send.tx {
TypedTransaction::Eip1559(ref tx) => tx.data.clone(),
_ => None,
};

// Using `send_transaction` instead of `coin_send.send()` so it gets the receipt.
// Unfortunately the returned `bool` is not available through the Ethereum API.
let receipt = request(
Expand All @@ -640,6 +647,15 @@ where

tracing::info!(tx_hash = ?receipt.transaction_hash, "coin sent");

request(
"eth_getTransactionByHash for input",
provider.get_transaction(receipt.transaction_hash).await,
|tx| match tx {
Some(tx) => tx.input == tx_input.unwrap_or_default(),
_ => false,
},
)?;

request(
"eth_getLogs",
mw.get_logs(&Filter::new().at_block_hash(receipt.block_hash.unwrap()))
Expand Down
31 changes: 19 additions & 12 deletions fendermint/eth/api/src/conv/from_tm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use std::str::FromStr;
use anyhow::{anyhow, Context};
use ethers_core::types::{self as et};
use fendermint_vm_actor_interface::eam::EthAddress;
use fendermint_vm_message::conv::from_fvm::to_eth_transaction_request;
use fendermint_vm_message::{chain::ChainMessage, signed::SignedMessage};
use fvm_shared::address::Address;
use fvm_shared::bigint::Zero;
Expand Down Expand Up @@ -211,30 +212,36 @@ pub fn to_eth_transaction(
// Based on https://github.com/filecoin-project/lotus/blob/6cc506f5cf751215be6badc94a960251c6453202/node/impl/full/eth.go#L2048
let sig =
to_eth_signature(msg.signature(), true).context("failed to convert to eth signature")?;
let msg = msg.message;

// Recover the original request; this method has better tests.
let tx = to_eth_transaction_request(&msg.message, &chain_id)
.context("failed to convert to tx request")?;

let tx = et::Transaction {
hash,
nonce: et::U256::from(msg.sequence),
nonce: tx.nonce.unwrap_or_default(),
block_hash: None,
block_number: None,
transaction_index: None,
from: to_eth_address(&msg.from).unwrap_or_default(),
to: to_eth_address(&msg.to),
value: to_eth_tokens(&msg.value)?,
gas: et::U256::from(msg.gas_limit),
max_fee_per_gas: Some(to_eth_tokens(&msg.gas_fee_cap)?),
max_priority_fee_per_gas: Some(to_eth_tokens(&msg.gas_premium)?),
from: tx.from.unwrap_or_default(),
to: tx.to.and_then(|to| to.as_address().cloned()),
value: tx.value.unwrap_or_default(),
gas: tx.gas.unwrap_or_default(),
max_fee_per_gas: tx.max_fee_per_gas,
max_priority_fee_per_gas: tx.max_priority_fee_per_gas,
// Strictly speaking a "Type 2" transaction should not need to set this, but we do because Blockscout
// has a database constraint that if a transaction is included in a block this can't be null.
gas_price: Some(to_eth_tokens(&msg.gas_fee_cap)? + to_eth_tokens(&msg.gas_premium)?),
input: et::Bytes::from(msg.params.bytes().to_vec()),
chain_id: Some(et::U256::from(u64::from(chain_id))),
gas_price: Some(
tx.max_fee_per_gas.unwrap_or_default()
+ tx.max_priority_fee_per_gas.unwrap_or_default(),
),
input: tx.data.unwrap_or_default(),
chain_id: tx.chain_id.map(|x| et::U256::from(x.as_u64())),
v: et::U64::from(sig.v),
r: sig.r,
s: sig.s,
transaction_type: Some(2u64.into()),
access_list: None,
access_list: Some(tx.access_list),
other: Default::default(),
};

Expand Down
8 changes: 8 additions & 0 deletions fendermint/testing/smoke-test/Makefile.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ env_files = [
{ path = "../scripts/ci.env", profile = "ci" },
]

[tasks.test-data-env]
script = """
cat << EOF > ${TEST_DATA_DIR}/.env
CMT_P2P_MAX_NUM_OUTBOUND_PEERS=0
CMT_CONSENSUS_TIMEOUT_COMMIT=1s
EOF
"""

[tasks.test]
clear = true
dependencies = ["simplecoin-example", "ethapi-example"]
Expand Down
40 changes: 39 additions & 1 deletion fendermint/vm/message/src/conv/from_eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,18 @@ pub fn to_fvm_tokens(value: &U256) -> TokenAmount {
#[cfg(test)]
mod tests {

use ethers_core::{
types::{transaction::eip2718::TypedTransaction, Bytes, TxHash},
utils::rlp,
};
use fendermint_testing::arb::ArbTokenAmount;
use fvm_shared::{chainid::ChainID, crypto::signature::Signature};
use quickcheck_macros::quickcheck;

use crate::conv::from_fvm::to_eth_tokens;
use crate::{
conv::{from_eth::to_fvm_message, from_fvm::to_eth_tokens},
signed::{DomainHash, SignedMessage},
};

use super::to_fvm_tokens;

Expand All @@ -93,4 +101,34 @@ mod tests {
}
true
}

#[test]
fn test_domain_hash() {
let expected_hash: TxHash =
"0x8fe4fd8e1c7c40dceed249c99a553bc218774f611cfefd8a48ede67b8f6e4725"
.parse()
.unwrap();

let raw_tx: Bytes = "0x02f86e87084472af917f2a8080808502540be400948ed26a19f0e0d6708546495611e9a298d9befb598203e880c080a0a37d03d98e50622ec3744ee368565c5e9469852a1d9111197608135928cd2430a010d1575c68602c96c89e9ec30fade44f5844bf34226044d2931afc60b0a8b2de".parse().unwrap();

let rlp = rlp::Rlp::new(&raw_tx);

let tx_hash = TxHash::from(ethers_core::utils::keccak256(rlp.as_raw()));
assert_eq!(tx_hash, expected_hash);

let (tx0, sig) = TypedTransaction::decode_signed(&rlp).expect("decode signed tx");
let chain_id: ChainID = tx0.chain_id().unwrap().as_u64().into();

let msg = SignedMessage {
message: to_fvm_message(tx0.as_eip1559_ref().unwrap()).expect("to_fvm_message"),
signature: Signature::new_secp256k1(sig.to_vec()),
};

let domain_hash = msg.domain_hash(&chain_id).expect("domain_hash");

match domain_hash {
Some(DomainHash::Eth(h)) => assert_eq!(h, tx_hash.0),
other => panic!("unexpected domain hash: {other:?}"),
}
}
}
21 changes: 11 additions & 10 deletions fendermint/vm/message/src/conv/from_fvm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use std::str::FromStr;

use anyhow::anyhow;
use ethers_core::types as et;
use ethers_core::types::transaction::eip2718::TypedTransaction;
use fendermint_crypto::{RecoveryId, Signature};
use fendermint_vm_actor_interface::eam::EthAddress;
use fendermint_vm_actor_interface::eam::EAM_ACTOR_ID;
Expand Down Expand Up @@ -94,11 +93,11 @@ pub fn to_eth_signature(sig: &FvmSignature, normalized: bool) -> anyhow::Result<
Ok(sig)
}

/// Turn an FVM `Message` back into an Ethereum transaction request, mainly for signature checking.
pub fn to_eth_typed_transaction(
/// Turn an FVM `Message` back into an Ethereum transaction request.
pub fn to_eth_transaction_request(
msg: &Message,
chain_id: &ChainID,
) -> anyhow::Result<TypedTransaction> {
) -> anyhow::Result<et::Eip1559TransactionRequest> {
let chain_id: u64 = (*chain_id).into();

let Message {
Expand Down Expand Up @@ -134,7 +133,7 @@ pub fn to_eth_typed_transaction(
tx.value = Some(to_eth_tokens(value)?);
}

Ok(tx.into())
Ok(tx)
}

#[cfg(test)]
Expand All @@ -158,7 +157,7 @@ pub mod tests {
tests::{EthMessage, KeyPair},
};

use super::{to_eth_signature, to_eth_tokens, to_eth_typed_transaction};
use super::{to_eth_signature, to_eth_tokens, to_eth_transaction_request};

#[quickcheck]
fn prop_to_eth_tokens(tokens: ArbTokenAmount) -> bool {
Expand Down Expand Up @@ -211,9 +210,9 @@ 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_typed_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");
let tx = to_eth_transaction_request(&msg0, &chain_id)
.expect("to_eth_transaction_request failed");
let msg1 = to_fvm_message(&tx).expect("to_fvm_message failed");

assert_eq!(msg1, msg0)
}
Expand All @@ -226,7 +225,9 @@ pub mod tests {

let chain_id = ChainID::from(chain_id);
let msg0 = msg.0;
let tx = to_eth_typed_transaction(&msg0, &chain_id).expect("to_eth_transaction failed");
let tx: TypedTransaction = to_eth_transaction_request(&msg0, &chain_id)
.expect("to_eth_transaction_request failed")
.into();

let wallet: Wallet<SigningKey> = Wallet::from_bytes(key_pair.sk.serialize().as_ref())
.expect("failed to create wallet")
Expand Down
13 changes: 9 additions & 4 deletions fendermint/vm/message/src/signed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use anyhow::anyhow;
use cid::multihash::MultihashDigest;
use cid::Cid;
use ethers_core::types as et;
use ethers_core::types::transaction::eip2718::TypedTransaction;
use fendermint_crypto::SecretKey;
use fendermint_vm_actor_interface::{eam, evm};
use fvm_ipld_encoding::tuple::{Deserialize_tuple, Serialize_tuple};
Expand Down Expand Up @@ -41,6 +42,7 @@ pub enum SignedMessageError {
/// which use a different algorithm than Ethereum.
///
/// We can potentially extend this list to include CID based indexing.
#[derive(Debug, Clone)]
pub enum DomainHash {
Eth([u8; 32]),
}
Expand Down Expand Up @@ -104,8 +106,9 @@ impl SignedMessage {
// work with regular accounts.
match maybe_eth_address(&message.from) {
Some(addr) => {
let tx = from_fvm::to_eth_typed_transaction(message, chain_id)
.map_err(SignedMessageError::Ethereum)?;
let tx: TypedTransaction = from_fvm::to_eth_transaction_request(message, chain_id)
.map_err(SignedMessageError::Ethereum)?
.into();

Ok(Signable::Ethereum((tx.sighash(), addr)))
}
Expand Down Expand Up @@ -155,8 +158,10 @@ impl SignedMessage {
chain_id: &ChainID,
) -> Result<Option<DomainHash>, SignedMessageError> {
if maybe_eth_address(&self.message.from).is_some() {
let tx = from_fvm::to_eth_typed_transaction(self.message(), chain_id)
.map_err(SignedMessageError::Ethereum)?;
let tx: TypedTransaction =
from_fvm::to_eth_transaction_request(self.message(), chain_id)
.map_err(SignedMessageError::Ethereum)?
.into();

let sig = from_fvm::to_eth_signature(self.signature(), true)
.map_err(SignedMessageError::Ethereum)?;
Expand Down
Loading