From 3108dcd9e1e12ce5a353f4ee289d68c6db21b76d Mon Sep 17 00:00:00 2001 From: Eval EXEC Date: Sun, 22 Sep 2024 01:26:55 +0800 Subject: [PATCH 1/3] `get_fee_rate_statistics` should aware `block_ext.txs_sizes` length is `block_ext.txs_fees` length + 1 Signed-off-by: Eval EXEC --- rpc/src/util/fee_rate.rs | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/rpc/src/util/fee_rate.rs b/rpc/src/util/fee_rate.rs index 2cea14ba15..fbb1086001 100644 --- a/rpc/src/util/fee_rate.rs +++ b/rpc/src/util/fee_rate.rs @@ -83,20 +83,16 @@ where target = std::cmp::min(self.provider.max_target(), target); let mut fee_rates = self.provider.collect(target, |mut fee_rates, block_ext| { - if block_ext.txs_fees.len() > 1 - && block_ext.cycles.is_some() - && block_ext.txs_sizes.is_some() - { - // block_ext.txs_fees, cycles, txs_sizes length is same + let txs_sizes = block_ext.txs_sizes.expect("expect txs_size's length >= 1"); + if txs_sizes.len() > 1 && block_ext.cycles.is_some() && !block_ext.txs_fees.is_empty() { + // block_ext.txs_fees's length == block_ext.cycles's length + // block_ext.txs_fees's length + 1 == txs_sizes's length for (fee, cycles, size) in itertools::izip!( block_ext.txs_fees, block_ext.cycles.expect("checked"), - block_ext.txs_sizes.expect("checked") - ) - // skip cellbase (first element in the Vec) - .skip(1) - { - let weight = get_transaction_weight(size as usize, cycles); + txs_sizes.iter().skip(1) // skip cellbase (first element in the Vec) + ) { + let weight = get_transaction_weight(*size as usize, cycles); if weight > 0 { fee_rates.push(FeeRate::calculate(fee, weight).as_u64()); } From 279f3aa01f219209e3c44b249b393adf708d98ea Mon Sep 17 00:00:00 2001 From: Eval EXEC Date: Sun, 22 Sep 2024 01:52:58 +0800 Subject: [PATCH 2/3] Add code comment for BlockExt --- util/types/src/core/extras.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/util/types/src/core/extras.rs b/util/types/src/core/extras.rs index d42eb66ce6..a7261a2fb5 100644 --- a/util/types/src/core/extras.rs +++ b/util/types/src/core/extras.rs @@ -10,7 +10,15 @@ use std::fmt; use std::num::ParseIntError; use std::str::FromStr; -/// TODO(doc): @quake +/// Represents a block's additional information. +/// +/// It is crucial to ensure that `txs_sizes` has one more element than `txs_fees`, and that `cycles` has the same length as `txs_fees`. +/// +/// `BlockTxsVerifier::verify()` skips the first transaction (the cellbase) in the block. Therefore, `txs_sizes` must have a length equal to `txs_fees` length + 1. +/// +/// Refer to: https://github.com/nervosnetwork/ckb/blob/44afc93cd88a1b52351831dce788d3023c52f37e/verification/contextual/src/contextual_block_verifier.rs#L455 +/// +/// Additionally, the `get_fee_rate_statistics` RPC function requires accurate `txs_sizes` and `txs_fees` data from `BlockExt`. #[derive(Clone, PartialEq, Default, Debug, Eq)] pub struct BlockExt { /// TODO(doc): @quake @@ -21,11 +29,14 @@ pub struct BlockExt { pub total_uncles_count: u64, /// TODO(doc): @quake pub verified: Option, - /// TODO(doc): @quake + /// Transaction fees for each transaction except the cellbase. + /// The length of `txs_fees` is equal to the length of `cycles`. pub txs_fees: Vec, - /// block txs consumed cycles + /// Execution cycles for each transaction except the cellbase. + /// The length of `cycles` is equal to the length of `txs_fees`. pub cycles: Option>, - /// block txs serialized sizes + /// Sizes of each transaction including the cellbase. + /// The length of `txs_sizes` is `txs_fees` length + 1. pub txs_sizes: Option>, } From cfd8a7f44d1b0deb592963cce11e93c9e68c6371 Mon Sep 17 00:00:00 2001 From: Eval EXEC Date: Sun, 22 Sep 2024 02:02:09 +0800 Subject: [PATCH 3/3] Fix get_fee_rate_statistics unit test --- rpc/src/tests/fee_rate.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/rpc/src/tests/fee_rate.rs b/rpc/src/tests/fee_rate.rs index e4d41ebbef..e805334dc1 100644 --- a/rpc/src/tests/fee_rate.rs +++ b/rpc/src/tests/fee_rate.rs @@ -54,13 +54,12 @@ fn test_fee_rate_statics() { total_uncles_count: 0, verified: None, - // first element in txs_fees is belong to cellbase - txs_fees: vec![ - Capacity::shannons(i * 1234), - Capacity::shannons(i * i * 100), - ], - // first element in cycles is belong to cellbase - cycles: Some(vec![0, i * 100]), + // txs_fees length is equal to block_ext.cycles length + // and txs_fees does not include cellbase + txs_fees: vec![Capacity::shannons(i * i * 100)], + // cycles does not include cellbase + cycles: Some(vec![i * 100]), + // txs_sizes length is equal to block_ext.txs_fees length + 1 // first element in txs_sizes is belong to cellbase txs_sizes: Some(vec![i * 5678, i * 100]), };