Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c8e0b33
Refactor: Narrow RateChange.untilEpoch to uint64 for gas optimization
pali101 May 8, 2025
07e2467
refactor: use uint64 for fromEpoch, toEpoch, and settleUpto in IArbit…
pali101 May 8, 2025
a73619c
refactor: shrink epoch fields to uint64, commission BPS to uint16, an…
pali101 May 8, 2025
5975af2
refactor: narrow epoch (from uint256 to uint64) and commision types (…
pali101 May 8, 2025
41bdc8e
refactor: explicit uint64/uint16 downcasts for internal functions and…
pali101 May 8, 2025
a5a5be0
test: adapt test helpers to uint64 epoch API
pali101 May 8, 2025
5600d7d
test: update MockArbiter to use uint64 for epoch parameters and settl…
pali101 May 8, 2025
446389b
test: adapt tests for uint64/uint16 support
pali101 May 8, 2025
1591fd7
refactor settleRailAndVerify: unpack into named SettlementResult to f…
pali101 May 9, 2025
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
131 changes: 75 additions & 56 deletions src/Payments.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ interface IArbiter {
// The actual payment amount determined by the arbiter after arbitration of a rail during settlement
uint256 modifiedAmount;
// The epoch up to and including which settlement should occur.
uint256 settleUpto;
uint64 settleUpto;
// A placeholder note for any additional information the arbiter wants to send to the caller of `settleRail`
string note;
}
Expand All @@ -24,9 +24,9 @@ interface IArbiter {
uint256 railId,
uint256 proposedAmount,
// the epoch up to and including which the rail has already been settled
uint256 fromEpoch,
uint64 fromEpoch,
// the epoch up to and including which arbitration is requested; payment will be arbitrated for (toEpoch - fromEpoch) epochs
uint256 toEpoch,
uint64 toEpoch,
uint256 rate
) external returns (ArbitrationResult memory result);
}
Expand All @@ -42,33 +42,48 @@ contract Payments is
using RateChangeQueue for RateChangeQueue.Queue;

// Maximum commission rate in basis points (100% = 10000 BPS)
uint256 public constant COMMISSION_MAX_BPS = 10000;
uint16 public constant COMMISSION_MAX_BPS = 10000;

uint256 public constant PAYMENT_FEE_BPS = 10; //(0.1 % fee)
uint16 public constant PAYMENT_FEE_BPS = 10; //(0.1 % fee)

struct Account {
uint256 funds;
uint256 lockupCurrent;
uint256 lockupRate;
// epoch up to and including which lockup has been settled for the account
uint256 lockupLastSettledAt;
uint64 lockupLastSettledAt;
}

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

@wjmelements wjmelements Jul 24, 2025

Choose a reason for hiding this comment

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

nice

Copy link
Collaborator

@wjmelements wjmelements Jul 24, 2025

Choose a reason for hiding this comment

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

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


// 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;
}
Comment on lines 57 to 87
Copy link
Collaborator

@silent-cipher silent-cipher Jun 5, 2025

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@pali101 pali101 Jun 13, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@ZenGround0 ZenGround0 Jul 2, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@ZenGround0 ZenGround0 Jul 2, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

@wjmelements wjmelements Jul 24, 2025

Choose a reason for hiding this comment

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

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.


struct OperatorApproval {
Expand Down Expand Up @@ -96,12 +111,12 @@ contract Payments is
address operator;
address arbiter;
uint256 paymentRate;
uint256 lockupPeriod;
uint64 lockupPeriod;
uint256 lockupFixed;
uint256 settledUpTo;
uint256 endEpoch;
uint64 settledUpTo;
uint64 endEpoch;
// Operator commission rate in basis points (e.g., 100 BPS = 1%)
uint256 commissionRateBps;
uint16 commissionRateBps;
}

// token => client => operator => Approval
Expand All @@ -118,7 +133,7 @@ contract Payments is
struct RailInfo {
uint256 railId; // The rail ID
bool isTerminated; // True if rail is terminated
uint256 endEpoch; // End epoch for terminated rails (0 for active rails)
uint64 endEpoch; // End epoch for terminated rails (0 for active rails)
}

// token => payee => array of railIds
Expand All @@ -135,7 +150,7 @@ contract Payments is
uint256 totalNetPayeeAmount;
uint256 totalPaymentFee;
uint256 totalOperatorCommission;
uint256 processedEpoch;
uint64 processedEpoch;
string note;
}

Expand Down Expand Up @@ -473,7 +488,7 @@ contract Payments is
address from,
address to,
address arbiter,
uint256 commissionRateBps
uint16 commissionRateBps
)
external
nonReentrant
Expand Down Expand Up @@ -503,7 +518,7 @@ contract Payments is
rail.to = to;
rail.operator = operator;
rail.arbiter = arbiter;
rail.settledUpTo = block.number;
rail.settledUpTo = uint64(block.number);
rail.endEpoch = 0;
rail.commissionRateBps = commissionRateBps;

