Skip to content

Commit 0c6bcfd

Browse files
sipal0rinc
andcommitted
feefrac: support both rounding up and down for Evaluate
Co-Authored-By: l0rinc <pap.lorinc@gmail.com>
1 parent ecf956e commit 0c6bcfd

File tree

3 files changed

+126
-67
lines changed

3 files changed

+126
-67
lines changed

src/test/feefrac_tests.cpp

Lines changed: 63 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -17,26 +17,43 @@ BOOST_AUTO_TEST_CASE(feefrac_operators)
1717
FeeFrac empty{0, 0};
1818
FeeFrac zero_fee{0, 1}; // zero-fee allowed
1919

20-
BOOST_CHECK_EQUAL(zero_fee.EvaluateFee(0), 0);
21-
BOOST_CHECK_EQUAL(zero_fee.EvaluateFee(1), 0);
22-
BOOST_CHECK_EQUAL(zero_fee.EvaluateFee(1000000), 0);
23-
BOOST_CHECK_EQUAL(zero_fee.EvaluateFee(0x7fffffff), 0);
24-
25-
BOOST_CHECK_EQUAL(p1.EvaluateFee(0), 0);
26-
BOOST_CHECK_EQUAL(p1.EvaluateFee(1), 10);
27-
BOOST_CHECK_EQUAL(p1.EvaluateFee(100000000), 1000000000);
28-
BOOST_CHECK_EQUAL(p1.EvaluateFee(0x7fffffff), int64_t(0x7fffffff) * 10);
20+
BOOST_CHECK_EQUAL(zero_fee.EvaluateFeeDown(0), 0);
21+
BOOST_CHECK_EQUAL(zero_fee.EvaluateFeeDown(1), 0);
22+
BOOST_CHECK_EQUAL(zero_fee.EvaluateFeeDown(1000000), 0);
23+
BOOST_CHECK_EQUAL(zero_fee.EvaluateFeeDown(0x7fffffff), 0);
24+
BOOST_CHECK_EQUAL(zero_fee.EvaluateFeeUp(0), 0);
25+
BOOST_CHECK_EQUAL(zero_fee.EvaluateFeeUp(1), 0);
26+
BOOST_CHECK_EQUAL(zero_fee.EvaluateFeeUp(1000000), 0);
27+
BOOST_CHECK_EQUAL(zero_fee.EvaluateFeeUp(0x7fffffff), 0);
28+
29+
BOOST_CHECK_EQUAL(p1.EvaluateFeeDown(0), 0);
30+
BOOST_CHECK_EQUAL(p1.EvaluateFeeDown(1), 10);
31+
BOOST_CHECK_EQUAL(p1.EvaluateFeeDown(100000000), 1000000000);
32+
BOOST_CHECK_EQUAL(p1.EvaluateFeeDown(0x7fffffff), int64_t(0x7fffffff) * 10);
33+
BOOST_CHECK_EQUAL(p1.EvaluateFeeUp(0), 0);
34+
BOOST_CHECK_EQUAL(p1.EvaluateFeeUp(1), 10);
35+
BOOST_CHECK_EQUAL(p1.EvaluateFeeUp(100000000), 1000000000);
36+
BOOST_CHECK_EQUAL(p1.EvaluateFeeUp(0x7fffffff), int64_t(0x7fffffff) * 10);
2937

