FM-114: Ethereum signature check (#127)

* FM-114: Move fvm<->evm conversion to the message crate

* FM-114: fvm <-> eth tx mapping

* FM-114: Check ethereum signature

* FM-114: Check the fields which aren't part of the signature too

fendermint/app/Cargo.toml

@@ -38,7 +38,7 @@ fendermint_eth_api = { path = "../eth/api" }
 fendermint_vm_actor_interface = { path = "../vm/actor_interface" }
 fendermint_vm_core = { path = "../vm/core" }
 fendermint_vm_interpreter = { path = "../vm/interpreter", features = ["bundle"] }
-fendermint_vm_message = { path = "../vm/message", features = ["secp256k1"] }
+fendermint_vm_message = { path = "../vm/message" }
 fendermint_vm_genesis = { path = "../vm/genesis" }
 
 cid = { workspace = true }

fendermint/eth/api/examples/ethers.rs

@@ -179,9 +179,9 @@ impl TestAccount {
 // - eth_getTransactionByHash
 // - eth_getTransactionReceipt
 // - eth_feeHistory
+// - eth_sendRawTransaction
 //
 // DOING:
-// - eth_sendRawTransaction
 //
 // TODO:
 // - eth_newBlockFilter

fendermint/eth/api/src/conv/from_eth.rs

@@ -4,94 +4,11 @@
 //! Helper methods to convert between Ethereum and FVM data formats.
 
 use anyhow::Context;
-use ethers_core::types::{Eip1559TransactionRequest, NameOrAddress, H160, H256, U256};
-use fendermint_vm_actor_interface::{
-    eam::{self, EthAddress},
-    evm,
-};
-use fvm_ipld_encoding::RawBytes;
-use fvm_shared::{
-    address::Address,
-    bigint::{BigInt, Sign},
-    econ::TokenAmount,
-    message::Message,
-};
+use ethers_core::types::H256;
 
-// https://github.com/filecoin-project/lotus/blob/594c52b96537a8c8728389b446482a2d7ea5617c/chain/types/ethtypes/eth_transactions.go#L152
-pub fn to_fvm_message(tx: &Eip1559TransactionRequest) -> anyhow::Result<Message> {
-    // FIP-55 says that we should use `InvokeContract` for transfers instead of `METHOD_SEND`,
-    // because if we are sending to some Ethereum actor by ID using `METHOD_SEND`, they will
-    // get the tokens but the contract might not provide any way of retrieving them.
-    // The `Account` actor has been modified to accept any method call, so it will not fail
-    // even if it receives tokens using `InvokeContract`.
-    let (method_num, to) = match tx.to {
-        None => (eam::Method::CreateExternal as u64, eam::EAM_ACTOR_ADDR),
-        Some(NameOrAddress::Address(to)) => {
-            let to = to_fvm_address(to);
-            (evm::Method::InvokeContract as u64, to)
-        }
-        Some(NameOrAddress::Name(_)) => {
-            anyhow::bail!("Turning name to address would require ENS which is not supported.")
-        }
-    };
-
-    // The `from` of the transaction is inferred from the signature.
-    // As long as the client and the server use the same hashing scheme,
-    // this should be usable as a delegated address.
-    let from = to_fvm_address(tx.from.unwrap_or_default());
-
-    let msg = Message {
-        version: 0,
-        from,
-        to,
-        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()),
-        gas_limit: tx
-            .gas
-            .map(|gas| gas.min(U256::from(u64::MAX)).as_u64())
-            .unwrap_or_default(),
-        gas_fee_cap: to_fvm_tokens(&tx.max_fee_per_gas.unwrap_or_default()),
-        gas_premium: to_fvm_tokens(&tx.max_fee_per_gas.unwrap_or_default()),
-    };
-
-    Ok(msg)
-}
-
-pub fn to_fvm_address(addr: H160) -> Address {
-    Address::from(EthAddress(addr.0))
-}
-
-pub fn to_fvm_tokens(value: &U256) -> TokenAmount {
-    let mut bz = [0u8; 256 / 8];
-    value.to_big_endian(&mut bz);
-    let atto = BigInt::from_bytes_be(Sign::Plus, &bz);
-    TokenAmount::from_atto(atto)
-}
+pub use fendermint_vm_message::conv::from_eth::*;
 
 pub fn to_tm_hash(value: &H256) -> anyhow::Result<tendermint::Hash> {
     tendermint::Hash::try_from(value.as_bytes().to_vec())
         .context("failed to convert to Tendermint Hash")
 }
-
-#[cfg(test)]
-mod tests {
-
-    use fendermint_testing::arb::ArbTokenAmount;
-    use quickcheck_macros::quickcheck;
-
-    use crate::conv::from_fvm::to_eth_tokens;
-
-    use super::to_fvm_tokens;
-
-    #[quickcheck]
-    fn prop_to_token_amount(tokens: ArbTokenAmount) -> bool {
-        let tokens0 = tokens.0;
-        if let Ok(value) = to_eth_tokens(&tokens0) {
-            let tokens1 = to_fvm_tokens(&value);
-            return tokens0 == tokens1;
-        }
-        true
-    }
-}

fendermint/eth/api/src/conv/from_fvm.rs

@@ -3,148 +3,4 @@
 
 //! Helper methods to convert between FVM and Ethereum data formats.
 
-use std::str::FromStr;
-
-use anyhow::anyhow;
-use ethers_core::types::{self as et};
-use fendermint_vm_actor_interface::eam::EthAddress;
-use fendermint_vm_actor_interface::eam::EAM_ACTOR_ID;
-use fvm_shared::bigint::BigInt;
-use fvm_shared::crypto::signature::Signature;
-use fvm_shared::crypto::signature::SignatureType;
-use fvm_shared::crypto::signature::SECP_SIG_LEN;
-use fvm_shared::message::Message;
-use fvm_shared::{address::Payload, econ::TokenAmount};
-use lazy_static::lazy_static;
-use libsecp256k1::RecoveryId;
-
-lazy_static! {
-    static ref MAX_U256: BigInt = BigInt::from_str(&et::U256::MAX.to_string()).unwrap();
-}
-
-pub fn to_eth_tokens(amount: &TokenAmount) -> anyhow::Result<et::U256> {
-    if amount.atto() > &MAX_U256 {
-        Err(anyhow!("TokenAmount > U256.MAX"))
-    } else {
-        let (_sign, bz) = amount.atto().to_bytes_be();
-        Ok(et::U256::from_big_endian(&bz))
-    }
-}
-
-pub fn to_eth_from_address(msg: &Message) -> anyhow::Result<et::H160> {
-    match msg.from.payload() {
-        Payload::Secp256k1(h) => Ok(et::H160::from_slice(h)),
-        Payload::Delegated(d) if d.namespace() == EAM_ACTOR_ID && d.subaddress().len() == 20 => {
-            Ok(et::H160::from_slice(d.subaddress()))
-        }
-        other => Err(anyhow!("unexpected `from` address payload: {other:?}")),
-    }
-}
-
-pub fn to_eth_to_address(msg: &Message) -> Option<et::H160> {
-    match msg.to.payload() {
-        Payload::Secp256k1(h) => Some(et::H160::from_slice(h)),
-        Payload::Delegated(d) if d.namespace() == EAM_ACTOR_ID && d.subaddress().len() == 20 => {
-            Some(et::H160::from_slice(d.subaddress()))
-        }
-        Payload::Actor(h) => Some(et::H160::from_slice(h)),
-        Payload::ID(id) => Some(et::H160::from_slice(&EthAddress::from_id(*id).0)),
-        _ => None, // BLS or an invalid delegated address. Just move on.
-    }
-}
-
-fn parse_secp256k1(
-    sig: &[u8],
-) -> anyhow::Result<(libsecp256k1::RecoveryId, libsecp256k1::Signature)> {
-    if sig.len() != SECP_SIG_LEN {
-        return Err(anyhow!("unexpected Secp256k1 length: {}", sig.len()));
-    }
-
-    // generate types to recover key from
-    let rec_id = RecoveryId::parse(sig[64])?;
-
-    // Signature value without recovery byte
-    let mut s = [0u8; 64];
-    s.clone_from_slice(&sig[..64]);
-
-    // generate Signature
-    let sig = libsecp256k1::Signature::parse_standard(&s)?;
-
-    Ok((rec_id, sig))
-}
-
-pub fn to_eth_signature(sig: &Signature) -> anyhow::Result<et::Signature> {
-    let (v, sig) = match sig.sig_type {
-        SignatureType::Secp256k1 => parse_secp256k1(&sig.bytes)?,
-        other => return Err(anyhow!("unexpected signature type: {other:?}")),
-    };
-
-    let sig = et::Signature {
-        v: et::U64::from(v.serialize()).as_u64(),
-        r: et::U256::from_big_endian(sig.r.b32().as_ref()),
-        s: et::U256::from_big_endian(sig.s.b32().as_ref()),
-    };
-
-    Ok(sig)
-}
-
-#[cfg(test)]
-mod tests {
-
-    use std::str::FromStr;
-
-    use fendermint_testing::arb::ArbTokenAmount;
-    use fendermint_vm_message::signed::SignedMessage;
-    use fvm_shared::{bigint::BigInt, chainid::ChainID, econ::TokenAmount};
-    use libsecp256k1::SecretKey;
-    use quickcheck_macros::quickcheck;
-    use rand::{rngs::StdRng, SeedableRng};
-
-    use super::{to_eth_signature, to_eth_tokens};
-
-    #[quickcheck]
-    fn prop_to_eth_tokens(tokens: ArbTokenAmount) -> bool {
-        let tokens = tokens.0;
-        if let Ok(u256_from_tokens) = to_eth_tokens(&tokens) {
-            let tokens_as_str = tokens.atto().to_str_radix(10);
-            let u256_from_str = ethers_core::types::U256::from_dec_str(&tokens_as_str).unwrap();
-            return u256_from_str == u256_from_tokens;
-        }
-        true
-    }
-
-    #[test]
-    fn test_to_eth_tokens() {
-        let atto = BigInt::from_str(
-            "99191064924191451313862974502415542781658129482631472725645205117646186753315",
-        )
-        .unwrap();
-
-        let tokens = TokenAmount::from_atto(atto);
-
-        to_eth_tokens(&tokens).unwrap();
-    }
-
-    #[quickcheck]
-    fn prop_signature(msg: SignedMessage, seed: u64, chain_id: u64) -> Result<(), String> {
-        let chain_id = ChainID::from(chain_id);
-
-        let mut rng = StdRng::seed_from_u64(seed);
-        let sk = SecretKey::random(&mut rng);
-
-        let msg = SignedMessage::new_secp256k1(msg.into_message(), &sk, &chain_id)
-            .map_err(|e| format!("failed to sign: {e}"))?;
-
-        let sig0 = msg.signature();
-
-        let sig1 =
-            to_eth_signature(sig0).map_err(|e| format!("failed to convert signature: {e}"))?;
-
-        let sig2 = fvm_shared::crypto::signature::Signature::new_secp256k1(sig1.to_vec());
-
-        if *sig0 != sig2 {
-            return Err(format!("signatures don't match: {sig0:?} != {sig2:?}"));
-        }
-        Ok(())
-    }
-}
+pub use fendermint_vm_message::conv::from_fvm::*;

fendermint/eth/api/src/conv/from_tm.rs

@@ -17,7 +17,7 @@ use tendermint::abci::response::DeliverTx;
 use tendermint::abci::EventAttribute;
 use tendermint_rpc::endpoint;
 
-use super::from_fvm::{to_eth_from_address, to_eth_signature, to_eth_to_address, to_eth_tokens};
+use super::from_fvm::{to_eth_address, to_eth_signature, to_eth_tokens};
 
 // Values taken from https://github.com/filecoin-project/lotus/blob/6e7dc9532abdb3171427347710df4c860f1957a2/chain/types/ethtypes/eth_types.go#L199
 
@@ -153,8 +153,8 @@ pub fn to_eth_transaction(
         block_hash: None,
         block_number: None,
         transaction_index: None,
-        from: to_eth_from_address(&msg)?,
-        to: to_eth_to_address(&msg),
+        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)?),
@@ -258,8 +258,8 @@ pub fn to_eth_receipt(
         transaction_index,
         block_hash: Some(block_hash),
         block_number: Some(block_number),
-        from: to_eth_from_address(&msg)?,
-        to: to_eth_to_address(&msg),
+        from: to_eth_address(&msg.from).unwrap_or_default(),
+        to: to_eth_address(&msg.to),
         cumulative_gas_used,
         gas_used: Some(et::U256::from(result.tx_result.gas_used)),
         contract_address,

fendermint/rpc/Cargo.toml

@@ -24,7 +24,7 @@ fvm_ipld_encoding = { workspace = true }
 fvm_shared = { workspace = true }
 
 fendermint_vm_actor_interface = { path = "../vm/actor_interface" }
-fendermint_vm_message = { path = "../vm/message", features = ["secp256k1"] }
+fendermint_vm_message = { path = "../vm/message" }
 
 [dev-dependencies]
 clap = { workspace = true }

fendermint/rpc/src/message.rs

@@ -17,6 +17,9 @@ use libsecp256k1::{PublicKey, SecretKey};
 use crate::B64_ENGINE;
 
 /// Factory methods for signed transaction payload construction.
+///
+/// It assumes the sender is an `f1` type address, it won't work with `f410` addresses.
+/// For those one must use the Ethereum API, with a suitable client library such as [ethers].
 pub struct MessageFactory {
     sk: SecretKey,
     addr: Address,

fendermint/testing/src/arb/mod.rs

@@ -4,6 +4,7 @@ use fvm_shared::{
     address::Address,
     bigint::{BigInt, Integer, Sign, MAX_BIGINT_SIZE},
     econ::TokenAmount,
+    message::Message,
 };
 use quickcheck::{Arbitrary, Gen};
 
@@ -38,3 +39,18 @@ impl Arbitrary for ArbAddress {
         Self(Address::from_bytes(&bz).unwrap())
     }
 }
+
+#[derive(Clone, Debug)]
+pub struct ArbMessage(pub Message);
+
+impl Arbitrary for ArbMessage {
+    fn arbitrary(g: &mut Gen) -> Self {
+        let mut message = Message::arbitrary(g);
+        message.gas_fee_cap = ArbTokenAmount::arbitrary(g).0;
+        message.gas_premium = ArbTokenAmount::arbitrary(g).0;
+        message.value = ArbTokenAmount::arbitrary(g).0;
+        message.to = ArbAddress::arbitrary(g).0;
+        message.from = ArbAddress::arbitrary(g).0;
+        Self(message)
+    }
+}

fendermint/vm/actor_interface/src/eam.rs

@@ -50,11 +50,16 @@ impl EthAddress {
         hash20.copy_from_slice(&hash32.digest()[12..]);
         Ok(Self(hash20))
     }
+
+    /// Indicate whether this hash is really an actor ID.
+    pub fn is_masked_id(&self) -> bool {
+        self.0[0] == 0xff && self.0[1..].starts_with(&[0u8; 11])
+    }
 }
 
 impl From<EthAddress> for Address {
     fn from(value: EthAddress) -> Address {
-        if value.0[0] == 0xff {
+        if value.is_masked_id() {
             let mut bytes = [0u8; 8];
             bytes.copy_from_slice(&value.0[12..]);
             let id = u64::from_be_bytes(bytes);

fendermint/vm/interpreter/src/signed.rs

@@ -53,6 +53,7 @@ where
 
         match msg.verify(chain_id) {
             Err(SignedMessageError::Ipld(e)) => Err(anyhow!(e)),
+            Err(SignedMessageError::Ethereum(e)) => Err(e),
             Err(SignedMessageError::InvalidSignature(s)) => {
                 // TODO: We can penalize the validator for including an invalid signature.
                 Ok((state, Err(InvalidSignature(s))))
@@ -97,6 +98,7 @@ where
 
         match verify_result {
             Err(SignedMessageError::Ipld(e)) => Err(anyhow!(e)),
+            Err(SignedMessageError::Ethereum(e)) => Err(e),
             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.