From 3238b33731e838aa018c5537a478ec7889dcb5bb Mon Sep 17 00:00:00 2001 From: Bogdan Kovtun Date: Wed, 18 Sep 2024 10:43:51 +0500 Subject: [PATCH 1/8] Unify types file structure --- contracts/types/Duration.sol | 81 ++++++++++++++-------- contracts/types/ETHValue.sol | 74 +++++++++++++++----- contracts/types/IndexOneBased.sol | 27 +++++++- contracts/types/PercentD16.sol | 47 ++++++++++++- contracts/types/SharesValue.sol | 45 +++++++++--- contracts/types/Timestamp.sol | 43 ++++++++---- test/unit/libraries/AssetsAccounting.t.sol | 8 +-- test/unit/libraries/EscrowState.t.sol | 6 +- test/utils/testing-assert-eq-extender.sol | 10 +++ 9 files changed, 258 insertions(+), 83 deletions(-) diff --git a/contracts/types/Duration.sol b/contracts/types/Duration.sol index f095cd7d..3a412e2c 100644 --- a/contracts/types/Duration.sol +++ b/contracts/types/Duration.sol @@ -3,20 +3,37 @@ pragma solidity 0.8.26; import {Timestamp, Timestamps} from "./Timestamp.sol"; +// --- +// Type definition +// --- + type Duration is uint32; +// --- +// Errors +// --- + error DurationOverflow(); error DurationUnderflow(); +error TimestampUnderflow(); -// the max possible duration is ~ 106 years -uint256 constant MAX_VALUE = type(uint32).max; +// --- +// Assign global operations +// --- -using {lt as <, lte as <=, gt as >, gte as >=, eq as ==, notEq as !=} for Duration global; using {plus as +, minus as -} for Duration global; +using {lt as <, lte as <=, eq as ==, neq as !=, gt as >, gte as >=} for Duration global; using {addTo, plusSeconds, minusSeconds, multipliedBy, dividedBy, toSeconds} for Duration global; // --- -// Comparison Ops +// Constants +// --- + +/// @dev The max possible duration is about 106 years +uint256 constant MAX_DURATION_VALUE = type(uint32).max; + +// --- +// Comparison operations // --- function lt(Duration d1, Duration d2) pure returns (bool) { @@ -27,39 +44,47 @@ function lte(Duration d1, Duration d2) pure returns (bool) { return Duration.unwrap(d1) <= Duration.unwrap(d2); } -function gt(Duration d1, Duration d2) pure returns (bool) { - return Duration.unwrap(d1) > Duration.unwrap(d2); +function eq(Duration d1, Duration d2) pure returns (bool) { + return Duration.unwrap(d1) == Duration.unwrap(d2); } -function gte(Duration d1, Duration d2) pure returns (bool) { - return Duration.unwrap(d1) >= Duration.unwrap(d2); +function neq(Duration d1, Duration d2) pure returns (bool) { + return Duration.unwrap(d1) != Duration.unwrap(d2); } -function eq(Duration d1, Duration d2) pure returns (bool) { - return Duration.unwrap(d1) == Duration.unwrap(d2); +function gte(Duration d1, Duration d2) pure returns (bool) { + return Duration.unwrap(d1) >= Duration.unwrap(d2); } -function notEq(Duration d1, Duration d2) pure returns (bool) { - return !(d1 == d2); +function gt(Duration d1, Duration d2) pure returns (bool) { + return Duration.unwrap(d1) > Duration.unwrap(d2); } // --- -// Arithmetic Operations +// Arithmetic operations // --- function plus(Duration d1, Duration d2) pure returns (Duration) { - return toDuration(Duration.unwrap(d1) + Duration.unwrap(d2)); + unchecked { + return Durations.from(d1.toSeconds() + d2.toSeconds()); + } } function minus(Duration d1, Duration d2) pure returns (Duration) { if (d1 < d2) { revert DurationUnderflow(); } - return Duration.wrap(Duration.unwrap(d1) - Duration.unwrap(d2)); + unchecked { + return Duration.wrap(uint32(d1.toSeconds() - d2.toSeconds())); + } } +// --- +// Custom operations +// --- + function plusSeconds(Duration d, uint256 seconds_) pure returns (Duration) { - return toDuration(Duration.unwrap(d) + seconds_); + return Durations.from(Duration.unwrap(d) + seconds_); } function minusSeconds(Duration d, uint256 seconds_) pure returns (Duration) { @@ -75,7 +100,7 @@ function dividedBy(Duration d, uint256 divisor) pure returns (Duration) { } function multipliedBy(Duration d, uint256 multiplicand) pure returns (Duration) { - return toDuration(Duration.unwrap(d) * multiplicand); + return Durations.from(multiplicand * d.toSeconds()); } function addTo(Duration d, Timestamp t) pure returns (Timestamp) { @@ -83,32 +108,32 @@ function addTo(Duration d, Timestamp t) pure returns (Timestamp) { } // --- -// Conversion Ops +// Conversion operations // --- -function toDuration(uint256 value) pure returns (Duration) { - if (value > MAX_VALUE) { - revert DurationOverflow(); - } - return Duration.wrap(uint32(value)); -} - function toSeconds(Duration d) pure returns (uint256) { return Duration.unwrap(d); } +// --- +// Namespaced helper methods +// --- + library Durations { Duration internal constant ZERO = Duration.wrap(0); Duration internal constant MIN = ZERO; - Duration internal constant MAX = Duration.wrap(uint32(MAX_VALUE)); + Duration internal constant MAX = Duration.wrap(uint32(MAX_DURATION_VALUE)); function from(uint256 seconds_) internal pure returns (Duration res) { - res = toDuration(seconds_); + if (seconds_ > MAX_DURATION_VALUE) { + revert DurationOverflow(); + } + res = Duration.wrap(uint32(seconds_)); } function between(Timestamp t1, Timestamp t2) internal pure returns (Duration res) { - res = toDuration(t1.toSeconds() - t2.toSeconds()); + res = from(t1 > t2 ? t1.toSeconds() - t2.toSeconds() : t2.toSeconds() - t1.toSeconds()); } function min(Duration d1, Duration d2) internal pure returns (Duration res) { diff --git a/contracts/types/ETHValue.sol b/contracts/types/ETHValue.sol index 9a85a1b9..5f822a2a 100644 --- a/contracts/types/ETHValue.sol +++ b/contracts/types/ETHValue.sol @@ -3,50 +3,86 @@ pragma solidity 0.8.26; import {Address} from "@openzeppelin/contracts/utils/Address.sol"; +// --- +// Type definition +// --- + type ETHValue is uint128; +// --- +// Errors +// --- + error ETHValueOverflow(); error ETHValueUnderflow(); -using {plus as +, minus as -, lt as <, gt as >, eq as ==, neq as !=} for ETHValue global; -using {toUint256} for ETHValue global; -using {sendTo} for ETHValue global; +// --- +// Assign global operations +// --- -function sendTo(ETHValue value, address payable recipient) { - Address.sendValue(recipient, value.toUint256()); -} +using {lt as <, gt as >, eq as ==, neq as !=} for ETHValue global; +using {plus as +, minus as -} for ETHValue global; +using {toUint256, sendTo} for ETHValue global; -function plus(ETHValue v1, ETHValue v2) pure returns (ETHValue) { - return ETHValues.from(ETHValue.unwrap(v1) + ETHValue.unwrap(v2)); +// --- +// Comparison operations +// --- + +function lt(ETHValue v1, ETHValue v2) pure returns (bool) { + return ETHValue.unwrap(v1) < ETHValue.unwrap(v2); } -function minus(ETHValue v1, ETHValue v2) pure returns (ETHValue) { - if (v1 < v2) { - revert ETHValueUnderflow(); - } - return ETHValues.from(ETHValue.unwrap(v1) - ETHValue.unwrap(v2)); +function eq(ETHValue v1, ETHValue v2) pure returns (bool) { + return ETHValue.unwrap(v1) == ETHValue.unwrap(v2); } -function lt(ETHValue v1, ETHValue v2) pure returns (bool) { - return ETHValue.unwrap(v1) < ETHValue.unwrap(v2); +function neq(ETHValue v1, ETHValue v2) pure returns (bool) { + return ETHValue.unwrap(v1) != ETHValue.unwrap(v2); } function gt(ETHValue v1, ETHValue v2) pure returns (bool) { return ETHValue.unwrap(v1) > ETHValue.unwrap(v2); } -function eq(ETHValue v1, ETHValue v2) pure returns (bool) { - return ETHValue.unwrap(v1) == ETHValue.unwrap(v2); +// --- +// Arithmetic operations +// --- + +function plus(ETHValue v1, ETHValue v2) pure returns (ETHValue) { + unchecked { + return ETHValues.from(v1.toUint256() + v2.toUint256()); + } } -function neq(ETHValue v1, ETHValue v2) pure returns (bool) { - return ETHValue.unwrap(v1) != ETHValue.unwrap(v2); +function minus(ETHValue v1, ETHValue v2) pure returns (ETHValue) { + if (v1 < v2) { + revert ETHValueUnderflow(); + } + unchecked { + return ETHValues.from(ETHValue.unwrap(v1) - ETHValue.unwrap(v2)); + } } +// --- +// Custom operations +// --- + +function sendTo(ETHValue value, address payable recipient) { + Address.sendValue(recipient, value.toUint256()); +} + +// --- +// Conversion operations +// --- + function toUint256(ETHValue value) pure returns (uint256) { return ETHValue.unwrap(value); } +// --- +// Namespaced helper methods +// --- + library ETHValues { ETHValue internal constant ZERO = ETHValue.wrap(0); diff --git a/contracts/types/IndexOneBased.sol b/contracts/types/IndexOneBased.sol index b080af75..68315dbe 100644 --- a/contracts/types/IndexOneBased.sol +++ b/contracts/types/IndexOneBased.sol @@ -1,17 +1,38 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.26; +// --- +// Type definition +// --- + type IndexOneBased is uint32; +// --- +// Errors +// --- + error IndexOneBasedOverflow(); error IndexOneBasedUnderflow(); -using {neq as !=, isEmpty, isNotEmpty, toZeroBasedValue} for IndexOneBased global; +// --- +// Assign global operations +// --- + +using {neq as !=} for IndexOneBased global; +using {isEmpty, isNotEmpty, toZeroBasedValue} for IndexOneBased global; + +// --- +// Comparison operations +// --- function neq(IndexOneBased i1, IndexOneBased i2) pure returns (bool) { return IndexOneBased.unwrap(i1) != IndexOneBased.unwrap(i2); } +// --- +// Custom operations +// --- + function isEmpty(IndexOneBased index) pure returns (bool) { return IndexOneBased.unwrap(index) == 0; } @@ -29,6 +50,10 @@ function toZeroBasedValue(IndexOneBased index) pure returns (uint256) { } } +// --- +// Namespaced helper methods +// --- + library IndicesOneBased { function fromOneBasedValue(uint256 oneBasedIndexValue) internal pure returns (IndexOneBased) { if (oneBasedIndexValue > type(uint32).max) { diff --git a/contracts/types/PercentD16.sol b/contracts/types/PercentD16.sol index c865cb96..e06184f1 100644 --- a/contracts/types/PercentD16.sol +++ b/contracts/types/PercentD16.sol @@ -1,13 +1,34 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.26; +// --- +// Type definition +// --- + type PercentD16 is uint256; +// --- +// Errors +// --- +error PercentD16Overflow(); + +// --- +// Constants +// --- + uint256 constant HUNDRED_PERCENTS_UINT256 = 100 * 10 ** 16; -error Overflow(); +// --- +// Assign global operations +// --- -using {lt as <, lte as <=, gte as >=, gt as >, minus as -, plus as +} for PercentD16 global; +using {lt as <, lte as <=, eq as ==, gte as >=, gt as >} for PercentD16 global; +using {minus as -, plus as +} for PercentD16 global; +using {toUint256} for PercentD16 global; + +// --- +// Comparison operations +// --- function lt(PercentD16 a, PercentD16 b) pure returns (bool) { return PercentD16.unwrap(a) < PercentD16.unwrap(b); @@ -17,6 +38,10 @@ function lte(PercentD16 a, PercentD16 b) pure returns (bool) { return PercentD16.unwrap(a) <= PercentD16.unwrap(b); } +function eq(PercentD16 a, PercentD16 b) pure returns (bool) { + return PercentD16.unwrap(a) == PercentD16.unwrap(b); +} + function gt(PercentD16 a, PercentD16 b) pure returns (bool) { return PercentD16.unwrap(a) > PercentD16.unwrap(b); } @@ -25,9 +50,13 @@ function gte(PercentD16 a, PercentD16 b) pure returns (bool) { return PercentD16.unwrap(a) >= PercentD16.unwrap(b); } +// --- +// Arithmetic operations +// --- + function minus(PercentD16 a, PercentD16 b) pure returns (PercentD16) { if (b > a) { - revert Overflow(); + revert PercentD16Overflow(); } return PercentD16.wrap(PercentD16.unwrap(a) - PercentD16.unwrap(b)); } @@ -36,6 +65,18 @@ function plus(PercentD16 a, PercentD16 b) pure returns (PercentD16) { return PercentD16.wrap(PercentD16.unwrap(a) + PercentD16.unwrap(b)); } +// --- +// Conversion operations +// --- + +function toUint256(PercentD16 value) pure returns (uint256) { + return PercentD16.unwrap(value); +} + +// --- +// Namespaced helper methods +// --- + library PercentsD16 { function fromBasisPoints(uint256 bpValue) internal pure returns (PercentD16) { return PercentD16.wrap(HUNDRED_PERCENTS_UINT256 * bpValue / 100_00); diff --git a/contracts/types/SharesValue.sol b/contracts/types/SharesValue.sol index 1ff60455..76c84590 100644 --- a/contracts/types/SharesValue.sol +++ b/contracts/types/SharesValue.sol @@ -3,20 +3,29 @@ pragma solidity 0.8.26; import {ETHValue, ETHValues} from "./ETHValue.sol"; +// --- +// Type definition +// --- + type SharesValue is uint128; +// --- +// Errors +// --- + error SharesValueOverflow(); -using {plus as +, minus as -, eq as ==, lt as <} for SharesValue global; -using {toUint256} for SharesValue global; +// --- +// Assign global operations +// --- -function plus(SharesValue v1, SharesValue v2) pure returns (SharesValue) { - return SharesValue.wrap(SharesValue.unwrap(v1) + SharesValue.unwrap(v2)); -} +using {eq as ==, lt as <} for SharesValue global; +using {plus as +, minus as -} for SharesValue global; +using {toUint256} for SharesValue global; -function minus(SharesValue v1, SharesValue v2) pure returns (SharesValue) { - return SharesValue.wrap(SharesValue.unwrap(v1) - SharesValue.unwrap(v2)); -} +// --- +// Comparison operations +// --- function lt(SharesValue v1, SharesValue v2) pure returns (bool) { return SharesValue.unwrap(v1) < SharesValue.unwrap(v2); @@ -26,10 +35,30 @@ function eq(SharesValue v1, SharesValue v2) pure returns (bool) { return SharesValue.unwrap(v1) == SharesValue.unwrap(v2); } +// --- +// Arithmetic operations +// --- + +function plus(SharesValue v1, SharesValue v2) pure returns (SharesValue) { + return SharesValue.wrap(SharesValue.unwrap(v1) + SharesValue.unwrap(v2)); +} + +function minus(SharesValue v1, SharesValue v2) pure returns (SharesValue) { + return SharesValue.wrap(SharesValue.unwrap(v1) - SharesValue.unwrap(v2)); +} + +// --- +// Custom operations +// --- + function toUint256(SharesValue v) pure returns (uint256) { return SharesValue.unwrap(v); } +// --- +// Namespaced helper methods +// --- + library SharesValues { SharesValue internal constant ZERO = SharesValue.wrap(0); diff --git a/contracts/types/Timestamp.sol b/contracts/types/Timestamp.sol index f153f71b..410ba70d 100644 --- a/contracts/types/Timestamp.sol +++ b/contracts/types/Timestamp.sol @@ -1,26 +1,33 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.26; +// --- +// Type definition +// --- + type Timestamp is uint40; +// --- +// Errors +// --- + error TimestampOverflow(); -error TimestampUnderflow(); -uint256 constant MAX_TIMESTAMP_VALUE = type(uint40).max; +// --- +// Assign global operations +// --- -using {lt as <} for Timestamp global; -using {gt as >} for Timestamp global; -using {gte as >=} for Timestamp global; -using {lte as <=} for Timestamp global; -using {eq as ==} for Timestamp global; -using {notEq as !=} for Timestamp global; +using {lt as <, lte as <=, eq as ==, neq as !=, gt as >, gte as >=} for Timestamp global; +using {isZero, isNotZero, toSeconds} for Timestamp global; -using {isZero} for Timestamp global; -using {isNotZero} for Timestamp global; -using {toSeconds} for Timestamp global; +// --- +// Constants +// --- + +uint256 constant MAX_TIMESTAMP_VALUE = type(uint40).max; // --- -// Comparison Ops +// Comparison operations // --- function lt(Timestamp t1, Timestamp t2) pure returns (bool) { @@ -43,10 +50,14 @@ function eq(Timestamp t1, Timestamp t2) pure returns (bool) { return Timestamp.unwrap(t1) == Timestamp.unwrap(t2); } -function notEq(Timestamp t1, Timestamp t2) pure returns (bool) { +function neq(Timestamp t1, Timestamp t2) pure returns (bool) { return !(t1 == t2); } +// --- +// Custom operations +// --- + function isZero(Timestamp t) pure returns (bool) { return Timestamp.unwrap(t) == 0; } @@ -56,14 +67,16 @@ function isNotZero(Timestamp t) pure returns (bool) { } // --- -// Conversion Ops +// Conversion operations // --- function toSeconds(Timestamp t) pure returns (uint256) { return Timestamp.unwrap(t); } -uint256 constant MAX_VALUE = type(uint40).max; +// --- +// Namespaced helper methods +// --- library Timestamps { Timestamp internal constant ZERO = Timestamp.wrap(0); diff --git a/test/unit/libraries/AssetsAccounting.t.sol b/test/unit/libraries/AssetsAccounting.t.sol index 27c3642d..723bacbd 100644 --- a/test/unit/libraries/AssetsAccounting.t.sol +++ b/test/unit/libraries/AssetsAccounting.t.sol @@ -1111,7 +1111,7 @@ contract AssetsAccountingUnitTests is UnitTest { _accountingContext.unstETHRecords[unstETHIds[0]].shares = SharesValues.from(123); claimableAmountsPrepared[0] = uint256(type(uint128).max - 2); - vm.expectRevert(stdError.arithmeticError); + vm.expectRevert(ETHValueOverflow.selector); AssetsAccounting.accountUnstETHFinalized(_accountingContext, unstETHIds, claimableAmountsPrepared); } @@ -1410,7 +1410,7 @@ contract AssetsAccountingUnitTests is UnitTest { ETHValues.from(uint256(type(uint128).max) / 2 + 1); } - vm.expectRevert(stdError.arithmeticError); + vm.expectRevert(ETHValueOverflow.selector); AssetsAccounting.accountUnstETHWithdraw(_accountingContext, holder, unstETHIds); } @@ -1499,10 +1499,6 @@ contract AssetsAccountingUnitTests is UnitTest { assertEq(a.toUint256(), b.toUint256()); } - function assertEq(ETHValue a, ETHValue b) internal { - assertEq(a.toUint256(), b.toUint256()); - } - function assertEq(UnstETHRecordStatus a, UnstETHRecordStatus b) internal { assertEq(uint256(a), uint256(b)); } diff --git a/test/unit/libraries/EscrowState.t.sol b/test/unit/libraries/EscrowState.t.sol index 9018073f..a86f9594 100644 --- a/test/unit/libraries/EscrowState.t.sol +++ b/test/unit/libraries/EscrowState.t.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.26; -import {Duration, Durations, MAX_VALUE as DURATION_MAX_VALUE} from "contracts/types/Duration.sol"; +import {Duration, Durations, MAX_DURATION_VALUE} from "contracts/types/Duration.sol"; import {Timestamp, Timestamps, MAX_TIMESTAMP_VALUE, TimestampOverflow} from "contracts/types/Timestamp.sol"; import {EscrowState, State} from "contracts/libraries/EscrowState.sol"; @@ -239,8 +239,8 @@ contract EscrowStateUnitTests is UnitTest { } function test_checkWithdrawalsDelayPassed_RevertWhen_EthWithdrawalsDelayOverflow() external { - Duration rageQuitExtensionPeriodDuration = Durations.from(DURATION_MAX_VALUE / 2); - Duration rageQuitEthWithdrawalsDelay = Durations.from(DURATION_MAX_VALUE / 2 + 1); + Duration rageQuitExtensionPeriodDuration = Durations.from(MAX_DURATION_VALUE / 2); + Duration rageQuitEthWithdrawalsDelay = Durations.from(MAX_DURATION_VALUE / 2 + 1); _context.rageQuitExtensionPeriodStartedAt = Timestamps.from(MAX_TIMESTAMP_VALUE - 1); _context.rageQuitExtensionPeriodDuration = rageQuitExtensionPeriodDuration; diff --git a/test/utils/testing-assert-eq-extender.sol b/test/utils/testing-assert-eq-extender.sol index 5d350c77..421f3a62 100644 --- a/test/utils/testing-assert-eq-extender.sol +++ b/test/utils/testing-assert-eq-extender.sol @@ -3,9 +3,11 @@ pragma solidity 0.8.26; import {Test} from "forge-std/Test.sol"; +import {ETHValue} from "contracts/types/ETHValue.sol"; import {Duration} from "contracts/types/Duration.sol"; import {Timestamp} from "contracts/types/Timestamp.sol"; import {PercentD16} from "contracts/types/PercentD16.sol"; +import {IndexOneBased} from "contracts/types/IndexOneBased.sol"; import {Status as ProposalStatus} from "contracts/libraries/ExecutableProposals.sol"; import {State as DualGovernanceState} from "contracts/DualGovernance.sol"; @@ -49,4 +51,12 @@ contract TestingAssertEqExtender is Test { function assertEq(PercentD16 a, PercentD16 b) internal { assertEq(PercentD16.unwrap(a), PercentD16.unwrap(b)); } + + function assertEq(ETHValue a, ETHValue b) internal { + assertEq(ETHValue.unwrap(a), ETHValue.unwrap(b)); + } + + function assertEq(IndexOneBased a, IndexOneBased b) internal { + assertEq(IndexOneBased.unwrap(a), IndexOneBased.unwrap(b)); + } } From b700b71507c5395dc99eb42e9662ce1dc67cbbcc Mon Sep 17 00:00:00 2001 From: Bogdan Kovtun Date: Wed, 18 Sep 2024 10:44:40 +0500 Subject: [PATCH 2/8] Add tests for Duration, ETHValue, IndexOneBased --- test/unit/types/Duration.t.sol | 219 ++++++++++++++++++++++++++++ test/unit/types/ETHValue.t.sol | 135 +++++++++++++++++ test/unit/types/IndexOneBased.t.sol | 45 ++++++ 3 files changed, 399 insertions(+) create mode 100644 test/unit/types/Duration.t.sol create mode 100644 test/unit/types/ETHValue.t.sol create mode 100644 test/unit/types/IndexOneBased.t.sol diff --git a/test/unit/types/Duration.t.sol b/test/unit/types/Duration.t.sol new file mode 100644 index 00000000..49c7b5e5 --- /dev/null +++ b/test/unit/types/Duration.t.sol @@ -0,0 +1,219 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.26; + +import {Math} from "@openzeppelin/contracts/utils/math/Math.sol"; + +import { + Duration, Durations, MAX_DURATION_VALUE, DurationOverflow, DurationUnderflow +} from "contracts/types/Duration.sol"; +import {Timestamp, TimestampOverflow, MAX_TIMESTAMP_VALUE} from "contracts/types/Timestamp.sol"; + +import {stdError} from "forge-std/StdError.sol"; +import {UnitTest} from "test/utils/unit-test.sol"; + +contract DurationTests is UnitTest { + uint256 private constant MAX_SECONDS_VALUE = type(uint256).max - MAX_DURATION_VALUE; + // --- + // Conversion operations + // --- + + function testFuzz_toSeconds_HappyPath(Duration d) external { + assertEq(d.toSeconds(), Duration.unwrap(d)); + } + + // --- + // Comparison operations + // --- + + function testFuzz_lt_HappyPath(Duration d1, Duration d2) external { + assertEq(d1 < d2, d1.toSeconds() < d2.toSeconds()); + } + + function testFuzz_lte_HappyPath(Duration d1, Duration d2) external { + assertEq(d1 <= d2, d1.toSeconds() <= d2.toSeconds()); + } + + function testFuzz_eq_HappyPath(Duration d1, Duration d2) external { + assertEq(d1 == d2, d1.toSeconds() == d2.toSeconds()); + } + + function testFuzz_neq_HappyPath(Duration d1, Duration d2) external { + assertEq(d1 != d2, d1.toSeconds() != d2.toSeconds()); + } + + function testFuzz_gt_HappyPath(Duration d1, Duration d2) external { + assertEq(d1 > d2, d1.toSeconds() > d2.toSeconds()); + } + + function testFuzz_gte_HappyPath(Duration d1, Duration d2) external { + assertEq(d1 >= d2, d1.toSeconds() >= d2.toSeconds()); + } + + // --- + // Arithmetic operations + // --- + + function testFuzz_plus_HappyPath(Duration d1, Duration d2) external { + uint256 expectedResult = d1.toSeconds() + d2.toSeconds(); + vm.assume(expectedResult <= MAX_DURATION_VALUE); + + assertEq(d1 + d2, Duration.wrap(uint32(expectedResult))); + } + + function testFuzz_plus_Overflow(Duration d1, Duration d2) external { + vm.assume(d1.toSeconds() + d2.toSeconds() > MAX_DURATION_VALUE); + vm.expectRevert(DurationOverflow.selector); + d1 + d2; + } + + function testFuzz_minus_HappyPath(Duration d1, Duration d2) external { + vm.assume(d1 >= d2); + assertEq(d1 - d2, Durations.from(d1.toSeconds() - d2.toSeconds())); + } + + function testFuzz_minus_Underflow(Duration d1, Duration d2) external { + vm.assume(d1 < d2); + vm.expectRevert(DurationUnderflow.selector); + d1 - d2; + } + + // --- + // Custom operations + // --- + + // --- + // plusSeconds() + // --- + + function testFuzz_plusSeconds_HappyPath(Duration d, uint256 seconds_) external { + vm.assume(seconds_ < MAX_SECONDS_VALUE); + vm.assume(d.toSeconds() + seconds_ <= MAX_DURATION_VALUE); + + assertEq(d.plusSeconds(seconds_), Duration.wrap(uint32(d.toSeconds() + seconds_))); + } + + function testFuzz_plusSeconds_Overflow(Duration d, uint256 seconds_) external { + vm.assume(seconds_ < MAX_SECONDS_VALUE); + vm.assume(d.toSeconds() + seconds_ > MAX_DURATION_VALUE); + + vm.expectRevert(DurationOverflow.selector); + d.plusSeconds(seconds_); + } + + // --- + // minusSeconds() + // --- + + function testFuzz_minusSeconds_HappyPath(Duration d, uint256 seconds_) external { + vm.assume(seconds_ <= d.toSeconds()); + + assertEq(d.minusSeconds(seconds_), Duration.wrap(uint32(d.toSeconds() - seconds_))); + } + + function testFuzz_minusSeconds_Overflow(Duration d, uint256 seconds_) external { + vm.assume(seconds_ > d.toSeconds()); + + vm.expectRevert(DurationUnderflow.selector); + d.minusSeconds(seconds_); + } + + // --- + // dividedBy() + // --- + + function testFuzz_dividedBy_HappyPath(Duration d, uint256 divisor) external { + vm.assume(divisor != 0); + assertEq(d.dividedBy(divisor), Duration.wrap(uint32(d.toSeconds() / divisor))); + } + + function testFuzz_dividedBy_RevertOn_DivisorIsZero(Duration d) external { + vm.expectRevert(stdError.divisionError); + d.dividedBy(0); + } + + // --- + // multipliedBy() + // --- + + function testFuzz_multipliedBy_HappyPath(Duration d, uint256 multiplicand) external { + (bool isSuccess, uint256 expectedResult) = Math.tryMul(d.toSeconds(), multiplicand); + vm.assume(isSuccess && expectedResult <= MAX_DURATION_VALUE); + assertEq(d.multipliedBy(multiplicand), Duration.wrap(uint32(expectedResult))); + } + + function testFuzz_multipliedBy_RevertOn_ResultOverflow(Duration d, uint256 multiplicand) external { + (bool isSuccess, uint256 expectedResult) = Math.tryMul(d.toSeconds(), multiplicand); + vm.assume(isSuccess && expectedResult > MAX_DURATION_VALUE); + vm.expectRevert(DurationOverflow.selector); + d.multipliedBy(multiplicand); + } + + // --- + // addTo() + // --- + + function testFuzz_addTo_HappyPath(Duration d, Timestamp t) external { + (bool isSuccess, uint256 expectedResult) = Math.tryAdd(t.toSeconds(), d.toSeconds()); + vm.assume(isSuccess && expectedResult <= MAX_TIMESTAMP_VALUE); + assertEq(d.addTo(t), Timestamp.wrap(uint40(expectedResult))); + } + + function testFuzz_addTo_RevertOn_Overflow(Duration d, Timestamp t) external { + (bool isSuccess, uint256 expectedResult) = Math.tryAdd(t.toSeconds(), d.toSeconds()); + vm.assume(isSuccess && expectedResult > MAX_TIMESTAMP_VALUE); + vm.expectRevert(TimestampOverflow.selector); + d.addTo(t); + } + + // --- + // Namespaced helper methods + // --- + + function test_ZERO_CorrectValue() external { + assertEq(Durations.ZERO, Duration.wrap(0)); + } + + function test_MIN_CorrectValue() external { + assertEq(Durations.MIN, Duration.wrap(0)); + } + + function test_MAX_CorrectValue() external { + assertEq(Durations.MAX, Duration.wrap(uint32(MAX_DURATION_VALUE))); + } + + function testFuzz_from_HappyPath(uint256 seconds_) external { + vm.assume(seconds_ <= MAX_DURATION_VALUE); + assertEq(Durations.from(seconds_), Duration.wrap(uint32(seconds_))); + } + + function testFuzz_from_RevertOn_Overflow(uint256 seconds_) external { + vm.assume(seconds_ > MAX_DURATION_VALUE); + vm.expectRevert(DurationOverflow.selector); + Durations.from(seconds_); + } + + function testFuzz_between_HappyPath(Timestamp t1, Timestamp t2) external { + uint256 t1Seconds = t1.toSeconds(); + uint256 t2Seconds = t2.toSeconds(); + uint256 expectedValue = t1Seconds > t2Seconds ? t1Seconds - t2Seconds : t2Seconds - t1Seconds; + + vm.assume(expectedValue <= MAX_DURATION_VALUE); + + assertEq(Durations.between(t1, t2), Duration.wrap(uint32(expectedValue))); + } + + function testFuzz_between_RevertOn_Overflow(Timestamp t1, Timestamp t2) external { + uint256 t1Seconds = t1.toSeconds(); + uint256 t2Seconds = t2.toSeconds(); + uint256 expectedValue = t1Seconds > t2Seconds ? t1Seconds - t2Seconds : t2Seconds - t1Seconds; + + vm.assume(expectedValue > MAX_DURATION_VALUE); + + vm.expectRevert(DurationOverflow.selector); + Durations.between(t1, t2); + } + + function testFuzz_min_HappyPath(Duration d1, Duration d2) external { + assertEq(Durations.min(d1, d2), Durations.from(Math.min(d1.toSeconds(), d2.toSeconds()))); + } +} diff --git a/test/unit/types/ETHValue.t.sol b/test/unit/types/ETHValue.t.sol new file mode 100644 index 00000000..3318ab34 --- /dev/null +++ b/test/unit/types/ETHValue.t.sol @@ -0,0 +1,135 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.26; + +import {Address} from "@openzeppelin/contracts/utils/Address.sol"; + +import {ETHValue, ETHValues, ETHValueOverflow, ETHValueUnderflow} from "contracts/types/ETHValue.sol"; + +import {UnitTest} from "test/utils/unit-test.sol"; + +contract ETHTransfersForbiddenStub { + error ETHTransfersForbidden(); + + receive() external payable { + revert ETHTransfersForbidden(); + } +} + +contract ETHValueTests is UnitTest { + uint256 internal constant _MAX_ETH_SEND = 1_000_000 ether; + uint256 internal constant _MAX_ETH_VALUE = type(uint128).max; + address internal immutable _RECIPIENT = makeAddr("RECIPIENT"); + + // --- + // Comparison operations + // --- + + function testFuzz_lt_HappyPath(ETHValue v1, ETHValue v2) external { + assertEq(v1 < v2, ETHValue.unwrap(v1) < ETHValue.unwrap(v2)); + } + + function testFuzz_eq_HappyPath(ETHValue v1, ETHValue v2) external { + assertEq(v1 == v2, ETHValue.unwrap(v1) == ETHValue.unwrap(v2)); + } + + function testFuzz_neq_HappyPath(ETHValue v1, ETHValue v2) external { + assertEq(v1 != v2, ETHValue.unwrap(v1) != ETHValue.unwrap(v2)); + } + + function testFuzz_gt_HappyPath(ETHValue v1, ETHValue v2) external { + assertEq(v1 > v2, ETHValue.unwrap(v1) > ETHValue.unwrap(v2)); + } + + // --- + // Arithmetic operations + // --- + + function testFuzz_plus_HappyPath(ETHValue v1, ETHValue v2) external { + uint256 expectedResult = v1.toUint256() + v2.toUint256(); + vm.assume(expectedResult <= _MAX_ETH_VALUE); + assertEq(v1 + v2, ETHValue.wrap(uint128(expectedResult))); + } + + function testFuzz_plus_Overflow(ETHValue v1, ETHValue v2) external { + uint256 expectedResult = v1.toUint256() + v2.toUint256(); + vm.assume(expectedResult > _MAX_ETH_VALUE); + vm.expectRevert(ETHValueOverflow.selector); + v1 + v2; + } + + function testFuzz_minus_HappyPath(ETHValue v1, ETHValue v2) external { + vm.assume(v1 > v2); + uint256 expectedResult = v1.toUint256() - v2.toUint256(); + assertEq(v1 - v2, ETHValue.wrap(uint128(expectedResult))); + } + + function testFuzz_minus_Overflow(ETHValue v1, ETHValue v2) external { + vm.assume(v1 < v2); + vm.expectRevert(ETHValueUnderflow.selector); + v1 - v2; + } + + // --- + // Custom operations + // --- + + function testFuzz_sendTo_HappyPath(ETHValue amount, uint256 balance) external { + vm.assume(balance <= _MAX_ETH_SEND); + vm.assume(amount.toUint256() <= balance); + + vm.deal(address(this), balance); + + assertEq(_RECIPIENT.balance, 0); + + amount.sendTo(payable(_RECIPIENT)); + assertEq(_RECIPIENT.balance, amount.toUint256()); + } + + function testFuzz_sendTo_RevertOn_InsufficientBalance(ETHValue amount, uint256 balance) external { + vm.assume(balance <= _MAX_ETH_SEND); + vm.assume(amount.toUint256() > balance); + + vm.deal(address(this), balance); + + vm.expectRevert(abi.encodeWithSelector(Address.AddressInsufficientBalance.selector, address(this))); + this.sendTo__external(amount, payable(_RECIPIENT)); + } + + function testFuzz_sendTo_RevertOn_ETHTransfersForbidden(ETHValue amount, uint256 balance) external { + vm.assume(balance <= _MAX_ETH_SEND); + vm.assume(amount.toUint256() <= balance); + + vm.deal(address(this), balance); + + assertEq(_RECIPIENT.balance, 0); + + ETHTransfersForbiddenStub ethTransfersForbiddenStub = new ETHTransfersForbiddenStub(); + vm.expectRevert(Address.FailedInnerCall.selector); + this.sendTo__external(amount, payable(address(ethTransfersForbiddenStub))); + } + + function testFuzz_toUint256_HappyPath(ETHValue amount) external { + assertEq(amount.toUint256(), ETHValue.unwrap(amount)); + } + + function testFuzz_from_HappyPath(uint256 amount) external { + vm.assume(amount <= _MAX_ETH_VALUE); + assertEq(ETHValues.from(amount), ETHValue.wrap(uint128(amount))); + } + + function testFuzz_from_RevertOn_Overflow(uint256 amount) external { + vm.assume(amount > _MAX_ETH_VALUE); + vm.expectRevert(ETHValueOverflow.selector); + ETHValues.from(amount); + } + + function testFuzz_fromAddressBalance_HappyPath(ETHValue balance) external { + vm.assume(balance.toUint256() <= _MAX_ETH_SEND); + vm.deal(address(this), balance.toUint256()); + assertEq(balance, ETHValue.wrap(uint128(address(this).balance))); + } + + function sendTo__external(ETHValue amount, address payable recipient) external { + amount.sendTo(recipient); + } +} diff --git a/test/unit/types/IndexOneBased.t.sol b/test/unit/types/IndexOneBased.t.sol new file mode 100644 index 00000000..f855bf25 --- /dev/null +++ b/test/unit/types/IndexOneBased.t.sol @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.26; + +import {IndicesOneBased, IndexOneBased, IndexOneBasedUnderflow} from "contracts/types/IndexOneBased.sol"; + +import {UnitTest} from "test/utils/unit-test.sol"; + +contract IndexOneBasedUnitTests is UnitTest { + // --- + // Comparison operations + // --- + + function testFuzz_neq_HappyPath(IndexOneBased i1, IndexOneBased i2) external { + assertEq(i1 != i2, IndexOneBased.unwrap(i1) != IndexOneBased.unwrap(i2)); + } + + function test_isEmpty_HappyPath() external { + assertTrue(IndexOneBased.wrap(0).isEmpty()); + assertFalse(IndicesOneBased.fromOneBasedValue(1).isEmpty()); + } + + function testFuzz_isEmpty_HappyPath(IndexOneBased i1) external { + assertEq(i1.isEmpty(), IndexOneBased.unwrap(i1) == 0); + } + + function test_isNotEmpty_HappyPath() external { + assertTrue(IndicesOneBased.fromOneBasedValue(1).isNotEmpty()); + assertFalse(IndexOneBased.wrap(0).isNotEmpty()); + } + + function testFuzz_isNotEmpty_HappyPath(IndexOneBased i1) external { + assertEq(i1.isNotEmpty(), IndexOneBased.unwrap(i1) != 0); + } + + function testFuzz_toZeroBasedValue_HappyPath(IndexOneBased index) external { + vm.assume(IndexOneBased.unwrap(index) > 0); + assertEq(index.toZeroBasedValue(), IndexOneBased.unwrap(index) - 1); + } + + function test_toZeroBasedValue_RevertOn_EmptyIndex(IndexOneBased index) external { + IndexOneBased emptyIndex = IndexOneBased.wrap(0); + vm.expectRevert(IndexOneBasedUnderflow.selector); + emptyIndex.toZeroBasedValue(); + } +} From 7081e650b3af83a442c0c14904c8ab85036a8f73 Mon Sep 17 00:00:00 2001 From: Bogdan Kovtun Date: Mon, 23 Sep 2024 13:06:30 +0400 Subject: [PATCH 3/8] Custom types code standardization. Add tests for all types. --- contracts/libraries/AssetsAccounting.sol | 6 +- contracts/libraries/DualGovernanceConfig.sol | 4 +- contracts/types/Duration.sol | 96 +++-- contracts/types/ETHValue.sol | 44 ++- contracts/types/IndexOneBased.sol | 21 +- contracts/types/PercentD16.sol | 79 ++-- contracts/types/SharesValue.sol | 58 +-- contracts/types/Timestamp.sol | 52 +-- test/unit/libraries/AssetsAccounting.t.sol | 20 +- .../unit/libraries/DualGovernanceConfig.t.sol | 4 +- test/unit/libraries/ExecutableProposals.t.sol | 10 +- test/unit/types/Duration.t.sol | 234 +++++++++--- test/unit/types/ETHValue.t.sol | 35 +- test/unit/types/IndexOneBased.t.sol | 65 +++- test/unit/types/PercentD16.t.sol | 342 ++++++++++++++++++ test/unit/types/SharesValue.t.sol | 167 +++++++++ test/unit/types/Timestamp.t.sol | 189 ++++++++++ test/utils/testing-assert-eq-extender.sol | 5 + 18 files changed, 1218 insertions(+), 213 deletions(-) create mode 100644 test/unit/types/PercentD16.t.sol create mode 100644 test/unit/types/SharesValue.t.sol create mode 100644 test/unit/types/Timestamp.t.sol diff --git a/contracts/libraries/AssetsAccounting.sol b/contracts/libraries/AssetsAccounting.sol index c47af26c..a57b1769 100644 --- a/contracts/libraries/AssetsAccounting.sol +++ b/contracts/libraries/AssetsAccounting.sol @@ -167,8 +167,10 @@ library AssetsAccounting { _checkNonZeroShares(stETHSharesToWithdraw); assets.stETHLockedShares = SharesValues.ZERO; - ethWithdrawn = - SharesValues.calcETHValue(self.stETHTotals.claimedETH, stETHSharesToWithdraw, self.stETHTotals.lockedShares); + ethWithdrawn = ETHValues.from( + self.stETHTotals.claimedETH.toUint256() * stETHSharesToWithdraw.toUint256() + / self.stETHTotals.lockedShares.toUint256() + ); emit ETHWithdrawn(holder, stETHSharesToWithdraw, ethWithdrawn); } diff --git a/contracts/libraries/DualGovernanceConfig.sol b/contracts/libraries/DualGovernanceConfig.sol index 4c723896..f95f3d1d 100644 --- a/contracts/libraries/DualGovernanceConfig.sol +++ b/contracts/libraries/DualGovernanceConfig.sol @@ -119,9 +119,9 @@ library DualGovernanceConfig { return vetoSignallingMinDuration + Durations.from( - PercentD16.unwrap(rageQuitSupport - firstSealRageQuitSupport) + (rageQuitSupport - firstSealRageQuitSupport).toUint256() * (vetoSignallingMaxDuration - vetoSignallingMinDuration).toSeconds() - / PercentD16.unwrap(secondSealRageQuitSupport - firstSealRageQuitSupport) + / (secondSealRageQuitSupport - firstSealRageQuitSupport).toUint256() ); } diff --git a/contracts/types/Duration.sol b/contracts/types/Duration.sol index 3a412e2c..6fdd12cb 100644 --- a/contracts/types/Duration.sol +++ b/contracts/types/Duration.sol @@ -10,27 +10,27 @@ import {Timestamp, Timestamps} from "./Timestamp.sol"; type Duration is uint32; // --- -// Errors +// Assign global operations // --- -error DurationOverflow(); -error DurationUnderflow(); -error TimestampUnderflow(); +using {lt as <, lte as <=, eq as ==, neq as !=, gte as >=, gt as >} for Duration global; +using {addTo, plusSeconds, minusSeconds, multipliedBy, dividedBy, toSeconds} for Duration global; +using {plus as +, minus as -} for Duration global; // --- -// Assign global operations +// Errors // --- -using {plus as +, minus as -} for Duration global; -using {lt as <, lte as <=, eq as ==, neq as !=, gt as >, gte as >=} for Duration global; -using {addTo, plusSeconds, minusSeconds, multipliedBy, dividedBy, toSeconds} for Duration global; +error DivisionByZero(); +error DurationOverflow(); +error DurationUnderflow(); // --- // Constants // --- -/// @dev The max possible duration is about 106 years -uint256 constant MAX_DURATION_VALUE = type(uint32).max; +/// @dev The maximum possible duration is approximately 136 years (assuming 365 days per year). +uint32 constant MAX_DURATION_VALUE = type(uint32).max; // --- // Comparison operations @@ -60,22 +60,38 @@ function gt(Duration d1, Duration d2) pure returns (bool) { return Duration.unwrap(d1) > Duration.unwrap(d2); } +// --- +// Conversion operations +// --- + +function toSeconds(Duration d) pure returns (uint256) { + return Duration.unwrap(d); +} + // --- // Arithmetic operations // --- function plus(Duration d1, Duration d2) pure returns (Duration) { unchecked { + /// @dev Both `d1.toSeconds()` and `d2.toSeconds()` are <= type(uint32).max. Therefore, their + /// sum is <= type(uint256).max. return Durations.from(d1.toSeconds() + d2.toSeconds()); } } function minus(Duration d1, Duration d2) pure returns (Duration) { - if (d1 < d2) { + uint256 d1Seconds = d1.toSeconds(); + uint256 d2Seconds = d2.toSeconds(); + + if (d1Seconds < d2Seconds) { revert DurationUnderflow(); } + unchecked { - return Duration.wrap(uint32(d1.toSeconds() - d2.toSeconds())); + /// @dev Subtraction is safe because `d1Seconds` >= `d2Seconds`. + /// Both `d1Seconds` and `d2Seconds` <= `type(uint32).max`, so the difference fits within `uint32`. + return Duration.wrap(uint32(d1Seconds - d2Seconds)); } } @@ -83,20 +99,30 @@ function minus(Duration d1, Duration d2) pure returns (Duration) { // Custom operations // --- -function plusSeconds(Duration d, uint256 seconds_) pure returns (Duration) { - return Durations.from(Duration.unwrap(d) + seconds_); +function plusSeconds(Duration d, uint256 secondsToAdd) pure returns (Duration) { + return Durations.from(d.toSeconds() + secondsToAdd); } -function minusSeconds(Duration d, uint256 seconds_) pure returns (Duration) { - uint256 durationValue = Duration.unwrap(d); - if (durationValue < seconds_) { +function minusSeconds(Duration d, uint256 secondsToSubtract) pure returns (Duration) { + uint256 durationSeconds = d.toSeconds(); + + if (durationSeconds < secondsToSubtract) { revert DurationUnderflow(); } - return Duration.wrap(uint32(durationValue - seconds_)); + + unchecked { + /// @dev Subtraction is safe because `durationSeconds` >= `secondsToSubtract`. + /// Both `durationSeconds` and `secondsToSubtract` <= `type(uint32).max`, + /// so the difference fits within `uint32`. + return Duration.wrap(uint32(durationSeconds - secondsToSubtract)); + } } function dividedBy(Duration d, uint256 divisor) pure returns (Duration) { - return Duration.wrap(uint32(Duration.unwrap(d) / divisor)); + if (divisor == 0) { + revert DivisionByZero(); + } + return Duration.wrap(uint32(d.toSeconds() / divisor)); } function multipliedBy(Duration d, uint256 multiplicand) pure returns (Duration) { @@ -104,15 +130,11 @@ function multipliedBy(Duration d, uint256 multiplicand) pure returns (Duration) } function addTo(Duration d, Timestamp t) pure returns (Timestamp) { - return Timestamps.from(t.toSeconds() + d.toSeconds()); -} - -// --- -// Conversion operations -// --- - -function toSeconds(Duration d) pure returns (uint256) { - return Duration.unwrap(d); + unchecked { + /// @dev Both `t.toSeconds()` <= `type(uint40).max` and `d.toSeconds()` <= `type(uint32).max`, so their + /// sum fits within `uint256`. + return Timestamps.from(t.toSeconds() + d.toSeconds()); + } } // --- @@ -122,18 +144,22 @@ function toSeconds(Duration d) pure returns (uint256) { library Durations { Duration internal constant ZERO = Duration.wrap(0); - Duration internal constant MIN = ZERO; - Duration internal constant MAX = Duration.wrap(uint32(MAX_DURATION_VALUE)); - - function from(uint256 seconds_) internal pure returns (Duration res) { - if (seconds_ > MAX_DURATION_VALUE) { + function from(uint256 durationInSeconds) internal pure returns (Duration res) { + if (durationInSeconds > MAX_DURATION_VALUE) { revert DurationOverflow(); } - res = Duration.wrap(uint32(seconds_)); + res = Duration.wrap(uint32(durationInSeconds)); } function between(Timestamp t1, Timestamp t2) internal pure returns (Duration res) { - res = from(t1 > t2 ? t1.toSeconds() - t2.toSeconds() : t2.toSeconds() - t1.toSeconds()); + uint256 t1Seconds = t1.toSeconds(); + uint256 t2Seconds = t2.toSeconds(); + + unchecked { + /// @dev Calculating the absolute difference between `t1Seconds` and `t2Seconds`. + /// Both are <= `type(uint40).max`, so their difference fits within `uint256`. + res = from(t1Seconds > t2Seconds ? t1Seconds - t2Seconds : t2Seconds - t1Seconds); + } } function min(Duration d1, Duration d2) internal pure returns (Duration res) { diff --git a/contracts/types/ETHValue.sol b/contracts/types/ETHValue.sol index 5f822a2a..01543b53 100644 --- a/contracts/types/ETHValue.sol +++ b/contracts/types/ETHValue.sol @@ -9,6 +9,14 @@ import {Address} from "@openzeppelin/contracts/utils/Address.sol"; type ETHValue is uint128; +// --- +// Assign global operations +// --- + +using {lt as <, eq as ==, neq as !=, gt as >} for ETHValue global; +using {toUint256, sendTo} for ETHValue global; +using {plus as +, minus as -} for ETHValue global; + // --- // Errors // --- @@ -17,12 +25,10 @@ error ETHValueOverflow(); error ETHValueUnderflow(); // --- -// Assign global operations +// Constants // --- -using {lt as <, gt as >, eq as ==, neq as !=} for ETHValue global; -using {plus as +, minus as -} for ETHValue global; -using {toUint256, sendTo} for ETHValue global; +uint128 constant MAX_ETH_VALUE = type(uint128).max; // --- // Comparison operations @@ -44,22 +50,38 @@ function gt(ETHValue v1, ETHValue v2) pure returns (bool) { return ETHValue.unwrap(v1) > ETHValue.unwrap(v2); } +// --- +// Conversion operations +// --- + +function toUint256(ETHValue value) pure returns (uint256) { + return ETHValue.unwrap(value); +} + // --- // Arithmetic operations // --- function plus(ETHValue v1, ETHValue v2) pure returns (ETHValue) { unchecked { + /// @dev Both `v1.toUint256()` and `v2.toUint256()` are <= type(uint128).max. Therefore, their + /// sum is <= type(uint256).max. return ETHValues.from(v1.toUint256() + v2.toUint256()); } } function minus(ETHValue v1, ETHValue v2) pure returns (ETHValue) { - if (v1 < v2) { + uint256 v1Value = v1.toUint256(); + uint256 v2Value = v2.toUint256(); + + if (v1Value < v2Value) { revert ETHValueUnderflow(); } + unchecked { - return ETHValues.from(ETHValue.unwrap(v1) - ETHValue.unwrap(v2)); + /// @dev Subtraction is safe because `v1Value` >= `v2Value`. + /// Both `v1Value` and `v2Value` <= `type(uint128).max`, so the difference fits within `uint128`. + return ETHValue.wrap(uint128(v1Value - v2Value)); } } @@ -71,14 +93,6 @@ function sendTo(ETHValue value, address payable recipient) { Address.sendValue(recipient, value.toUint256()); } -// --- -// Conversion operations -// --- - -function toUint256(ETHValue value) pure returns (uint256) { - return ETHValue.unwrap(value); -} - // --- // Namespaced helper methods // --- @@ -87,7 +101,7 @@ library ETHValues { ETHValue internal constant ZERO = ETHValue.wrap(0); function from(uint256 value) internal pure returns (ETHValue) { - if (value > type(uint128).max) { + if (value > MAX_ETH_VALUE) { revert ETHValueOverflow(); } return ETHValue.wrap(uint128(value)); diff --git a/contracts/types/IndexOneBased.sol b/contracts/types/IndexOneBased.sol index 68315dbe..847a4a14 100644 --- a/contracts/types/IndexOneBased.sol +++ b/contracts/types/IndexOneBased.sol @@ -7,6 +7,13 @@ pragma solidity 0.8.26; type IndexOneBased is uint32; +// --- +// Assign global operations +// --- + +using {neq as !=} for IndexOneBased global; +using {isEmpty, isNotEmpty, toZeroBasedValue} for IndexOneBased global; + // --- // Errors // --- @@ -15,11 +22,10 @@ error IndexOneBasedOverflow(); error IndexOneBasedUnderflow(); // --- -// Assign global operations +// Constants // --- -using {neq as !=} for IndexOneBased global; -using {isEmpty, isNotEmpty, toZeroBasedValue} for IndexOneBased global; +uint32 constant MAX_INDEX_ONE_BASED_VALUE = type(uint32).max; // --- // Comparison operations @@ -38,7 +44,7 @@ function isEmpty(IndexOneBased index) pure returns (bool) { } function isNotEmpty(IndexOneBased index) pure returns (bool) { - return IndexOneBased.unwrap(index) != 0; + return IndexOneBased.unwrap(index) > 0; } function toZeroBasedValue(IndexOneBased index) pure returns (uint256) { @@ -46,6 +52,8 @@ function toZeroBasedValue(IndexOneBased index) pure returns (uint256) { revert IndexOneBasedUnderflow(); } unchecked { + /// @dev Subtraction is safe because `index` is not zero. + /// The result fits within `uint32`, so casting to `uint256` is safe. return IndexOneBased.unwrap(index) - 1; } } @@ -56,7 +64,10 @@ function toZeroBasedValue(IndexOneBased index) pure returns (uint256) { library IndicesOneBased { function fromOneBasedValue(uint256 oneBasedIndexValue) internal pure returns (IndexOneBased) { - if (oneBasedIndexValue > type(uint32).max) { + if (oneBasedIndexValue == 0) { + revert IndexOneBasedUnderflow(); + } + if (oneBasedIndexValue > MAX_INDEX_ONE_BASED_VALUE) { revert IndexOneBasedOverflow(); } return IndexOneBased.wrap(uint32(oneBasedIndexValue)); diff --git a/contracts/types/PercentD16.sol b/contracts/types/PercentD16.sol index e06184f1..d1f725cd 100644 --- a/contracts/types/PercentD16.sol +++ b/contracts/types/PercentD16.sol @@ -5,26 +5,31 @@ pragma solidity 0.8.26; // Type definition // --- -type PercentD16 is uint256; +type PercentD16 is uint128; // --- -// Errors +// Assign global operations // --- -error PercentD16Overflow(); + +using {lt as <, lte as <=, eq as ==, gte as >=, gt as >} for PercentD16 global; +using {toUint256} for PercentD16 global; +using {minus as -, plus as +} for PercentD16 global; // --- -// Constants +// Errors // --- -uint256 constant HUNDRED_PERCENTS_UINT256 = 100 * 10 ** 16; +error DivisionByZero(); +error PercentD16Overflow(); +error PercentD16Underflow(); // --- -// Assign global operations +// Constants // --- -using {lt as <, lte as <=, eq as ==, gte as >=, gt as >} for PercentD16 global; -using {minus as -, plus as +} for PercentD16 global; -using {toUint256} for PercentD16 global; +uint128 constant HUNDRED_PERCENT_BP = 100_00; +uint128 constant MAX_PERCENT_D16 = type(uint128).max; +uint128 constant HUNDRED_PERCENT_D16 = 100 * 10 ** 16; // --- // Comparison operations @@ -42,35 +47,47 @@ function eq(PercentD16 a, PercentD16 b) pure returns (bool) { return PercentD16.unwrap(a) == PercentD16.unwrap(b); } +function gte(PercentD16 a, PercentD16 b) pure returns (bool) { + return PercentD16.unwrap(a) >= PercentD16.unwrap(b); +} + function gt(PercentD16 a, PercentD16 b) pure returns (bool) { return PercentD16.unwrap(a) > PercentD16.unwrap(b); } -function gte(PercentD16 a, PercentD16 b) pure returns (bool) { - return PercentD16.unwrap(a) >= PercentD16.unwrap(b); +// --- +// Conversion operations +// --- + +function toUint256(PercentD16 value) pure returns (uint256) { + return PercentD16.unwrap(value); } // --- // Arithmetic operations // --- -function minus(PercentD16 a, PercentD16 b) pure returns (PercentD16) { - if (b > a) { - revert PercentD16Overflow(); +function plus(PercentD16 a, PercentD16 b) pure returns (PercentD16) { + unchecked { + /// @dev Both `a.toUint256()` and `b.toUint256()` are <= type(uint128).max. Therefore, their + /// sum is <= type(uint256).max. + return PercentsD16.from(a.toUint256() + b.toUint256()); } - return PercentD16.wrap(PercentD16.unwrap(a) - PercentD16.unwrap(b)); } -function plus(PercentD16 a, PercentD16 b) pure returns (PercentD16) { - return PercentD16.wrap(PercentD16.unwrap(a) + PercentD16.unwrap(b)); -} +function minus(PercentD16 a, PercentD16 b) pure returns (PercentD16) { + uint256 aValue = a.toUint256(); + uint256 bValue = b.toUint256(); -// --- -// Conversion operations -// --- + if (aValue < bValue) { + revert PercentD16Underflow(); + } -function toUint256(PercentD16 value) pure returns (uint256) { - return PercentD16.unwrap(value); + unchecked { + /// @dev Subtraction is safe because `aValue` >= `bValue`. + /// Both `aValue` and `bValue` <= `type(uint128).max`, so the difference fits within `uint128`. + return PercentD16.wrap(uint128(aValue - bValue)); + } } // --- @@ -78,11 +95,21 @@ function toUint256(PercentD16 value) pure returns (uint256) { // --- library PercentsD16 { - function fromBasisPoints(uint256 bpValue) internal pure returns (PercentD16) { - return PercentD16.wrap(HUNDRED_PERCENTS_UINT256 * bpValue / 100_00); + function from(uint256 value) internal pure returns (PercentD16) { + if (value > MAX_PERCENT_D16) { + revert PercentD16Overflow(); + } + return PercentD16.wrap(uint128(value)); } function fromFraction(uint256 numerator, uint256 denominator) internal pure returns (PercentD16) { - return PercentD16.wrap(HUNDRED_PERCENTS_UINT256 * numerator / denominator); + if (denominator == 0) { + revert DivisionByZero(); + } + return from(HUNDRED_PERCENT_D16 * numerator / denominator); + } + + function fromBasisPoints(uint256 bpValue) internal pure returns (PercentD16) { + return from(HUNDRED_PERCENT_D16 * bpValue / HUNDRED_PERCENT_BP); } } diff --git a/contracts/types/SharesValue.sol b/contracts/types/SharesValue.sol index 76c84590..a2d663b6 100644 --- a/contracts/types/SharesValue.sol +++ b/contracts/types/SharesValue.sol @@ -1,27 +1,32 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.26; -import {ETHValue, ETHValues} from "./ETHValue.sol"; - // --- // Type definition // --- type SharesValue is uint128; +// --- +// Assign global operations +// --- + +using {lt as <, eq as ==} for SharesValue global; +using {toUint256} for SharesValue global; +using {plus as +, minus as -} for SharesValue global; + // --- // Errors // --- error SharesValueOverflow(); +error SharesValueUnderflow(); // --- -// Assign global operations +// Constants // --- -using {eq as ==, lt as <} for SharesValue global; -using {plus as +, minus as -} for SharesValue global; -using {toUint256} for SharesValue global; +uint128 constant MAX_SHARES_VALUE = type(uint128).max; // --- // Comparison operations @@ -35,24 +40,39 @@ function eq(SharesValue v1, SharesValue v2) pure returns (bool) { return SharesValue.unwrap(v1) == SharesValue.unwrap(v2); } +// --- +// Conversion operations +// --- + +function toUint256(SharesValue v) pure returns (uint256) { + return SharesValue.unwrap(v); +} + // --- // Arithmetic operations // --- function plus(SharesValue v1, SharesValue v2) pure returns (SharesValue) { - return SharesValue.wrap(SharesValue.unwrap(v1) + SharesValue.unwrap(v2)); + unchecked { + /// @dev Both `v1.toUint256()` and `v2.toUint256()` are <= type(uint128).max. Therefore, their + /// sum is <= type(uint256).max. + return SharesValues.from(v1.toUint256() + v2.toUint256()); + } } function minus(SharesValue v1, SharesValue v2) pure returns (SharesValue) { - return SharesValue.wrap(SharesValue.unwrap(v1) - SharesValue.unwrap(v2)); -} + uint256 v1Value = v1.toUint256(); + uint256 v2Value = v2.toUint256(); -// --- -// Custom operations -// --- + if (v1Value < v2Value) { + revert SharesValueUnderflow(); + } -function toUint256(SharesValue v) pure returns (uint256) { - return SharesValue.unwrap(v); + unchecked { + /// @dev Subtraction is safe because `v1Value` >= `v2Value`. + /// Both `v1Value` and `v2Value` <= `type(uint128).max`, so the difference fits within `uint128`. + return SharesValue.wrap(uint128(v1Value - v2Value)); + } } // --- @@ -63,17 +83,9 @@ library SharesValues { SharesValue internal constant ZERO = SharesValue.wrap(0); function from(uint256 value) internal pure returns (SharesValue) { - if (value > type(uint128).max) { + if (value > MAX_SHARES_VALUE) { revert SharesValueOverflow(); } return SharesValue.wrap(uint128(value)); } - - function calcETHValue( - ETHValue totalPooled, - SharesValue share, - SharesValue total - ) internal pure returns (ETHValue) { - return ETHValues.from(totalPooled.toUint256() * share.toUint256() / total.toUint256()); - } } diff --git a/contracts/types/Timestamp.sol b/contracts/types/Timestamp.sol index 410ba70d..b65bca30 100644 --- a/contracts/types/Timestamp.sol +++ b/contracts/types/Timestamp.sol @@ -8,23 +8,24 @@ pragma solidity 0.8.26; type Timestamp is uint40; // --- -// Errors +// Assign global operations // --- -error TimestampOverflow(); +using {lt as <, lte as <=, eq as ==, neq as !=, gte as >=, gt as >} for Timestamp global; +using {isZero, isNotZero, toSeconds} for Timestamp global; // --- -// Assign global operations +// Errors // --- -using {lt as <, lte as <=, eq as ==, neq as !=, gt as >, gte as >=} for Timestamp global; -using {isZero, isNotZero, toSeconds} for Timestamp global; +error TimestampOverflow(); // --- // Constants // --- -uint256 constant MAX_TIMESTAMP_VALUE = type(uint40).max; +/// @dev The maximum value for a `Timestamp`, corresponding to approximately the year 36812. +uint40 constant MAX_TIMESTAMP_VALUE = type(uint40).max; // --- // Comparison operations @@ -34,14 +35,6 @@ function lt(Timestamp t1, Timestamp t2) pure returns (bool) { return Timestamp.unwrap(t1) < Timestamp.unwrap(t2); } -function gt(Timestamp t1, Timestamp t2) pure returns (bool) { - return Timestamp.unwrap(t1) > Timestamp.unwrap(t2); -} - -function gte(Timestamp t1, Timestamp t2) pure returns (bool) { - return Timestamp.unwrap(t1) >= Timestamp.unwrap(t2); -} - function lte(Timestamp t1, Timestamp t2) pure returns (bool) { return Timestamp.unwrap(t1) <= Timestamp.unwrap(t2); } @@ -51,19 +44,15 @@ function eq(Timestamp t1, Timestamp t2) pure returns (bool) { } function neq(Timestamp t1, Timestamp t2) pure returns (bool) { - return !(t1 == t2); + return Timestamp.unwrap(t1) != Timestamp.unwrap(t2); } -// --- -// Custom operations -// --- - -function isZero(Timestamp t) pure returns (bool) { - return Timestamp.unwrap(t) == 0; +function gte(Timestamp t1, Timestamp t2) pure returns (bool) { + return Timestamp.unwrap(t1) >= Timestamp.unwrap(t2); } -function isNotZero(Timestamp t) pure returns (bool) { - return Timestamp.unwrap(t) != 0; +function gt(Timestamp t1, Timestamp t2) pure returns (bool) { + return Timestamp.unwrap(t1) > Timestamp.unwrap(t2); } // --- @@ -74,6 +63,18 @@ function toSeconds(Timestamp t) pure returns (uint256) { return Timestamp.unwrap(t); } +// --- +// Custom operations +// --- + +function isZero(Timestamp t) pure returns (bool) { + return Timestamp.unwrap(t) == 0; +} + +function isNotZero(Timestamp t) pure returns (bool) { + return Timestamp.unwrap(t) > 0; +} + // --- // Namespaced helper methods // --- @@ -81,14 +82,13 @@ function toSeconds(Timestamp t) pure returns (uint256) { library Timestamps { Timestamp internal constant ZERO = Timestamp.wrap(0); - Timestamp internal constant MIN = ZERO; - Timestamp internal constant MAX = Timestamp.wrap(uint40(MAX_TIMESTAMP_VALUE)); - function max(Timestamp t1, Timestamp t2) internal pure returns (Timestamp) { return t1 > t2 ? t1 : t2; } function now() internal view returns (Timestamp res) { + /// @dev Skipping the check that `block.timestamp` <= `MAX_TIMESTAMP_VALUE` for gas efficiency. + /// Overflow is possible only after approximately 34,000 years from the Unix epoch. res = Timestamp.wrap(uint40(block.timestamp)); } diff --git a/test/unit/libraries/AssetsAccounting.t.sol b/test/unit/libraries/AssetsAccounting.t.sol index 723bacbd..b1a6af0f 100644 --- a/test/unit/libraries/AssetsAccounting.t.sol +++ b/test/unit/libraries/AssetsAccounting.t.sol @@ -6,7 +6,7 @@ import {stdError} from "forge-std/StdError.sol"; import {Math} from "@openzeppelin/contracts/utils/math/Math.sol"; import {ETHValue, ETHValues, ETHValueOverflow, ETHValueUnderflow} from "contracts/types/ETHValue.sol"; -import {SharesValue, SharesValues, SharesValueOverflow} from "contracts/types/SharesValue.sol"; +import {SharesValue, SharesValues, SharesValueOverflow, SharesValueUnderflow} from "contracts/types/SharesValue.sol"; import {IndicesOneBased} from "contracts/types/IndexOneBased.sol"; import {Durations} from "contracts/types/Duration.sol"; import {Timestamps} from "contracts/types/Timestamp.sol"; @@ -149,7 +149,7 @@ contract AssetsAccountingUnitTests is UnitTest { _accountingContext.stETHTotals.lockedShares = totalLockedShares; _accountingContext.assets[holder].stETHLockedShares = shares; - vm.expectRevert(stdError.arithmeticError); + vm.expectRevert(SharesValueUnderflow.selector); AssetsAccounting.accountStETHSharesUnlock(_accountingContext, holder, shares); } @@ -620,7 +620,7 @@ contract AssetsAccountingUnitTests is UnitTest { withdrawalRequestStatuses[0].isClaimed = false; _accountingContext.unstETHRecords[unstETHIds[0]].status = UnstETHRecordStatus.NotLocked; - vm.expectRevert(stdError.arithmeticError); + vm.expectRevert(SharesValueOverflow.selector); AssetsAccounting.accountUnstETHLock(_accountingContext, holder, unstETHIds, withdrawalRequestStatuses); } @@ -644,7 +644,7 @@ contract AssetsAccountingUnitTests is UnitTest { withdrawalRequestStatuses[0].isClaimed = false; _accountingContext.unstETHRecords[unstETHIds[0]].status = UnstETHRecordStatus.NotLocked; - vm.expectRevert(stdError.arithmeticError); + vm.expectRevert(SharesValueOverflow.selector); AssetsAccounting.accountUnstETHLock(_accountingContext, holder, unstETHIds, withdrawalRequestStatuses); } @@ -880,7 +880,7 @@ contract AssetsAccountingUnitTests is UnitTest { _accountingContext.unstETHRecords[unstETHIds[0]].index = IndicesOneBased.fromOneBasedValue(1); _accountingContext.assets[holder].unstETHIds.push(unstETHIds[0]); - vm.expectRevert(stdError.arithmeticError); + vm.expectRevert(SharesValueUnderflow.selector); AssetsAccounting.accountUnstETHUnlock(_accountingContext, holder, unstETHIds); } @@ -919,7 +919,7 @@ contract AssetsAccountingUnitTests is UnitTest { _accountingContext.unstETHRecords[unstETHIds[0]].index = IndicesOneBased.fromOneBasedValue(1); _accountingContext.assets[holder].unstETHIds.push(unstETHIds[0]); - vm.expectRevert(stdError.arithmeticError); + vm.expectRevert(SharesValueUnderflow.selector); AssetsAccounting.accountUnstETHUnlock(_accountingContext, holder, unstETHIds); } @@ -1135,7 +1135,7 @@ contract AssetsAccountingUnitTests is UnitTest { _accountingContext.unstETHRecords[unstETHIds[0]].shares = SharesValues.from(type(uint64).max); claimableAmountsPrepared[0] = 1; - vm.expectRevert(stdError.arithmeticError); + vm.expectRevert(SharesValueUnderflow.selector); AssetsAccounting.accountUnstETHFinalized(_accountingContext, unstETHIds, claimableAmountsPrepared); } @@ -1447,7 +1447,7 @@ contract AssetsAccountingUnitTests is UnitTest { _accountingContext.unstETHTotals.unfinalizedShares = totalUnfinalizedShares; _accountingContext.stETHTotals.lockedShares = totalLockedShares; - vm.expectRevert(stdError.arithmeticError); + vm.expectRevert(SharesValueOverflow.selector); AssetsAccounting.getLockedAssetsTotals(_accountingContext); } @@ -1495,10 +1495,6 @@ contract AssetsAccountingUnitTests is UnitTest { assertEq(_accountingContext.unstETHTotals.finalizedETH, finalizedETH); } - function assertEq(SharesValue a, SharesValue b) internal { - assertEq(a.toUint256(), b.toUint256()); - } - function assertEq(UnstETHRecordStatus a, UnstETHRecordStatus b) internal { assertEq(uint256(a), uint256(b)); } diff --git a/test/unit/libraries/DualGovernanceConfig.t.sol b/test/unit/libraries/DualGovernanceConfig.t.sol index 5a172192..872567e2 100644 --- a/test/unit/libraries/DualGovernanceConfig.t.sol +++ b/test/unit/libraries/DualGovernanceConfig.t.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.26; import {Timestamp, Timestamps} from "contracts/types/Timestamp.sol"; -import {Duration, Durations} from "contracts/types/Duration.sol"; +import {Duration, Durations, MAX_DURATION_VALUE} from "contracts/types/Duration.sol"; import {PercentD16, PercentsD16} from "contracts/types/PercentD16.sol"; import {DualGovernanceConfig, PercentD16} from "contracts/libraries/DualGovernanceConfig.sol"; @@ -315,7 +315,7 @@ contract DualGovernanceConfigTest is UnitTest { vm.assume( config.rageQuitEthWithdrawalsMinDelay.toSeconds() - + config.rageQuitEthWithdrawalsDelayGrowth.toSeconds() * (rageQuitRound + 1) <= Durations.MAX.toSeconds() + + config.rageQuitEthWithdrawalsDelayGrowth.toSeconds() * (rageQuitRound + 1) <= MAX_DURATION_VALUE ); vm.assume(config.rageQuitEthWithdrawalsMinDelay <= config.rageQuitEthWithdrawalsMaxDelay); diff --git a/test/unit/libraries/ExecutableProposals.t.sol b/test/unit/libraries/ExecutableProposals.t.sol index 30dc25fd..c67cff1b 100644 --- a/test/unit/libraries/ExecutableProposals.t.sol +++ b/test/unit/libraries/ExecutableProposals.t.sol @@ -3,7 +3,7 @@ pragma solidity 0.8.26; import {Vm} from "forge-std/Test.sol"; -import {Duration, Durations} from "contracts/types/Duration.sol"; +import {Duration, Durations, MAX_DURATION_VALUE} from "contracts/types/Duration.sol"; import {Timestamp, Timestamps} from "contracts/types/Timestamp.sol"; import {ITimelock} from "contracts/interfaces/ITimelock.sol"; @@ -71,7 +71,7 @@ contract ExecutableProposalsUnitTests is UnitTest { } function testFuzz_schedule_proposal(Duration delay) external { - vm.assume(delay > Durations.ZERO && delay <= Durations.MAX); + vm.assume(delay > Durations.ZERO && delay.toSeconds() <= MAX_DURATION_VALUE); _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); @@ -114,7 +114,7 @@ contract ExecutableProposalsUnitTests is UnitTest { } function testFuzz_cannot_schedule_proposal_before_delay_passed(Duration delay) external { - vm.assume(delay > Durations.ZERO && delay <= Durations.MAX); + vm.assume(delay > Durations.ZERO && delay.toSeconds() <= MAX_DURATION_VALUE); _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); @@ -137,7 +137,7 @@ contract ExecutableProposalsUnitTests is UnitTest { } function testFuzz_execute_proposal(Duration delay) external { - vm.assume(delay > Durations.ZERO && delay <= Durations.MAX); + vm.assume(delay > Durations.ZERO && delay.toSeconds() <= MAX_DURATION_VALUE); _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); uint256 proposalId = _proposals.getProposalsCount(); @@ -203,7 +203,7 @@ contract ExecutableProposalsUnitTests is UnitTest { } function testFuzz_cannot_execute_before_delay_passed(Duration delay) external { - vm.assume(delay > Durations.ZERO && delay <= Durations.MAX); + vm.assume(delay > Durations.ZERO && delay.toSeconds() <= MAX_DURATION_VALUE); _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); uint256 proposalId = _proposals.getProposalsCount(); _proposals.schedule(proposalId, Durations.ZERO); diff --git a/test/unit/types/Duration.t.sol b/test/unit/types/Duration.t.sol index 49c7b5e5..f9d45d59 100644 --- a/test/unit/types/Duration.t.sol +++ b/test/unit/types/Duration.t.sol @@ -4,55 +4,153 @@ pragma solidity 0.8.26; import {Math} from "@openzeppelin/contracts/utils/math/Math.sol"; import { - Duration, Durations, MAX_DURATION_VALUE, DurationOverflow, DurationUnderflow + Duration, + Durations, + MAX_DURATION_VALUE, + DivisionByZero, + DurationOverflow, + DurationUnderflow } from "contracts/types/Duration.sol"; import {Timestamp, TimestampOverflow, MAX_TIMESTAMP_VALUE} from "contracts/types/Timestamp.sol"; -import {stdError} from "forge-std/StdError.sol"; import {UnitTest} from "test/utils/unit-test.sol"; contract DurationTests is UnitTest { - uint256 private constant MAX_SECONDS_VALUE = type(uint256).max - MAX_DURATION_VALUE; // --- - // Conversion operations + // Comparison operations // --- - function testFuzz_toSeconds_HappyPath(Duration d) external { - assertEq(d.toSeconds(), Duration.unwrap(d)); - } - // --- - // Comparison operations + // lt() // --- + function test_lt_HappyPath_ReturnsTrue() external { + assertTrue(Duration.wrap(1) < Duration.wrap(2)); + assertTrue(Duration.wrap(0) < Durations.from(MAX_DURATION_VALUE)); + assertTrue(Durations.from(MAX_DURATION_VALUE - 1) < Durations.from(MAX_DURATION_VALUE)); + } + + function test_lt_HappyPath_ReturnsFalse() external { + assertFalse(Duration.wrap(0) < Duration.wrap(0)); + assertFalse(Duration.wrap(1) < Duration.wrap(0)); + assertFalse(Durations.from(MAX_DURATION_VALUE) < Durations.from(MAX_DURATION_VALUE - 1)); + } + function testFuzz_lt_HappyPath(Duration d1, Duration d2) external { assertEq(d1 < d2, d1.toSeconds() < d2.toSeconds()); } + // --- + // lte() + // --- + + function test_lte_HappyPath_ReturnsTrue() external { + assertTrue(Duration.wrap(0) <= Duration.wrap(0)); + assertTrue(Duration.wrap(1) <= Duration.wrap(2)); + assertTrue(Duration.wrap(2) <= Duration.wrap(2)); + assertTrue(Duration.wrap(0) <= Durations.from(MAX_DURATION_VALUE)); + assertTrue(Durations.from(MAX_DURATION_VALUE - 1) <= Durations.from(MAX_DURATION_VALUE)); + assertTrue(Durations.from(MAX_DURATION_VALUE) <= Durations.from(MAX_DURATION_VALUE)); + } + + function test_lte_HappyPath_ReturnsFalse() external { + assertFalse(Duration.wrap(1) <= Duration.wrap(0)); + assertFalse(Durations.from(MAX_DURATION_VALUE) <= Durations.from(MAX_DURATION_VALUE - 1)); + } + function testFuzz_lte_HappyPath(Duration d1, Duration d2) external { assertEq(d1 <= d2, d1.toSeconds() <= d2.toSeconds()); } + // --- + // eq() + // --- + + function test_eq_HappyPath_ReturnsTrue() external { + assertTrue(Duration.wrap(0) == Duration.wrap(0)); + assertTrue(Duration.wrap(MAX_DURATION_VALUE / 2) == Duration.wrap(MAX_DURATION_VALUE / 2)); + assertTrue(Duration.wrap(MAX_DURATION_VALUE) == Duration.wrap(MAX_DURATION_VALUE)); + } + + function test_eq_HappyPath_ReturnsFalse() external { + assertFalse(Duration.wrap(0) == Duration.wrap(1)); + assertFalse(Duration.wrap(MAX_DURATION_VALUE / 2) == Duration.wrap(MAX_DURATION_VALUE)); + assertFalse(Duration.wrap(0) == Duration.wrap(MAX_DURATION_VALUE)); + } + function testFuzz_eq_HappyPath(Duration d1, Duration d2) external { assertEq(d1 == d2, d1.toSeconds() == d2.toSeconds()); } + // --- + // neq() + // --- + + function test_neq_HappyPath_ReturnsTrue() external { + assertTrue(Duration.wrap(0) != Duration.wrap(1)); + assertTrue(Duration.wrap(MAX_DURATION_VALUE / 2) != Duration.wrap(MAX_DURATION_VALUE)); + assertTrue(Duration.wrap(0) != Duration.wrap(MAX_DURATION_VALUE)); + } + + function test_neq_HappyPath_ReturnsFalse() external { + assertFalse(Duration.wrap(0) != Duration.wrap(0)); + assertFalse(Duration.wrap(MAX_DURATION_VALUE / 2) != Duration.wrap(MAX_DURATION_VALUE / 2)); + assertFalse(Duration.wrap(MAX_DURATION_VALUE) != Duration.wrap(MAX_DURATION_VALUE)); + } + function testFuzz_neq_HappyPath(Duration d1, Duration d2) external { assertEq(d1 != d2, d1.toSeconds() != d2.toSeconds()); } - function testFuzz_gt_HappyPath(Duration d1, Duration d2) external { - assertEq(d1 > d2, d1.toSeconds() > d2.toSeconds()); + // --- + // gte + // --- + + function test_gte_HappyPath_ReturnsTrue() external { + assertTrue(Duration.wrap(0) >= Duration.wrap(0)); + assertTrue(Duration.wrap(5) >= Duration.wrap(3)); + assertTrue(Duration.wrap(MAX_DURATION_VALUE) >= Duration.wrap(MAX_DURATION_VALUE / 2)); + assertTrue(Duration.wrap(MAX_DURATION_VALUE) >= Duration.wrap(MAX_DURATION_VALUE)); + } + + function test_gte_HappyPath_ReturnsFalse() external { + assertFalse(Duration.wrap(0) >= Duration.wrap(1)); + assertFalse(Duration.wrap(5) >= Duration.wrap(9)); + assertFalse(Duration.wrap(MAX_DURATION_VALUE / 2) >= Duration.wrap(MAX_DURATION_VALUE)); } function testFuzz_gte_HappyPath(Duration d1, Duration d2) external { assertEq(d1 >= d2, d1.toSeconds() >= d2.toSeconds()); } + // --- + // gt + // --- + + function test_gt_HappyPath_ReturnsTrue() external { + assertTrue(Duration.wrap(1) > Duration.wrap(0)); + assertTrue(Duration.wrap(5) > Duration.wrap(3)); + assertTrue(Duration.wrap(MAX_DURATION_VALUE) > Duration.wrap(MAX_DURATION_VALUE / 2)); + } + + function test_gt_HappyPath_ReturnsFalse() external { + assertFalse(Duration.wrap(0) > Duration.wrap(0)); + assertFalse(Duration.wrap(5) > Duration.wrap(9)); + assertFalse(Duration.wrap(MAX_DURATION_VALUE / 2) > Duration.wrap(MAX_DURATION_VALUE)); + } + + function testFuzz_gt_HappyPath(Duration d1, Duration d2) external { + assertEq(d1 > d2, d1.toSeconds() > d2.toSeconds()); + } + // --- // Arithmetic operations // --- + // --- + // plus() + // --- + function testFuzz_plus_HappyPath(Duration d1, Duration d2) external { uint256 expectedResult = d1.toSeconds() + d2.toSeconds(); vm.assume(expectedResult <= MAX_DURATION_VALUE); @@ -63,9 +161,13 @@ contract DurationTests is UnitTest { function testFuzz_plus_Overflow(Duration d1, Duration d2) external { vm.assume(d1.toSeconds() + d2.toSeconds() > MAX_DURATION_VALUE); vm.expectRevert(DurationOverflow.selector); - d1 + d2; + this.external__plus(d1, d2); } + // --- + // minus() + // --- + function testFuzz_minus_HappyPath(Duration d1, Duration d2) external { vm.assume(d1 >= d2); assertEq(d1 - d2, Durations.from(d1.toSeconds() - d2.toSeconds())); @@ -74,7 +176,7 @@ contract DurationTests is UnitTest { function testFuzz_minus_Underflow(Duration d1, Duration d2) external { vm.assume(d1 < d2); vm.expectRevert(DurationUnderflow.selector); - d1 - d2; + this.external__minus(d1, d2); } // --- @@ -85,36 +187,36 @@ contract DurationTests is UnitTest { // plusSeconds() // --- - function testFuzz_plusSeconds_HappyPath(Duration d, uint256 seconds_) external { - vm.assume(seconds_ < MAX_SECONDS_VALUE); - vm.assume(d.toSeconds() + seconds_ <= MAX_DURATION_VALUE); + function testFuzz_plusSeconds_HappyPath(Duration d, uint256 secondsToAdd) external { + vm.assume(secondsToAdd < type(uint256).max - MAX_DURATION_VALUE); + vm.assume(d.toSeconds() + secondsToAdd <= MAX_DURATION_VALUE); - assertEq(d.plusSeconds(seconds_), Duration.wrap(uint32(d.toSeconds() + seconds_))); + assertEq(d.plusSeconds(secondsToAdd), Duration.wrap(uint32(d.toSeconds() + secondsToAdd))); } - function testFuzz_plusSeconds_Overflow(Duration d, uint256 seconds_) external { - vm.assume(seconds_ < MAX_SECONDS_VALUE); - vm.assume(d.toSeconds() + seconds_ > MAX_DURATION_VALUE); + function testFuzz_plusSeconds_Overflow(Duration d, uint256 secondsToAdd) external { + vm.assume(secondsToAdd < type(uint256).max - MAX_DURATION_VALUE); + vm.assume(d.toSeconds() + secondsToAdd > MAX_DURATION_VALUE); vm.expectRevert(DurationOverflow.selector); - d.plusSeconds(seconds_); + this.external__plusSeconds(d, secondsToAdd); } // --- // minusSeconds() // --- - function testFuzz_minusSeconds_HappyPath(Duration d, uint256 seconds_) external { - vm.assume(seconds_ <= d.toSeconds()); + function testFuzz_minusSeconds_HappyPath(Duration d, uint256 secondsToAdd) external { + vm.assume(secondsToAdd <= d.toSeconds()); - assertEq(d.minusSeconds(seconds_), Duration.wrap(uint32(d.toSeconds() - seconds_))); + assertEq(d.minusSeconds(secondsToAdd), Duration.wrap(uint32(d.toSeconds() - secondsToAdd))); } - function testFuzz_minusSeconds_Overflow(Duration d, uint256 seconds_) external { - vm.assume(seconds_ > d.toSeconds()); + function testFuzz_minusSeconds_Overflow(Duration d, uint256 secondsToSubtract) external { + vm.assume(secondsToSubtract > d.toSeconds()); vm.expectRevert(DurationUnderflow.selector); - d.minusSeconds(seconds_); + this.external__minusSeconds(d, secondsToSubtract); } // --- @@ -127,8 +229,8 @@ contract DurationTests is UnitTest { } function testFuzz_dividedBy_RevertOn_DivisorIsZero(Duration d) external { - vm.expectRevert(stdError.divisionError); - d.dividedBy(0); + vm.expectRevert(DivisionByZero.selector); + this.external__dividedBy(d, 0); } // --- @@ -145,7 +247,7 @@ contract DurationTests is UnitTest { (bool isSuccess, uint256 expectedResult) = Math.tryMul(d.toSeconds(), multiplicand); vm.assume(isSuccess && expectedResult > MAX_DURATION_VALUE); vm.expectRevert(DurationOverflow.selector); - d.multipliedBy(multiplicand); + this.external__multipliedBy(d, multiplicand); } // --- @@ -162,34 +264,34 @@ contract DurationTests is UnitTest { (bool isSuccess, uint256 expectedResult) = Math.tryAdd(t.toSeconds(), d.toSeconds()); vm.assume(isSuccess && expectedResult > MAX_TIMESTAMP_VALUE); vm.expectRevert(TimestampOverflow.selector); - d.addTo(t); + this.external__addTo(d, t); } // --- - // Namespaced helper methods + // Conversion operations // --- - function test_ZERO_CorrectValue() external { - assertEq(Durations.ZERO, Duration.wrap(0)); + function testFuzz_toSeconds_HappyPath(Duration d) external { + assertEq(d.toSeconds(), Duration.unwrap(d)); } - function test_MIN_CorrectValue() external { - assertEq(Durations.MIN, Duration.wrap(0)); - } + // --- + // Namespaced helper methods + // --- - function test_MAX_CorrectValue() external { - assertEq(Durations.MAX, Duration.wrap(uint32(MAX_DURATION_VALUE))); + function test_ZERO_CorrectValue() external { + assertEq(Durations.ZERO, Duration.wrap(0)); } - function testFuzz_from_HappyPath(uint256 seconds_) external { - vm.assume(seconds_ <= MAX_DURATION_VALUE); - assertEq(Durations.from(seconds_), Duration.wrap(uint32(seconds_))); + function testFuzz_from_HappyPath(uint256 durationInSeconds) external { + vm.assume(durationInSeconds <= MAX_DURATION_VALUE); + assertEq(Durations.from(durationInSeconds), Duration.wrap(uint32(durationInSeconds))); } - function testFuzz_from_RevertOn_Overflow(uint256 seconds_) external { - vm.assume(seconds_ > MAX_DURATION_VALUE); + function testFuzz_from_RevertOn_Overflow(uint256 durationInSeconds) external { + vm.assume(durationInSeconds > MAX_DURATION_VALUE); vm.expectRevert(DurationOverflow.selector); - Durations.from(seconds_); + this.external__from(durationInSeconds); } function testFuzz_between_HappyPath(Timestamp t1, Timestamp t2) external { @@ -210,10 +312,50 @@ contract DurationTests is UnitTest { vm.assume(expectedValue > MAX_DURATION_VALUE); vm.expectRevert(DurationOverflow.selector); - Durations.between(t1, t2); + this.external__between(t1, t2); } function testFuzz_min_HappyPath(Duration d1, Duration d2) external { assertEq(Durations.min(d1, d2), Durations.from(Math.min(d1.toSeconds(), d2.toSeconds()))); } + + // --- + // Helper test methods + // --- + + function external__plus(Duration d1, Duration d2) external returns (Duration) { + return d1 + d2; + } + + function external__minus(Duration d1, Duration d2) external returns (Duration) { + return d1 - d2; + } + + function external__plusSeconds(Duration d, uint256 secondsToAdd) external returns (Duration) { + return d.plusSeconds(secondsToAdd); + } + + function external__minusSeconds(Duration d, uint256 secondsToSubtract) external returns (Duration) { + return d.minusSeconds(secondsToSubtract); + } + + function external__dividedBy(Duration d, uint256 divisor) external returns (Duration) { + return d.dividedBy(divisor); + } + + function external__multipliedBy(Duration d, uint256 multiplicand) external returns (Duration) { + return d.multipliedBy(multiplicand); + } + + function external__addTo(Duration d, Timestamp t) external returns (Timestamp) { + return d.addTo(t); + } + + function external__from(uint256 valueInSeconds) external returns (Duration) { + return Durations.from(valueInSeconds); + } + + function external__between(Timestamp t1, Timestamp t2) external returns (Duration) { + return Durations.between(t1, t2); + } } diff --git a/test/unit/types/ETHValue.t.sol b/test/unit/types/ETHValue.t.sol index 3318ab34..2b887ff9 100644 --- a/test/unit/types/ETHValue.t.sol +++ b/test/unit/types/ETHValue.t.sol @@ -3,7 +3,7 @@ pragma solidity 0.8.26; import {Address} from "@openzeppelin/contracts/utils/Address.sol"; -import {ETHValue, ETHValues, ETHValueOverflow, ETHValueUnderflow} from "contracts/types/ETHValue.sol"; +import {ETHValue, ETHValues, ETHValueOverflow, ETHValueUnderflow, MAX_ETH_VALUE} from "contracts/types/ETHValue.sol"; import {UnitTest} from "test/utils/unit-test.sol"; @@ -17,7 +17,6 @@ contract ETHTransfersForbiddenStub { contract ETHValueTests is UnitTest { uint256 internal constant _MAX_ETH_SEND = 1_000_000 ether; - uint256 internal constant _MAX_ETH_VALUE = type(uint128).max; address internal immutable _RECIPIENT = makeAddr("RECIPIENT"); // --- @@ -46,15 +45,15 @@ contract ETHValueTests is UnitTest { function testFuzz_plus_HappyPath(ETHValue v1, ETHValue v2) external { uint256 expectedResult = v1.toUint256() + v2.toUint256(); - vm.assume(expectedResult <= _MAX_ETH_VALUE); + vm.assume(expectedResult <= MAX_ETH_VALUE); assertEq(v1 + v2, ETHValue.wrap(uint128(expectedResult))); } function testFuzz_plus_Overflow(ETHValue v1, ETHValue v2) external { uint256 expectedResult = v1.toUint256() + v2.toUint256(); - vm.assume(expectedResult > _MAX_ETH_VALUE); + vm.assume(expectedResult > MAX_ETH_VALUE); vm.expectRevert(ETHValueOverflow.selector); - v1 + v2; + this.external__plus(v1, v2); } function testFuzz_minus_HappyPath(ETHValue v1, ETHValue v2) external { @@ -66,7 +65,7 @@ contract ETHValueTests is UnitTest { function testFuzz_minus_Overflow(ETHValue v1, ETHValue v2) external { vm.assume(v1 < v2); vm.expectRevert(ETHValueUnderflow.selector); - v1 - v2; + this.external__minus(v1, v2); } // --- @@ -92,7 +91,7 @@ contract ETHValueTests is UnitTest { vm.deal(address(this), balance); vm.expectRevert(abi.encodeWithSelector(Address.AddressInsufficientBalance.selector, address(this))); - this.sendTo__external(amount, payable(_RECIPIENT)); + this.external__sendTo(amount, payable(_RECIPIENT)); } function testFuzz_sendTo_RevertOn_ETHTransfersForbidden(ETHValue amount, uint256 balance) external { @@ -105,7 +104,7 @@ contract ETHValueTests is UnitTest { ETHTransfersForbiddenStub ethTransfersForbiddenStub = new ETHTransfersForbiddenStub(); vm.expectRevert(Address.FailedInnerCall.selector); - this.sendTo__external(amount, payable(address(ethTransfersForbiddenStub))); + this.external__sendTo(amount, payable(address(ethTransfersForbiddenStub))); } function testFuzz_toUint256_HappyPath(ETHValue amount) external { @@ -113,14 +112,14 @@ contract ETHValueTests is UnitTest { } function testFuzz_from_HappyPath(uint256 amount) external { - vm.assume(amount <= _MAX_ETH_VALUE); + vm.assume(amount <= MAX_ETH_VALUE); assertEq(ETHValues.from(amount), ETHValue.wrap(uint128(amount))); } function testFuzz_from_RevertOn_Overflow(uint256 amount) external { - vm.assume(amount > _MAX_ETH_VALUE); + vm.assume(amount > MAX_ETH_VALUE); vm.expectRevert(ETHValueOverflow.selector); - ETHValues.from(amount); + this.external__from(amount); } function testFuzz_fromAddressBalance_HappyPath(ETHValue balance) external { @@ -129,7 +128,19 @@ contract ETHValueTests is UnitTest { assertEq(balance, ETHValue.wrap(uint128(address(this).balance))); } - function sendTo__external(ETHValue amount, address payable recipient) external { + function external__sendTo(ETHValue amount, address payable recipient) external { amount.sendTo(recipient); } + + function external__plus(ETHValue v1, ETHValue v2) external returns (ETHValue) { + return v1 + v2; + } + + function external__minus(ETHValue v1, ETHValue v2) external returns (ETHValue) { + return v1 - v2; + } + + function external__from(uint256 amount) external returns (ETHValue) { + return ETHValues.from(amount); + } } diff --git a/test/unit/types/IndexOneBased.t.sol b/test/unit/types/IndexOneBased.t.sol index f855bf25..3ce7e887 100644 --- a/test/unit/types/IndexOneBased.t.sol +++ b/test/unit/types/IndexOneBased.t.sol @@ -1,7 +1,13 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.26; -import {IndicesOneBased, IndexOneBased, IndexOneBasedUnderflow} from "contracts/types/IndexOneBased.sol"; +import { + IndicesOneBased, + IndexOneBased, + IndexOneBasedOverflow, + IndexOneBasedUnderflow, + MAX_INDEX_ONE_BASED_VALUE +} from "contracts/types/IndexOneBased.sol"; import {UnitTest} from "test/utils/unit-test.sol"; @@ -14,6 +20,14 @@ contract IndexOneBasedUnitTests is UnitTest { assertEq(i1 != i2, IndexOneBased.unwrap(i1) != IndexOneBased.unwrap(i2)); } + // --- + // Custom operations + // --- + + // --- + // isEmpty() + // --- + function test_isEmpty_HappyPath() external { assertTrue(IndexOneBased.wrap(0).isEmpty()); assertFalse(IndicesOneBased.fromOneBasedValue(1).isEmpty()); @@ -23,6 +37,10 @@ contract IndexOneBasedUnitTests is UnitTest { assertEq(i1.isEmpty(), IndexOneBased.unwrap(i1) == 0); } + // --- + // isNotEmpty() + // --- + function test_isNotEmpty_HappyPath() external { assertTrue(IndicesOneBased.fromOneBasedValue(1).isNotEmpty()); assertFalse(IndexOneBased.wrap(0).isNotEmpty()); @@ -32,6 +50,10 @@ contract IndexOneBasedUnitTests is UnitTest { assertEq(i1.isNotEmpty(), IndexOneBased.unwrap(i1) != 0); } + // --- + // toZeroBasedValue() + // --- + function testFuzz_toZeroBasedValue_HappyPath(IndexOneBased index) external { vm.assume(IndexOneBased.unwrap(index) > 0); assertEq(index.toZeroBasedValue(), IndexOneBased.unwrap(index) - 1); @@ -40,6 +62,45 @@ contract IndexOneBasedUnitTests is UnitTest { function test_toZeroBasedValue_RevertOn_EmptyIndex(IndexOneBased index) external { IndexOneBased emptyIndex = IndexOneBased.wrap(0); vm.expectRevert(IndexOneBasedUnderflow.selector); - emptyIndex.toZeroBasedValue(); + this.external__toZeroBasedValue(emptyIndex); + } + + // --- + // Namespaced helper methods + // --- + + // --- + // fromOneBasedValue() + // --- + + function testFuzz_fromOneBasedValue_HappyPath(uint256 oneBasedIndexValue) external { + vm.assume(oneBasedIndexValue > 0); + vm.assume(oneBasedIndexValue <= MAX_INDEX_ONE_BASED_VALUE); + + assertEq(IndicesOneBased.fromOneBasedValue(oneBasedIndexValue), IndexOneBased.wrap(uint32(oneBasedIndexValue))); + } + + function test_fromOneBasedValue_RevertOn_ZeroIndex() external { + vm.expectRevert(IndexOneBasedUnderflow.selector); + this.external__fromOneBasedValue(0); + } + + function testFuzz_fromOneBasedValue_RevertOn_Overflow(uint256 oneBasedIndexValue) external { + vm.assume(oneBasedIndexValue > MAX_INDEX_ONE_BASED_VALUE); + + vm.expectRevert(IndexOneBasedOverflow.selector); + this.external__fromOneBasedValue(oneBasedIndexValue); + } + + // --- + // Helper test methods + // --- + + function external__toZeroBasedValue(IndexOneBased oneBasedIndex) external pure returns (uint256) { + return oneBasedIndex.toZeroBasedValue(); + } + + function external__fromOneBasedValue(uint256 oneBasedIndexValue) external pure returns (IndexOneBased) { + return IndicesOneBased.fromOneBasedValue(oneBasedIndexValue); } } diff --git a/test/unit/types/PercentD16.t.sol b/test/unit/types/PercentD16.t.sol new file mode 100644 index 00000000..c07558dd --- /dev/null +++ b/test/unit/types/PercentD16.t.sol @@ -0,0 +1,342 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.26; + +import {Math} from "@openzeppelin/contracts/utils/math/Math.sol"; + +import { + PercentD16, + PercentsD16, + DivisionByZero, + PercentD16Underflow, + PercentD16Overflow, + MAX_PERCENT_D16, + HUNDRED_PERCENT_BP, + HUNDRED_PERCENT_D16 +} from "contracts/types/PercentD16.sol"; + +import {UnitTest} from "test/utils/unit-test.sol"; +import {stdError} from "forge-std/StdError.sol"; + +contract PercentD16UnitTests is UnitTest { + // --- + // Comparison operations + // --- + + // --- + // lt() + // --- + + function testFuzz_lt_HappyPath(PercentD16 a, PercentD16 b) external { + assertEq(a < b, a.toUint256() < b.toUint256()); + } + + // --- + // lte() + // --- + + function testFuzz_lte_HappyPath(PercentD16 a, PercentD16 b) external { + assertEq(a <= b, a.toUint256() <= b.toUint256()); + } + + // --- + // eq() + // --- + + function testFuzz_eq_HappyPath(PercentD16 a, PercentD16 b) external { + assertEq(a == b, a.toUint256() == b.toUint256()); + } + + // --- + // gt() + // --- + + function testFuzz_gt_HappyPath(PercentD16 a, PercentD16 b) external { + assertEq(a > b, a.toUint256() > b.toUint256()); + } + + // --- + // gte() + // --- + + function testFuzz_gte_HappyPath(PercentD16 a, PercentD16 b) external { + assertEq(a >= b, a.toUint256() >= b.toUint256()); + } + + // --- + // Arithmetic operations + // --- + + // --- + // plus + // --- + + function test_plus_HappyPath() external { + assertEq(PercentsD16.from(0) + PercentsD16.from(0), PercentsD16.from(0)); + assertEq(PercentsD16.from(0) + PercentsD16.from(1), PercentsD16.from(1)); + assertEq(PercentsD16.from(0) + PercentsD16.from(1), PercentsD16.from(1)); + assertEq(PercentsD16.from(500) + PercentsD16.from(20), PercentsD16.from(520)); + assertEq(PercentsD16.from(0) + PercentsD16.from(MAX_PERCENT_D16), PercentsD16.from(MAX_PERCENT_D16)); + assertEq(PercentsD16.from(MAX_PERCENT_D16) + PercentsD16.from(0), PercentsD16.from(MAX_PERCENT_D16)); + + assertEq( + PercentsD16.from(MAX_PERCENT_D16 / 2) + PercentsD16.from(MAX_PERCENT_D16 / 2 + 1), + PercentsD16.from(MAX_PERCENT_D16) + ); + } + + function test_plus_RevertOn_Overflow() external { + vm.expectRevert(PercentD16Overflow.selector); + this.external__plus(PercentsD16.from(MAX_PERCENT_D16), PercentsD16.from(1)); + + vm.expectRevert(PercentD16Overflow.selector); + this.external__plus(PercentsD16.from(MAX_PERCENT_D16), PercentsD16.from(MAX_PERCENT_D16)); + + vm.expectRevert(PercentD16Overflow.selector); + this.external__plus(PercentsD16.from(MAX_PERCENT_D16 / 2 + 1), PercentsD16.from(MAX_PERCENT_D16 / 2 + 1)); + } + + function testFuzz_plus_HappyPath(PercentD16 a, PercentD16 b) external { + vm.assume(a.toUint256() + b.toUint256() <= MAX_PERCENT_D16); + assertEq(a + b, PercentD16.wrap(uint128(a.toUint256() + b.toUint256()))); + } + + function testFuzz_plus_RevertOn_Overflow(PercentD16 a, PercentD16 b) external { + vm.assume(a.toUint256() + b.toUint256() > MAX_PERCENT_D16); + vm.expectRevert(PercentD16Overflow.selector); + this.external__plus(a, b); + } + + // --- + // minus + // --- + + function test_minus_HappyPath() external { + assertEq(PercentsD16.from(5) - PercentsD16.from(2), PercentsD16.from(3)); + assertEq(PercentsD16.from(0) - PercentsD16.from(0), PercentsD16.from(0)); + assertEq(PercentsD16.from(1) - PercentsD16.from(0), PercentsD16.from(1)); + } + + function test_minus_RevertOn_Underflow() external { + vm.expectRevert(PercentD16Underflow.selector); + this.external__minus(PercentsD16.from(0), PercentsD16.from(1)); + + vm.expectRevert(PercentD16Underflow.selector); + this.external__minus(PercentsD16.from(4), PercentsD16.from(5)); + } + + function testFuzz_minus_HappyPath(PercentD16 a, PercentD16 b) external { + vm.assume(a >= b); + assertEq(a - b, PercentD16.wrap(uint128(a.toUint256() - b.toUint256()))); + } + + function testFuzz_minus_RevertOn_Underflow(PercentD16 a, PercentD16 b) external { + vm.assume(a < b); + vm.expectRevert(PercentD16Underflow.selector); + this.external__minus(a, b); + } + + // --- + // Conversion operations + // --- + + // --- + // toUint256() + // --- + + function test_toUint256_HappyPath() external { + assertEq(PercentsD16.from(0).toUint256(), 0); + assertEq(PercentsD16.from(1).toUint256(), 1); + assertEq(PercentsD16.from(MAX_PERCENT_D16 / 2).toUint256(), MAX_PERCENT_D16 / 2); + assertEq(PercentsD16.from(MAX_PERCENT_D16 - 1).toUint256(), MAX_PERCENT_D16 - 1); + assertEq(PercentsD16.from(MAX_PERCENT_D16).toUint256(), MAX_PERCENT_D16); + } + + function testFuzz_toUint256_HappyPath(PercentD16 a) external { + assertEq(a.toUint256(), PercentD16.unwrap(a)); + } + + // --- + // Namespaced helper methods + // --- + + // --- + // from() + // --- + + function test_from_HappyPath() external { + assertEq(PercentsD16.from(0), PercentD16.wrap(0)); + assertEq(PercentsD16.from(1), PercentD16.wrap(1)); + assertEq(PercentsD16.from(MAX_PERCENT_D16 / 2), PercentD16.wrap(uint128(MAX_PERCENT_D16 / 2))); + assertEq(PercentsD16.from(MAX_PERCENT_D16 - 1), PercentD16.wrap(uint128(MAX_PERCENT_D16 - 1))); + assertEq(PercentsD16.from(MAX_PERCENT_D16), PercentD16.wrap(uint128(MAX_PERCENT_D16))); + } + + function test_from_RevertOn_Overflow() external { + vm.expectRevert(PercentD16Overflow.selector); + this.external__from(uint256(MAX_PERCENT_D16) + 1); + + vm.expectRevert(PercentD16Overflow.selector); + this.external__from(type(uint256).max); + } + + function testFuzz_from_HappyPath(uint256 a) external { + vm.assume(a <= MAX_PERCENT_D16); + assertEq(PercentsD16.from(a), PercentD16.wrap(uint128(a))); + } + + function testFuzz_from_RevertOn_Overflow(uint256 a) external { + vm.assume(a > MAX_PERCENT_D16); + vm.expectRevert(PercentD16Overflow.selector); + this.external__from(a); + } + + // --- + // fromFraction() + // --- + + function test_fromFraction() external { + assertEq(PercentsD16.fromFraction({numerator: 0, denominator: 1}), PercentsD16.from(0)); + assertEq(PercentsD16.fromFraction({numerator: 0, denominator: 33}), PercentsD16.from(0)); + assertEq(PercentsD16.fromFraction({numerator: 1, denominator: 1}), PercentsD16.from(100 * 10 ** 16)); + assertEq(PercentsD16.fromFraction({numerator: 1, denominator: 2}), PercentsD16.from(50 * 10 ** 16)); + assertEq(PercentsD16.fromFraction({numerator: 5, denominator: 2}), PercentsD16.from(250 * 10 ** 16)); + assertEq(PercentsD16.fromFraction({numerator: 2, denominator: 5}), PercentsD16.from(40 * 10 ** 16)); + assertEq(PercentsD16.fromFraction({numerator: 1, denominator: 100}), PercentsD16.from(1 * 10 ** 16)); + assertEq(PercentsD16.fromFraction({numerator: 2, denominator: 1000}), PercentsD16.from(0.2 * 10 ** 16)); + + assertEq( + PercentsD16.fromFraction({numerator: MAX_PERCENT_D16 / HUNDRED_PERCENT_D16, denominator: 1}), + PercentsD16.from(MAX_PERCENT_D16 / HUNDRED_PERCENT_D16 * HUNDRED_PERCENT_D16) + ); + + assertEq( + PercentsD16.fromFraction({ + numerator: MAX_PERCENT_D16 / HUNDRED_PERCENT_D16, + denominator: HUNDRED_PERCENT_D16 + }), + PercentsD16.from(MAX_PERCENT_D16 / HUNDRED_PERCENT_D16) + ); + } + + function test_fromFraction_RevertOn_DenominatorIsZero() external { + vm.expectRevert(DivisionByZero.selector); + this.external__fromFraction({numerator: 1, denominator: 0}); + } + + function test_fromFraction_RevertOn_ArithmeticErrors() external { + vm.expectRevert(stdError.arithmeticError); + this.external__fromFraction({numerator: type(uint256).max, denominator: 1}); + + vm.expectRevert(stdError.arithmeticError); + this.external__fromFraction({numerator: type(uint256).max, denominator: type(uint256).max}); + + vm.expectRevert(stdError.arithmeticError); + this.external__fromFraction({numerator: type(uint256).max / HUNDRED_PERCENT_D16 + 1, denominator: 1}); + } + + function test_fromFraction_RevertOn_PercentD16Overflow() external { + vm.expectRevert(PercentD16Overflow.selector); + this.external__fromFraction({numerator: uint256(MAX_PERCENT_D16) + 1, denominator: 1}); + + vm.expectRevert(PercentD16Overflow.selector); + this.external__fromFraction({numerator: MAX_PERCENT_D16 / 1000, denominator: 100}); + } + + function testFuzz_fromFraction_HappyPath(uint256 numerator, uint256 denominator) external { + vm.assume(numerator <= MAX_PERCENT_D16 / HUNDRED_PERCENT_D16); + vm.assume(denominator > 0); + assertEq( + PercentsD16.fromFraction(numerator, denominator), + PercentD16.wrap(uint128(HUNDRED_PERCENT_D16 * numerator / denominator)) + ); + } + + function testFuzz_fromFraction_RevertOn_ArithmeticErrors(uint256 numerator, uint256 denominator) external { + (bool isSuccess,) = Math.tryMul(numerator, HUNDRED_PERCENT_D16); + vm.assume(!isSuccess); + vm.assume(denominator > 0); + + vm.expectRevert(stdError.arithmeticError); + this.external__fromFraction(numerator, denominator); + } + + function testFuzz_fromFraction_RevertOn_PercentD16Overflow(uint256 numerator, uint256 denominator) external { + (bool isSuccess,) = Math.tryMul(numerator, HUNDRED_PERCENT_D16); + vm.assume(isSuccess); + + vm.assume(denominator > 0); + vm.assume(HUNDRED_PERCENT_D16 * numerator / denominator > MAX_PERCENT_D16); + + vm.expectRevert(PercentD16Overflow.selector); + this.external__fromFraction(numerator, denominator); + } + + // --- + // fromBasisPoints() + // --- + + function test_fromBasisPoints_HappyPath() external { + assertEq(PercentsD16.fromBasisPoints(0), PercentsD16.from(0)); + assertEq(PercentsD16.fromBasisPoints(42_42), PercentsD16.from(42.42 * 10 ** 16)); + assertEq(PercentsD16.fromBasisPoints(100_00), PercentsD16.from(100 * 10 ** 16)); + assertEq(PercentsD16.fromBasisPoints(3000_00), PercentsD16.from(3000 * 10 ** 16)); + assertEq( + PercentsD16.fromBasisPoints(uint256(HUNDRED_PERCENT_BP) * MAX_PERCENT_D16 / HUNDRED_PERCENT_D16), + PercentsD16.from( + uint256(MAX_PERCENT_D16) * HUNDRED_PERCENT_BP / HUNDRED_PERCENT_D16 * HUNDRED_PERCENT_D16 + / HUNDRED_PERCENT_BP + ) + ); + } + + function test_fromBasisPoints_RevertOn_ArithmeticErrors() external { + vm.expectRevert(stdError.arithmeticError); + this.external__fromBasisPoints(type(uint256).max); + + vm.expectRevert(stdError.arithmeticError); + this.external__fromBasisPoints(type(uint256).max / HUNDRED_PERCENT_D16 * HUNDRED_PERCENT_BP); + + vm.expectRevert(stdError.arithmeticError); + this.external__fromBasisPoints(type(uint256).max / HUNDRED_PERCENT_D16 + 1); + } + + function test_fromBasisPoints_RevertOn_PercentD16Overflow() external { + vm.expectRevert(PercentD16Overflow.selector); + this.external__fromBasisPoints(MAX_PERCENT_D16); + + vm.expectRevert(PercentD16Overflow.selector); + this.external__fromBasisPoints(MAX_PERCENT_D16 / HUNDRED_PERCENT_D16 * HUNDRED_PERCENT_BP * 10); + } + + function testFuzz_fromBasisPoints(uint256 value) external { + vm.assume(value <= MAX_PERCENT_D16 / HUNDRED_PERCENT_D16); + assertEq( + PercentsD16.fromBasisPoints(value), + PercentD16.wrap(uint128(value * HUNDRED_PERCENT_D16 / HUNDRED_PERCENT_BP)) + ); + } + + // --- + // Helper test methods + // --- + + function external__fromBasisPoints(uint256 bpValue) external returns (PercentD16) { + return PercentsD16.fromBasisPoints(bpValue); + } + + function external__fromFraction(uint256 numerator, uint256 denominator) external returns (PercentD16) { + return PercentsD16.fromFraction(numerator, denominator); + } + + function external__plus(PercentD16 a, PercentD16 b) external returns (PercentD16) { + return a + b; + } + + function external__minus(PercentD16 a, PercentD16 b) external returns (PercentD16) { + return a - b; + } + + function external__from(uint256 value) external returns (PercentD16) { + return PercentsD16.from(value); + } +} diff --git a/test/unit/types/SharesValue.t.sol b/test/unit/types/SharesValue.t.sol new file mode 100644 index 00000000..3c7becfa --- /dev/null +++ b/test/unit/types/SharesValue.t.sol @@ -0,0 +1,167 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.26; + +import { + SharesValue, + SharesValues, + SharesValueOverflow, + SharesValueUnderflow, + MAX_SHARES_VALUE +} from "contracts/types/SharesValue.sol"; + +import {UnitTest} from "test/utils/unit-test.sol"; + +contract ETHTransfersForbiddenStub { + error ETHTransfersForbidden(); + + receive() external payable { + revert ETHTransfersForbidden(); + } +} + +contract SharesValueTests is UnitTest { + // --- + // Comparison operations + // --- + + // --- + // lt() + // --- + + function testFuzz_lt_HappyPath(SharesValue v1, SharesValue v2) external { + assertEq(v1 < v2, SharesValue.unwrap(v1) < SharesValue.unwrap(v2)); + } + + // --- + // eq() + // --- + + function testFuzz_eq_HappyPath(SharesValue v1, SharesValue v2) external { + assertEq(v1 == v2, SharesValue.unwrap(v1) == SharesValue.unwrap(v2)); + } + + // --- + // Arithmetic operations + // --- + + // --- + // plus() + // --- + + function test_plus_HappyPath() external { + assertEq(SharesValues.from(0) + SharesValues.from(0), SharesValue.wrap(0)); + assertEq(SharesValues.from(1) + SharesValues.from(0), SharesValue.wrap(1)); + assertEq(SharesValues.from(0) + SharesValues.from(1), SharesValue.wrap(1)); + assertEq(SharesValues.from(0) + SharesValues.from(1), SharesValue.wrap(1)); + assertEq( + SharesValues.from(MAX_SHARES_VALUE / 2) + SharesValues.from(MAX_SHARES_VALUE / 2), + SharesValue.wrap(type(uint128).max - 1) + ); + assertEq(SharesValues.from(MAX_SHARES_VALUE) + SharesValues.from(0), SharesValue.wrap(type(uint128).max)); + } + + function test_plus_RevertOn_Overflow() external { + vm.expectRevert(SharesValueOverflow.selector); + this.external__plus(SharesValues.from(MAX_SHARES_VALUE), SharesValues.from(1)); + + vm.expectRevert(SharesValueOverflow.selector); + this.external__plus(SharesValues.from(MAX_SHARES_VALUE / 2 + 1), SharesValues.from(MAX_SHARES_VALUE / 2 + 1)); + } + + function testFuzz_plus_HappyPath(SharesValue v1, SharesValue v2) external { + uint256 expectedResult = v1.toUint256() + v2.toUint256(); + vm.assume(expectedResult <= MAX_SHARES_VALUE); + assertEq(v1 + v2, SharesValue.wrap(uint128(expectedResult))); + } + + function testFuzz_plus_RevertOn_Overflow(SharesValue v1, SharesValue v2) external { + uint256 expectedResult = v1.toUint256() + v2.toUint256(); + vm.assume(expectedResult > MAX_SHARES_VALUE); + vm.expectRevert(SharesValueOverflow.selector); + this.external__plus(v1, v2); + } + + // --- + // minus() + // --- + + function test_minus_HappyPath() external { + assertEq(SharesValues.from(0) - SharesValues.from(0), SharesValue.wrap(0)); + assertEq(SharesValues.from(1) - SharesValues.from(0), SharesValue.wrap(1)); + assertEq(SharesValues.from(1) - SharesValues.from(1), SharesValue.wrap(0)); + assertEq( + SharesValues.from(MAX_SHARES_VALUE) - SharesValues.from(1), SharesValue.wrap(uint128(MAX_SHARES_VALUE - 1)) + ); + + assertEq(SharesValues.from(MAX_SHARES_VALUE) - SharesValues.from(MAX_SHARES_VALUE), SharesValue.wrap(0)); + } + + function test_minus_RevertOn_SharesValueUnderflow() external { + vm.expectRevert(SharesValueUnderflow.selector); + this.external__minus(SharesValues.from(0), SharesValues.from(1)); + + vm.expectRevert(SharesValueUnderflow.selector); + this.external__minus(SharesValues.from(0), SharesValues.from(MAX_SHARES_VALUE)); + } + + function testFuzz_minus_HappyPath(SharesValue v1, SharesValue v2) external { + vm.assume(SharesValue.unwrap(v1) > SharesValue.unwrap(v2)); + uint256 expectedResult = v1.toUint256() - v2.toUint256(); + assertEq(v1 - v2, SharesValue.wrap(uint128(expectedResult))); + } + + function testFuzz_minus_Overflow(SharesValue v1, SharesValue v2) external { + vm.assume(v1 < v2); + vm.expectRevert(SharesValueUnderflow.selector); + this.external__minus(v1, v2); + } + + // --- + // Custom operations + // --- + + // --- + // toUint256() + // --- + + function test_toUint256_HappyPath() external { + assertEq(SharesValues.from(0).toUint256(), 0); + assertEq(SharesValues.from(MAX_SHARES_VALUE / 2).toUint256(), MAX_SHARES_VALUE / 2); + assertEq(SharesValues.from(MAX_SHARES_VALUE).toUint256(), MAX_SHARES_VALUE); + } + + function testFuzz_toUint256_HappyPath(SharesValue amount) external { + assertEq(amount.toUint256(), SharesValue.unwrap(amount)); + } + + // --- + // from() + // --- + + function testFuzz_from_HappyPath(uint256 amount) external { + vm.assume(amount <= MAX_SHARES_VALUE); + assertEq(SharesValues.from(amount), SharesValue.wrap(uint128(amount))); + } + + function testFuzz_from_RevertOn_Overflow(uint256 amount) external { + vm.assume(amount > MAX_SHARES_VALUE); + vm.expectRevert(SharesValueOverflow.selector); + this.external__from(amount); + } + + // --- + // Helper test methods + // --- + + function external__plus(SharesValue a, SharesValue b) external { + a + b; + } + + function external__minus(SharesValue a, SharesValue b) external { + a - b; + } + + function external__from(uint256 amount) external returns (SharesValue) { + SharesValues.from(amount); + } +} diff --git a/test/unit/types/Timestamp.t.sol b/test/unit/types/Timestamp.t.sol new file mode 100644 index 00000000..f12d02c7 --- /dev/null +++ b/test/unit/types/Timestamp.t.sol @@ -0,0 +1,189 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.26; + +import {Math} from "@openzeppelin/contracts/utils/math/Math.sol"; + +import {Timestamp, Timestamps, TimestampOverflow, MAX_TIMESTAMP_VALUE} from "contracts/types/Timestamp.sol"; + +import {UnitTest} from "test/utils/unit-test.sol"; + +contract TimestampTests is UnitTest { + // --- + // Constants + // --- + + function test_MAX_TIMESTAMP_VALUE_HappyPath() external { + assertEq(MAX_TIMESTAMP_VALUE, type(uint40).max); + } + + // --- + // Comparison operations + // --- + + // --- + // lt() + // --- + + function testFuzz_lt_HappyPath(Timestamp t1, Timestamp t2) external { + assertEq(t1 < t2, t1.toSeconds() < t2.toSeconds()); + } + + // --- + // lte() + // --- + + function testFuzz_lte_HappyPath(Timestamp t1, Timestamp t2) external { + assertEq(t1 <= t2, t1.toSeconds() <= t2.toSeconds()); + } + + // --- + // eq() + // --- + + function testFuzz_eq_HappyPath(Timestamp t1, Timestamp t2) external { + assertEq(t1 == t2, t1.toSeconds() == t2.toSeconds()); + } + + // --- + // neq() + // --- + + function testFuzz_neq_HappyPath(Timestamp t1, Timestamp t2) external { + assertEq(t1 != t2, t1.toSeconds() != t2.toSeconds()); + } + + // --- + // gte() + // --- + + function testFuzz_gte_HappyPath(Timestamp t1, Timestamp t2) external { + assertEq(t1 >= t2, t1.toSeconds() >= t2.toSeconds()); + } + + // --- + // gt() + // --- + + function testFuzz_gt_HappyPath(Timestamp t1, Timestamp t2) external { + assertEq(t1 > t2, t1.toSeconds() > t2.toSeconds()); + } + + // --- + // Conversion operations + // --- + + function testFuzz_toSeconds_HappyPath(Timestamp t) external { + assertEq(t.toSeconds(), Timestamp.unwrap(t)); + } + + // --- + // Custom operations + // --- + + // --- + // isZero() + // --- + + function test_isZero_HappyPath_ReturnsTrue() external { + assertTrue(Timestamp.wrap(0).isZero()); + } + + function test_isZero_HappyPath_ReturnFalse() external { + assertFalse(Timestamp.wrap(1).isZero()); + assertFalse(Timestamp.wrap(MAX_TIMESTAMP_VALUE / 2).isZero()); + assertFalse(Timestamp.wrap(MAX_TIMESTAMP_VALUE).isZero()); + } + + function testFuzz_isZero_HappyPath(Timestamp t) external { + assertEq(t.isZero(), t == Timestamps.ZERO); + } + + // --- + // isNotZero() + // --- + + function test_isNotZero_HappyPath_ReturnFalse() external { + assertTrue(Timestamp.wrap(1).isNotZero()); + assertTrue(Timestamp.wrap(MAX_TIMESTAMP_VALUE / 2).isNotZero()); + assertTrue(Timestamp.wrap(MAX_TIMESTAMP_VALUE).isNotZero()); + } + + function test_isNotZero_HappyPath_ReturnsFalse() external { + assertFalse(Timestamp.wrap(0).isNotZero()); + } + + function testFuzz_isNotZero_HappyPath(Timestamp t) external { + assertEq(t.isNotZero(), t != Timestamps.ZERO); + } + + // --- + // Namespaced helper methods + // --- + + // --- + // now() + // --- + + function testFuzz_max_HappyPath(Timestamp t1, Timestamp t2) external { + assertEq(Timestamps.max(t1, t2), Timestamps.from(Math.max(t1.toSeconds(), t2.toSeconds()))); + } + + function test_now_HappyPath() external { + assertEq(Timestamps.now(), Timestamps.from(block.timestamp)); + + vm.warp(12 hours); + assertEq(Timestamps.now(), Timestamps.from(block.timestamp)); + + vm.warp(30 days); + assertEq(Timestamps.now(), Timestamps.from(block.timestamp)); + + vm.warp(365 days); + assertEq(Timestamps.now(), Timestamps.from(block.timestamp)); + + vm.warp(100 * 365 days); + assertEq(Timestamps.now(), Timestamps.from(block.timestamp)); + + vm.warp(1_000 * 365 days); + assertEq(Timestamps.now(), Timestamps.from(block.timestamp)); + + vm.warp(10_000 * 365 days); + assertEq(Timestamps.now(), Timestamps.from(block.timestamp)); + + vm.warp(34_000 * 365 days); + assertEq(Timestamps.now(), Timestamps.from(block.timestamp)); + } + + function test_now_InvalidValueAfterApprox34000Years() external { + vm.warp(MAX_TIMESTAMP_VALUE); // MAX_TIMESTAMP_VALUE is ~ 36812 year + assertEq(Timestamps.now().toSeconds(), block.timestamp); + + // After the ~34800 years the uint40 timestamp value will overflow and conversion + // of block.timestamp to uint40 will start return incorrect values. + vm.warp(uint256(MAX_TIMESTAMP_VALUE) + 1); + assertEq(Timestamps.now().toSeconds(), 0); + } + + // --- + // from() + // --- + + function testFuzz_from_HappyPath(uint256 value) external { + vm.assume(value <= MAX_TIMESTAMP_VALUE); + assertEq(Timestamps.from(value), Timestamp.wrap(uint40(value))); + } + + function testFuzz_from_RevertOn_Overflow(uint256 value) external { + vm.assume(value > MAX_TIMESTAMP_VALUE); + + vm.expectRevert(TimestampOverflow.selector); + this.external__from(value); + } + + // --- + // Helper test methods + // --- + + function external__from(uint256 value) external returns (Timestamp) { + return Timestamps.from(value); + } +} diff --git a/test/utils/testing-assert-eq-extender.sol b/test/utils/testing-assert-eq-extender.sol index 421f3a62..fcac37da 100644 --- a/test/utils/testing-assert-eq-extender.sol +++ b/test/utils/testing-assert-eq-extender.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.26; import {Test} from "forge-std/Test.sol"; import {ETHValue} from "contracts/types/ETHValue.sol"; +import {SharesValue} from "contracts/types/SharesValue.sol"; import {Duration} from "contracts/types/Duration.sol"; import {Timestamp} from "contracts/types/Timestamp.sol"; import {PercentD16} from "contracts/types/PercentD16.sol"; @@ -56,6 +57,10 @@ contract TestingAssertEqExtender is Test { assertEq(ETHValue.unwrap(a), ETHValue.unwrap(b)); } + function assertEq(SharesValue a, SharesValue b) internal { + assertEq(SharesValue.unwrap(a), SharesValue.unwrap(b)); + } + function assertEq(IndexOneBased a, IndexOneBased b) internal { assertEq(IndexOneBased.unwrap(a), IndexOneBased.unwrap(b)); } From 7871b80d37dbca123b805757062c6a2043e7ee14 Mon Sep 17 00:00:00 2001 From: Bogdan Kovtun Date: Mon, 23 Sep 2024 13:40:56 +0400 Subject: [PATCH 4/8] Add missing owner assume for committee tests --- test/unit/committees/EmergencyActivationCommittee.t.sol | 1 + test/unit/committees/EmergencyExecutionCommittee.t.sol | 1 + test/unit/committees/TiebreakerSubCommittee.t.sol | 1 + 3 files changed, 3 insertions(+) diff --git a/test/unit/committees/EmergencyActivationCommittee.t.sol b/test/unit/committees/EmergencyActivationCommittee.t.sol index 1cd2159c..6f204903 100644 --- a/test/unit/committees/EmergencyActivationCommittee.t.sol +++ b/test/unit/committees/EmergencyActivationCommittee.t.sol @@ -30,6 +30,7 @@ contract EmergencyActivationCommitteeUnitTest is UnitTest { uint256 _quorum, address _emergencyProtectedTimelock ) external { + vm.assume(_owner != address(0)); vm.assume(_quorum > 0 && _quorum <= committeeMembers.length); EmergencyActivationCommittee localCommittee = new EmergencyActivationCommittee(_owner, committeeMembers, _quorum, _emergencyProtectedTimelock); diff --git a/test/unit/committees/EmergencyExecutionCommittee.t.sol b/test/unit/committees/EmergencyExecutionCommittee.t.sol index cc047b30..0fb0b9ae 100644 --- a/test/unit/committees/EmergencyExecutionCommittee.t.sol +++ b/test/unit/committees/EmergencyExecutionCommittee.t.sol @@ -42,6 +42,7 @@ contract EmergencyExecutionCommitteeUnitTest is UnitTest { uint256 _quorum, address _emergencyProtectedTimelock ) external { + vm.assume(_owner != address(0)); vm.assume(_quorum > 0 && _quorum <= committeeMembers.length); EmergencyExecutionCommittee localCommittee = new EmergencyExecutionCommittee(_owner, committeeMembers, _quorum, _emergencyProtectedTimelock); diff --git a/test/unit/committees/TiebreakerSubCommittee.t.sol b/test/unit/committees/TiebreakerSubCommittee.t.sol index 774ae27c..17a7575a 100644 --- a/test/unit/committees/TiebreakerSubCommittee.t.sol +++ b/test/unit/committees/TiebreakerSubCommittee.t.sol @@ -43,6 +43,7 @@ contract TiebreakerSubCommitteeUnitTest is UnitTest { } function test_constructor_HappyPath(address _owner, uint256 _quorum, address _tiebreakerCore) external { + vm.assume(_owner != address(0)); vm.assume(_quorum > 0 && _quorum <= committeeMembers.length); new TiebreakerSubCommittee(_owner, committeeMembers, _quorum, _tiebreakerCore); } From 03f9e56f95729ef4778aa3760dfe7c9b70bf15d1 Mon Sep 17 00:00:00 2001 From: Bogdan Kovtun Date: Wed, 25 Sep 2024 13:52:26 +0400 Subject: [PATCH 5/8] Update section comments in the type files --- contracts/types/Duration.sol | 14 +++++++------- contracts/types/ETHValue.sol | 14 +++++++------- contracts/types/IndexOneBased.sol | 10 +++++----- contracts/types/PercentD16.sol | 12 ++++++------ contracts/types/SharesValue.sol | 12 ++++++------ contracts/types/Timestamp.sol | 12 ++++++------ 6 files changed, 37 insertions(+), 37 deletions(-) diff --git a/contracts/types/Duration.sol b/contracts/types/Duration.sol index 6fdd12cb..c669bb75 100644 --- a/contracts/types/Duration.sol +++ b/contracts/types/Duration.sol @@ -4,13 +4,13 @@ pragma solidity 0.8.26; import {Timestamp, Timestamps} from "./Timestamp.sol"; // --- -// Type definition +// Type Definition // --- type Duration is uint32; // --- -// Assign global operations +// Assign Global Operations // --- using {lt as <, lte as <=, eq as ==, neq as !=, gte as >=, gt as >} for Duration global; @@ -33,7 +33,7 @@ error DurationUnderflow(); uint32 constant MAX_DURATION_VALUE = type(uint32).max; // --- -// Comparison operations +// Comparison Operations // --- function lt(Duration d1, Duration d2) pure returns (bool) { @@ -61,7 +61,7 @@ function gt(Duration d1, Duration d2) pure returns (bool) { } // --- -// Conversion operations +// Conversion Operations // --- function toSeconds(Duration d) pure returns (uint256) { @@ -69,7 +69,7 @@ function toSeconds(Duration d) pure returns (uint256) { } // --- -// Arithmetic operations +// Arithmetic Operations // --- function plus(Duration d1, Duration d2) pure returns (Duration) { @@ -96,7 +96,7 @@ function minus(Duration d1, Duration d2) pure returns (Duration) { } // --- -// Custom operations +// Custom Operations // --- function plusSeconds(Duration d, uint256 secondsToAdd) pure returns (Duration) { @@ -138,7 +138,7 @@ function addTo(Duration d, Timestamp t) pure returns (Timestamp) { } // --- -// Namespaced helper methods +// Namespaced Helper Methods // --- library Durations { diff --git a/contracts/types/ETHValue.sol b/contracts/types/ETHValue.sol index 01543b53..bd296412 100644 --- a/contracts/types/ETHValue.sol +++ b/contracts/types/ETHValue.sol @@ -4,13 +4,13 @@ pragma solidity 0.8.26; import {Address} from "@openzeppelin/contracts/utils/Address.sol"; // --- -// Type definition +// Type Definition // --- type ETHValue is uint128; // --- -// Assign global operations +// Assign Global Operations // --- using {lt as <, eq as ==, neq as !=, gt as >} for ETHValue global; @@ -31,7 +31,7 @@ error ETHValueUnderflow(); uint128 constant MAX_ETH_VALUE = type(uint128).max; // --- -// Comparison operations +// Comparison Operations // --- function lt(ETHValue v1, ETHValue v2) pure returns (bool) { @@ -51,7 +51,7 @@ function gt(ETHValue v1, ETHValue v2) pure returns (bool) { } // --- -// Conversion operations +// Conversion Operations // --- function toUint256(ETHValue value) pure returns (uint256) { @@ -59,7 +59,7 @@ function toUint256(ETHValue value) pure returns (uint256) { } // --- -// Arithmetic operations +// Arithmetic Operations // --- function plus(ETHValue v1, ETHValue v2) pure returns (ETHValue) { @@ -86,7 +86,7 @@ function minus(ETHValue v1, ETHValue v2) pure returns (ETHValue) { } // --- -// Custom operations +// Custom Operations // --- function sendTo(ETHValue value, address payable recipient) { @@ -94,7 +94,7 @@ function sendTo(ETHValue value, address payable recipient) { } // --- -// Namespaced helper methods +// Namespaced Helper Methods // --- library ETHValues { diff --git a/contracts/types/IndexOneBased.sol b/contracts/types/IndexOneBased.sol index 847a4a14..79e01974 100644 --- a/contracts/types/IndexOneBased.sol +++ b/contracts/types/IndexOneBased.sol @@ -2,13 +2,13 @@ pragma solidity 0.8.26; // --- -// Type definition +// Type Definition // --- type IndexOneBased is uint32; // --- -// Assign global operations +// Assign Global Operations // --- using {neq as !=} for IndexOneBased global; @@ -28,7 +28,7 @@ error IndexOneBasedUnderflow(); uint32 constant MAX_INDEX_ONE_BASED_VALUE = type(uint32).max; // --- -// Comparison operations +// Comparison Operations // --- function neq(IndexOneBased i1, IndexOneBased i2) pure returns (bool) { @@ -36,7 +36,7 @@ function neq(IndexOneBased i1, IndexOneBased i2) pure returns (bool) { } // --- -// Custom operations +// Custom Operations // --- function isEmpty(IndexOneBased index) pure returns (bool) { @@ -59,7 +59,7 @@ function toZeroBasedValue(IndexOneBased index) pure returns (uint256) { } // --- -// Namespaced helper methods +// Namespaced Helper Methods // --- library IndicesOneBased { diff --git a/contracts/types/PercentD16.sol b/contracts/types/PercentD16.sol index d1f725cd..300ae848 100644 --- a/contracts/types/PercentD16.sol +++ b/contracts/types/PercentD16.sol @@ -2,13 +2,13 @@ pragma solidity 0.8.26; // --- -// Type definition +// Type Definition // --- type PercentD16 is uint128; // --- -// Assign global operations +// Assign Global Operations // --- using {lt as <, lte as <=, eq as ==, gte as >=, gt as >} for PercentD16 global; @@ -32,7 +32,7 @@ uint128 constant MAX_PERCENT_D16 = type(uint128).max; uint128 constant HUNDRED_PERCENT_D16 = 100 * 10 ** 16; // --- -// Comparison operations +// Comparison Operations // --- function lt(PercentD16 a, PercentD16 b) pure returns (bool) { @@ -56,7 +56,7 @@ function gt(PercentD16 a, PercentD16 b) pure returns (bool) { } // --- -// Conversion operations +// Conversion Operations // --- function toUint256(PercentD16 value) pure returns (uint256) { @@ -64,7 +64,7 @@ function toUint256(PercentD16 value) pure returns (uint256) { } // --- -// Arithmetic operations +// Arithmetic Operations // --- function plus(PercentD16 a, PercentD16 b) pure returns (PercentD16) { @@ -91,7 +91,7 @@ function minus(PercentD16 a, PercentD16 b) pure returns (PercentD16) { } // --- -// Namespaced helper methods +// Namespaced Helper Methods // --- library PercentsD16 { diff --git a/contracts/types/SharesValue.sol b/contracts/types/SharesValue.sol index a2d663b6..f6511f06 100644 --- a/contracts/types/SharesValue.sol +++ b/contracts/types/SharesValue.sol @@ -2,13 +2,13 @@ pragma solidity 0.8.26; // --- -// Type definition +// Type Definition // --- type SharesValue is uint128; // --- -// Assign global operations +// Assign Global Operations // --- using {lt as <, eq as ==} for SharesValue global; @@ -29,7 +29,7 @@ error SharesValueUnderflow(); uint128 constant MAX_SHARES_VALUE = type(uint128).max; // --- -// Comparison operations +// Comparison Operations // --- function lt(SharesValue v1, SharesValue v2) pure returns (bool) { @@ -41,7 +41,7 @@ function eq(SharesValue v1, SharesValue v2) pure returns (bool) { } // --- -// Conversion operations +// Conversion Operations // --- function toUint256(SharesValue v) pure returns (uint256) { @@ -49,7 +49,7 @@ function toUint256(SharesValue v) pure returns (uint256) { } // --- -// Arithmetic operations +// Arithmetic Operations // --- function plus(SharesValue v1, SharesValue v2) pure returns (SharesValue) { @@ -76,7 +76,7 @@ function minus(SharesValue v1, SharesValue v2) pure returns (SharesValue) { } // --- -// Namespaced helper methods +// Namespaced Helper Methods // --- library SharesValues { diff --git a/contracts/types/Timestamp.sol b/contracts/types/Timestamp.sol index b65bca30..9c702c06 100644 --- a/contracts/types/Timestamp.sol +++ b/contracts/types/Timestamp.sol @@ -2,13 +2,13 @@ pragma solidity 0.8.26; // --- -// Type definition +// Type Definition // --- type Timestamp is uint40; // --- -// Assign global operations +// Assign Global Operations // --- using {lt as <, lte as <=, eq as ==, neq as !=, gte as >=, gt as >} for Timestamp global; @@ -28,7 +28,7 @@ error TimestampOverflow(); uint40 constant MAX_TIMESTAMP_VALUE = type(uint40).max; // --- -// Comparison operations +// Comparison Operations // --- function lt(Timestamp t1, Timestamp t2) pure returns (bool) { @@ -56,7 +56,7 @@ function gt(Timestamp t1, Timestamp t2) pure returns (bool) { } // --- -// Conversion operations +// Conversion Operations // --- function toSeconds(Timestamp t) pure returns (uint256) { @@ -64,7 +64,7 @@ function toSeconds(Timestamp t) pure returns (uint256) { } // --- -// Custom operations +// Custom Operations // --- function isZero(Timestamp t) pure returns (bool) { @@ -76,7 +76,7 @@ function isNotZero(Timestamp t) pure returns (bool) { } // --- -// Namespaced helper methods +// Namespaced Helper Methods // --- library Timestamps { From b90ec9ebc68e039e6c1c056f5b4990bdb26f9e1d Mon Sep 17 00:00:00 2001 From: Bogdan Kovtun Date: Sun, 29 Sep 2024 18:41:16 +0400 Subject: [PATCH 6/8] Remove Durations.between() method --- contracts/types/Duration.sol | 11 ------ test/unit/libraries/EmergencyProtection.t.sol | 10 ++--- test/unit/libraries/EscrowState.t.sol | 38 ++++++++----------- test/unit/types/Duration.t.sol | 25 ------------ 4 files changed, 21 insertions(+), 63 deletions(-) diff --git a/contracts/types/Duration.sol b/contracts/types/Duration.sol index c669bb75..0e7bec5a 100644 --- a/contracts/types/Duration.sol +++ b/contracts/types/Duration.sol @@ -151,17 +151,6 @@ library Durations { res = Duration.wrap(uint32(durationInSeconds)); } - function between(Timestamp t1, Timestamp t2) internal pure returns (Duration res) { - uint256 t1Seconds = t1.toSeconds(); - uint256 t2Seconds = t2.toSeconds(); - - unchecked { - /// @dev Calculating the absolute difference between `t1Seconds` and `t2Seconds`. - /// Both are <= `type(uint40).max`, so their difference fits within `uint256`. - res = from(t1Seconds > t2Seconds ? t1Seconds - t2Seconds : t2Seconds - t1Seconds); - } - } - function min(Duration d1, Duration d2) internal pure returns (Duration res) { res = d1 < d2 ? d1 : d2; } diff --git a/test/unit/libraries/EmergencyProtection.t.sol b/test/unit/libraries/EmergencyProtection.t.sol index 4789953d..4fbb01ab 100644 --- a/test/unit/libraries/EmergencyProtection.t.sol +++ b/test/unit/libraries/EmergencyProtection.t.sol @@ -34,7 +34,7 @@ contract EmergencyProtectionTest is UnitTest { function test_activateEmergencyMode_RevertOn_ProtectionExpired() external { Duration untilExpiration = - Durations.between(ctx.emergencyProtectionEndsAfter, Timestamps.from(block.timestamp)).plusSeconds(1); + Durations.from(ctx.emergencyProtectionEndsAfter.toSeconds() - Timestamps.now().toSeconds()).plusSeconds(1); _wait(untilExpiration); @@ -210,7 +210,7 @@ contract EmergencyProtectionTest is UnitTest { assertFalse(EmergencyProtection.isEmergencyModeDurationPassed(ctx)); Duration untilExpiration = - Durations.between(ctx.emergencyModeEndsAfter, Timestamps.from(block.timestamp)).plusSeconds(1); + Durations.from(ctx.emergencyModeEndsAfter.toSeconds() - Timestamps.now().toSeconds()).plusSeconds(1); _wait(untilExpiration); assertTrue(EmergencyProtection.isEmergencyModeDurationPassed(ctx)); @@ -220,7 +220,7 @@ contract EmergencyProtectionTest is UnitTest { assertTrue(EmergencyProtection.isEmergencyProtectionEnabled(ctx)); Duration untilExpiration = - Durations.between(ctx.emergencyProtectionEndsAfter, Timestamps.from(block.timestamp)).plusSeconds(1); + Durations.from(ctx.emergencyProtectionEndsAfter.toSeconds() - Timestamps.now().toSeconds()).plusSeconds(1); _wait(untilExpiration); assertFalse(EmergencyProtection.isEmergencyProtectionEnabled(ctx)); @@ -232,13 +232,13 @@ contract EmergencyProtectionTest is UnitTest { assertTrue(EmergencyProtection.isEmergencyProtectionEnabled(ctx)); Duration untilExpiration = - Durations.between(ctx.emergencyModeEndsAfter, Timestamps.from(block.timestamp)).plusSeconds(1); + Durations.from(ctx.emergencyModeEndsAfter.toSeconds() - Timestamps.now().toSeconds()).plusSeconds(1); _wait(untilExpiration); assertTrue(EmergencyProtection.isEmergencyProtectionEnabled(ctx)); untilExpiration = - Durations.between(ctx.emergencyProtectionEndsAfter, Timestamps.from(block.timestamp)).plusSeconds(1); + Durations.from(ctx.emergencyProtectionEndsAfter.toSeconds() - Timestamps.now().toSeconds()).plusSeconds(1); assertTrue(EmergencyProtection.isEmergencyProtectionEnabled(ctx)); diff --git a/test/unit/libraries/EscrowState.t.sol b/test/unit/libraries/EscrowState.t.sol index a86f9594..c7685468 100644 --- a/test/unit/libraries/EscrowState.t.sol +++ b/test/unit/libraries/EscrowState.t.sol @@ -195,14 +195,10 @@ contract EscrowStateUnitTests is UnitTest { _context.rageQuitExtensionPeriodDuration = rageQuitExtensionPeriodDuration; _context.rageQuitEthWithdrawalsDelay = rageQuitEthWithdrawalsDelay; - _wait( - Durations.between( - (rageQuitExtensionPeriodDuration + rageQuitEthWithdrawalsDelay).plusSeconds(1).addTo( - rageQuitExtensionPeriodStartedAt - ), - Timestamps.now() - ) - ); + Duration totalWithdrawalsDelay = rageQuitExtensionPeriodDuration + rageQuitEthWithdrawalsDelay; + Timestamp withdrawalsAllowedAt = totalWithdrawalsDelay.plusSeconds(1).addTo(rageQuitExtensionPeriodStartedAt); + + _wait(Durations.from(withdrawalsAllowedAt.toSeconds() - Timestamps.now().toSeconds())); EscrowState.checkEthWithdrawalsDelayPassed(_context); } @@ -226,12 +222,10 @@ contract EscrowStateUnitTests is UnitTest { _context.rageQuitExtensionPeriodDuration = rageQuitExtensionPeriodDuration; _context.rageQuitEthWithdrawalsDelay = rageQuitEthWithdrawalsDelay; - _wait( - Durations.between( - (rageQuitExtensionPeriodDuration + rageQuitEthWithdrawalsDelay).addTo(rageQuitExtensionPeriodStartedAt), - Timestamps.now() - ) - ); + Duration totalWithdrawalsDelay = rageQuitExtensionPeriodDuration + rageQuitEthWithdrawalsDelay; + Timestamp withdrawalsAllowedAfter = totalWithdrawalsDelay.addTo(rageQuitExtensionPeriodStartedAt); + + _wait(Durations.from(withdrawalsAllowedAfter.toSeconds() - Timestamps.now().toSeconds())); vm.expectRevert(EscrowState.EthWithdrawalsDelayNotPassed.selector); @@ -276,11 +270,10 @@ contract EscrowStateUnitTests is UnitTest { _context.rageQuitExtensionPeriodStartedAt = rageQuitExtensionPeriodStartedAt; _context.rageQuitExtensionPeriodDuration = rageQuitExtensionPeriodDuration; - _wait( - Durations.between( - rageQuitExtensionPeriodDuration.plusSeconds(1).addTo(rageQuitExtensionPeriodStartedAt), Timestamps.now() - ) - ); + Timestamp rageQuitExtensionPeriodPassedAfter = + rageQuitExtensionPeriodDuration.plusSeconds(1).addTo(rageQuitExtensionPeriodStartedAt); + + _wait(Durations.from(rageQuitExtensionPeriodPassedAfter.toSeconds() - Timestamps.now().toSeconds())); bool res = EscrowState.isRageQuitExtensionPeriodPassed(_context); assertTrue(res); } @@ -296,9 +289,10 @@ contract EscrowStateUnitTests is UnitTest { _context.rageQuitExtensionPeriodStartedAt = rageQuitExtensionPeriodStartedAt; _context.rageQuitExtensionPeriodDuration = rageQuitExtensionPeriodDuration; - _wait( - Durations.between(rageQuitExtensionPeriodDuration.addTo(rageQuitExtensionPeriodStartedAt), Timestamps.now()) - ); + Timestamp rageQuitExtensionPeriodPassedAt = + rageQuitExtensionPeriodDuration.addTo(rageQuitExtensionPeriodStartedAt); + + _wait(Durations.from(rageQuitExtensionPeriodPassedAt.toSeconds() - Timestamps.now().toSeconds())); bool res = EscrowState.isRageQuitExtensionPeriodPassed(_context); assertFalse(res); } diff --git a/test/unit/types/Duration.t.sol b/test/unit/types/Duration.t.sol index f9d45d59..d1151730 100644 --- a/test/unit/types/Duration.t.sol +++ b/test/unit/types/Duration.t.sol @@ -294,27 +294,6 @@ contract DurationTests is UnitTest { this.external__from(durationInSeconds); } - function testFuzz_between_HappyPath(Timestamp t1, Timestamp t2) external { - uint256 t1Seconds = t1.toSeconds(); - uint256 t2Seconds = t2.toSeconds(); - uint256 expectedValue = t1Seconds > t2Seconds ? t1Seconds - t2Seconds : t2Seconds - t1Seconds; - - vm.assume(expectedValue <= MAX_DURATION_VALUE); - - assertEq(Durations.between(t1, t2), Duration.wrap(uint32(expectedValue))); - } - - function testFuzz_between_RevertOn_Overflow(Timestamp t1, Timestamp t2) external { - uint256 t1Seconds = t1.toSeconds(); - uint256 t2Seconds = t2.toSeconds(); - uint256 expectedValue = t1Seconds > t2Seconds ? t1Seconds - t2Seconds : t2Seconds - t1Seconds; - - vm.assume(expectedValue > MAX_DURATION_VALUE); - - vm.expectRevert(DurationOverflow.selector); - this.external__between(t1, t2); - } - function testFuzz_min_HappyPath(Duration d1, Duration d2) external { assertEq(Durations.min(d1, d2), Durations.from(Math.min(d1.toSeconds(), d2.toSeconds()))); } @@ -354,8 +333,4 @@ contract DurationTests is UnitTest { function external__from(uint256 valueInSeconds) external returns (Duration) { return Durations.from(valueInSeconds); } - - function external__between(Timestamp t1, Timestamp t2) external returns (Duration) { - return Durations.between(t1, t2); - } } From 3a0791a5aa595de3a587410c2341cbe7b481b5e1 Mon Sep 17 00:00:00 2001 From: Bogdan Kovtun Date: Sun, 29 Sep 2024 18:58:34 +0400 Subject: [PATCH 7/8] Add comments to from() methods for the types --- contracts/types/Duration.sol | 2 ++ contracts/types/ETHValue.sol | 2 ++ contracts/types/IndexOneBased.sol | 2 ++ contracts/types/PercentD16.sol | 2 ++ contracts/types/SharesValue.sol | 2 ++ contracts/types/Timestamp.sol | 17 ++++++++++------- 6 files changed, 20 insertions(+), 7 deletions(-) diff --git a/contracts/types/Duration.sol b/contracts/types/Duration.sol index 0e7bec5a..768f6698 100644 --- a/contracts/types/Duration.sol +++ b/contracts/types/Duration.sol @@ -148,6 +148,8 @@ library Durations { if (durationInSeconds > MAX_DURATION_VALUE) { revert DurationOverflow(); } + /// @dev Casting `durationInSeconds` to `uint32` is safe as the check ensures it is less than or equal + /// to `MAX_DURATION_VALUE`, which fits within the `uint32`. res = Duration.wrap(uint32(durationInSeconds)); } diff --git a/contracts/types/ETHValue.sol b/contracts/types/ETHValue.sol index bd296412..9cf8f30b 100644 --- a/contracts/types/ETHValue.sol +++ b/contracts/types/ETHValue.sol @@ -104,6 +104,8 @@ library ETHValues { if (value > MAX_ETH_VALUE) { revert ETHValueOverflow(); } + /// @dev Casting `value` to `uint128` is safe as the check ensures it is less than or equal + /// to `MAX_ETH_VALUE`, which fits within the `uint128`. return ETHValue.wrap(uint128(value)); } diff --git a/contracts/types/IndexOneBased.sol b/contracts/types/IndexOneBased.sol index 79e01974..538c0b90 100644 --- a/contracts/types/IndexOneBased.sol +++ b/contracts/types/IndexOneBased.sol @@ -70,6 +70,8 @@ library IndicesOneBased { if (oneBasedIndexValue > MAX_INDEX_ONE_BASED_VALUE) { revert IndexOneBasedOverflow(); } + /// @dev Casting `oneBasedIndexValue` to `uint32` is safe as the check ensures it is less than or equal + /// to `MAX_INDEX_ONE_BASED_VALUE`, which fits within the `uint32`. return IndexOneBased.wrap(uint32(oneBasedIndexValue)); } } diff --git a/contracts/types/PercentD16.sol b/contracts/types/PercentD16.sol index 300ae848..162ca6e6 100644 --- a/contracts/types/PercentD16.sol +++ b/contracts/types/PercentD16.sol @@ -99,6 +99,8 @@ library PercentsD16 { if (value > MAX_PERCENT_D16) { revert PercentD16Overflow(); } + /// @dev Casting `value` to `uint128` is safe as the check ensures it is less than or equal + /// to `MAX_PERCENT_D16`, which fits within the `uint128`. return PercentD16.wrap(uint128(value)); } diff --git a/contracts/types/SharesValue.sol b/contracts/types/SharesValue.sol index f6511f06..a5aa466b 100644 --- a/contracts/types/SharesValue.sol +++ b/contracts/types/SharesValue.sol @@ -86,6 +86,8 @@ library SharesValues { if (value > MAX_SHARES_VALUE) { revert SharesValueOverflow(); } + /// @dev Casting `value` to `uint128` is safe as the check ensures it is less than or equal + /// to `MAX_SHARES_VALUE`, which fits within the `uint128`. return SharesValue.wrap(uint128(value)); } } diff --git a/contracts/types/Timestamp.sol b/contracts/types/Timestamp.sol index 9c702c06..7da52292 100644 --- a/contracts/types/Timestamp.sol +++ b/contracts/types/Timestamp.sol @@ -82,8 +82,14 @@ function isNotZero(Timestamp t) pure returns (bool) { library Timestamps { Timestamp internal constant ZERO = Timestamp.wrap(0); - function max(Timestamp t1, Timestamp t2) internal pure returns (Timestamp) { - return t1 > t2 ? t1 : t2; + function from(uint256 timestampInSeconds) internal pure returns (Timestamp res) { + if (timestampInSeconds > MAX_TIMESTAMP_VALUE) { + revert TimestampOverflow(); + } + + /// @dev Casting `timestampInSeconds` to `uint40` is safe as the check ensures it is less than or equal + /// to `MAX_TIMESTAMP_VALUE`, which fits within the `uint40`. + return Timestamp.wrap(uint40(timestampInSeconds)); } function now() internal view returns (Timestamp res) { @@ -92,10 +98,7 @@ library Timestamps { res = Timestamp.wrap(uint40(block.timestamp)); } - function from(uint256 value) internal pure returns (Timestamp res) { - if (value > MAX_TIMESTAMP_VALUE) { - revert TimestampOverflow(); - } - return Timestamp.wrap(uint40(value)); + function max(Timestamp t1, Timestamp t2) internal pure returns (Timestamp) { + return t1 > t2 ? t1 : t2; } } From c4e127a6fe84f122c7d8677bb9461bed295da92e Mon Sep 17 00:00:00 2001 From: Aleksandr Tarelkin Date: Wed, 2 Oct 2024 15:33:49 +0300 Subject: [PATCH 8/8] fix: types tests --- test/unit/types/Duration.t.sol | 2 ++ test/unit/types/PercentD16.t.sol | 2 +- test/unit/types/SharesValue.t.sol | 1 - 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/test/unit/types/Duration.t.sol b/test/unit/types/Duration.t.sol index d1151730..0b7a3921 100644 --- a/test/unit/types/Duration.t.sol +++ b/test/unit/types/Duration.t.sol @@ -74,6 +74,7 @@ contract DurationTests is UnitTest { function test_eq_HappyPath_ReturnsFalse() external { assertFalse(Duration.wrap(0) == Duration.wrap(1)); + assertFalse(Duration.wrap(1) == Duration.wrap(0)); assertFalse(Duration.wrap(MAX_DURATION_VALUE / 2) == Duration.wrap(MAX_DURATION_VALUE)); assertFalse(Duration.wrap(0) == Duration.wrap(MAX_DURATION_VALUE)); } @@ -88,6 +89,7 @@ contract DurationTests is UnitTest { function test_neq_HappyPath_ReturnsTrue() external { assertTrue(Duration.wrap(0) != Duration.wrap(1)); + assertTrue(Duration.wrap(1) != Duration.wrap(0)); assertTrue(Duration.wrap(MAX_DURATION_VALUE / 2) != Duration.wrap(MAX_DURATION_VALUE)); assertTrue(Duration.wrap(0) != Duration.wrap(MAX_DURATION_VALUE)); } diff --git a/test/unit/types/PercentD16.t.sol b/test/unit/types/PercentD16.t.sol index c07558dd..a3b965e6 100644 --- a/test/unit/types/PercentD16.t.sol +++ b/test/unit/types/PercentD16.t.sol @@ -73,7 +73,7 @@ contract PercentD16UnitTests is UnitTest { function test_plus_HappyPath() external { assertEq(PercentsD16.from(0) + PercentsD16.from(0), PercentsD16.from(0)); assertEq(PercentsD16.from(0) + PercentsD16.from(1), PercentsD16.from(1)); - assertEq(PercentsD16.from(0) + PercentsD16.from(1), PercentsD16.from(1)); + assertEq(PercentsD16.from(1) + PercentsD16.from(0), PercentsD16.from(1)); assertEq(PercentsD16.from(500) + PercentsD16.from(20), PercentsD16.from(520)); assertEq(PercentsD16.from(0) + PercentsD16.from(MAX_PERCENT_D16), PercentsD16.from(MAX_PERCENT_D16)); assertEq(PercentsD16.from(MAX_PERCENT_D16) + PercentsD16.from(0), PercentsD16.from(MAX_PERCENT_D16)); diff --git a/test/unit/types/SharesValue.t.sol b/test/unit/types/SharesValue.t.sol index 3c7becfa..06d634b2 100644 --- a/test/unit/types/SharesValue.t.sol +++ b/test/unit/types/SharesValue.t.sol @@ -52,7 +52,6 @@ contract SharesValueTests is UnitTest { assertEq(SharesValues.from(0) + SharesValues.from(0), SharesValue.wrap(0)); assertEq(SharesValues.from(1) + SharesValues.from(0), SharesValue.wrap(1)); assertEq(SharesValues.from(0) + SharesValues.from(1), SharesValue.wrap(1)); - assertEq(SharesValues.from(0) + SharesValues.from(1), SharesValue.wrap(1)); assertEq( SharesValues.from(MAX_SHARES_VALUE / 2) + SharesValues.from(MAX_SHARES_VALUE / 2), SharesValue.wrap(type(uint128).max - 1)