Expand All @@ -524,7 +539,7 @@ contract Payments is
/// @custom:constraint Operator must have sufficient lockup allowance to cover any increases the lockup period or the fixed lockup.
function modifyRailLockup(
uint256 railId,
uint256 period,
uint64 period,
uint256 lockupFixed
)
external
Expand All @@ -545,7 +560,7 @@ contract Payments is

function modifyTerminatedRailLockup(
Rail storage rail,
uint256 period,
uint64 period,
uint256 lockupFixed
) internal {
require(
Expand Down Expand Up @@ -580,7 +595,7 @@ contract Payments is

function modifyNonTerminatedRailLockup(
Rail storage rail,
uint256 period,
uint64 period,
uint256 lockupFixed
) internal {
Account storage payer = accounts[rail.token][rail.from];
Expand Down Expand Up @@ -740,7 +755,7 @@ contract Payments is
uint256 newRate,
uint256 oneTimePayment
) internal {
uint256 endEpoch = maxSettlementEpochForTerminatedRail(rail);
uint64 endEpoch = maxSettlementEpochForTerminatedRail(rail);
require(
newRate == 0 && oneTimePayment == 0,
"for terminated rails beyond last settlement epoch, both new rate and one-time payment must be 0"
Expand Down Expand Up @@ -778,7 +793,7 @@ contract Payments is

// 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));
Copy link
Collaborator

@silent-cipher silent-cipher Jun 5, 2025

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@pali101 pali101 Jun 13, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@ZenGround0 ZenGround0 Jul 2, 2025

Choose a reason for hiding this comment

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

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

require(
settledUpto == block.number,
"failed to settle rail up to current epoch"
Expand All @@ -795,15 +810,15 @@ contract Payments is
// For arbitrated rails, we need to enqueue the old rate.
// This ensures that the old rate is applied up to and including the current block.
// The new rate will be applicable starting from the next block.
rail.rateChangeQueue.enqueue(oldRate, block.number);
rail.rateChangeQueue.enqueue(oldRate, uint64(block.number));
}
}

function calculateAndPayFees(
uint256 amount,
address token,
address operator,
uint256 commissionRateBps
uint16 commissionRateBps
)
internal
returns (
Expand Down Expand Up @@ -900,12 +915,12 @@ contract Payments is
uint256 totalNetPayeeAmount,
uint256 totalPaymentFee,
uint256 totalOperatorCommission,
uint256 finalSettledEpoch,
uint64 finalSettledEpoch,
string memory note
)
{
// Verify the current epoch is greater than the max settlement epoch
uint256 maxSettleEpoch = maxSettlementEpochForTerminatedRail(
uint64 maxSettleEpoch = maxSettlementEpochForTerminatedRail(
rails[railId]
);
require(
Expand All @@ -927,7 +942,7 @@ contract Payments is
/// @return note Additional information about the settlement (especially from arbitration).
function settleRail(
uint256 railId,
uint256 untilEpoch
uint64 untilEpoch
)
public
nonReentrant
Expand All @@ -939,7 +954,7 @@ contract Payments is
uint256 totalNetPayeeAmount,
uint256 totalPaymentFee,
uint256 totalOperatorCommission,
uint256 finalSettledEpoch,
uint64 finalSettledEpoch,
string memory note
)
{
Expand All @@ -948,7 +963,7 @@ contract Payments is

function settleRailInternal(
uint256 railId,
uint256 untilEpoch,
uint64 untilEpoch,
bool skipArbitration
)
internal
Expand All @@ -957,7 +972,7 @@ contract Payments is
uint256 totalNetPayeeAmount,
uint256 totalPaymentFee,
uint256 totalOperatorCommission,
uint256 finalSettledEpoch,
uint64 finalSettledEpoch,
string memory note
)
{
Expand All @@ -983,14 +998,14 @@ contract Payments is
}

// Calculate the maximum settlement epoch based on account lockup
uint256 maxSettlementEpoch;
uint64 maxSettlementEpoch;
if (!isRailTerminated(rail)) {
maxSettlementEpoch = min(untilEpoch, payer.lockupLastSettledAt);
maxSettlementEpoch = min64(untilEpoch, payer.lockupLastSettledAt);
} else {
maxSettlementEpoch = min(untilEpoch, rail.endEpoch);
maxSettlementEpoch = min64(untilEpoch, rail.endEpoch);
}

uint256 startEpoch = rail.settledUpTo;
uint64 startEpoch = rail.settledUpTo;
// Nothing to settle (already settled or zero-duration)
if (startEpoch >= maxSettlementEpoch) {
return (
Expand Down Expand Up @@ -1079,12 +1094,12 @@ contract Payments is
uint256 totalNetPayeeAmount,
uint256 totalPaymentFee,
uint256 totalOperatorCommission,
uint256 finalEpoch,
uint64 finalEpoch,
string memory regularNote,
string memory finalizedNote
)
internal
returns (uint256, uint256, uint256, uint256, uint256, string memory)
returns (uint256, uint256, uint256, uint256, uint64, string memory)
{
// Check if rail is a terminated rail that's now fully settled
if (
Expand Down Expand Up @@ -1137,8 +1152,8 @@ contract Payments is
function _settleWithRateChanges(
uint256 railId,
uint256 currentRate,
uint256 startEpoch,
uint256 targetEpoch,
uint64 startEpoch,
uint64 targetEpoch,
bool skipArbitration
)
internal
Expand All @@ -1165,7 +1180,7 @@ contract Payments is
// Process each segment until we reach the target epoch or hit an early exit condition
while (state.processedEpoch < targetEpoch) {
(
uint256 segmentEndBoundary,
uint64 segmentEndBoundary,
uint256 segmentRate
) = _getNextSegmentBoundary(
rateQueue,
Expand Down Expand Up @@ -1252,9 +1267,9 @@ contract Payments is
function _getNextSegmentBoundary(
RateChangeQueue.Queue storage rateQueue,
uint256 currentRate,
uint256 processedEpoch,
uint256 targetEpoch
) internal view returns (uint256 segmentEndBoundary, uint256 segmentRate) {
uint64 processedEpoch,
uint64 targetEpoch
) internal view returns (uint64 segmentEndBoundary, uint256 segmentRate) {
// Default boundary is the target we want to reach
segmentEndBoundary = targetEpoch;
segmentRate = currentRate;
Expand All @@ -1270,15 +1285,15 @@ contract Payments is
);

// Boundary is the minimum of our target or the next rate change epoch
segmentEndBoundary = min(targetEpoch, nextRateChange.untilEpoch);
segmentEndBoundary = min64(targetEpoch, nextRateChange.untilEpoch);
segmentRate = nextRateChange.rate;
}
}

function _settleSegment(
uint256 railId,
uint256 epochStart,
uint256 epochEnd,
uint64 epochStart,
uint64 epochEnd,
uint256 rate,
bool skipArbitration
)
Expand All @@ -1303,7 +1318,7 @@ contract Payments is
// Calculate the default settlement values (without arbitration)
uint256 duration = epochEnd - epochStart;
uint256 settledAmount = rate * duration;
uint256 settledUntilEpoch = epochEnd;
uint64 settledUntilEpoch = epochEnd;
note = "";

// If this rail has an arbiter and we're not skipping arbitration, let it decide on the final settlement amount
Expand Down Expand Up @@ -1397,9 +1412,9 @@ contract Payments is
// returns the actual epoch upto and including which the lockup was settled
function settleAccountLockup(
Account storage account
) internal returns (uint256) {
uint256 currentEpoch = block.number;
uint256 elapsedTime = currentEpoch - account.lockupLastSettledAt;
) internal returns (uint64) {
uint64 currentEpoch = uint64(block.number);
uint64 elapsedTime = currentEpoch - account.lockupLastSettledAt;

if (elapsedTime <= 0) {
return account.lockupLastSettledAt;
Expand Down Expand Up @@ -1431,7 +1446,7 @@ contract Payments is
}

// Round down to the nearest whole epoch
uint256 fractionalEpochs = availableFunds / account.lockupRate;
uint64 fractionalEpochs = uint64(availableFunds / account.lockupRate);

// Apply lockup up to this point
account.lockupCurrent += account.lockupRate * fractionalEpochs;
Expand All @@ -1443,7 +1458,7 @@ contract Payments is

function remainingEpochsForTerminatedRail(
Rail storage rail
) internal view returns (uint256) {
) internal view returns (uint64) {
require(isRailTerminated(rail), "rail is not terminated");

// If current block beyond end epoch, return 0
Expand All @@ -1452,7 +1467,7 @@ contract Payments is
}

// Return the number of epochs (blocks) remaining until end epoch
return rail.endEpoch - block.number;
return rail.endEpoch - uint64(block.number);
}

function isRailTerminated(Rail storage rail) internal view returns (bool) {
Expand All @@ -1466,7 +1481,7 @@ contract Payments is
// Get the final settlement epoch for a terminated rail
function maxSettlementEpochForTerminatedRail(
Rail storage rail
) internal view returns (uint256) {
) internal view returns (uint64) {
require(isRailTerminated(rail), "rail is not terminated");
return rail.endEpoch;
}
Expand Down Expand Up @@ -1670,3 +1685,7 @@ contract Payments is
function min(uint256 a, uint256 b) pure returns (uint256) {
return a < b ? a : b;
}

function min64(uint64 a, uint64 b) pure returns (uint64) {
return a < b ? a : b;
}
Loading