diff --git a/src/test/feefrac_tests.cpp b/src/test/feefrac_tests.cpp index 03addeeff59..8da79c20718 100644 --- a/src/test/feefrac_tests.cpp +++ b/src/test/feefrac_tests.cpp @@ -17,26 +17,43 @@ BOOST_AUTO_TEST_CASE(feefrac_operators) FeeFrac empty{0, 0}; FeeFrac zero_fee{0, 1}; // zero-fee allowed - BOOST_CHECK_EQUAL(zero_fee.EvaluateFee(0), 0); - BOOST_CHECK_EQUAL(zero_fee.EvaluateFee(1), 0); - BOOST_CHECK_EQUAL(zero_fee.EvaluateFee(1000000), 0); - BOOST_CHECK_EQUAL(zero_fee.EvaluateFee(0x7fffffff), 0); + BOOST_CHECK_EQUAL(zero_fee.EvaluateFeeDown(0), 0); + BOOST_CHECK_EQUAL(zero_fee.EvaluateFeeDown(1), 0); + BOOST_CHECK_EQUAL(zero_fee.EvaluateFeeDown(1000000), 0); + BOOST_CHECK_EQUAL(zero_fee.EvaluateFeeDown(0x7fffffff), 0); + BOOST_CHECK_EQUAL(zero_fee.EvaluateFeeUp(0), 0); + BOOST_CHECK_EQUAL(zero_fee.EvaluateFeeUp(1), 0); + BOOST_CHECK_EQUAL(zero_fee.EvaluateFeeUp(1000000), 0); + BOOST_CHECK_EQUAL(zero_fee.EvaluateFeeUp(0x7fffffff), 0); - BOOST_CHECK_EQUAL(p1.EvaluateFee(0), 0); - BOOST_CHECK_EQUAL(p1.EvaluateFee(1), 10); - BOOST_CHECK_EQUAL(p1.EvaluateFee(100000000), 1000000000); - BOOST_CHECK_EQUAL(p1.EvaluateFee(0x7fffffff), int64_t(0x7fffffff) * 10); + BOOST_CHECK_EQUAL(p1.EvaluateFeeDown(0), 0); + BOOST_CHECK_EQUAL(p1.EvaluateFeeDown(1), 10); + BOOST_CHECK_EQUAL(p1.EvaluateFeeDown(100000000), 1000000000); + BOOST_CHECK_EQUAL(p1.EvaluateFeeDown(0x7fffffff), int64_t(0x7fffffff) * 10); + BOOST_CHECK_EQUAL(p1.EvaluateFeeUp(0), 0); + BOOST_CHECK_EQUAL(p1.EvaluateFeeUp(1), 10); + BOOST_CHECK_EQUAL(p1.EvaluateFeeUp(100000000), 1000000000); + BOOST_CHECK_EQUAL(p1.EvaluateFeeUp(0x7fffffff), int64_t(0x7fffffff) * 10); FeeFrac neg{-1001, 100}; - BOOST_CHECK_EQUAL(neg.EvaluateFee(0), 0); - BOOST_CHECK_EQUAL(neg.EvaluateFee(1), -11); - BOOST_CHECK_EQUAL(neg.EvaluateFee(2), -21); - BOOST_CHECK_EQUAL(neg.EvaluateFee(3), -31); - BOOST_CHECK_EQUAL(neg.EvaluateFee(100), -1001); - BOOST_CHECK_EQUAL(neg.EvaluateFee(101), -1012); - BOOST_CHECK_EQUAL(neg.EvaluateFee(100000000), -1001000000); - BOOST_CHECK_EQUAL(neg.EvaluateFee(100000001), -1001000011); - BOOST_CHECK_EQUAL(neg.EvaluateFee(0x7fffffff), -21496311307); + BOOST_CHECK_EQUAL(neg.EvaluateFeeDown(0), 0); + BOOST_CHECK_EQUAL(neg.EvaluateFeeDown(1), -11); + BOOST_CHECK_EQUAL(neg.EvaluateFeeDown(2), -21); + BOOST_CHECK_EQUAL(neg.EvaluateFeeDown(3), -31); + BOOST_CHECK_EQUAL(neg.EvaluateFeeDown(100), -1001); + BOOST_CHECK_EQUAL(neg.EvaluateFeeDown(101), -1012); + BOOST_CHECK_EQUAL(neg.EvaluateFeeDown(100000000), -1001000000); + BOOST_CHECK_EQUAL(neg.EvaluateFeeDown(100000001), -1001000011); + BOOST_CHECK_EQUAL(neg.EvaluateFeeDown(0x7fffffff), -21496311307); + BOOST_CHECK_EQUAL(neg.EvaluateFeeUp(0), 0); + BOOST_CHECK_EQUAL(neg.EvaluateFeeUp(1), -10); + BOOST_CHECK_EQUAL(neg.EvaluateFeeUp(2), -20); + BOOST_CHECK_EQUAL(neg.EvaluateFeeUp(3), -30); + BOOST_CHECK_EQUAL(neg.EvaluateFeeUp(100), -1001); + BOOST_CHECK_EQUAL(neg.EvaluateFeeUp(101), -1011); + BOOST_CHECK_EQUAL(neg.EvaluateFeeUp(100000000), -1001000000); + BOOST_CHECK_EQUAL(neg.EvaluateFeeUp(100000001), -1001000010); + BOOST_CHECK_EQUAL(neg.EvaluateFeeUp(0x7fffffff), -21496311306); BOOST_CHECK(empty == FeeFrac{}); // same as no-args @@ -88,15 +105,22 @@ BOOST_AUTO_TEST_CASE(feefrac_operators) BOOST_CHECK(oversized_1 << oversized_2); BOOST_CHECK(oversized_1 != oversized_2); - BOOST_CHECK_EQUAL(oversized_1.EvaluateFee(0), 0); - BOOST_CHECK_EQUAL(oversized_1.EvaluateFee(1), 1152921); - BOOST_CHECK_EQUAL(oversized_1.EvaluateFee(2), 2305843); - BOOST_CHECK_EQUAL(oversized_1.EvaluateFee(1548031267), 1784758530396540); + BOOST_CHECK_EQUAL(oversized_1.EvaluateFeeDown(0), 0); + BOOST_CHECK_EQUAL(oversized_1.EvaluateFeeDown(1), 1152921); + BOOST_CHECK_EQUAL(oversized_1.EvaluateFeeDown(2), 2305843); + BOOST_CHECK_EQUAL(oversized_1.EvaluateFeeDown(1548031267), 1784758530396540); + BOOST_CHECK_EQUAL(oversized_1.EvaluateFeeUp(0), 0); + BOOST_CHECK_EQUAL(oversized_1.EvaluateFeeUp(1), 1152922); + BOOST_CHECK_EQUAL(oversized_1.EvaluateFeeUp(2), 2305843); + BOOST_CHECK_EQUAL(oversized_1.EvaluateFeeUp(1548031267), 1784758530396541); - // Test cases on the threshold where FeeFrac::EvaluateFee start using Mul/Div. - BOOST_CHECK_EQUAL(FeeFrac(0x1ffffffff, 123456789).EvaluateFee(98765432), 6871947728); - BOOST_CHECK_EQUAL(FeeFrac(0x200000000, 123456789).EvaluateFee(98765432), 6871947729); - BOOST_CHECK_EQUAL(FeeFrac(0x200000001, 123456789).EvaluateFee(98765432), 6871947730); + // Test cases on the threshold where FeeFrac::Evaluate start using Mul/Div. + BOOST_CHECK_EQUAL(FeeFrac(0x1ffffffff, 123456789).EvaluateFeeDown(98765432), 6871947728); + BOOST_CHECK_EQUAL(FeeFrac(0x200000000, 123456789).EvaluateFeeDown(98765432), 6871947729); + BOOST_CHECK_EQUAL(FeeFrac(0x200000001, 123456789).EvaluateFeeDown(98765432), 6871947730); + BOOST_CHECK_EQUAL(FeeFrac(0x1ffffffff, 123456789).EvaluateFeeUp(98765432), 6871947729); + BOOST_CHECK_EQUAL(FeeFrac(0x200000000, 123456789).EvaluateFeeUp(98765432), 6871947730); + BOOST_CHECK_EQUAL(FeeFrac(0x200000001, 123456789).EvaluateFeeUp(98765432), 6871947731); // Tests paths that use double arithmetic FeeFrac busted{(static_cast(INT32_MAX)) + 1, INT32_MAX}; @@ -108,12 +132,18 @@ BOOST_AUTO_TEST_CASE(feefrac_operators) BOOST_CHECK(max_fee <= max_fee); BOOST_CHECK(max_fee >= max_fee); - BOOST_CHECK_EQUAL(max_fee.EvaluateFee(0), 0); - BOOST_CHECK_EQUAL(max_fee.EvaluateFee(1), 977888); - BOOST_CHECK_EQUAL(max_fee.EvaluateFee(2), 1955777); - BOOST_CHECK_EQUAL(max_fee.EvaluateFee(3), 2933666); - BOOST_CHECK_EQUAL(max_fee.EvaluateFee(1256796054), 1229006664189047); - BOOST_CHECK_EQUAL(max_fee.EvaluateFee(INT32_MAX), 2100000000000000); + BOOST_CHECK_EQUAL(max_fee.EvaluateFeeDown(0), 0); + BOOST_CHECK_EQUAL(max_fee.EvaluateFeeDown(1), 977888); + BOOST_CHECK_EQUAL(max_fee.EvaluateFeeDown(2), 1955777); + BOOST_CHECK_EQUAL(max_fee.EvaluateFeeDown(3), 2933666); + BOOST_CHECK_EQUAL(max_fee.EvaluateFeeDown(1256796054), 1229006664189047); + BOOST_CHECK_EQUAL(max_fee.EvaluateFeeDown(INT32_MAX), 2100000000000000); + BOOST_CHECK_EQUAL(max_fee.EvaluateFeeUp(0), 0); + BOOST_CHECK_EQUAL(max_fee.EvaluateFeeUp(1), 977889); + BOOST_CHECK_EQUAL(max_fee.EvaluateFeeUp(2), 1955778); + BOOST_CHECK_EQUAL(max_fee.EvaluateFeeUp(3), 2933667); + BOOST_CHECK_EQUAL(max_fee.EvaluateFeeUp(1256796054), 1229006664189048); + BOOST_CHECK_EQUAL(max_fee.EvaluateFeeUp(INT32_MAX), 2100000000000000); FeeFrac max_fee2{1, 1}; BOOST_CHECK(max_fee >= max_fee2); diff --git a/src/test/fuzz/feefrac.cpp b/src/test/fuzz/feefrac.cpp index b0f6f95ceaa..6da552e01f4 100644 --- a/src/test/fuzz/feefrac.cpp +++ b/src/test/fuzz/feefrac.cpp @@ -109,20 +109,24 @@ FUZZ_TARGET(feefrac_div_fallback) { // Verify the behavior of FeeFrac::DivFallback over all possible inputs. - // Construct a 96-bit signed value num, and positive 31-bit value den. + // Construct a 96-bit signed value num, a positive 31-bit value den, and rounding mode. FuzzedDataProvider provider(buffer.data(), buffer.size()); auto num_high = provider.ConsumeIntegral(); auto num_low = provider.ConsumeIntegral(); std::pair num{num_high, num_low}; auto den = provider.ConsumeIntegralInRange(1, std::numeric_limits::max()); + auto round_down = provider.ConsumeBool(); // Predict the sign of the actual result. bool is_negative = num_high < 0; - // Evaluate absolute value using arith_uint256. If the actual result is negative, the absolute - // value of the quotient is the rounded-up quotient of the absolute values. + // Evaluate absolute value using arith_uint256. If the actual result is negative and we are + // rounding down, or positive and we are rounding up, the absolute value of the quotient is + // the rounded-up quotient of the absolute values. auto num_abs = Abs256(num); auto den_abs = Abs256(den); - auto quot_abs = is_negative ? (num_abs + den_abs - 1) / den_abs : num_abs / den_abs; + auto quot_abs = (is_negative == round_down) ? + (num_abs + den_abs - 1) / den_abs : + num_abs / den_abs; // If the result is not representable by an int64_t, bail out. if ((is_negative && quot_abs > MAX_ABS_INT64) || (!is_negative && quot_abs >= MAX_ABS_INT64)) { @@ -130,8 +134,8 @@ FUZZ_TARGET(feefrac_div_fallback) } // Verify the behavior of FeeFrac::DivFallback. - auto res = FeeFrac::DivFallback(num, den); - assert((res < 0) == is_negative); + auto res = FeeFrac::DivFallback(num, den, round_down); + assert(res == 0 || (res < 0) == is_negative); assert(Abs256(res) == quot_abs); } @@ -142,41 +146,47 @@ FUZZ_TARGET(feefrac_mul_div) // - The combination of FeeFrac::MulFallback + FeeFrac::DivFallback. // - FeeFrac::Evaluate. - // Construct a 32-bit signed multiplicand, a 64-bit signed multiplicand, and a positive 31-bit - // divisor. + // Construct a 32-bit signed multiplicand, a 64-bit signed multiplicand, a positive 31-bit + // divisor, and a rounding mode. FuzzedDataProvider provider(buffer.data(), buffer.size()); auto mul32 = provider.ConsumeIntegral(); auto mul64 = provider.ConsumeIntegral(); auto div = provider.ConsumeIntegralInRange(1, std::numeric_limits::max()); + auto round_down = provider.ConsumeBool(); // Predict the sign of the overall result. bool is_negative = ((mul32 < 0) && (mul64 > 0)) || ((mul32 > 0) && (mul64 < 0)); - // Evaluate absolute value using arith_uint256. If the actual result is negative, the absolute - // value of the quotient is the rounded-up quotient of the absolute values. + // Evaluate absolute value using arith_uint256. If the actual result is negative and we are + // rounding down or positive and we rounding up, the absolute value of the quotient is the + // rounded-up quotient of the absolute values. auto prod_abs = Abs256(mul32) * Abs256(mul64); auto div_abs = Abs256(div); - auto quot_abs = is_negative ? (prod_abs + div_abs - 1) / div_abs : prod_abs / div_abs; + auto quot_abs = (is_negative == round_down) ? + (prod_abs + div_abs - 1) / div_abs : + prod_abs / div_abs; // If the result is not representable by an int64_t, bail out. if ((is_negative && quot_abs > MAX_ABS_INT64) || (!is_negative && quot_abs >= MAX_ABS_INT64)) { // If 0 <= mul32 <= div, then the result is guaranteed to be representable. In the context - // of the Evaluate call below, this corresponds to 0 <= at_size <= feefrac.size. + // of the Evaluate{Down,Up} calls below, this corresponds to 0 <= at_size <= feefrac.size. assert(mul32 < 0 || mul32 > div); return; } // Verify the behavior of FeeFrac::Mul + FeeFrac::Div. - auto res = FeeFrac::Div(FeeFrac::Mul(mul64, mul32), div); - assert((res < 0) == is_negative); + auto res = FeeFrac::Div(FeeFrac::Mul(mul64, mul32), div, round_down); + assert(res == 0 || (res < 0) == is_negative); assert(Abs256(res) == quot_abs); // Verify the behavior of FeeFrac::MulFallback + FeeFrac::DivFallback. - auto res_fallback = FeeFrac::DivFallback(FeeFrac::MulFallback(mul64, mul32), div); + auto res_fallback = FeeFrac::DivFallback(FeeFrac::MulFallback(mul64, mul32), div, round_down); assert(res == res_fallback); - // Verify the behavior of FeeFrac::Evaluate. + // Verify the behavior of FeeFrac::Evaluate{Down,Up}. if (mul32 > 0) { - auto res_fee = FeeFrac{mul64, div}.EvaluateFee(mul32); + auto res_fee = round_down ? + FeeFrac{mul64, div}.EvaluateFeeDown(mul32) : + FeeFrac{mul64, div}.EvaluateFeeUp(mul32); assert(res == res_fee); } } diff --git a/src/util/feefrac.h b/src/util/feefrac.h index b3098e0d48c..6c2fd934eeb 100644 --- a/src/util/feefrac.h +++ b/src/util/feefrac.h @@ -47,14 +47,15 @@ struct FeeFrac return {high + (low >> 32), static_cast(low)}; } - /** Helper function for 96/32 signed division, rounding towards negative infinity. This is a - * fallback version, separate so it can be tested on platforms where it isn't actually needed. + /** Helper function for 96/32 signed division, rounding towards negative infinity (if + * round_down) or positive infinity (if !round_down). This is a fallback version, separate so + * that it can be tested on platforms where it isn't actually needed. * * The exact behavior with negative n does not really matter, but this implementation chooses - * to always round down, for consistency and testability. + * to be consistent for testability reasons. * * The result must fit in an int64_t, and d must be strictly positive. */ - static inline int64_t DivFallback(std::pair n, int32_t d) noexcept + static inline int64_t DivFallback(std::pair n, int32_t d, bool round_down) noexcept { Assume(d > 0); // Compute quot_high = n.first / d, so the result becomes @@ -64,11 +65,13 @@ struct FeeFrac // Evaluate the parenthesized expression above, so the result becomes // n_low / d + (quot_high * 2**32) int64_t n_low = ((n.first % d) << 32) + n.second; - // Evaluate the division so the result becomes quot_low + quot_high * 2**32. We need this - // division to round down however, while the / operator rounds towards zero. In case n_low - // is negative and not a multiple of size, we thus need a correction. + // Evaluate the division so the result becomes quot_low + quot_high * 2**32. It is possible + // that the / operator here rounds in the wrong direction (if n_low is not a multiple of + // size, and is (if round_down) negative, or (if !round_down) positive). If so, make a + // correction. int64_t quot_low = n_low / d; - quot_low -= (n_low % d) < 0; + int32_t mod_low = n_low % d; + quot_low += (mod_low > 0) - (mod_low && round_down); // Combine and return the result return (quot_high << 32) + quot_low; } @@ -85,13 +88,14 @@ struct FeeFrac * version relying on __int128. * * The result must fit in an int64_t, and d must be strictly positive. */ - static inline int64_t Div(__int128 n, int32_t d) noexcept + static inline int64_t Div(__int128 n, int32_t d, bool round_down) noexcept { Assume(d > 0); // Compute the division. int64_t quot = n / d; - // Make it round down. - return quot - ((n % d) < 0); + int32_t mod = n % d; + // Correct result if the / operator above rounded in the wrong direction. + return quot + (mod > 0) - (mod && round_down); } #else static constexpr auto Mul = MulFallback; @@ -186,23 +190,35 @@ struct FeeFrac /** Compute the fee for a given size `at_size` using this object's feerate. * * This effectively corresponds to evaluating (this->fee * at_size) / this->size, with the - * result rounded down (even for negative feerates). + * result rounded towards negative infinity (if RoundDown) or towards positive infinity + * (if !RoundDown). * * Requires this->size > 0, at_size >= 0, and that the correct result fits in a int64_t. This * is guaranteed to be the case when 0 <= at_size <= this->size. */ + template int64_t EvaluateFee(int32_t at_size) const noexcept { Assume(size > 0); Assume(at_size >= 0); if (fee >= 0 && fee < 0x200000000) [[likely]] { // Common case where (this->fee * at_size) is guaranteed to fit in a uint64_t. - return (uint64_t(fee) * at_size) / uint32_t(size); + if constexpr (RoundDown) { + return (uint64_t(fee) * at_size) / uint32_t(size); + } else { + return (uint64_t(fee) * at_size + size - 1U) / uint32_t(size); + } } else { // Otherwise, use Mul and Div. - return Div(Mul(fee, at_size), size); + return Div(Mul(fee, at_size), size, RoundDown); } } + +public: + /** Compute the fee for a given size `at_size` using this object's feerate, rounding down. */ + int64_t EvaluateFeeDown(int32_t at_size) const noexcept { return EvaluateFee(at_size); } + /** Compute the fee for a given size `at_size` using this object's feerate, rounding up. */ + int64_t EvaluateFeeUp(int32_t at_size) const noexcept { return EvaluateFee(at_size); } }; /** Compare the feerate diagrams implied by the provided sorted chunks data.