From 026537b2ddd41c0b9ef529d59368fc624a488409 Mon Sep 17 00:00:00 2001 From: zhangsoledad <787953403@qq.com> Date: Wed, 22 Mar 2023 17:26:18 +0800 Subject: [PATCH] chore: improve error message --- resource/ckb.toml | 2 +- rpc/src/tests/error.rs | 9 ++++++--- .../relay/transaction_relay_low_fee_rate.rs | 2 +- test/template/ckb.toml | 1 - tx-pool/src/component/entry.rs | 16 +++++++++++----- tx-pool/src/pool.rs | 5 ++++- tx-pool/src/process.rs | 2 +- util/types/src/core/fee_rate.rs | 2 +- util/types/src/core/tx_pool.rs | 4 ++-- 9 files changed, 27 insertions(+), 16 deletions(-) diff --git a/resource/ckb.toml b/resource/ckb.toml index 2eb5f3336a4..7cb1f2321d0 100644 --- a/resource/ckb.toml +++ b/resource/ckb.toml @@ -133,7 +133,7 @@ enable_deprecated_rpc = false # {{ [tx_pool] max_tx_pool_size = 180_000_000 # 180mb -min_fee_rate = 1_000 # shannons/KB +min_fee_rate = 1_000 # Here fee_rate are calculated directly using size in units of shannons/KB max_tx_verify_cycles = 70_000_000 max_ancestors_count = 25 diff --git a/rpc/src/tests/error.rs b/rpc/src/tests/error.rs index 8235ab180df..fc32e7f253f 100644 --- a/rpc/src/tests/error.rs +++ b/rpc/src/tests/error.rs @@ -22,7 +22,7 @@ fn test_submit_transaction_error() { let min_fee_rate = FeeRate::from_u64(500); let reject = Reject::LowFeeRate(min_fee_rate, 100, 50); assert_eq!( - "PoolRejectedTransactionByMinFeeRate: The min fee rate is 500 shannons/KB, so the transaction fee should be 100 shannons at least, but only got 50", + "PoolRejectedTransactionByMinFeeRate: The min fee rate is 500 shannons/KW, so the transaction fee should be 100 shannons at least, but only got 50", RPCError::from_submit_transaction_reject(&reject).message ); @@ -32,9 +32,12 @@ fn test_submit_transaction_error() { RPCError::from_submit_transaction_reject(&reject).message ); - let reject = Reject::Full("size".to_owned()); + let reject = Reject::Full(format!( + "the fee_rate for this transaction is: {}", + FeeRate::from_u64(500) + )); assert_eq!( - "PoolIsFull: Transaction are replaced because the pool is full size", + "PoolIsFull: Transaction are replaced because the pool is full, the fee_rate for this transaction is: 500 shannons/KW", RPCError::from_submit_transaction_reject(&reject).message ); diff --git a/test/src/specs/relay/transaction_relay_low_fee_rate.rs b/test/src/specs/relay/transaction_relay_low_fee_rate.rs index 51a3c8b9e14..dfe086636db 100644 --- a/test/src/specs/relay/transaction_relay_low_fee_rate.rs +++ b/test/src/specs/relay/transaction_relay_low_fee_rate.rs @@ -36,7 +36,7 @@ impl Spec for TransactionRelayLowFeeRate { node1, 0, 10, - "reject tx The min fee rate is 1000 shannons/KB, so the transaction fee should be 242 shannons at least, but only got 0" + "reject tx The min fee rate is 1000 shannons/KW, so the transaction fee should be 242 shannons at least, but only got 0" ) .is_some()); } diff --git a/test/template/ckb.toml b/test/template/ckb.toml index b9bce4c01ca..3ca23dc799f 100644 --- a/test/template/ckb.toml +++ b/test/template/ckb.toml @@ -78,7 +78,6 @@ enable_deprecated_rpc = true [tx_pool] max_tx_pool_size = 180_000_000 # 180mb -max_cycles = 200_000_000_000 min_fee_rate = 0 # shannons/KB max_tx_verify_cycles = 70_000_000 max_ancestors_count = 25 diff --git a/tx-pool/src/component/entry.rs b/tx-pool/src/component/entry.rs index a96b01f54c7..6a1d6028ccb 100644 --- a/tx-pool/src/component/entry.rs +++ b/tx-pool/src/component/entry.rs @@ -99,6 +99,12 @@ impl TxEntry { EvictKey::from(self) } + /// Returns fee rate + pub fn fee_rate(&self) -> FeeRate { + let weight = get_transaction_weight(self.size, self.cycles); + FeeRate::calculate(self.fee, weight) + } + /// Update ancestor state for add an entry pub fn add_entry_weight(&mut self, entry: &TxEntry) { self.ancestors_count = self.ancestors_count.saturating_add(1); @@ -184,9 +190,10 @@ impl Ord for TxEntry { } /// Currently we do not have trace descendants, -/// so we take the simplest strategy to evict the transactions by first comparing the fee_rate, -/// and then taking the one with the largest timestamp, -/// which also means that the fewer descendants may exist. +/// so first take the simplest strategy, +/// first compare fee_rate, select the smallest fee_rate, +/// and then select the latest timestamp, for eviction, +/// the latest timestamp which also means that the fewer descendants may exist. #[derive(Eq, PartialEq, Clone, Debug)] pub struct EvictKey { fee_rate: FeeRate, @@ -195,9 +202,8 @@ pub struct EvictKey { impl From<&TxEntry> for EvictKey { fn from(entry: &TxEntry) -> Self { - let weight = get_transaction_weight(entry.size, entry.cycles); EvictKey { - fee_rate: FeeRate::calculate(entry.fee, weight), + fee_rate: entry.fee_rate(), timestamp: entry.timestamp, } } diff --git a/tx-pool/src/pool.rs b/tx-pool/src/pool.rs index 73f0e028323..3443ff0c766 100644 --- a/tx-pool/src/pool.rs +++ b/tx-pool/src/pool.rs @@ -42,7 +42,10 @@ macro_rules! evict_for_trim_size { "removed by size limit {} timestamp({})", tx_hash, entry.timestamp ); - let reject = Reject::Full(format!("fee: {}", entry.fee)); + let reject = Reject::Full(format!( + "the fee_rate for this transaction is: {}", + entry.fee_rate() + )); $callbacks.call_reject($self, &entry, reject); } } diff --git a/tx-pool/src/process.rs b/tx-pool/src/process.rs index f1df3e5fc9c..3907c1e11db 100644 --- a/tx-pool/src/process.rs +++ b/tx-pool/src/process.rs @@ -959,7 +959,7 @@ fn _update_tx_pool_for_reorg( // Remove expired transaction from pending tx_pool.remove_expired(callbacks); - // Remove transactions from the pool until its size < size_limit. + // Remove transactions from the pool until its size <= size_limit. tx_pool.limit_size(callbacks); } diff --git a/util/types/src/core/fee_rate.rs b/util/types/src/core/fee_rate.rs index 8bb2caee30f..edb4cc8f0ce 100644 --- a/util/types/src/core/fee_rate.rs +++ b/util/types/src/core/fee_rate.rs @@ -39,6 +39,6 @@ impl FeeRate { impl ::std::fmt::Display for FeeRate { fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result { - write!(f, "{}", self.0) + write!(f, "{} shannons/KW", self.0) } } diff --git a/util/types/src/core/tx_pool.rs b/util/types/src/core/tx_pool.rs index b3ff9360568..b1eac056f15 100644 --- a/util/types/src/core/tx_pool.rs +++ b/util/types/src/core/tx_pool.rs @@ -14,7 +14,7 @@ use std::collections::HashMap; #[derive(Error, Debug, Clone)] pub enum Reject { /// Transaction fee lower than config - #[error("The min fee rate is {0} shannons/KB, so the transaction fee should be {1} shannons at least, but only got {2}")] + #[error("The min fee rate is {0}, so the transaction fee should be {1} shannons at least, but only got {2}")] LowFeeRate(FeeRate, u64, u64), /// Transaction exceeded maximum ancestors count limit @@ -26,7 +26,7 @@ pub enum Reject { ExceededTransactionSizeLimit(u64, u64), /// Transaction are replaced because the pool is full - #[error("Transaction are replaced because the pool is full {0}")] + #[error("Transaction are replaced because the pool is full, {0}")] Full(String), /// Transaction already exist in transaction_pool