Merge pull request #4107 from chenyukang/fix-tx-rejected-err

Add reason to PoolTransactionReject Malformed capacity error

rpc/src/error.rs

@@ -171,7 +171,7 @@ impl RPCError {
             }
             Reject::Full(_) => RPCError::PoolIsFull,
             Reject::Duplicated(_) => RPCError::PoolRejectedDuplicatedTransaction,
-            Reject::Malformed(_) => RPCError::PoolRejectedMalformedTransaction,
+            Reject::Malformed(_, _) => RPCError::PoolRejectedMalformedTransaction,
             Reject::DeclaredWrongCycles(..) => RPCError::PoolRejectedMalformedTransaction,
             Reject::Resolve(_) => RPCError::TransactionFailedToResolve,
             Reject::Verification(_) => RPCError::TransactionFailedToVerify,

rpc/src/tests/error.rs

@@ -47,7 +47,7 @@ fn test_submit_transaction_error() {
             RPCError::from_submit_transaction_reject(&reject).message
         );
 
-    let reject = Reject::Malformed("cellbase like".to_owned());
+    let reject = Reject::Malformed("cellbase like".to_owned(), "".to_owned());
     assert_eq!(
         "PoolRejectedMalformedTransaction: Malformed cellbase like transaction",
         RPCError::from_submit_transaction_reject(&reject).message

test/src/main.rs

@@ -526,6 +526,7 @@ fn all_specs() -> Vec<Box<dyn Spec>> {
         Box::new(HandlingDescendantsOfCommitted),
         Box::new(ProposeOutOfOrder),
         Box::new(SubmitTransactionWhenItsParentInGap),
+        Box::new(MalformedTx),
         Box::new(SubmitTransactionWhenItsParentInProposed),
         Box::new(ProposeTransactionButParentNot),
         Box::new(ProposalExpireRuleForCommittingAndExpiredAtOneTime),

test/src/specs/mining/fee.rs

@@ -6,8 +6,14 @@ use crate::util::transaction::always_success_transaction;
 use crate::{Node, Spec};
 use crate::{DEFAULT_TX_PROPOSAL_WINDOW, FINALIZATION_DELAY_LENGTH};
 use ckb_types::core::TransactionBuilder;
+use ckb_types::packed::CellInput;
 use ckb_types::packed::CellOutput;
+use ckb_types::packed::OutPoint;
 use ckb_types::prelude::*;
+use ckb_types::{
+    core::{capacity_bytes, Capacity},
+    packed::CellOutputBuilder,
+};
 use rand::{thread_rng, Rng};
 
 pub struct FeeOfTransaction;
@@ -240,3 +246,45 @@ impl Spec for ProposeDuplicated {
         check_fee(node);
     }
 }
+
+pub struct MalformedTx;
+impl Spec for MalformedTx {
+    fn run(&self, nodes: &mut Vec<Node>) {
+        let node0 = &nodes[0];
+
+        node0.mine_until_out_bootstrap_period();
+        let tx0 = node0.new_transaction_spend_tip_cellbase();
+
+        let output = CellOutputBuilder::default()
+            .capacity(capacity_bytes!(1000).pack())
+            .build();
+
+        let child = tx0
+            .as_advanced_builder()
+            .set_inputs(vec![{
+                CellInput::new_builder()
+                    .previous_output(OutPoint::new(tx0.hash(), 0))
+                    .build()
+            }])
+            .set_outputs(vec![output])
+            .build();
+
+        let ret = node0
+            .rpc_client()
+            .send_transaction_result(tx0.data().into());
+        assert!(ret.is_ok());
+
+        node0.mine_until_transaction_confirm(&tx0.hash());
+
+        let ret = node0
+            .rpc_client()
+            .send_transaction_result(child.data().into());
+
+        assert!(ret.is_err());
+        let message = ret.unwrap_err().to_string();
+        assert!(
+            message.contains("Malformed Overflow transaction")
+                && message.contains("expect (outputs capacity) <= (inputs capacity)")
+        );
+    }
+}

tx-pool/src/component/tests/recent_reject.rs

@@ -15,14 +15,14 @@ fn test_basic() {
     for i in 0..80u64 {
         let key = Byte32::new(blake2b_256(i.to_le_bytes()));
         recent_reject
-            .put(&key, Reject::Malformed(i.to_string()))
+            .put(&key, Reject::Malformed(i.to_string(), Default::default()))
             .unwrap();
     }
 
     for i in 0..80u64 {
         let key = Byte32::new(blake2b_256(i.to_le_bytes()));
         let reject: ckb_jsonrpc_types::PoolTransactionReject =
-            Reject::Malformed(i.to_string()).into();
+            Reject::Malformed(i.to_string(), Default::default()).into();
         assert_eq!(
             recent_reject.get(&key).unwrap().unwrap(),
             serde_json::to_string(&reject).unwrap()
@@ -32,7 +32,7 @@ fn test_basic() {
     for i in 0..80u64 {
         let key = Byte32::new(blake2b_256(i.to_le_bytes()));
         recent_reject
-            .put(&key, Reject::Malformed(i.to_string()))
+            .put(&key, Reject::Malformed(i.to_string(), Default::default()))
             .unwrap();
     }
 

tx-pool/src/util.rs

@@ -33,7 +33,12 @@ pub(crate) fn check_tx_fee(
 ) -> Result<Capacity, Reject> {
     let fee = DaoCalculator::new(snapshot.consensus(), &snapshot.borrow_as_data_loader())
         .transaction_fee(rtx)
-        .map_err(|err| Reject::Malformed(format!("{err}")))?;
+        .map_err(|err| {
+            Reject::Malformed(
+                format!("{err}"),
+                "expect (outputs capacity) <= (inputs capacity)".to_owned(),
+            )
+        })?;
     // Theoretically we cannot use size as weight directly to calculate fee_rate,
     // here min fee rate is used as a cheap check,
     // so we will use size to calculate fee_rate directly
@@ -68,7 +73,10 @@ pub(crate) fn non_contextual_verify(
     }
     // cellbase is only valid in a block, not as a loose transaction
     if tx.is_cellbase() {
-        return Err(Reject::Malformed("cellbase like".to_owned()));
+        return Err(Reject::Malformed(
+            "cellbase like".to_owned(),
+            Default::default(),
+        ));
     }
 
     Ok(())

util/jsonrpc-types/src/pool.rs

@@ -264,7 +264,7 @@ impl From<Reject> for PoolTransactionReject {
             }
             Reject::Full(..) => Self::Full(format!("{reject}")),
             Reject::Duplicated(_) => Self::Duplicated(format!("{reject}")),
-            Reject::Malformed(_) => Self::Malformed(format!("{reject}")),
+            Reject::Malformed(_, _) => Self::Malformed(format!("{reject}")),
             Reject::DeclaredWrongCycles(..) => Self::DeclaredWrongCycles(format!("{reject}")),
             Reject::Resolve(_) => Self::Resolve(format!("{reject}")),
             Reject::Verification(_) => Self::Verification(format!("{reject}")),

util/types/src/core/tests/tx_pool.rs

@@ -22,7 +22,7 @@ fn test_if_is_malformed_tx() {
     let reject = Reject::Duplicated(Default::default());
     assert!(!reject.is_malformed_tx());
 
-    let reject = Reject::Malformed(Default::default());
+    let reject = Reject::Malformed(Default::default(), Default::default());
     assert!(reject.is_malformed_tx());
 
     for error in vec![

util/types/src/core/tx_pool.rs

@@ -38,7 +38,7 @@ pub enum Reject {
 
     /// Malformed transaction
     #[error("Malformed {0} transaction")]
-    Malformed(String),
+    Malformed(String, String),
 
     /// Declared wrong cycles
     #[error("Declared wrong cycles {0}, actual {1}")]
@@ -83,7 +83,7 @@ impl Reject {
     /// Returns true if the reject reason is malformed tx.
     pub fn is_malformed_tx(&self) -> bool {
         match self {
-            Reject::Malformed(_) => true,
+            Reject::Malformed(_, _) => true,
             Reject::DeclaredWrongCycles(..) => true,
             Reject::Verification(err) => is_malformed_from_verification(err),
             Reject::Resolve(OutPointError::OverMaxDepExpansionLimit) => true,