feat: re-broadcast when duplicated transaction submit

nervosnetwork · Aug 1, 2020 · 2701205 · 2701205
1 parent 392511c
commit 2701205
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 37 deletions.
diff --git a/rpc/src/error.rs b/rpc/src/error.rs
@@ -73,6 +73,27 @@ impl RPCError {
         }
     }
 
+    pub fn from_submit_transaction_reject(reject: &Reject) -> Error {
+        let code = match reject {
+            Reject::LowFeeRate(_, _) => RPCError::PoolRejectedTransactionByMinFeeRate,
+            Reject::ExceededMaximumAncestorsCount => {
+                RPCError::PoolRejectedTransactionByMaxAncestorsCountLimit
+            }
+            Reject::Full(_, _) => RPCError::PoolIsFull,
+            Reject::Duplicated(_) => RPCError::PoolRejectedDuplicatedTransaction,
+            Reject::Malformed(_) => RPCError::PoolRejectedMalformedTransaction,
+        };
+        RPCError::custom_with_error(code, reject)
+    }
+
+    pub fn downcast_submit_transaction_reject(err: &CKBError) -> Option<&Reject> {
+        use ckb_error::ErrorKind::SubmitTransaction;
+        match err.kind() {
+            SubmitTransaction => err.downcast_ref::<Reject>(),
+            _ => None,
+        }
+    }
+
     pub fn from_ckb_error(err: CKBError) -> Error {
         use ckb_error::ErrorKind::*;
         match err.kind() {
@@ -82,24 +103,6 @@ impl RPCError {
                 RPCError::TransactionFailedToVerify,
                 err.unwrap_cause_or_self(),
             ),
-            SubmitTransaction => {
-                let reject = match err.downcast_ref::<Reject>() {
-                    Some(err) => err,
-                    None => return Self::ckb_internal_error(err),
-                };
-
-                let kind = match *reject {
-                    Reject::LowFeeRate(_, _) => RPCError::PoolRejectedTransactionByMinFeeRate,
-                    Reject::ExceededMaximumAncestorsCount => {
-                        RPCError::PoolRejectedTransactionByMaxAncestorsCountLimit
-                    }
-                    Reject::Full(_, _) => RPCError::PoolIsFull,
-                    Reject::Duplicated(_) => RPCError::PoolRejectedDuplicatedTransaction,
-                    Reject::Malformed(_) => RPCError::PoolRejectedMalformedTransaction,
-                };
-
-                RPCError::custom_with_error(kind, reject)
-            }
             Internal => {
                 let internal_err = match err.downcast_ref::<InternalError>() {
                     Some(err) => err,
@@ -159,35 +162,41 @@ mod tests {
     }
 
     #[test]
-    fn test_submit_tx_error_from_ckb_error() {
+    fn test_submit_transaction_error() {
         let err: CKBError = Reject::LowFeeRate(100, 50).into();
         assert_eq!(
             "PoolRejectedTransactionByMinFeeRate: Transaction fee rate must >= 100 shannons/KB, got: 50",
-            RPCError::from_ckb_error(err).message
+            RPCError::from_submit_transaction_reject(RPCError::downcast_submit_transaction_reject(&err).unwrap()).message
         );
 
         let err: CKBError = Reject::ExceededMaximumAncestorsCount.into();
         assert_eq!(
             "PoolRejectedTransactionByMaxAncestorsCountLimit: Transaction exceeded maximum ancestors count limit, try send it later",
-            RPCError::from_ckb_error(err).message
+            RPCError::from_submit_transaction_reject(RPCError::downcast_submit_transaction_reject(&err).unwrap()).message
         );
 
         let err: CKBError = Reject::Full("size".to_owned(), 10).into();
         assert_eq!(
             "PoolIsFull: Transaction pool exceeded maximum size limit(10), try send it later",
-            RPCError::from_ckb_error(err).message
+            RPCError::from_submit_transaction_reject(
+                RPCError::downcast_submit_transaction_reject(&err).unwrap()
+            )
+            .message
         );
 
         let err: CKBError = Reject::Duplicated(Byte32::new([0; 32])).into();
         assert_eq!(
             "PoolRejectedDuplicatedTransaction: Transaction(Byte32(0x0000000000000000000000000000000000000000000000000000000000000000)) already exist in transaction_pool",
-            RPCError::from_ckb_error(err).message
+            RPCError::from_submit_transaction_reject(RPCError::downcast_submit_transaction_reject(&err).unwrap()).message
         );
 
         let err: CKBError = Reject::Malformed("cellbase like".to_owned()).into();
         assert_eq!(
             "PoolRejectedMalformedTransaction: Malformed cellbase like transaction",
-            RPCError::from_ckb_error(err).message
+            RPCError::from_submit_transaction_reject(
+                RPCError::downcast_submit_transaction_reject(&err).unwrap()
+            )
+            .message
         );
     }
 

diff --git a/rpc/src/module/pool.rs b/rpc/src/module/pool.rs
@@ -7,6 +7,7 @@ use ckb_network::PeerIndex;
 use ckb_script::IllTransactionChecker;
 use ckb_shared::shared::Shared;
 use ckb_sync::SyncShared;
+use ckb_tx_pool::error::Reject;
 use ckb_types::{core, packed, prelude::*, H256};
 use ckb_verification::{Since, SinceMetric};
 use jsonrpc_core::Result;
@@ -100,20 +101,31 @@ impl PoolRpc for PoolRpcImpl {
             return Err(RPCError::ckb_internal_error(e));
         }
 
+        let broadcast = |tx_hash: packed::Byte32| {
+            // workaround: we are using `PeerIndex(usize::max)` to indicate that tx hash source is itself.
+            let peer_index = PeerIndex::new(usize::max_value());
+            self.sync_shared
+                .state()
+                .tx_hashes()
+                .entry(peer_index)
+                .or_default()
+                .insert(tx_hash);
+        };
+        let tx_hash = tx.hash();
         match submit_txs.unwrap() {
             Ok(_) => {
-                // workaround: we are using `PeerIndex(usize::max)` to indicate that tx hash source is itself.
-                let peer_index = PeerIndex::new(usize::max_value());
-                let hash = tx.hash();
-                self.sync_shared
-                    .state()
-                    .tx_hashes()
-                    .entry(peer_index)
-                    .or_default()
-                    .insert(hash.clone());
-                Ok(hash.unpack())
+                broadcast(tx_hash.clone());
+                Ok(tx_hash.unpack())
             }
-            Err(e) => Err(RPCError::from_ckb_error(e)),
+            Err(e) => match RPCError::downcast_submit_transaction_reject(&e) {
+                Some(reject) => {
+                    if let Reject::Duplicated(_) = reject {
+                        broadcast(tx_hash);
+                    }
+                    Err(RPCError::from_submit_transaction_reject(reject))
+                }
+                None => Err(RPCError::from_ckb_error(e)),
+            },
         }
     }
 

diff --git a/test/src/specs/tx_pool/collision.rs b/test/src/specs/tx_pool/collision.rs
@@ -30,7 +30,7 @@ impl Spec for TransactionHashCollisionDifferentWitnessHashes {
             .err()
             .unwrap()
             .to_string()
-            .contains("PoolTransactionDuplicated"));
+            .contains("PoolRejectedDuplicatedTransaction"));
     }
 }
 
@@ -54,7 +54,7 @@ impl Spec for DuplicatedTransaction {
             .err()
             .unwrap()
             .to_string()
-            .contains("PoolTransactionDuplicated"));
+            .contains("PoolRejectedDuplicatedTransaction"));
     }
 }