fix: rlp decode of SignedTransaction with interoperation signature

axonweb3 · Nov 14, 2023 · 04a7ae3 · 04a7ae3
1 parent a22b709
commit 04a7ae3
Show file tree

Hide file tree

Showing 10 changed files with 51 additions and 130 deletions.
diff --git a/core/api/src/jsonrpc/impl/web3.rs b/core/api/src/jsonrpc/impl/web3.rs
@@ -1,26 +1,19 @@
 use std::{sync::Arc, time::Duration};
 
-use ckb_types::core::cell::{CellProvider, CellStatus};
-use ckb_types::prelude::Entity;
 use jsonrpsee::core::RpcResult;
 
 use common_apm::metrics_rpc;
-use protocol::traits::{APIAdapter, Context, Interoperation};
+use protocol::traits::{APIAdapter, Context};
 use protocol::types::{
-    Block, BlockNumber, Bytes, CellDepWithPubKey, EthAccountProof, Hash, Hasher, Header, Hex,
-    Proposal, Receipt, SignatureComponents, SignatureR, SignatureS, SignedTransaction, TxResp,
-    UnverifiedTransaction, BASE_FEE_PER_GAS, H160, H256, MAX_FEE_HISTORY, MAX_RPC_GAS_CAP,
-    MIN_TRANSACTION_GAS_LIMIT, U256,
+    Block, BlockNumber, Bytes, EthAccountProof, Hash, Header, Hex, Proposal, Receipt,
+    SignedTransaction, TxResp, UnverifiedTransaction, BASE_FEE_PER_GAS, H160, H256,
+    MAX_FEE_HISTORY, MAX_RPC_GAS_CAP, MIN_TRANSACTION_GAS_LIMIT, U256,
 };
 use protocol::{
-    async_trait, ckb_blake2b_256, codec::ProtocolCodec, lazy::PROTOCOL_VERSION, tokio::time::sleep,
-    ProtocolResult, MEMPOOL_REFRESH_TIMEOUT,
+    async_trait, codec::ProtocolCodec, lazy::PROTOCOL_VERSION, tokio::time::sleep, ProtocolResult,
+    MEMPOOL_REFRESH_TIMEOUT,
 };
 
-use core_executor::system_contract::DataProvider;
-use core_interoperation::utils::is_dummy_out_point;
-use core_interoperation::InteroperationImpl;
-
 use crate::jsonrpc::web3_types::{
     BlockCount, BlockId, FeeHistoryEmpty, FeeHistoryWithReward, FeeHistoryWithoutReward,
     RichTransactionOrHash, Web3Block, Web3CallRequest, Web3FeeHistory, Web3Filter, Web3Log,
@@ -191,80 +184,6 @@ impl<Adapter: APIAdapter> Web3RpcImpl<Adapter> {
             reward,
         ))
     }
