Skip to content
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,19 @@ pub fn processRewardsAndPenalties(
const balances = try state.balancesSlice(allocator);
defer allocator.free(balances);

// Use saturating arithmetic instead of checked (`std.math.add`):
// - Overflow is impossible: max ETH supply ~120M ETH = ~1.2e17 Gwei,
// u64 max = ~1.8e19, giving 154x headroom even with all rewards summed.
// - Saturating ops are branchless, enabling SIMD auto-vectorization of the
// balance loop (~3.4x speedup on the inner loop for 500k+ validators).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This comment is very helpful for explaining the rationale. To better align with the style guide's preference for prose in comments, consider rephrasing it as a paragraph.

From the style guide:

Comments are sentences, with a space after the slash, with a capital letter and a full stop, or a colon if they relate to something that follows. Comments are well-written prose describing the code, not just scribblings in the margin. (lines 304-306)

    // Use saturating arithmetic for performance. Saturating operations are branchless, which
    // enables SIMD auto-vectorization of the balance update loop. This is safe because a
    // balance overflow is mathematically impossible: the maximum total ETH supply (~1.2e17 Gwei)
    // is more than 150x smaller than the max u64 value (~1.8e19 Gwei).
References
  1. Comments should be well-written prose in the form of sentences, rather than lists of points. (link)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 29d60ec. Added std.debug.assert for the invariant and rewrote the comment as prose.

if (slashing_penalties) |slashings| {
for (rewards, penalties, balances, 0..) |reward, penalty, *balance, i| {
const slashing: u64 = if (i < slashings.len) slashings[i] else 0;
balance.* = (try std.math.add(u64, balance.*, reward)) -| penalty -| slashing;
balance.* = ((balance.* +| reward) -| penalty) -| slashing;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To uphold the style guide's emphasis on safety (Safety, performance, and developer experience. In that order., line 5), it's best to add a std.debug.assert here. This will verify the no-overflow assumption during testing without impacting performance in ReleaseFast builds. This also serves as executable documentation for the invariant you've identified.

From the style guide:

Assert all function arguments and return values, pre/postconditions and invariants. (line 51)

I've also removed a redundant set of parentheses for cleaner code.

            std.debug.assert(balance.* <= std.math.maxInt(u64) - reward);
            balance.* = (balance.* +| reward) -| penalty -| slashing;
References
  1. The style guide prioritizes safety over performance and mandates asserting all pre/postconditions and invariants. Adding a debug assertion maintains safety during testing without compromising release performance. (link)

}
} else {
for (rewards, penalties, balances) |reward, penalty, *balance| {
balance.* = (try std.math.add(u64, balance.*, reward)) -| penalty;
balance.* = (balance.* +| reward) -| penalty;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similarly, please add a std.debug.assert here to ensure safety during testing, in line with the style guide's emphasis on assertions.

            std.debug.assert(balance.* <= std.math.maxInt(u64) - reward);
            balance.* = (balance.* +| reward) -| penalty;
References
  1. The style guide prioritizes safety over performance and mandates asserting all pre/postconditions and invariants. Adding a debug assertion maintains safety during testing without compromising release performance. (link)

}
}

Expand Down
Loading