3038
FeeFrac neg{-1001, 100};
31-
BOOST_CHECK_EQUAL(neg.EvaluateFee(0), 0);
32-
BOOST_CHECK_EQUAL(neg.EvaluateFee(1), -11);
33-
BOOST_CHECK_EQUAL(neg.EvaluateFee(2), -21);
34-
BOOST_CHECK_EQUAL(neg.EvaluateFee(3), -31);
35-
BOOST_CHECK_EQUAL(neg.EvaluateFee(100), -1001);
36-
BOOST_CHECK_EQUAL(neg.EvaluateFee(101), -1012);
37-
BOOST_CHECK_EQUAL(neg.EvaluateFee(100000000), -1001000000);
38-
BOOST_CHECK_EQUAL(neg.EvaluateFee(100000001), -1001000011);
39-
BOOST_CHECK_EQUAL(neg.EvaluateFee(0x7fffffff), -21496311307);
39+
BOOST_CHECK_EQUAL(neg.EvaluateFeeDown(0), 0);
40+
BOOST_CHECK_EQUAL(neg.EvaluateFeeDown(1), -11);
41+
BOOST_CHECK_EQUAL(neg.EvaluateFeeDown(2), -21);
42+
BOOST_CHECK_EQUAL(neg.EvaluateFeeDown(3), -31);
43+
BOOST_CHECK_EQUAL(neg.EvaluateFeeDown(100), -1001);
44+
BOOST_CHECK_EQUAL(neg.EvaluateFeeDown(101), -1012);
45+
BOOST_CHECK_EQUAL(neg.EvaluateFeeDown(100000000), -1001000000);
46+
BOOST_CHECK_EQUAL(neg.EvaluateFeeDown(100000001), -1001000011);
47+
BOOST_CHECK_EQUAL(neg.EvaluateFeeDown(0x7fffffff), -21496311307);
48+
BOOST_CHECK_EQUAL(neg.EvaluateFeeUp(0), 0);
49+
BOOST_CHECK_EQUAL(neg.EvaluateFeeUp(1), -10);
50+
BOOST_CHECK_EQUAL(neg.EvaluateFeeUp(2), -20);
51+
BOOST_CHECK_EQUAL(neg.EvaluateFeeUp(3), -30);
52+
BOOST_CHECK_EQUAL(neg.EvaluateFeeUp(100), -1001);
53+
BOOST_CHECK_EQUAL(neg.EvaluateFeeUp(101), -1011);
54+
BOOST_CHECK_EQUAL(neg.EvaluateFeeUp(100000000), -1001000000);
55+
BOOST_CHECK_EQUAL(neg.EvaluateFeeUp(100000001), -1001000010);
56+
BOOST_CHECK_EQUAL(neg.EvaluateFeeUp(0x7fffffff), -21496311306);
4057

4158
BOOST_CHECK(empty == FeeFrac{}); // same as no-args
4259

@@ -88,15 +105,22 @@ BOOST_AUTO_TEST_CASE(feefrac_operators)
88105
BOOST_CHECK(oversized_1 << oversized_2);
89106
BOOST_CHECK(oversized_1 != oversized_2);
90107

91-
BOOST_CHECK_EQUAL(oversized_1.EvaluateFee(0), 0);
92-
BOOST_CHECK_EQUAL(oversized_1.EvaluateFee(1), 1152921);
93-
BOOST_CHECK_EQUAL(oversized_1.EvaluateFee(2), 2305843);
94-
BOOST_CHECK_EQUAL(oversized_1.EvaluateFee(1548031267), 1784758530396540);
95-
96-
// Test cases on the threshold where FeeFrac::EvaluateFee start using Mul/Div.
97-
BOOST_CHECK_EQUAL(FeeFrac(0x1ffffffff, 123456789).EvaluateFee(98765432), 6871947728);
98-
BOOST_CHECK_EQUAL(FeeFrac(0x200000000, 123456789).EvaluateFee(98765432), 6871947729);
99-
BOOST_CHECK_EQUAL(FeeFrac(0x200000001, 123456789).EvaluateFee(98765432), 6871947730);
108+
BOOST_CHECK_EQUAL(oversized_1.EvaluateFeeDown(0), 0);
109+
BOOST_CHECK_EQUAL(oversized_1.EvaluateFeeDown(1), 1152921);
110+
BOOST_CHECK_EQUAL(oversized_1.EvaluateFeeDown(2), 2305843);
111+
BOOST_CHECK_EQUAL(oversized_1.EvaluateFeeDown(1548031267), 1784758530396540);
112+
BOOST_CHECK_EQUAL(oversized_1.EvaluateFeeUp(0), 0);
113+
BOOST_CHECK_EQUAL(oversized_1.EvaluateFeeUp(1), 1152922);
114+
BOOST_CHECK_EQUAL(oversized_1.EvaluateFeeUp(2), 2305843);
115+
BOOST_CHECK_EQUAL(oversized_1.EvaluateFeeUp(1548031267), 1784758530396541);
116+
117+
// Test cases on the threshold where FeeFrac::Evaluate start using Mul/Div.
118+
BOOST_CHECK_EQUAL(FeeFrac(0x1ffffffff, 123456789).EvaluateFeeDown(98765432), 6871947728);
119+
BOOST_CHECK_EQUAL(FeeFrac(0x200000000, 123456789).EvaluateFeeDown(98765432), 6871947729);
120+
BOOST_CHECK_EQUAL(FeeFrac(0x200000001, 123456789).EvaluateFeeDown(98765432), 6871947730);
121+
BOOST_CHECK_EQUAL(FeeFrac(0x1ffffffff, 123456789).EvaluateFeeUp(98765432), 6871947729);
122+
BOOST_CHECK_EQUAL(FeeFrac(0x200000000, 123456789).EvaluateFeeUp(98765432), 6871947730);
123+
BOOST_CHECK_EQUAL(FeeFrac(0x200000001, 123456789).EvaluateFeeUp(98765432), 6871947731);
100124

