Reduce uint256 fields to smaller integer types for gas and storage savings #85

pali101 · 2025-05-09T06:29:21Z

Closes #75

Summary

Most struct fields were uint256 even though their values never approach 2²⁵⁶ - 1. This PR:

Narrows each field to the smallest unsigned integer that can hold its max value (uint64 for epochs and uint16 for bps.
Reorders fields so they tightly pack into 32-byte storage slots.

Changes

Rail struct
- lockupPeriod - uint64
- settledUpTo - uint64
- endEpoch - uint64
- commissionRateBps - uint16
Account struct
- lockupLastSettledAt - uint64
PayeeCommissionLimit struct
- maxbps - uint16
Reordered all small width fields to maximize slot packing.

Gas and size impact

We ran forge test --gas-report on Foundry and compared the before/after outputs (see attached .txt files). Overall, core rail-operations (e.g. createRail, getRail, modifyRailLockup, terminateRail) saw 13–31% per-call gas savings on average and median, at the cost of a modest 0.75% increase in overall deployment cost. See details below:

Metric	Before	After	Δ	% Δ
Payments.sol deploy cost	4 760 515 gas	4 796 126 gas	+ 35 611	+ 0.75 %
Payments.sol deploy size	21 661 bytes	21 825 bytes	+ 164 bytes	+ 0.76 %
Fees.getAllAccumulatedFees	1 198 711 gas	1 175 751 gas	– 30 974	– 2.58 %
createRail (avg)	207 288 gas	180 275 gas	– 27 013	– 13.03 %
createRail (median)	275 281 gas	196 063 gas	– 790218	– 28.77 %
getRail (avg)	7 716 gas	5 659 gas	– 2 057	– 26.65 %
getRail (median)	8 735 gas	6 417 gas	– 2 318	– 26.53 %
modifyRailLockup (avg)	73 396 gas	58 175 gas	– 15 221	– 20.73 %
modifyRailLockup (median)	70 535 gas	50 493 gas	– 20 042	– 28.41 %
terminateRail (avg)	51 516 gas	37 456 gas	– 14 060	– 27.29 %
terminateRail (median)	59 308 gas	40 509 gas	– 18 799	– 31.69 %
settleRail (avg)	104 750 gas	101 935 gas	– 2 815	– 2.68 %
settleRail (median)	140 446 gas	140 773 gas	+ 327	+ 0.23 %

Optimized gas report (struct-packing)
Baseline gas report (pre-optimization)

Testing

All 79 existing Solidity tests pass unchanged.

cc @Stebalien - I've implemented the struct‐packing optimizations per #75, would love your review.

aarshkshah1992 · 2025-05-21T17:52:50Z

Hey @pali101 Thanks for the PR ! But we're not looking to make deep changes related to gas savings for now as we're still in the mode of adding features to this contract and flushing out bugs/additional features.

I would recommend taking a look at open issues to see if something catches your eye.

aarshkshah1992 · 2025-05-21T18:15:09Z

@pali101 Actually, this is good. Please can you rebase the PR on top of main and I can then review/merge this ?

…er interface

…d repack Rail struct Improve storage packing and reduce per-rail gas costs.

…uint256 to unit16) in public interfaces Affected Signatures: - setPayeeMaxCommission(address token, uint16 maxBps) - createRail(address token, address from, address to, address arbiter, uint16 commissionRateBps) - modifyRailLockup(uint256 railId, uint64 period, uint256 lockupFixed) - settleRail(uint256 railId, uint64 untilEpoch) - getRail(uint256 railId) -> returns RailView { // narrowed: uint64 lockupPeriod; uint64 settledUpTo; uint64 endEpoch; uint16 commissionRateBps; // other fields unchanged } - getRailsForPayerAndToken(address payer, address token) → returns RailInfo[] { uint64 endEpoch; other fields unchanged } - getRailsForPayeeAndToken(address payee, address token) → returns RailInfo[] { uint64 endEpoch; other fields unchanged } - RateChangeQueue.enqueue(Queue storage, uint256 rate, uint64 untilEpoch) - removed setPayeeMaxCommission function

… introduce min64 helper function - Add `min64(uint64 a, uint64 b)` to unambiguously compute the smaller of two 64-bit values. - Update settlement logic to call `min64` for all uint64 comparisons. - Ensure explicit `uint64(...)` casts when writing block numbers or division results back into 64-bit state fields.

- PaymentsTestHelpers: - getAccountData() now unpacks lockupLastSettledAt as uint64 and return Payments.Account matching new signature - assertAccountState expectedLastSettled parameter changed to uint64 - setupRailWithParameters accepts lockupPeriod as uint64 - RailSettlementHelpers: - SettlementResult.settledUpto field changed to uint64 - settleRailAndVerify and terminateAndSettleRail signatures updated to use uint64 - Added explicit `uint64(block.number)` casts where needed

…eUpto - Change `arbitratePayment` signature to `uint64 fromEpoch, uint64 toEpoch` per IArbiter - Convert `customUpto` storage and `setCustomValues` param to `uint64` - Compute and return 64-bit `settleUpto` values (e.g. cast reducedEndEpoch)

Update the following test files to use uint64 for epoch/block parameters and uint16 for BPS fields: - AccountLockupSettlement.t.sol - Fees.t.sol - RailGetters.t.sol - RateChangeQueue.t.sol - AccountManagement.t.sol - OperatorApproval.t.sol - RailSettlement.t.sol

…ix 'stack too deep'

pali101 · 2025-05-23T07:51:27Z

@aarshkshah1992 I've rebased it on top of main and it's ready for review. Let me know if you'd like any additional changes - happy to adjust as needed.
I'd also be glad to help with any other open issues!

silent-cipher

Please conduct a comprehensive audit of all uint64/uint256 type usage throughout the contract

silent-cipher · 2025-06-05T06:45:56Z

src/Payments.sol

    struct Rail {
+        // slot 0: 20 B + 8 B = 28 B used (4 B free)
        address token;
+        uint64 lockupPeriod;
+
+        // slot 1: 20 B + 8 B = 28 B used
        address from;
+        // epoch up to and including which this rail has been settled
+        uint64 settledUpTo;
+
+        // slot 2: 20 B + 8 B = 28 B used
        address to;
+        uint64 endEpoch; // Final epoch up to which the rail can be settled (0 if not terminated)
+
+        // slot 3: 20 B + 2 B = 22 B used
        address operator;
+        // Operator commission rate in basis points (e.g., 100 BPS = 1%)
+        uint16 commissionRateBps;
+
+        // slot 4: 20 B used
        address arbiter;
+
+        // slot 5: 32 B used
        uint256 paymentRate;
-        uint256 lockupPeriod;
+
+        // slot 6: 32 B used
        uint256 lockupFixed;
-        // epoch up to and including which this rail has been settled
-        uint256 settledUpTo;
+
+        // RateChangeQueue.Queue is 3×32 B (mapping + head + tail) slots 7, 8, 9
        RateChangeQueue.Queue rateChangeQueue;
-        uint256 endEpoch; // Final epoch up to which the rail can be settled (0 if not terminated)
-        // Operator commission rate in basis points (e.g., 100 BPS = 1%)
-        uint256 commissionRateBps;
    }


typical user may find these slot packing details overwhelming and may go out of sync easily

Thanks for the feedback! The slot comments are just for internal developer reference to understand the storage layout and optimize packing. While they may need manual updates, actual access to variables doesn’t change - just their arrangement for better packing and gas efficiency. They don’t impact typical users or contract behavior.

These comments are useful but are too much clutter for this place in the code. Rail struct comments should be exclusively about the semantic values of the fields. I think the correct place for this is either in a top level struct comment where you outline the complete packing behavior in one place OR in a small design document / in the README which is linked to in a top level struct comment.

A benefit of doing it this way is that the packing design will be more readable. I'm having trouble parsing the meaning of the numbers here and understanding how much space is actually being saved. An ascii diagram would probably be worth it.

Because the contract is a proxy, notation of the storage slots is an important part of the upkeep of the contract. The notes are important therefore. It would also be advantageous to declare all storage in a single file, to reduce the likelihood of underhanded or accidental slot shiftings between upgrades.

silent-cipher · 2025-06-05T06:46:47Z

src/Payments.sol

        // If there is no arbiter, settle the rail immediately
        if (rail.arbiter == address(0)) {
-            (, , , , uint256 settledUpto, ) = settleRail(railId, block.number);
+            (, , , , uint256 settledUpto, ) = settleRail(railId, uint64(block.number));


Type inconsistency throughout:

rail.settledUpTo is uint64 but compared with uint256 block.number

settledUpto should be uint64 not uint256

Mixed type comparisons without explicit casting

This was an intentional choice to avoid explicit type casting, which can introduce additional gas costs. Solidity safely allows comparisons between uint64 and uint256 as long as values are within range.

For example, comparing uint64(1) with uint256(1) evaluates correctly, and even values like uint64(123456) compared to uint256(123456) yield accurate results. Since block.number is well within the uint64 range under current and foreseeable network conditions, the logic remains safe and correct.

Avoiding the cast here is a small but deliberate gas optimization.

I understand the comparison response but shouldne't settledUpto here be declared as uint64?

aarshkshah1992 · 2025-06-16T07:04:32Z

@pali101 This is not a priority for now and so wont be merging it for a while as we're still figuring out the important work items for the payments contract and will have to prioritize accordingly. Just keeping it open for now

cc @silent-cipher @rjan90

rjan90 · 2025-07-24T12:34:35Z

I'm moving this PR back to draft to indicate that it's not expected to land anytime soon, given the upcoming code freeze and external audit.

Depending on feedback from the audit or other future input, gas and storage savings work might become a priority again. However, at this point, I don't anticipate us moving forward with it in Milestone 2.

wjmelements · 2025-07-24T21:33:14Z

src/Payments.sol

    struct Rail {
+        // slot 0: 20 B + 8 B = 28 B used (4 B free)
        address token;
+        uint64 lockupPeriod;


consider uint96 instead of uint64 in combination with address160 since you aren't using the other32

wjmelements · 2025-07-25T00:05:39Z

Another strategy for compressing large structs is to compress most of their information with a hash. The usual way to do this is to emit the uncompressed information in an event log, and have the end user resupply it later as calldata, which can then be verified by comparison to the stored hash.

pali101 mentioned this pull request May 9, 2025

Reduce uint256 fields to smaller integer types for gas and storage savings #75

Open

rjan90 added this to Payments May 21, 2025

rjan90 moved this to ⌨️ In Progress in Payments May 21, 2025

rjan90 assigned pali101 May 21, 2025

aarshkshah1992 closed this May 21, 2025

github-project-automation bot moved this from ⌨️ In Progress to 🎉 Done in Payments May 21, 2025

aarshkshah1992 reopened this May 21, 2025

rjan90 moved this from 🎉 Done to 🔎 Awaiting review in Payments May 22, 2025

pali101 force-pushed the opt/struct-packing branch from 3c7c597 to 5aca14d Compare May 23, 2025 03:54

pali101 added 9 commits May 23, 2025 09:26

Refactor: Narrow RateChange.untilEpoch to uint64 for gas optimization

c8e0b33

refactor: use uint64 for fromEpoch, toEpoch, and settleUpto in IArbit…

07e2467

…er interface

refactor: shrink epoch fields to uint64, commission BPS to uint16, an…

a73619c

…d repack Rail struct Improve storage packing and reduce per-rail gas costs.

refactor settleRailAndVerify: unpack into named SettlementResult to f…

1591fd7

…ix 'stack too deep'

pali101 force-pushed the opt/struct-packing branch from 5aca14d to 1591fd7 Compare May 23, 2025 07:38

silent-cipher reviewed Jun 5, 2025

View reviewed changes

rjan90 requested review from ZenGround0 and removed request for ZenGround0 July 2, 2025 13:52

BigLep added this to FS Jul 17, 2025

github-project-automation bot moved this to 📌 Triage in FS Jul 17, 2025

rjan90 marked this pull request as draft July 24, 2025 12:34

rjan90 added this to the MX: Priority and sequencing TBD milestone Jul 24, 2025

rjan90 moved this from 📌 Triage to 🐱 Todo in FS Jul 24, 2025

rjan90 moved this from 🔎 Awaiting review to ⌨️ In Progress in Payments Jul 24, 2025

wjmelements reviewed Jul 24, 2025

View reviewed changes

Reduce uint256 fields to smaller integer types for gas and storage savings #85

Are you sure you want to change the base?

Reduce uint256 fields to smaller integer types for gas and storage savings #85

Uh oh!

Conversation

pali101 commented May 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Gas and size impact

Testing

Uh oh!

aarshkshah1992 commented May 21, 2025

Uh oh!

aarshkshah1992 commented May 21, 2025

Uh oh!

pali101 commented May 23, 2025

Uh oh!

silent-cipher 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!

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!

aarshkshah1992 commented Jun 16, 2025

Uh oh!

rjan90 commented Jul 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wjmelements commented Jul 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

pali101 commented May 9, 2025 •

edited

Loading