Skip to content

Commit ce4a852

Browse files
author
MarcoFalke
committed
Merge bitcoin/bitcoin#21848: refactor: Make CFeeRate constructor architecture-independent
fafd121 refactor: Make CFeeRate constructor architecture-independent (MarcoFalke) Pull request description: Currently the constructor is architecture dependent. This is confusing for several reasons: * It is impossible to create a transaction larger than the max value of `uint32_t`, so a 64-bit `size_t` is not needed * Policy (and consensus) code should be arch-independent * The current code will print spurious compile errors when compiled on 32-bit systems: ``` policy/feerate.cpp:23:22: warning: result of comparison of constant 9223372036854775807 with expression of type 'size_t' (aka 'unsigned int') is always true [-Wtautological-constant-out-of-range-compare] assert(nBytes_ <= uint64_t(std::numeric_limits<int64_t>::max())); ``` Fix all issues by making it arch-independent. Also, fix `{}` style according to dev notes. ACKs for top commit: theStack: re-ACK fafd121 promag: Code review ACK fafd121. Tree-SHA512: e16f75bad9ee8088b87e873906d9b5633449417a6996a226a2f37d33a2b7d4f2fd91df68998a77e52163de20b40c57fadabe7fe3502e599cbb98494178591833
2 parents 5990009 + fafd121 commit ce4a852

File tree

4 files changed

+16
-22
lines changed

4 files changed

+16
-22
lines changed

src/policy/feerate.cpp

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,29 +7,26 @@
77

88
#include <tinyformat.h>
99

10-
CFeeRate::CFeeRate(const CAmount& nFeePaid, size_t nBytes_)
10+
CFeeRate::CFeeRate(const CAmount& nFeePaid, uint32_t num_bytes)
1111
{
12-
assert(nBytes_ <= uint64_t(std::numeric_limits<int64_t>::max()));
13-
int64_t nSize = int64_t(nBytes_);
12+
const int64_t nSize{num_bytes};
1413

15-
if (nSize > 0)
14+
if (nSize > 0) {
1615
nSatoshisPerK = nFeePaid * 1000 / nSize;
17-
else
16+
} else {
1817
nSatoshisPerK = 0;
18+
}
1919
}
2020

21-
CAmount CFeeRate::GetFee(size_t nBytes_) const
21+
CAmount CFeeRate::GetFee(uint32_t num_bytes) const
2222
{
23-
assert(nBytes_ <= uint64_t(std::numeric_limits<int64_t>::max()));
24-
int64_t nSize = int64_t(nBytes_);
23+
const int64_t nSize{num_bytes};
2524

2625
CAmount nFee = nSatoshisPerK * nSize / 1000;
2726

2827
if (nFee == 0 && nSize != 0) {
29-
if (nSatoshisPerK > 0)
30-
nFee = CAmount(1);
31-
if (nSatoshisPerK < 0)
32-
nFee = CAmount(-1);
28+
if (nSatoshisPerK > 0) nFee = CAmount(1);
29+
if (nSatoshisPerK < 0) nFee = CAmount(-1);
3330
}
3431

3532
return nFee;

src/policy/feerate.h

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,20 +39,17 @@ class CFeeRate
3939
// We've previously had bugs creep in from silent double->int conversion...
4040
static_assert(std::is_integral<I>::value, "CFeeRate should be used without floats");
4141
}
42-
/** Constructor for a fee rate in satoshis per kvB (sat/kvB). The size in bytes must not exceed (2^63 - 1).
42+
/** Constructor for a fee rate in satoshis per kvB (sat/kvB).
4343
*
44-
* Passing an nBytes value of COIN (1e8) returns a fee rate in satoshis per vB (sat/vB),
44+
* Passing a num_bytes value of COIN (1e8) returns a fee rate in satoshis per vB (sat/vB),
4545
* e.g. (nFeePaid * 1e8 / 1e3) == (nFeePaid / 1e5),
4646
* where 1e5 is the ratio to convert from BTC/kvB to sat/vB.
47-
*
48-
* @param[in] nFeePaid CAmount fee rate to construct with
49-
* @param[in] nBytes size_t bytes (units) to construct with
5047
*/
51-
CFeeRate(const CAmount& nFeePaid, size_t nBytes);
48+
CFeeRate(const CAmount& nFeePaid, uint32_t num_bytes);
5249
/**
5350
* Return the fee in satoshis for the given size in bytes.
5451
*/
55-
CAmount GetFee(size_t nBytes) const;
52+
CAmount GetFee(uint32_t num_bytes) const;
5653
/**
5754
* Return the fee in satoshis for a size of 1000 bytes
5855
*/

src/test/amount_tests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ BOOST_AUTO_TEST_CASE(GetFeeTest)
8383
BOOST_CHECK(CFeeRate(CAmount(26), 789) == CFeeRate(32));
8484
BOOST_CHECK(CFeeRate(CAmount(27), 789) == CFeeRate(34));
8585
// Maximum size in bytes, should not crash
86-
CFeeRate(MAX_MONEY, std::numeric_limits<size_t>::max() >> 1).GetFeePerK();
86+
CFeeRate(MAX_MONEY, std::numeric_limits<uint32_t>::max()).GetFeePerK();
8787
}
8888

8989
BOOST_AUTO_TEST_CASE(BinaryOperatorTest)

src/test/fuzz/fee_rate.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ FUZZ_TARGET(fee_rate)
2020
const CFeeRate fee_rate{satoshis_per_k};
2121

2222
(void)fee_rate.GetFeePerK();
23-
const size_t bytes = fuzzed_data_provider.ConsumeIntegral<size_t>();
24-
if (!MultiplicationOverflow(static_cast<int64_t>(bytes), satoshis_per_k) && bytes <= static_cast<uint64_t>(std::numeric_limits<int64_t>::max())) {
23+
const auto bytes = fuzzed_data_provider.ConsumeIntegral<uint32_t>();
24+
if (!MultiplicationOverflow(int64_t{bytes}, satoshis_per_k)) {
2525
(void)fee_rate.GetFee(bytes);
2626
}
2727
(void)fee_rate.ToString();

0 commit comments

Comments
 (0)