101125
// Tests paths that use double arithmetic
102126
FeeFrac busted{(static_cast<int64_t>(INT32_MAX)) + 1, INT32_MAX};
@@ -108,12 +132,18 @@ BOOST_AUTO_TEST_CASE(feefrac_operators)
108132
BOOST_CHECK(max_fee <= max_fee);
109133
BOOST_CHECK(max_fee >= max_fee);
110134

111-
BOOST_CHECK_EQUAL(max_fee.EvaluateFee(0), 0);
112-
BOOST_CHECK_EQUAL(max_fee.EvaluateFee(1), 977888);
113-
BOOST_CHECK_EQUAL(max_fee.EvaluateFee(2), 1955777);
114-
BOOST_CHECK_EQUAL(max_fee.EvaluateFee(3), 2933666);
115-
BOOST_CHECK_EQUAL(max_fee.EvaluateFee(1256796054), 1229006664189047);
116-
BOOST_CHECK_EQUAL(max_fee.EvaluateFee(INT32_MAX), 2100000000000000);
135+
BOOST_CHECK_EQUAL(max_fee.EvaluateFeeDown(0), 0);
136+
BOOST_CHECK_EQUAL(max_fee.EvaluateFeeDown(1), 977888);
137+
BOOST_CHECK_EQUAL(max_fee.EvaluateFeeDown(2), 1955777);
138+
BOOST_CHECK_EQUAL(max_fee.EvaluateFeeDown(3), 2933666);
139+
BOOST_CHECK_EQUAL(max_fee.EvaluateFeeDown(1256796054), 1229006664189047);
140+
BOOST_CHECK_EQUAL(max_fee.EvaluateFeeDown(INT32_MAX), 2100000000000000);
141+
BOOST_CHECK_EQUAL(max_fee.EvaluateFeeUp(0), 0);
142+
BOOST_CHECK_EQUAL(max_fee.EvaluateFeeUp(1), 977889);
143+
BOOST_CHECK_EQUAL(max_fee.EvaluateFeeUp(2), 1955778);
144+
BOOST_CHECK_EQUAL(max_fee.EvaluateFeeUp(3), 2933667);
145+
BOOST_CHECK_EQUAL(max_fee.EvaluateFeeUp(1256796054), 1229006664189048);
146+
BOOST_CHECK_EQUAL(max_fee.EvaluateFeeUp(INT32_MAX), 2100000000000000);
117147

118148
FeeFrac max_fee2{1, 1};
119149
BOOST_CHECK(max_fee >= max_fee2);

