Skip to content
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

Change types and add modifier to getter #296

Merged
merged 5 commits into from
Oct 14, 2024
Merged

Change types and add modifier to getter #296

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
703b9ec
fix: use uint256 for scaledOngoingDebt
andreivladbrg Oct 10, 2024
662d018
test: remove undeeded down casting
andreivladbrg Oct 11, 2024
cdedfd1
fix: use ">" in depletionTimeOf
andreivladbrg Oct 12, 2024
9ff1175
address last review comments
andreivladbrg Oct 13, 2024
98c2bfc
test: lower the rps bound range
andreivladbrg Oct 14, 2024
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
2 changes: 1 addition & 1 deletion benchmark/Flow.Gas.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ contract Flow_Gas_Test is Integration_Test {
);

// Deposit amount on an incremented stream ID to make stream solvent.
deposit(++streamId, flow.uncoveredDebtOf(streamId) + DEPOSIT_AMOUNT_6D);
deposit(++streamId, uint128(flow.uncoveredDebtOf(streamId)) + DEPOSIT_AMOUNT_6D);

// {flow.withdraw} (on a solvent stream).
computeGas(
Expand Down
24 changes: 12 additions & 12 deletions benchmark/results/SablierFlow.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@

| Function | Gas Usage |
| ----------------------------- | --------- |
| `adjustRatePerSecond` | 44009 |
| `create` | 113794 |
| `deposit` | 30152 |
| `depositViaBroker` | 22142 |
| `pause` | 9340 |
| `refund` | 11671 |
| `restart` | 7106 |
| `void (solvent stream)` | 10389 |
| `void (insolvent stream)` | 36795 |
| `withdraw (insolvent stream)` | 57634 |
| `withdraw (solvent stream)` | 40047 |
| `withdrawMax` | 52200 |
| `adjustRatePerSecond` | 43628 |
| `create` | 113659 |
| `deposit` | 30035 |
| `depositViaBroker` | 21953 |
| `pause` | 8983 |
| `refund` | 11534 |
| `restart` | 7031 |
| `void (solvent stream)` | 9517 |
| `void (insolvent stream)` | 36241 |
| `withdraw (insolvent stream)` | 57034 |
| `withdraw (solvent stream)` | 39502 |
| `withdrawMax` | 51379 |
56 changes: 29 additions & 27 deletions src/SablierFlow.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ contract SablierFlow is
override
notNull(streamId)
notPaused(streamId)
returns (uint40 depletionTime)
returns (uint256 depletionTime)
{
uint128 balance = _streams[streamId].balance;

Expand All @@ -69,15 +69,15 @@ contract SablierFlow is
return 0;
}

uint128 snapshotDebt = _streams[streamId].snapshotDebt;
uint256 snapshotDebt = _streams[streamId].snapshotDebt;

// If the stream has uncovered debt, return zero.
if (snapshotDebt + _ongoingDebtOf(streamId) >= balance) {
if (snapshotDebt + _ongoingDebtOf(streamId) > balance) {
return 0;
}

uint8 tokenDecimals = _streams[streamId].tokenDecimals;
uint128 solvencyAmount;
uint256 tokenDecimals = _streams[streamId].tokenDecimals;
uint256 solvencyAmount;

// Depletion time is defined as the UNIX timestamp beyond which the total debt exceeds stream balance.
// So we calculate it by solving: debt at depletion time = stream balance + 1. This ensures that we find the
Expand All @@ -87,16 +87,16 @@ contract SablierFlow is
if (tokenDecimals == 18) {
solvencyAmount = (balance - snapshotDebt + 1);
} else {
uint128 scaleFactor = (10 ** (18 - tokenDecimals)).toUint128();
uint256 scaleFactor = (10 ** (18 - tokenDecimals));
solvencyAmount = (balance - snapshotDebt + 1) * scaleFactor;
}
uint256 solvencyPeriod = solvencyAmount / _streams[streamId].ratePerSecond.unwrap();
return _streams[streamId].snapshotTime + solvencyPeriod.toUint40();
return _streams[streamId].snapshotTime + solvencyPeriod;
}
}

