diff --git a/src/Grants.sol b/src/Grants.sol index 2265898a..8ff4dffb 100644 --- a/src/Grants.sol +++ b/src/Grants.sol @@ -80,11 +80,7 @@ contract Grants { uint256 perPeriodAmount, address cancelAuthority ) external { - _mustBeNonZeroAddress(to); - _mustNotBeSelf(to); - _mustBeNonZero(period); - _mustBeNonZero(periodCount); - _mustBeEqualOrExceedMinAmount(perPeriodAmount); + validateVestingSchedule(to, period, periodCount, perPeriodAmount); token.safeTransferFrom(msg.sender, address(this), perPeriodAmount * periodCount); @@ -100,6 +96,24 @@ contract Grants { emit VestingScheduleAdded(to, schedule); } + /** + * @notice Checks if the vesting schedule parameters are valid. + * @param to Recipient of the tokens under the vesting schedule. + * @param period Duration of each period in seconds. + * @param periodCount Number of periods in the vesting schedule. + * @param perPeriodAmount Amount of tokens to be released each period. + */ + function validateVestingSchedule(address to, uint256 period, uint32 periodCount, uint256 perPeriodAmount) + public + view + { + _mustBeNonZeroAddress(to); + _mustNotBeSelf(to); + _mustBeNonZero(period); + _mustBeNonZero(periodCount); + _mustBeEqualOrExceedMinAmount(perPeriodAmount); + } + /** * @notice Claims all vested tokens available for msg.sender up to the current block timestamp. * @param start Start page of the vesting schedules to claim from. diff --git a/src/Rewards.sol b/src/Rewards.sol index 027495a0..af7cf2cd 100644 --- a/src/Rewards.sol +++ b/src/Rewards.sol @@ -228,7 +228,7 @@ contract Rewards is AccessControl, EIP712 { _mustBeExpectedSequence(reward.recipient, reward.sequence); _mustBeFromAuthorizedOracle(digestReward(reward), signature); - _checkedUpdateQuota(); + _checkedResetClaimed(); _checkedUpdateClaimed(reward.amount); // Safe to increment the sequence after checking this is the expected number (no overflow for the age of universe even with 1000 reward claims per second) @@ -251,7 +251,7 @@ contract Rewards is AccessControl, EIP712 { _mustBeFromAuthorizedOracle(digest, signature); - _checkedUpdateQuota(); + _checkedResetClaimed(); uint256 batchSum = _batchSum(batch); uint256 submitterRewardAmount = (batchSum * batchSubmitterRewardBasisPoints) / BASIS_POINTS_DIVISOR; @@ -284,11 +284,10 @@ contract Rewards is AccessControl, EIP712 { } /** - * @dev Internal function to update the rewards quota if the current block timestamp is greater than or equal to the quota renewal timestamp. * @notice This function resets the rewards claimed to 0 and updates the quota renewal timestamp based on the reward period. * @notice The following operations are safe based on the constructor's requirements for longer than the age of the universe. */ - function _checkedUpdateQuota() internal { + function _checkedResetClaimed() internal { if (block.timestamp >= quotaRenewalTimestamp) { claimed = 0; diff --git a/src/bridge/BridgeBase.sol b/src/bridge/BridgeBase.sol index 42b50b40..d7697395 100644 --- a/src/bridge/BridgeBase.sol +++ b/src/bridge/BridgeBase.sol @@ -56,15 +56,20 @@ abstract contract BridgeBase { /// @notice Error to indicate that the number of oracles exceeds the allowed maximum. error MaxOraclesExceeded(); + /// @notice Error to indicate that the number of oracles is less than the minimum required. + error NotEnoughOracles(); + + /// @notice Error to indicate an invalid value for the minimum votes. + error InvalidZeroForMinVotes(); + /// @notice Initializes the contract with specified parameters. /// @param bridgeOracles Array of oracle accounts. /// @param token Contract address of the NODL token. /// @param minVotes Minimum required votes to consider a proposal valid. /// @param minDelay Blocks to wait before a passed proposal can be executed. constructor(address[] memory bridgeOracles, NODL token, uint8 minVotes, uint256 minDelay) { - require(bridgeOracles.length >= minVotes, "Not enough oracles"); - require(minVotes > 0, "Votes must be more than zero"); - + _mustNotBeZeroMinVotes(minVotes); + _mustHaveEnoughOracles(bridgeOracles.length, minVotes); _mustNotExceedMaxOracles(bridgeOracles.length); for (uint256 i = 0; i < bridgeOracles.length; i++) { @@ -171,6 +176,12 @@ abstract contract BridgeBase { } } + function _mustHaveEnoughOracles(uint256 length, uint256 minOracles) internal pure { + if (length < minOracles) { + revert NotEnoughOracles(); + } + } + function _mustBeAnOracle(address maybeOracle) internal view { if (!isOracle[maybeOracle]) { revert NotAnOracle(maybeOracle); @@ -183,6 +194,12 @@ abstract contract BridgeBase { } } + function _mustNotBeZeroMinVotes(uint8 value) internal pure { + if (value == 0) { + revert InvalidZeroForMinVotes(); + } + } + function _mustNotHaveVotedYet(bytes32 proposal, address oracle) internal view { if (voted[oracle][proposal]) { revert AlreadyVoted(proposal, oracle); diff --git a/src/bridge/GrantsMigration.sol b/src/bridge/GrantsMigration.sol index 76f3f01d..37c07fde 100644 --- a/src/bridge/GrantsMigration.sol +++ b/src/bridge/GrantsMigration.sol @@ -7,6 +7,9 @@ import {NODL} from "../NODL.sol"; import {BridgeBase} from "./BridgeBase.sol"; contract GrantsMigration is BridgeBase { + /// @notice Maximum number of vesting schedules allowed per proposal. The 100 limit is coming from the Eden parachain. + uint8 public constant MAX_SCHEDULES = 100; + /// @dev Represents the vesting details of a proposal. struct Proposal { address target; // Address of the grant recipient. @@ -29,6 +32,21 @@ contract GrantsMigration is BridgeBase { // Events event Granted(bytes32 indexed proposal, address indexed user, uint256 amount, uint256 numOfSchedules); + /** + * @notice Error to indicate that the schedule array is empty. + */ + error EmptySchedules(); + + /** + * @notice Error to indicate that the schedule array is too large. + */ + error TooManySchedules(); + + /** + * @notice Error to indicate that the schedules do not add up to the total amount. + */ + error AmountMismatch(); + /** * @param bridgeOracles Array of addresses authorized to initiate and vote on proposals. * @param token Address of the NODL token used for grants. @@ -125,6 +143,22 @@ contract GrantsMigration is BridgeBase { uint256 amount, Grants.VestingSchedule[] memory schedules ) internal { + if (schedules.length == 0) { + revert EmptySchedules(); + } + if (schedules.length > MAX_SCHEDULES) { + revert TooManySchedules(); + } + uint256 totalAmount = 0; + for (uint256 i = 0; i < schedules.length; i++) { + grants.validateVestingSchedule( + user, schedules[i].period, schedules[i].periodCount, schedules[i].perPeriodAmount + ); + totalAmount += schedules[i].perPeriodAmount * schedules[i].periodCount; + } + if (totalAmount != amount) { + revert AmountMismatch(); + } proposals[proposal] = Proposal({target: user, amount: amount, schedules: schedules}); super._createVote(proposal, oracle, user, amount); } diff --git a/src/bridge/MigrationNFT.sol b/src/bridge/MigrationNFT.sol index 0140c5ab..b53cda2e 100644 --- a/src/bridge/MigrationNFT.sol +++ b/src/bridge/MigrationNFT.sol @@ -14,6 +14,10 @@ contract MigrationNFT is ERC721 { uint256[] public levels; string[] public levelToTokenURI; + error InvalidZeroHolders(); + + error InvalidZeroLevels(); + /** * @notice Mapping of token IDs to the levels they represent denominated from 1 (0 means token does not exists) */ @@ -49,20 +53,26 @@ contract MigrationNFT is ERC721 { uint256[] memory _levels, string[] memory _levelToTokenURI ) ERC721("OG ZK NODL", "OG_ZK_NODL") { - migration = _migration; - maxHolders = _maxHolders; - levels = _levels; - levelToTokenURI = _levelToTokenURI; - + if (_maxHolders == 0) { + revert InvalidZeroHolders(); + } + if (_levels.length == 0) { + revert InvalidZeroLevels(); + } if (_levels.length != _levelToTokenURI.length) { revert UnequalLengths(); } - for (uint256 i = 1; i < levels.length; i++) { - if (levels[i] <= levels[i - 1]) { + for (uint256 i = 1; i < _levels.length; i++) { + if (_levels[i] <= _levels[i - 1]) { revert UnsortedLevelsList(); } } + + migration = _migration; + maxHolders = _maxHolders; + levels = _levels; + levelToTokenURI = _levelToTokenURI; } /** diff --git a/test/bridge/GrantsMigration.t.sol b/test/bridge/GrantsMigration.t.sol index 8aa4f570..5ba8097b 100644 --- a/test/bridge/GrantsMigration.t.sol +++ b/test/bridge/GrantsMigration.t.sol @@ -187,7 +187,7 @@ contract GrantsMigrationTest is Test { function test_executionFailsIfInsufficientVotes() public { bytes32 paraTxHash = keccak256(abi.encodePacked("tx6")); vm.prank(oracles[0]); - migration.bridge(paraTxHash, user, 100, schedules); + migration.bridge(paraTxHash, user, amount, schedules); vm.roll(block.number + delay + 1); @@ -198,10 +198,10 @@ contract GrantsMigrationTest is Test { function test_executionFailsIfTooEarly() public { bytes32 paraTxHash = keccak256(abi.encodePacked("tx7")); vm.prank(oracles[0]); - migration.bridge(paraTxHash, user, 100, schedules); + migration.bridge(paraTxHash, user, amount, schedules); vm.prank(oracles[1]); - migration.bridge(paraTxHash, user, 100, schedules); + migration.bridge(paraTxHash, user, amount, schedules); vm.expectRevert(abi.encodeWithSelector(BridgeBase.NotYetWithdrawable.selector, paraTxHash)); migration.grant(paraTxHash); @@ -216,4 +216,46 @@ contract GrantsMigrationTest is Test { vm.expectRevert(abi.encodeWithSelector(BridgeBase.MaxOraclesExceeded.selector)); new GrantsMigration(manyOracles, nodl, grants, uint8(manyOracles.length / 2 + 1), delay); } + + function test_createProposalFailsWithEmptySchedules() public { + bytes32 paraTxHash = keccak256(abi.encodePacked("tx8")); + Grants.VestingSchedule[] memory emptySchedules; + vm.prank(oracles[0]); + vm.expectRevert(GrantsMigration.EmptySchedules.selector); + migration.bridge(paraTxHash, user, amount, emptySchedules); + } + + function test_createProposalFailIfTooManySchedules() public { + bytes32 paraTxHash = keccak256(abi.encodePacked("tx9")); + Grants.VestingSchedule[] memory manySchedules = new Grants.VestingSchedule[](migration.MAX_SCHEDULES() + 1); + for (uint256 i = 0; i < manySchedules.length; i++) { + manySchedules[i] = schedules[0]; + } + vm.prank(oracles[0]); + vm.expectRevert(GrantsMigration.TooManySchedules.selector); + migration.bridge(paraTxHash, user, amount, manySchedules); + } + + function test_createProposalFailsIfAmountMismatch() public { + bytes32 paraTxHash = keccak256(abi.encodePacked("tx10")); + vm.prank(oracles[0]); + vm.expectRevert(GrantsMigration.AmountMismatch.selector); + migration.bridge(paraTxHash, user, amount + 1, schedules); + } + + function test_createProposalFailsIfVestingScheduleInvalid() public { + bytes32 paraTxHash = keccak256(abi.encodePacked("tx11")); + Grants.VestingSchedule[] memory invalidSchedules = new Grants.VestingSchedule[](1); + invalidSchedules[0] = Grants.VestingSchedule({ + start: block.timestamp + 1 days, + period: 1 days, + periodCount: 5, + perPeriodAmount: 0, + cancelAuthority: user + }); + schedules[0].perPeriodAmount = 0; + vm.prank(oracles[0]); + vm.expectRevert(abi.encodeWithSelector(Grants.LowVestingAmount.selector)); + migration.bridge(paraTxHash, user, amount, invalidSchedules); + } } diff --git a/test/bridge/MigrationNFT.t.sol b/test/bridge/MigrationNFT.t.sol index 730fb2ac..710ef69a 100644 --- a/test/bridge/MigrationNFT.t.sol +++ b/test/bridge/MigrationNFT.t.sol @@ -61,6 +61,11 @@ contract MigrationNFTTest is Test { } } + function test_zeroHoldersWillRevert() public { + vm.expectRevert(MigrationNFT.InvalidZeroHolders.selector); + new MigrationNFT(migration, 0, levels, levelToTokenURI); + } + function test_enforceSorting() public { uint256[] memory unsortedLevels = new uint256[](5); unsortedLevels[0] = 1000;