src/test/fuzz/feefrac.cpp

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -110,33 +110,38 @@ FUZZ_TARGET(feefrac_div_fallback)
110110
{
111111
// Verify the behavior of FeeFrac::DivFallback over all possible inputs.
112112

113-
// Construct a 96-bit signed value num, and positive 31-bit value den.
113+
// Construct a 96-bit signed value num, a positive 31-bit value den, and rounding mode.
114114
FuzzedDataProvider provider(buffer.data(), buffer.size());
115115
auto num_high = provider.ConsumeIntegral<int64_t>();
116116
auto num_low = provider.ConsumeIntegral<uint32_t>();
117117
std::pair<int64_t, uint32_t> num{num_high, num_low};
118118
auto den = provider.ConsumeIntegralInRange<int32_t>(1, std::numeric_limits<int32_t>::max());
119+
auto round_down = provider.ConsumeBool();
119120

120121
// Predict the sign of the actual result.
121122
bool is_negative = num_high < 0;
122-
// Evaluate absolute value using arith_uint256. If the actual result is negative, the absolute
123-
// value of the quotient is the rounded-up quotient of the absolute values.
123+
// Evaluate absolute value using arith_uint256. If the actual result is negative and we are
124+
// rounding down, or positive and we are rounding up, the absolute value of the quotient is
125+
// the rounded-up quotient of the absolute values.
124126
auto num_abs = Abs256(num);
125127
auto den_abs = Abs256(den);
126-
auto quot_abs = is_negative ? (num_abs + den_abs - 1) / den_abs : num_abs / den_abs;
128+
auto quot_abs = (is_negative == round_down) ?
129+
(num_abs + den_abs - 1) / den_abs :
130+
num_abs / den_abs;
127131

128132
// If the result is not representable by an int64_t, bail out.
129133
if ((is_negative && quot_abs > MAX_ABS_INT64) || (!is_negative && quot_abs >= MAX_ABS_INT64)) {
130134
return;
131135
}
132136

133137
// Verify the behavior of FeeFrac::DivFallback.
134-
auto res = FeeFrac::DivFallback(num, den);
135-
assert((res < 0) == is_negative);
138+
auto res = FeeFrac::DivFallback(num, den, round_down);
139+
assert(res == 0 || (res < 0) == is_negative);
136140
assert(Abs256(res) == quot_abs);
137141

138142
// Compare approximately with floating-point.
139-
long double expect = std::floor(num_high * 4294967296.0L + num_low) / den;
143+
long double expect = round_down ? std::floor(num_high * 4294967296.0L + num_low) / den
144+
: std::ceil(num_high * 4294967296.0L + num_low) / den;
140145
// Expect to be accurate within 50 bits of precision, +- 1 sat.
141146
if (expect == 0.0L) {
142147
assert(res >= -1 && res <= 1);
@@ -156,40 +161,45 @@ FUZZ_TARGET(feefrac_mul_div)
156161
// - The combination of FeeFrac::MulFallback + FeeFrac::DivFallback.
157162
// - FeeFrac::Evaluate.
158163

159-
// Construct a 32-bit signed multiplicand, a 64-bit signed multiplicand, and a positive 31-bit
160-
// divisor.
164+
// Construct a 32-bit signed multiplicand, a 64-bit signed multiplicand, a positive 31-bit
165+
// divisor, and a rounding mode.
161166
FuzzedDataProvider provider(buffer.data(), buffer.size());
162167
auto mul32 = provider.ConsumeIntegral<int32_t>();
163168
auto mul64 = provider.ConsumeIntegral<int64_t>();
164169
auto div = provider.ConsumeIntegralInRange<int32_t>(1, std::numeric_limits<int32_t>::max());
170+
auto round_down = provider.ConsumeBool();
165171

166172
// Predict the sign of the overall result.
167173
bool is_negative = ((mul32 < 0) && (mul64 > 0)) || ((mul32 > 0) && (mul64 < 0));
168-
// Evaluate absolute value using arith_uint256. If the actual result is negative, the absolute
169-
// value of the quotient is the rounded-up quotient of the absolute values.
174+
// Evaluate absolute value using arith_uint256. If the actual result is negative and we are
175+
// rounding down or positive and we rounding up, the absolute value of the quotient is the
176+
// rounded-up quotient of the absolute values.
170177
auto prod_abs = Abs256(mul32) * Abs256(mul64);
171178
auto div_abs = Abs256(div);
172-
auto quot_abs = is_negative ? (prod_abs + div_abs - 1) / div_abs : prod_abs / div_abs;
179+
auto quot_abs = (is_negative == round_down) ?
180+
(prod_abs + div_abs - 1) / div_abs :
181+
prod_abs / div_abs;
173182

174183
// If the result is not representable by an int64_t, bail out.
175184
if ((is_negative && quot_abs > MAX_ABS_INT64) || (!is_negative && quot_abs >= MAX_ABS_INT64)) {
176185
// If 0 <= mul32 <= div, then the result is guaranteed to be representable. In the context
177-
// of the Evaluate call below, this corresponds to 0 <= at_size <= feefrac.size.
186+
// of the Evaluate{Down,Up} calls below, this corresponds to 0 <= at_size <= feefrac.size.
178187
assert(mul32 < 0 || mul32 > div);
179188
return;
180189
}
181190

182191
// Verify the behavior of FeeFrac::Mul + FeeFrac::Div.
183-
auto res = FeeFrac::Div(FeeFrac::Mul(mul64, mul32), div);
184-
assert((res < 0) == is_negative);
192+
auto res = FeeFrac::Div(FeeFrac::Mul(mul64, mul32), div, round_down);
193+
assert(res == 0 || (res < 0) == is_negative);
185194
assert(Abs256(res) == quot_abs);
186195

187196
// Verify the behavior of FeeFrac::MulFallback + FeeFrac::DivFallback.
188-
auto res_fallback = FeeFrac::DivFallback(FeeFrac::MulFallback(mul64, mul32), div);
197+
auto res_fallback = FeeFrac::DivFallback(FeeFrac::MulFallback(mul64, mul32), div, round_down);
189198
assert(res == res_fallback);
190199

191200
// Compare approximately with floating-point.
192-
long double expect = std::floor(static_cast<long double>(mul32) * mul64 / div);
201+
long double expect = round_down ? std::floor(static_cast<long double>(mul32) * mul64 / div)
202+
: std::ceil(static_cast<long double>(mul32) * mul64 / div);
193203
// Expect to be accurate within 50 bits of precision, +- 1 sat.
194204
if (expect == 0.0L) {
195205
assert(res >= -1 && res <= 1);
@@ -201,9 +211,11 @@ FUZZ_TARGET(feefrac_mul_div)
201211
assert(res <= expect * 0.999999999999999L + 1.0L);
202212
}
203213

204-
// Verify the behavior of FeeFrac::Evaluate.
214+
// Verify the behavior of FeeFrac::Evaluate{Down,Up}.
205215
if (mul32 >= 0) {
206-
auto res_fee = FeeFrac{mul64, div}.EvaluateFee(mul32);
216+
auto res_fee = round_down ?
217+
FeeFrac{mul64, div}.EvaluateFeeDown(mul32) :
218+
FeeFrac{mul64, div}.EvaluateFeeUp(mul32);
207219
assert(res == res_fee);
208220
}
209221
}

src/util/feefrac.h

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,15 @@ struct FeeFrac
4747
return {high + (low >> 32), static_cast<uint32_t>(low)};
4848
}
4949