/// @inheritdoc ISablierFlow
function ongoingDebtOf(uint256 streamId) external view override notNull(streamId) returns (uint128 ongoingDebt) {
function ongoingDebtOf(uint256 streamId) external view override notNull(streamId) returns (uint256 ongoingDebt) {
ongoingDebt = _ongoingDebtOf(streamId);
}

Expand Down Expand Up @@ -141,7 +141,7 @@ contract SablierFlow is
}

/// @inheritdoc ISablierFlow
function totalDebtOf(uint256 streamId) external view override notNull(streamId) returns (uint128 totalDebt) {
function totalDebtOf(uint256 streamId) external view override notNull(streamId) returns (uint256 totalDebt) {
totalDebt = _totalDebtOf(streamId);
}

Expand All @@ -151,7 +151,7 @@ contract SablierFlow is
view
override
notNull(streamId)
returns (uint128 uncoveredDebt)
returns (uint256 uncoveredDebt)
{
uncoveredDebt = _uncoveredDebtOf(streamId);
}
Expand Down Expand Up @@ -437,41 +437,42 @@ contract SablierFlow is
return 0;
}

uint128 totalDebt = _totalDebtOf(streamId);
uint256 totalDebt = _totalDebtOf(streamId);

// If the stream balance is less than or equal to the total debt, return the stream balance.
if (balance < totalDebt) {
return balance;
}

return totalDebt;
// At this point, the total debt fits within `uint128`, as it is less than or equal to the balance.
return totalDebt.toUint128();
smol-ninja marked this conversation as resolved.
Show resolved Hide resolved
}

