FIX: 27 shift only needed by Solidity

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

@@ -148,7 +148,8 @@ pub fn to_eth_transaction(
     hash: et::TxHash,
 ) -> anyhow::Result<et::Transaction> {
     // Based on https://github.com/filecoin-project/lotus/blob/6cc506f5cf751215be6badc94a960251c6453202/node/impl/full/eth.go#L2048
-    let sig = to_eth_signature(msg.signature()).context("failed to convert to eth signature")?;
+    let sig =
+        to_eth_signature(msg.signature(), true).context("failed to convert to eth signature")?;
     let msg = msg.message;
 
     let tx = et::Transaction {

fendermint/testing/contract-test/tests/staking/machine.rs

@@ -309,7 +309,7 @@ impl StateMachine for StakingMachine {
 
                 for (addr, secret_key) in signatories {
                     let signature = sign_secp256k1(secret_key, &checkpoint_hash);
-                    let signature = from_fvm::to_eth_signature(&signature).unwrap();
+                    let signature = from_fvm::to_eth_signature(&signature, false).unwrap();
                     signatures.push((*addr, signature.into()));
                 }
 

fendermint/vm/interpreter/src/fvm/state/ipc.rs

@@ -97,8 +97,6 @@ impl<DB: Blockstore> GatewayCaller<DB> {
         height: u64,
     ) -> anyhow::Result<[u8; 32]> {
         let msgs = self.bottom_up_msgs(state, height)?;
-
-        // TODO: Not sure this is the right way to hash a vector.
         Ok(abi_hash(msgs))
     }
 
@@ -163,7 +161,8 @@ impl<DB: Blockstore> GatewayCaller<DB> {
         let hash = abi_hash((checkpoint,));
 
         let signature = sign_secp256k1(secret_key, &hash);
-        let signature = from_fvm::to_eth_signature(&signature).context("invalid signature")?;
+        let signature =
+            from_fvm::to_eth_signature(&signature, false).context("invalid signature")?;
         let signature = et::Bytes::from(signature.to_vec());
 
         let tree =

fendermint/vm/message/src/conv/from_fvm.rs

@@ -69,16 +69,24 @@ fn parse_secp256k1(sig: &[u8]) -> anyhow::Result<(RecoveryId, Signature)> {
     Ok((rec_id, sig))
 }
 
-pub fn to_eth_signature(sig: &FvmSignature) -> anyhow::Result<et::Signature> {
+/// Convert an FVM signature, which is a normal Secp256k1 signature, to an Ethereum one,
+/// where the `v` is optionally shifted by 27 to make it compatible with Solidity.
+///
+/// In theory we could incorporate the chain ID into it as well, but that hasn't come up.
+///
+/// Ethers normalizes Ethereum signatures during conversion to RLP.
+pub fn to_eth_signature(sig: &FvmSignature, normalized: bool) -> 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:?}")),
     };
 
     // By adding 27 to the recovery ID we make this compatible with Ethereum,
     // so that we can verify such signatures in Solidity with e.g. openzeppelin ECDSA.sol
+    let shift = if normalized { 0 } else { 27 };
+
     let sig = et::Signature {
-        v: et::U64::from(v.serialize() + 27).as_u64(),
+        v: et::U64::from(v.serialize() + shift).as_u64(),
         r: et::U256::from_big_endian(sig.r.b32().as_ref()),
         s: et::U256::from_big_endian(sig.s.b32().as_ref()),
     };
@@ -171,6 +179,7 @@ pub mod tests {
         to_eth_tokens(&tokens).unwrap();
     }
 
+    /// Check that converting a signature from FVM to ETH and back preserves it.
     #[quickcheck]
     fn prop_signature(msg: SignedMessage, seed: u64, chain_id: u64) -> Result<(), String> {
         let chain_id = ChainID::from(chain_id);
@@ -183,8 +192,8 @@ pub mod tests {
 
         let sig0 = msg.signature();
 
-        let sig1 =
-            to_eth_signature(sig0).map_err(|e| format!("failed to convert signature: {e}"))?;
+        let sig1 = to_eth_signature(sig0, true)
+            .map_err(|e| format!("failed to convert signature: {e}"))?;
 
         let sig2 = fvm_shared::crypto::signature::Signature::new_secp256k1(sig1.to_vec());
 
@@ -205,6 +214,7 @@ pub mod tests {
         assert_eq!(msg1, msg0)
     }
 
+    /// Check that decoding a signed ETH transaction and converting to FVM can be verified with the signature produced by a Wallet.
     #[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.

fendermint/vm/message/src/signed.rs

@@ -127,8 +127,8 @@ impl SignedMessage {
             Signable::Ethereum((hash, from)) => {
                 // If the sender is ethereum, recover the public key from the signature (which verifies it),
                 // then turn it into an `EthAddress` and verify it matches the `from` of the message.
-                let sig =
-                    from_fvm::to_eth_signature(signature).map_err(SignedMessageError::Ethereum)?;
+                let sig = from_fvm::to_eth_signature(signature, true)
+                    .map_err(SignedMessageError::Ethereum)?;
 
                 let rec = sig
                     .recover(hash)
@@ -158,7 +158,7 @@ impl SignedMessage {
             let tx = from_fvm::to_eth_transaction(self.message(), chain_id)
                 .map_err(SignedMessageError::Ethereum)?;
 
-            let sig = from_fvm::to_eth_signature(self.signature())
+            let sig = from_fvm::to_eth_signature(self.signature(), true)
                 .map_err(SignedMessageError::Ethereum)?;
 
             let rlp = tx.rlp_signed(&sig);