50-
/** Helper function for 96/32 signed division, rounding towards negative infinity. This is a
51-
* fallback version, separate so it can be tested on platforms where it isn't actually needed.
50+
/** Helper function for 96/32 signed division, rounding towards negative infinity (if
51+
* round_down) or positive infinity (if !round_down). This is a fallback version, separate so
52+
* that it can be tested on platforms where it isn't actually needed.
5253
*
5354
* The exact behavior with negative n does not really matter, but this implementation chooses
54-
* to always round down, for consistency and testability.
55+
* to be consistent for testability reasons.
5556
*
5657
* The result must fit in an int64_t, and d must be strictly positive. */
57-
static inline int64_t DivFallback(std::pair<int64_t, uint32_t> n, int32_t d) noexcept
58+
static inline int64_t DivFallback(std::pair<int64_t, uint32_t> n, int32_t d, bool round_down) noexcept
5859
{
5960
Assume(d > 0);
6061
// Compute quot_high = n.first / d, so the result becomes
@@ -64,11 +65,13 @@ struct FeeFrac
6465
// Evaluate the parenthesized expression above, so the result becomes
6566
// n_low / d + (quot_high * 2**32)
6667
int64_t n_low = ((n.first % d) << 32) + n.second;
67-
// Evaluate the division so the result becomes quot_low + quot_high * 2**32. We need this
68-
// division to round down however, while the / operator rounds towards zero. In case n_low
69-
// is negative and not a multiple of size, we thus need a correction.
68+
// Evaluate the division so the result becomes quot_low + quot_high * 2**32. It is possible
69+
// that the / operator here rounds in the wrong direction (if n_low is not a multiple of
70+
// size, and is (if round_down) negative, or (if !round_down) positive). If so, make a
71+
// correction.
7072
int64_t quot_low = n_low / d;
71-
quot_low -= (n_low % d) < 0;
73+
int32_t mod_low = n_low % d;
74+
quot_low += (mod_low > 0) - (mod_low && round_down);
7275
// Combine and return the result
7376
return (quot_high << 32) + quot_low;
7477
}
@@ -81,17 +84,19 @@ struct FeeFrac
8184
return __int128{a} * b;
8285
}
8386