-
-    async fn extract_interoperation_tx_sender(
-        &self,
-        utx: &UnverifiedTransaction,
-        signature: &SignatureComponents,
-    ) -> RpcResult<H160> {
-        // Call CKB-VM mode
-        if signature.r[0] == 0 {
-            let r = rlp::decode::<CellDepWithPubKey>(&signature.r[1..])
-                .map_err(|e| RpcError::DecodeInteroperationSigR(e.to_string()))?;
-
-            return Ok(Hasher::digest(&r.pub_key).into());
-        }
-
-        // Verify by CKB-VM mode
-        let r = SignatureR::decode(&signature.r)
-            .map_err(|e| RpcError::DecodeInteroperationSigR(e.to_string()))?;
-        let s = SignatureS::decode(&signature.s)
-            .map_err(|e| RpcError::DecodeInteroperationSigS(e.to_string()))?;
-        let address_source = r.address_source();
-
-        let ckb_tx_view =
-            InteroperationImpl::dummy_transaction(r.clone(), s, Some(utx.signature_hash(true).0));
-        let dummy_input = r.dummy_input();
-
-        let input = ckb_tx_view
-            .inputs()
-            .get(address_source.index as usize)
-            .ok_or(RpcError::InvalidAddressSource)?;
-
-        log::debug!("[mempool]: verify interoperation tx sender \ntx view \n{:?}\ndummy input\n {:?}\naddress source\n{:?}\n", ckb_tx_view, dummy_input, address_source);
-
-        // Dummy input mode
-        if is_dummy_out_point(&input.previous_output()) {
-            log::debug!("[mempool]: verify interoperation tx dummy input mode.");
-
-            if let Some(cell) = dummy_input {
-                if address_source.type_ == 1 && cell.type_script.is_none() {
-                    return Err(RpcError::InvalidAddressSource.into());
-                }
-
-                let script_hash = if address_source.type_ == 0 {
-                    cell.lock_script_hash()
-                } else {
-                    cell.type_script_hash().unwrap()
-                };
-
-                return Ok(Hasher::digest(script_hash).into());
-            }
-
-            return Err(RpcError::MissingDummyInputCell.into());
-        }
-
-        // Reality input mode
-        let root = self
-            .adapter
-            .get_image_cell_root(Context::new())
-            .await
-            .map_err(|e| RpcError::Internal(e.to_string()))?;
-        match DataProvider::new(root).cell(&input.previous_output(), true) {
-            CellStatus::Live(cell) => {
-                let script_hash = if address_source.type_ == 0 {
-                    ckb_blake2b_256(cell.cell_output.lock().as_slice())
-                } else if let Some(type_script) = cell.cell_output.type_().to_opt() {
-                    ckb_blake2b_256(type_script.as_slice())
-                } else {
-                    return Err(RpcError::InvalidAddressSource.into());
-                };
-
-                Ok(Hasher::digest(script_hash).into())
-            }
-            _ => Err(RpcError::CannotFindImageCell.into()),
-        }
-    }
 }
 
 #[async_trait]
@@ -297,17 +216,7 @@ impl<Adapter: APIAdapter + 'static> Web3RpcServer for Web3RpcImpl<Adapter> {
         utx.check_hash()
             .map_err(|e| RpcError::Internal(e.to_string()))?;
 
-        let interoperation_sender = if let Some(sig) = utx.signature.as_ref() {
-            if sig.is_eth_sig() {
-                None
-            } else {
-                Some(self.extract_interoperation_tx_sender(&utx, sig).await?)
-            }
-        } else {
-            return Err(RpcError::TransactionIsNotSigned.into());
-        };
-
-        let stx = SignedTransaction::from_unverified(utx, interoperation_sender)
+        let stx = SignedTransaction::from_unverified(utx)
             .map_err(|e| RpcError::Internal(e.to_string()))?;
         let hash = stx.transaction.hash;
 

