Feat: refactor fixed-point math in `pox_5_make_reward_set` by aaronb-stacks · Pull Request #7087 · stacks-network/stacks-core

aaronb-stacks · 2026-04-07T20:10:15Z

This does a bit of refactoring of pox_5_make_reward_set and the uint module in stacks-common. In the first case, it introduces and uses a FixedPointU256 struct to perform the reward set math -- this changed the precision, but the previous precision was pretty unnecessary (64 fractional bits is plenty).

The refactor of the wide uint types should bring some performance gains -- using rust's native multiplication, subtraction, and addition operations for supporting wider types (carrying_* and borrowing_sub) allowing for in-place modification for the self-consuming implementations, and using reference args for the cloning implementations. This avoids a fair number of clones in multiplication in particular.

Add new FixedPointU256 struct to uint in common
Use fewer bits of scaling -- 128 bits is totally unnecessary
Add checked variants and some more efficient calculations to wide uints
Add proptests and extreme value tests to ensure PoX calculations do not overflow

* Add new FixedPointU256 struct to `uint` in common * Use fewer bits of scaling -- 128 bits is totally unnecessary * Add checked variants and some more efficient calculations to wide uints * Add proptests and extreme value tests to ensure PoX calculations do not overflow * Refactor wide uint macros to use `const`s, `Self`, `Self::N_WORDS` where possible

aaronb-stacks · 2026-04-08T15:49:31Z

stacks-common/src/util/uint.rs

-                for i in 0..(2 * $n_words) {
-                    let to_mul = (other >> (32 * i)).low_u32();
-                    me = me + (self.mul_u32(to_mul) << (32 * i));
-                }


Each "add" here can do 2-4 clones of the wide type (because of how it implements carry and because, despite consuming the input arg, it allocates new arrays for the return and carry). Each shift is another clone, and mul_u32 also clones. This means that each iteration of the loop performs 5-7 clones, so multiplication ends up performing (10-14 * N_WORDS) clones.

brice-stacks

This looks great! 🙏

changelog.d/7087-fixed-point-struct.added

coveralls · 2026-04-08T20:09:25Z

Coverage Report for CI Build 24153681013

Coverage increased (+39.0%) to 85.62%

Details

Coverage increased (+39.0%) from the base build.
Patch coverage: 59 uncovered changes across 2 files (462 of 521 lines covered, 88.68%).
1591 coverage regressions across 39 files.

Uncovered Changes

File	Changed	Covered	%
stackslib/src/chainstate/nakamoto/signer_set.rs	94	57	60.64%
stacks-common/src/util/uint.rs	427	405	94.85%

Coverage Regressions

1591 previously-covered lines in 39 files lost coverage.

Top 10 Files by Coverage Loss	Lines Losing Coverage	Coverage
stackslib/src/chainstate/stacks/boot/mod.rs	250	81.88%
stackslib/src/config/mod.rs	216	68.33%
stackslib/src/chainstate/stacks/db/transactions.rs	188	92.77%
clarity/src/vm/database/structures.rs	141	80.67%
stackslib/src/chainstate/nakamoto/signer_set.rs	133	72.48%
clarity/src/vm/database/clarity_db.rs	110	80.79%
stackslib/src/chainstate/coordinator/mod.rs	107	81.95%
stackslib/src/chainstate/stacks/db/blocks.rs	101	87.21%
stacks-node/src/burnchains/bitcoin_regtest_controller.rs	81	86.85%
pox-locking/src/events.rs	49	81.07%

Coverage Stats


Relevant Lines:	221232
Covered Lines:	189419
Line Coverage:	85.62%
Coverage Strength:	34364888.68 hits per line

💛 - Coveralls

brice-stacks

👍

jcnelson · 2026-04-09T20:40:21Z

stacks-common/src/util/uint.rs