84-
/** Helper function for 96/32 signed division, rounding towards negative infinity. This is a
87+
/** Helper function for 96/32 signed division, rounding towards negative infinity (if
88+
* round_down), or towards positive infinity (if !round_down). This is a
8589
* version relying on __int128.
8690
*
8791
* The result must fit in an int64_t, and d must be strictly positive. */
88-
static inline int64_t Div(__int128 n, int32_t d) noexcept
92+
static inline int64_t Div(__int128 n, int32_t d, bool round_down) noexcept
8993
{
9094
Assume(d > 0);
9195
// Compute the division.
9296
int64_t quot = n / d;
93-
// Make it round down.
94-
return quot - ((n % d) < 0);
97+
int32_t mod = n % d;
98+
// Correct result if the / operator above rounded in the wrong direction.
99+
return quot + (mod > 0) - (mod && round_down);
95100
}
96101
#else
97102
static constexpr auto Mul = MulFallback;
@@ -186,23 +191,35 @@ struct FeeFrac
186191
/** Compute the fee for a given size `at_size` using this object's feerate.
187192
*
188193
* This effectively corresponds to evaluating (this->fee * at_size) / this->size, with the
189-
* result rounded down (even for negative feerates).
194+
* result rounded towards negative infinity (if RoundDown) or towards positive infinity
195+
* (if !RoundDown).
190196
*
191197
* Requires this->size > 0, at_size >= 0, and that the correct result fits in a int64_t. This
192198
* is guaranteed to be the case when 0 <= at_size <= this->size.
193199
*/
200+
template<bool RoundDown>
194201
int64_t EvaluateFee(int32_t at_size) const noexcept
195202
{
196203
Assume(size > 0);
197204
Assume(at_size >= 0);
198205
if (fee >= 0 && fee < 0x200000000) [[likely]] {
199206
// Common case where (this->fee * at_size) is guaranteed to fit in a uint64_t.
200-
return (uint64_t(fee) * at_size) / uint32_t(size);
207+
if constexpr (RoundDown) {
208+
return (uint64_t(fee) * at_size) / uint32_t(size);
209+
} else {
210+
return (uint64_t(fee) * at_size + size - 1U) / uint32_t(size);
211+
}
201212
} else {
202213
// Otherwise, use Mul and Div.
203-
return Div(Mul(fee, at_size), size);
214+
return Div(Mul(fee, at_size), size, RoundDown);
204215
}
205216
}
217+
218+
public:
219+
/** Compute the fee for a given size `at_size` using this object's feerate, rounding down. */
220+
int64_t EvaluateFeeDown(int32_t at_size) const noexcept { return EvaluateFee<true>(at_size); }
221+
/** Compute the fee for a given size `at_size` using this object's feerate, rounding up. */
222+
int64_t EvaluateFeeUp(int32_t at_size) const noexcept { return EvaluateFee<false>(at_size); }
206223
};
207224

208225
/** Compare the feerate diagrams implied by the provided sorted chunks data.

0 commit comments

Comments
 (0)