diff --git a/core/api/src/jsonrpc/web3_types.rs b/core/api/src/jsonrpc/web3_types.rs
@@ -858,7 +858,7 @@ mod tests {
         // https://etherscan.io/getRawTx?tx=0x07c7388b03ab8403deeaefc551efbc632f8531f04dc9993a274dbba9bbb98cbf
         let tx = Hex::from_str("0x02f902f801728405f5e1008509898edcf78302ffb8943fc91a3afd70395cd496c647d5a6cc9d4b2b7fad8802c68af0bb140000b902843593564c000000000000000000000000000000000000000000000000000000000000006000000000000000000000000000000000000000000000000000000000000000a0000000000000000000000000000000000000000000000000000000006480c64700000000000000000000000000000000000000000000000000000000000000020b080000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000a00000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000002c68af0bb1400000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000002c68af0bb1400000000000000000000000000000000000000000004a715ce36374beaa635218d9700000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2000000000000000000000000c3681a720605bd6f8fe9a2fabff6a7cdecdc605dc080a0d253ee687ab2d9734a5073d64a0ba26bc3bc1cf4582005137bba05ef88616ea89e8ba79925267b17403fdf3ab47641b4aa52322dc385429cc92a7003c5d7c2").unwrap();
         let tx = UnverifiedTransaction::decode(tx).unwrap();
-        let tx = SignedTransaction::from_unverified(tx, None).unwrap();
+        let tx = SignedTransaction::from_unverified(tx).unwrap();
         let tx_json = serde_json::to_value(Web3Transaction::from(tx)).unwrap();
 
         assert_eq!(

diff --git a/core/consensus/benches/bench_wal.rs b/core/consensus/benches/bench_wal.rs
@@ -53,7 +53,7 @@ fn mock_sign_tx() -> SignedTransaction {
         .to_bytes();
     utx.signature = Some(signature.into());
 
-    SignedTransaction::from_unverified(utx, None).unwrap()
+    SignedTransaction::from_unverified(utx).unwrap()
 }
 
 fn criterion_save_wal(c: &mut Criterion) {

diff --git a/core/consensus/src/wal.rs b/core/consensus/src/wal.rs
@@ -364,7 +364,7 @@ mod tests {
                 .to_bytes();
         utx.signature = Some(signature.into());
 
-        SignedTransaction::from_unverified(utx, None).unwrap()
+        SignedTransaction::from_unverified(utx).unwrap()
     }
 
     pub fn mock_wal_txs(size: usize) -> Vec<SignedTransaction> {

diff --git a/core/storage/benches/bench.rs b/core/storage/benches/bench.rs
@@ -143,7 +143,7 @@ fn mock_signed_tx() -> SignedTransaction {
     .to_bytes();
     utx.signature = Some(signature.into());
 
-    SignedTransaction::from_unverified(utx, None).unwrap()
+    SignedTransaction::from_unverified(utx).unwrap()
 }
 
 fn get_random_bytes(len: usize) -> Bytes {

diff --git a/core/storage/src/tests/mod.rs b/core/storage/src/tests/mod.rs
@@ -42,7 +42,7 @@ fn mock_signed_tx() -> SignedTransaction {
     .to_bytes();
     utx.signature = Some(signature.into());
 
-    SignedTransaction::from_unverified(utx, None).unwrap()
+    SignedTransaction::from_unverified(utx).unwrap()
 }
 
 fn mock_receipt(hash: Hash) -> Receipt {

diff --git a/protocol/src/codec/transaction.rs b/protocol/src/codec/transaction.rs
@@ -340,19 +340,24 @@ impl Decodable for SignedTransaction {
             .as_ref()
             .ok_or(DecoderError::Custom("missing signature"))?;
 
-        let public = if sig.is_eth_sig() {
-            Public::from_slice(
+        let (public, sender_addr) = if sig.is_eth_sig() {
+            let public = Public::from_slice(
                 &secp256k1_recover(utx.signature_hash(true).as_bytes(), sig.as_bytes().as_ref())
                     .map_err(|_| DecoderError::Custom("recover signature"))?
                     .serialize_uncompressed()[1..65],
-            )
+            );
+            (public, public_to_address(&public))
         } else {
-            Public::zero()
+            (
+                Public::zero(),
+                sig.extract_interoperation_tx_sender()
+                    .map_err(|_| DecoderError::Custom("Invalid interoperation sender"))?,
+            )
         };
 
         Ok(SignedTransaction {
             transaction: utx,
-            sender:      public_to_address(&public),
+            sender:      sender_addr,
             public:      Some(public),
         })
     }
@@ -460,7 +465,7 @@ mod tests {
     fn test_signed_tx_codec() {
         let raw = hex_decode("02f8670582010582012c82012c825208945cf83df52a32165a7f392168ac009b168c9e89150180c001a0a68aeb0db4d84cf16da5a6918becefd254654854cfc23f0112ef78154ce84db89f4b0af1cbf12f5bfaec81c3d4d495717d720b574a05092f6b436c2ab255cd35").unwrap();
         let utx = UnverifiedTransaction::decode(&Rlp::new(&raw)).unwrap();
-        let origin = SignedTransaction::from_unverified(utx, None).unwrap();
+        let origin = SignedTransaction::from_unverified(utx).unwrap();
         let encode = origin.rlp_bytes().freeze().to_vec();
         let decode: SignedTransaction = rlp::decode(&encode).unwrap();
         assert_eq!(origin, decode);
@@ -559,7 +564,7 @@ mod tests {
         let test_vector = |tx_data: &str, address: &'static str| {
             let utx =
                 UnverifiedTransaction::decode(&Rlp::new(&hex_decode(tx_data).unwrap())).unwrap();
-            let signed = SignedTransaction::from_unverified(utx.clone(), None).unwrap();
+            let signed = SignedTransaction::from_unverified(utx.clone()).unwrap();
             assert_eq!(
                 signed.sender,
                 H160::from_slice(&hex_decode(address).unwrap())

diff --git a/protocol/src/types/batch.rs b/protocol/src/types/batch.rs
@@ -82,7 +82,7 @@ mod tests {
         .to_bytes();
         utx.signature = Some(signature.into());
 
-        SignedTransaction::from_unverified(utx, None).unwrap()
+        SignedTransaction::from_unverified(utx).unwrap()
     }
 
     #[test]

diff --git a/protocol/src/types/mod.rs b/protocol/src/types/mod.rs
@@ -84,6 +84,9 @@ pub enum TypesError {
 
     #[display(fmt = "InvalidBlockVersion {}", _0)]
     InvalidBlockVersion(u8),
+
+    #[display(fmt = "Decode interoperation signature R error {:?}", _0)]
+    DecodeInteroperationSigR(rlp::DecoderError),
 }
 
 impl Error for TypesError {}

diff --git a/protocol/src/types/transaction.rs b/protocol/src/types/transaction.rs
@@ -8,8 +8,8 @@ use serde::{Deserialize, Serialize};
 use common_crypto::secp256k1_recover;
 
 use crate::types::{
-    Bloom, Bytes, BytesMut, ExitReason, Hash, Hasher, Public, TxResp, TypesError, H160, H256, H520,
-    U256,
+    Bloom, Bytes, BytesMut, CellDepWithPubKey, ExitReason, Hash, Hasher, Public, TxResp,
+    TypesError, H160, H256, H520, U256,
 };
 use crate::ProtocolResult;
 
@@ -412,6 +412,18 @@ impl SignatureComponents {
         }
     }
 
+    pub(crate) fn extract_interoperation_tx_sender(&self) -> ProtocolResult<H160> {
+        // Only call CKB-VM mode is supported now
+        if self.r[0] == 0 {
+            let r = rlp::decode::<CellDepWithPubKey>(&self.r[1..])
+                .map_err(TypesError::DecodeInteroperationSigR)?;
+
+            return Ok(Hasher::digest(&r.pub_key).into());
+        }
+
+        Err(TypesError::InvalidSignatureRType.into())
+    }
+
     #[allow(clippy::len_without_is_empty)]
     pub fn len(&self) -> usize {
         self.r.len() + self.s.len() + 1
@@ -426,24 +438,19 @@ pub struct SignedTransaction {
 }
 
 impl SignedTransaction {
-    pub fn from_unverified(
-        utx: UnverifiedTransaction,
-        sender: Option<H160>,
-    ) -> ProtocolResult<Self> {
+    pub fn from_unverified(utx: UnverifiedTransaction) -> ProtocolResult<Self> {
         if utx.signature.is_none() {
             return Err(TypesError::Unsigned.into());
         }
 
         let hash = utx.signature_hash(true);
+        let sig = utx.signature.as_ref().unwrap();
 
-        if utx.signature.as_ref().unwrap().is_eth_sig() {
+        if sig.is_eth_sig() {
             let public = Public::from_slice(
-                &secp256k1_recover(
-                    hash.as_bytes(),
-                    utx.signature.as_ref().unwrap().as_bytes().as_ref(),
-                )
-                .map_err(TypesError::Crypto)?
-                .serialize_uncompressed()[1..65],
+                &secp256k1_recover(hash.as_bytes(), sig.as_bytes().as_ref())
+                    .map_err(TypesError::Crypto)?
+                    .serialize_uncompressed()[1..65],
             );
 
             return Ok(SignedTransaction {
@@ -453,14 +460,11 @@ impl SignedTransaction {
             });
         }
 
-        if sender.is_none() {
-            return Err(TypesError::MissingInteroperationSender.into());
-        }
-
+        // Otherwise it is an interoperation transaction
         Ok(SignedTransaction {
-            transaction: utx.calc_hash(),
-            sender:      sender.unwrap(),
+            sender:      sig.extract_interoperation_tx_sender()?,
             public:      Some(Public::zero()),
+            transaction: utx.calc_hash(),
         })
     }