From be14afca36fb25f7c651ed4c52b9d0b13f01625f Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 24 May 2021 11:11:55 +0200 Subject: [PATCH] Merge bitcoin/bitcoin#21848: refactor: Make CFeeRate constructor architecture-independent fafd121026c4f1e25d498983e4f88c119516552b 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::max())); ``` Fix all issues by making it arch-independent. Also, fix `{}` style according to dev notes. ACKs for top commit: theStack: re-ACK fafd121026c4f1e25d498983e4f88c119516552b promag: Code review ACK fafd121026c4f1e25d498983e4f88c119516552b. Tree-SHA512: e16f75bad9ee8088b87e873906d9b5633449417a6996a226a2f37d33a2b7d4f2fd91df68998a77e52163de20b40c57fadabe7fe3502e599cbb98494178591833 --- src/policy/feerate.cpp | 21 +++++++++------------ src/policy/feerate.h | 4 ++-- src/test/amount_tests.cpp | 2 +- src/test/fuzz/fee_rate.cpp | 4 ++-- 4 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/policy/feerate.cpp b/src/policy/feerate.cpp index 93bcb2fd54..2bd7eb87a1 100644 --- a/src/policy/feerate.cpp +++ b/src/policy/feerate.cpp @@ -7,29 +7,26 @@ #include -CFeeRate::CFeeRate(const CAmount& nFeePaid, size_t nBytes_) +CFeeRate::CFeeRate(const CAmount& nFeePaid, uint32_t num_bytes) { - assert(nBytes_ <= uint64_t(std::numeric_limits::max())); - int64_t nSize = int64_t(nBytes_); + const int64_t nSize{num_bytes}; - if (nSize > 0) + if (nSize > 0) { nSatoshisPerK = nFeePaid * 1000 / nSize; - else + } else { nSatoshisPerK = 0; + } } -CAmount CFeeRate::GetFee(size_t nBytes_) const +CAmount CFeeRate::GetFee(uint32_t num_bytes) const { - assert(nBytes_ <= uint64_t(std::numeric_limits::max())); - int64_t nSize = int64_t(nBytes_); + const int64_t nSize{num_bytes}; CAmount nFee = nSatoshisPerK * nSize / 1000; if (nFee == 0 && nSize != 0) { - if (nSatoshisPerK > 0) - nFee = CAmount(1); - if (nSatoshisPerK < 0) - nFee = CAmount(-1); + if (nSatoshisPerK > 0) nFee = CAmount(1); + if (nSatoshisPerK < 0) nFee = CAmount(-1); } return nFee; diff --git a/src/policy/feerate.h b/src/policy/feerate.h index 26a06e8603..e4bed40fd8 100644 --- a/src/policy/feerate.h +++ b/src/policy/feerate.h @@ -40,11 +40,11 @@ public: static_assert(std::is_integral::value, "CFeeRate should be used without floats"); } /** Constructor for a fee rate in satoshis per kB. The size in bytes must not exceed (2^63 - 1)*/ - CFeeRate(const CAmount& nFeePaid, size_t nBytes); + CFeeRate(const CAmount& nFeePaid, uint32_t num_bytes); /** * Return the fee in satoshis for the given size in bytes. */ - CAmount GetFee(size_t nBytes) const; + CAmount GetFee(uint32_t num_bytes) const; /** * Return the fee in satoshis for a size of 1000 bytes */ diff --git a/src/test/amount_tests.cpp b/src/test/amount_tests.cpp index cf2c65e681..34d89ef7bd 100644 --- a/src/test/amount_tests.cpp +++ b/src/test/amount_tests.cpp @@ -84,7 +84,7 @@ BOOST_AUTO_TEST_CASE(GetFeeTest) BOOST_CHECK(CFeeRate(CAmount(26), 789) == CFeeRate(32)); BOOST_CHECK(CFeeRate(CAmount(27), 789) == CFeeRate(34)); // Maximum size in bytes, should not crash - CFeeRate(MAX_MONEY, std::numeric_limits::max() >> 1).GetFeePerK(); + CFeeRate(MAX_MONEY, std::numeric_limits::max()).GetFeePerK(); } BOOST_AUTO_TEST_CASE(BinaryOperatorTest) diff --git a/src/test/fuzz/fee_rate.cpp b/src/test/fuzz/fee_rate.cpp index 2955213635..dff0e58000 100644 --- a/src/test/fuzz/fee_rate.cpp +++ b/src/test/fuzz/fee_rate.cpp @@ -20,8 +20,8 @@ FUZZ_TARGET(fee_rate) const CFeeRate fee_rate{satoshis_per_k}; (void)fee_rate.GetFeePerK(); - const size_t bytes = fuzzed_data_provider.ConsumeIntegral(); - if (!MultiplicationOverflow(static_cast(bytes), satoshis_per_k) && bytes <= static_cast(std::numeric_limits::max())) { + const auto bytes = fuzzed_data_provider.ConsumeIntegral(); + if (!MultiplicationOverflow(int64_t{bytes}, satoshis_per_k)) { (void)fee_rate.GetFee(bytes); } (void)fee_rate.ToString();