+impl<const SCALE: u16> FixedPointU256<SCALE> {
+    /// Compile-time assertion that `SCALE < 128`, ensuring that even
+    /// `u128::MAX` can always be represented without overflow.
+    const _ASSERT_SCALE: () = assert!(SCALE < 128, "SCALE must be less than 128");


You might want to make this 64 instead of 128, since it impacts how big the numbers can be under multiplication. For example, if SCALE = 127, then FixedPointU256::from_u64(1) * FixedPointU256::from_u64(1) would require computing 1 << 254. You couldn't even compute FixedPointU256::from_u64(10) * FixedPointU256::from_u64(1).

I think that's fine, though. Checked math would return None in those cases, and the other options would wrap. This assertion is basically here to avoid needing to make from_u128() and from_u64() fallible.

jcnelson · 2026-04-09T21:09:03Z

stacks-common/src/util/uint.rs

+    /// Returns None if overflowed
+    pub fn pow(&self, exp: u32) -> Option<Self> {
+        if exp == 0 {
+            return Some(Self::MINIMAL);


Shouldn't this be Some(Self::from_u64(1))?

jcnelson · 2026-04-09T21:11:11Z

stacks-common/src/util/uint.rs

+        let mut high = self.clone();
+        loop {
+            if high <= low {
+                return Some(high.min(low));


Wouldn't this just be high?

stacks-common/src/util/uint.rs

jcnelson · 2026-04-09T21:19:07Z

stacks-common/src/util/uint.rs

+
+    /// Find the `n`th root of self using binary search, trimming the scale to `self.scaling` on
+    ///  iterations
+    pub fn find_root_floor(&self, n: u32) -> Option<Self> {


I get that the algorithm calls for computing a weighted geomean, but you don't need to employ an nth-root function if you have an integer log-2 function (which I think is easier to come by and less risky to implement). I mentioned this elsewhere, but I'll repost it here:

You don't have to take the 15th-root of anything; you could instead compute this: ( 5 * ilog2(STX0) + 4 * ilog2(STX1) + 3 * ilog2(STX2) + 2 * ilog2(STX3) + ilog2(STX4) ) ( ------------------------------------------------------------------------------------ ) ( 5 * ilog2(BTC0) + 4 * ilog2(BTC1) + 3 * ilog2(BTC2) + 2 * ilog2(BTC3) + ilog2(BTC4) ) GeoMean(STX/BTC) = 2 where ilog2(x) is the integer base-2 logarithm of x (which is easy to compute on fix-point numbers).

The nice thing IMHO about ilog2(x) is that because its range is bounded for all reasonable values of x, it's trivial to implement as a look-up table. If needed, you could interpolate between two points in the table for extra precision.

Yeah, that's better.

Hmm. This means dealing with negative values, right?

If the units of STX and BTC are uSTX and satoshis, respectively, then the range of ilog2(x) would always be positive since you can't have a fractional uSTX or satoshi.

Actually, I'm not sure how much this alternative representation saves us in complexity. You're still computing a fractional power of 2 in essentially all cases, which necessitates an nth-root fixed point function (unless there's a fast fixed-point algorithm that works specifically for fractional powers of 2).

jcnelson · 2026-04-13T01:12:53Z

stacks-common/src/util/uint.rs

+            if high <= low {
+                return Some(high.min(low));
+            }
+            let guess = high.clone().add(&low)?.div(Uint256::from_u64(2))?;


You could perhaps speed up this algorithm if your initial guess was simply computed from an f64, cast to a fixed-point.

aaronb-stacks force-pushed the feat/improve-fixed-point branch from 772a43a to 427b3cd Compare April 8, 2026 15:34

aaronb-stacks changed the title ~~Feat: refactor fixed-point math in pox_5_make_reward_set (draft)~~ Feat: refactor fixed-point math in pox_5_make_reward_set Apr 8, 2026

aaronb-stacks requested review from brice-stacks, hstove-stacks and jcnelson April 8, 2026 15:35

aaronb-stacks commented Apr 8, 2026

View reviewed changes

brice-stacks approved these changes Apr 8, 2026

View reviewed changes

brice-stacks reviewed Apr 8, 2026

View reviewed changes

changelog.d/7087-fixed-point-struct.added Show resolved Hide resolved

aaronb-stacks added 2 commits April 8, 2026 14:03

refactor: one more round of improvement in uint, broader testing

8c17fb8

fix changelog location

3421703

brice-stacks approved these changes Apr 8, 2026

View reviewed changes

jcnelson reviewed Apr 9, 2026

View reviewed changes

stacks-common/src/util/uint.rs Show resolved Hide resolved

jcnelson reviewed Apr 9, 2026

View reviewed changes

jcnelson reviewed Apr 13, 2026

View reviewed changes

Conversation

aaronb-stacks commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brice-stacks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coveralls commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report for CI Build 24153681013

Coverage increased (+39.0%) to 85.62%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

brice-stacks left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aaronb-stacks commented Apr 7, 2026 •

edited

Loading

coveralls commented Apr 8, 2026 •

edited

Loading