/// @dev Calculates the ongoing debt accrued since last snapshot. Return 0 if the stream is paused or
/// `block.timestamp` is less than or equal to snapshot time.
function _ongoingDebtOf(uint256 streamId) internal view returns (uint128 ongoingDebt) {
function _ongoingDebtOf(uint256 streamId) internal view returns (uint256 ongoingDebt) {
smol-ninja marked this conversation as resolved.
Show resolved Hide resolved
uint40 blockTimestamp = uint40(block.timestamp);
uint40 snapshotTime = _streams[streamId].snapshotTime;

uint128 ratePerSecond = _streams[streamId].ratePerSecond.unwrap();
uint256 ratePerSecond = _streams[streamId].ratePerSecond.unwrap();

// Check:if the rate per second is zero or the `block.timestamp` is less than the `snapshotTime`.
if (ratePerSecond == 0 || blockTimestamp <= snapshotTime) {
return 0;
}

uint128 elapsedTime;
uint256 elapsedTime;

// Safe to use unchecked because subtraction cannot underflow.
unchecked {
// Calculate time elapsed since the last snapshot.
elapsedTime = blockTimestamp - snapshotTime;
}

uint8 tokenDecimals = _streams[streamId].tokenDecimals;

// Calculate the ongoing debt accrued by multiplying the elapsed time by the rate per second.
uint128 scaledOngoingDebt = elapsedTime * ratePerSecond;
uint256 scaledOngoingDebt = elapsedTime * ratePerSecond;

uint8 tokenDecimals = _streams[streamId].tokenDecimals;

// If the token decimals are 18, return the scaled ongoing debt and the `block.timestamp`.
if (tokenDecimals == 18) {
Expand All @@ -480,7 +481,7 @@ contract SablierFlow is

// Safe to use unchecked because we use {SafeCast}.
unchecked {
uint128 scaleFactor = (10 ** (18 - tokenDecimals)).toUint128();
uint256 scaleFactor = 10 ** (18 - tokenDecimals);
// Since debt is denoted in token decimals, descale the amount.
ongoingDebt = scaledOngoingDebt / scaleFactor;
}
Expand All @@ -494,16 +495,16 @@ contract SablierFlow is
/// @notice Calculates the total debt.
/// @dev The total debt is the sum of the snapshot debt and the ongoing debt. This value is independent of the
/// stream's balance.
function _totalDebtOf(uint256 streamId) internal view returns (uint128) {
function _totalDebtOf(uint256 streamId) internal view returns (uint256) {
// Calculate the total debt.
return _streams[streamId].snapshotDebt + _ongoingDebtOf(streamId);
}

/// @dev Calculates the uncovered debt.
function _uncoveredDebtOf(uint256 streamId) internal view returns (uint128) {
function _uncoveredDebtOf(uint256 streamId) internal view returns (uint256) {
uint128 balance = _streams[streamId].balance;

uint128 totalDebt = _totalDebtOf(streamId);
uint256 totalDebt = _totalDebtOf(streamId);

if (balance < totalDebt) {
return totalDebt - balance;
Expand All @@ -523,7 +524,7 @@ contract SablierFlow is
revert Errors.SablierFlow_RatePerSecondNotDifferent(streamId, newRatePerSecond);
}

uint128 ongoingDebt = _ongoingDebtOf(streamId);
uint256 ongoingDebt = _ongoingDebtOf(streamId);

// Update the snapshot debt only if the stream has ongoing debt.
if (ongoingDebt > 0) {
Expand Down Expand Up @@ -709,11 +710,12 @@ contract SablierFlow is
revert Errors.SablierFlow_Unauthorized({ streamId: streamId, caller: msg.sender });
}

uint128 debtToWriteOff = _uncoveredDebtOf(streamId);
uint256 debtToWriteOff = _uncoveredDebtOf(streamId);

// If the stream is solvent, update the total debt normally.
if (debtToWriteOff == 0) {
uint128 ongoingDebt = _ongoingDebtOf(streamId);
// It is safe to downcast because if there is no uncovered debt, the ongoing debt fits within `uint128`.
andreivladbrg marked this conversation as resolved.
Show resolved Hide resolved
uint256 ongoingDebt = _ongoingDebtOf(streamId);
if (ongoingDebt > 0) {
// Effect: Update the snapshot debt by adding the ongoing debt.
_streams[streamId].snapshotDebt += ongoingDebt;
Expand Down Expand Up @@ -764,7 +766,7 @@ contract SablierFlow is
}

// Calculate the total debt.
uint128 totalDebt = _totalDebtOf(streamId);
uint256 totalDebt = _totalDebtOf(streamId);

// Calculate the withdrawable amount.
uint128 balance = _streams[streamId].balance;
Expand All @@ -775,7 +777,7 @@ contract SablierFlow is
withdrawableAmount = balance;
} else {
// Otherwise, the withdrawable amount is the total debt.
withdrawableAmount = totalDebt;
withdrawableAmount = totalDebt.toUint128();
}

// Check: the withdraw amount is not greater than the withdrawable amount.
Expand Down
4 changes: 2 additions & 2 deletions src/abstracts/SablierFlowBase.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ abstract contract SablierFlowBase is
view
override
notNull(streamId)
returns (uint128 snapshotDebt)
returns (uint256 snapshotDebt)
{
snapshotDebt = _streams[streamId].snapshotDebt;
}
Expand Down Expand Up @@ -181,7 +181,7 @@ abstract contract SablierFlowBase is
}

/// @inheritdoc ISablierFlowBase
function isTransferable(uint256 streamId) external view override returns (bool result) {
function isTransferable(uint256 streamId) external view override notNull(streamId) returns (bool result) {
result = _streams[streamId].isTransferable;
}

Expand Down
16 changes: 8 additions & 8 deletions src/interfaces/ISablierFlow.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ interface ISablierFlow is
/// @param newRatePerSecond The new rate per second, denoted as a fixed-point number where 1e18 is 1 token
/// per second.
event AdjustFlowStream(
uint256 indexed streamId, uint128 totalDebt, UD21x18 oldRatePerSecond, UD21x18 newRatePerSecond
uint256 indexed streamId, uint256 totalDebt, UD21x18 oldRatePerSecond, UD21x18 newRatePerSecond
);

/// @notice Emitted when a Flow stream is created.
Expand Down Expand Up @@ -56,7 +56,7 @@ interface ISablierFlow is
/// @param recipient The address of the stream's recipient.
/// @param totalDebt The amount of tokens owed by the sender to the recipient, denoted in token's decimals.
event PauseFlowStream(
uint256 indexed streamId, address indexed sender, address indexed recipient, uint128 totalDebt
uint256 indexed streamId, address indexed sender, address indexed recipient, uint256 totalDebt
);

/// @notice Emitted when a sender is refunded from a stream.
Expand Down Expand Up @@ -84,8 +84,8 @@ interface ISablierFlow is
address indexed sender,
address indexed recipient,
address caller,
uint128 newTotalDebt,
uint128 writtenOffDebt
uint256 newTotalDebt,
uint256 writtenOffDebt
);

/// @notice Emitted when tokens are withdrawn from a stream by a recipient or an approved operator.
Expand Down Expand Up @@ -119,12 +119,12 @@ interface ISablierFlow is
/// there already is uncovered debt, it returns zero.
/// @dev Reverts if `streamId` references a paused or a null stream.
/// @param streamId The stream ID for the query.
function depletionTimeOf(uint256 streamId) external view returns (uint40 depletionTime);
function depletionTimeOf(uint256 streamId) external view returns (uint256 depletionTime);

/// @notice Returns the amount of debt accrued since the snapshot time until now, denoted in token's decimals.
/// @dev Reverts if `streamId` references a null stream.
/// @param streamId The stream ID for the query.
function ongoingDebtOf(uint256 streamId) external view returns (uint128 ongoingDebt);
function ongoingDebtOf(uint256 streamId) external view returns (uint256 ongoingDebt);

/// @notice Returns the amount that the sender can be refunded from the stream, denoted in token's decimals.
/// @dev Reverts if `streamId` references a null stream.
Expand All @@ -139,12 +139,12 @@ interface ISablierFlow is
/// @notice Returns the total amount owed by the sender to the recipient, denoted in token's decimals.
/// @dev Reverts if `streamId` references a null stream.
/// @param streamId The stream ID for the query.
function totalDebtOf(uint256 streamId) external view returns (uint128 totalDebt);
function totalDebtOf(uint256 streamId) external view returns (uint256 totalDebt);

/// @notice Returns the amount of debt not covered by the stream balance, denoted in token's decimals.
/// @dev Reverts if `streamId` references a null stream.
/// @param streamId The stream ID for the query.
function uncoveredDebtOf(uint256 streamId) external view returns (uint128 uncoveredDebt);
function uncoveredDebtOf(uint256 streamId) external view returns (uint256 uncoveredDebt);

/// @notice Calculates the amount that the recipient can withdraw from the stream, denoted in token decimals. This
/// is an alias for `coveredDebtOf`.
Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/ISablierFlowBase.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ interface ISablierFlowBase is
/// @notice Retrieves the snapshot debt of the stream, denoted in token's decimals.
/// @dev Reverts if `streamId` references a null stream.
/// @param streamId The stream ID for the query.
function getSnapshotDebt(uint256 streamId) external view returns (uint128 snapshotDebt);
function getSnapshotDebt(uint256 streamId) external view returns (uint256 snapshotDebt);

/// @notice Retrieves the snapshot time of the stream, which is a Unix timestamp.
/// @dev Reverts if `streamId` references a null stream.
Expand Down
2 changes: 1 addition & 1 deletion src/types/DataTypes.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,6 @@ library Flow {
IERC20 token;
uint8 tokenDecimals;
// slot 3
uint128 snapshotDebt;
uint256 snapshotDebt;
}
}
22 changes: 11 additions & 11 deletions tests/fork/Flow.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,20 +49,20 @@ contract Flow_Fork_Test is Fork_Test {
uint256 actualAggregateAmount;
UD21x18 actualRatePerSecond;
uint40 actualSnapshotTime;
uint128 actualSnapshotDebt;
uint256 actualSnapshotDebt;
uint128 actualStreamBalance;
uint256 actualStreamId;
uint256 actualTokenBalance;
uint128 actualTotalDebt;
uint256 actualTotalDebt;
// Expected values.
uint256 expectedAggregateAmount;
UD21x18 expectedRatePerSecond;
uint40 expectedSnapshotTime;
uint128 expectedSnapshotDebt;
uint256 expectedSnapshotDebt;
uint128 expectedStreamBalance;
uint256 expectedStreamId;
uint256 expectedTokenBalance;
uint128 expectedTotalDebt;
uint256 expectedTotalDebt;
}

/*//////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -251,9 +251,9 @@ contract Flow_Fork_Test is Fork_Test {
newRatePerSecond = ud21x18(newRatePerSecond.unwrap() + 1);
}

uint128 beforeSnapshotAmount = flow.getSnapshotDebt(streamId);
uint128 totalDebt = flow.totalDebtOf(streamId);
uint128 ongoingDebt = flow.ongoingDebtOf(streamId);
uint256 beforeSnapshotAmount = flow.getSnapshotDebt(streamId);
uint256 totalDebt = flow.totalDebtOf(streamId);
uint256 ongoingDebt = flow.ongoingDebtOf(streamId);

// Compute the snapshot time that will be stored post withdraw.
vars.expectedSnapshotTime = getBlockTimestamp();
Expand Down Expand Up @@ -443,7 +443,7 @@ contract Flow_Fork_Test is Fork_Test {
// If the refundable amount less than 1, deposit some funds.
if (flow.refundableAmountOf(streamId) <= 1) {
uint128 depositAmount =
flow.uncoveredDebtOf(streamId) + getDefaultDepositAmount(flow.getTokenDecimals(streamId));
uint128(flow.uncoveredDebtOf(streamId)) + getDefaultDepositAmount(flow.getTokenDecimals(streamId));
depositOnStream(streamId, depositAmount);
}

Expand Down Expand Up @@ -527,8 +527,8 @@ contract Flow_Fork_Test is Fork_Test {
// Make sure the requirements are respected.
address sender = flow.getSender(streamId);
address recipient = flow.getRecipient(streamId);
uint128 uncoveredDebt = flow.uncoveredDebtOf(streamId);
uint128 expectedTotalDebt;
uint256 uncoveredDebt = flow.uncoveredDebtOf(streamId);
uint256 expectedTotalDebt;

resetPrank({ msgSender: sender });

Expand Down Expand Up @@ -583,7 +583,7 @@ contract Flow_Fork_Test is Fork_Test {
);

uint256 initialTokenBalance = token.balanceOf(address(flow));
uint128 totalDebt = flow.totalDebtOf(streamId);
uint256 totalDebt = flow.totalDebtOf(streamId);

vars.expectedSnapshotTime =
withdrawAmount <= flow.getSnapshotDebt(streamId) ? flow.getSnapshotTime(streamId) : getBlockTimestamp();
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/concrete/adjust-rate-per-second/adjustRatePerSecond.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ contract AdjustRatePerSecond_Integration_Concrete_Test is Integration_Test {
uint40 expectedSnapshotTime = getBlockTimestamp() - ONE_MONTH;
assertEq(actualSnapshotTime, expectedSnapshotTime, "snapshot time");

uint128 actualSnapshotDebt = flow.getSnapshotDebt(defaultStreamId);
uint256 actualSnapshotDebt = flow.getSnapshotDebt(defaultStreamId);
uint128 expectedSnapshotDebt = 0;
assertEq(actualSnapshotDebt, expectedSnapshotDebt, "snapshot debt");

Expand Down
Loading
Loading