diff --git a/lib/forge-std b/lib/forge-std index 83c5d212..726a6ee5 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit 83c5d212a01f8950727da4095cdfe2654baccb5b +Subproject commit 726a6ee5fc8427a0013d6f624e486c9130c0e336 diff --git a/src/JBPermissions.sol b/src/JBPermissions.sol index 46f02fd0..aa8efbe6 100644 --- a/src/JBPermissions.sol +++ b/src/JBPermissions.sol @@ -13,6 +13,8 @@ contract JBPermissions is IJBPermissions { // --------------------------- custom errors ------------------------- // //*********************************************************************// + error JBPermissions_CantSetRootPermissionForWildcardProject(); + error JBPermissions_NoZeroPermission(); error JBPermissions_PermissionIdOutOfBounds(uint256 permissionId); error JBPermissions_Unauthorized(); @@ -210,6 +212,9 @@ contract JBPermissions is IJBPermissions { // Pack the permission IDs into a uint256. uint256 packed = _packedPermissions(permissionsData.permissionIds); + // Make sure the 0 permission is not set. + if (_includesPermission({permissions: packed, permissionId: 0})) revert JBPermissions_NoZeroPermission(); + // Enforce permissions. ROOT operators are allowed to set permissions so long as they are not setting another // ROOT permission. if ( diff --git a/test/TestPermissions.sol b/test/TestPermissions.sol index 8081506b..eae36693 100644 --- a/test/TestPermissions.sol +++ b/test/TestPermissions.sol @@ -122,72 +122,45 @@ contract TestPermissions_Local is TestBaseWorkflow, JBTest { } } - function testSetOperators() public { - // Pack up our permission data. - JBPermissionsData[] memory permData = new JBPermissionsData[](1); - - uint8[] memory permIds = new uint8[](256); - - // Push an index higher than 255. - for (uint256 i; i < 256; i++) { - permIds[i] = uint8(i); - - permData[0] = JBPermissionsData({operator: address(0), projectId: _projectOne, permissionIds: permIds}); - - // Set em. - vm.prank(_projectOwner); - _permissions.setPermissionsFor(_projectOwner, permData[0]); - - // Verify. - bool _check = _permissions.hasPermission(address(0), _projectOwner, _projectOne, permIds[i], true, true); - assertEq(_check, true); - } - } - function testHasPermissions( address _account, - address _operator, - uint64 _projectId, - uint8[] memory _u8_check_permissions, - uint8[] memory _u8_set_permissions + address _op, + uint56 _projectId, + uint8 _checkedId, + uint8[] memory _set_permissions ) public { - uint256[] memory _check_permissions = new uint256[](_u8_check_permissions.length); - uint8[] memory _set_permissions = new uint8[](_u8_set_permissions.length); - - // Check if all the items in `check_permissions` also exist in `set_permissions`. - bool _shouldHavePermissions = true; - for (uint256 _i; _i < _u8_check_permissions.length; _i++) { - bool _exists; - _check_permissions[_i] = _u8_check_permissions[_i]; - for (uint256 _j; _j < _u8_set_permissions.length; _j++) { - // We update this lots of times unnecesarily but no need to optimize this. - _set_permissions[_j] = _u8_set_permissions[_j % 256]; - // If we find this item we break and mark the flag. - if (_u8_check_permissions[_i] == _u8_set_permissions[_j]) { - _exists = true; - break; - } - } - - // If any item does not exist we should not have permission. - if (_exists == false) { - _shouldHavePermissions = false; - break; - } + vm.assume(_projectId != 0); + vm.assume(_set_permissions.length != 0); + + uint256[] memory _check_permissions = new uint256[](1); + _check_permissions[0] = _checkedId; + + bool hasPerm; + bool hasZero; + for (uint256 i; i < _set_permissions.length; i++) { + // Flag if index is zero, meaning set call should revert later. + if (_set_permissions[i] == 0) hasZero = true; + + // Flag if hasPermissions check will be true if the array contains it or has root ID. + if (_set_permissions[i] == _checkedId) hasPerm = true; } + // If setting with a zero permission id, will revert. + if (hasZero) vm.expectRevert(JBPermissions.JBPermissions_NoZeroPermission.selector); + // Set the permissions. vm.prank(_account); _permissions.setPermissionsFor( - _account, JBPermissionsData({operator: _operator, projectId: _projectId, permissionIds: _set_permissions}) + _account, JBPermissionsData({operator: _op, projectId: _projectId, permissionIds: _set_permissions}) ); - assertEq( - _permissions.hasPermissions(_operator, _account, _projectId, _check_permissions, false, false), - _shouldHavePermissions - ); + // Check permissions if the call didn't revert. + if (!hasZero) { + // Call has permissions and assert. + assertEq(_permissions.hasPermissions(_op, _account, _projectId, _check_permissions, false, false), hasPerm); + } } function testBasicAccessSetup() public { diff --git a/test/units/static/JBMultiTerminal/TestSendPayoutsOf.sol b/test/units/static/JBMultiTerminal/TestSendPayoutsOf.sol index 61f02152..3705ba5f 100644 --- a/test/units/static/JBMultiTerminal/TestSendPayoutsOf.sol +++ b/test/units/static/JBMultiTerminal/TestSendPayoutsOf.sol @@ -170,9 +170,8 @@ contract TestSendPayoutsOf_Local is JBMultiTerminalSetup { _terminal.sendPayoutsOf(_projectId, address(0), 0, 0, 0); } + // it will revert UNDER_MIN_TOKENS_PAID_OUT function test_WhenPayoutFailsDoNotTakeFee() external { - // it will revert UNDER_MIN_TOKENS_PAID_OUT - // needed for terminal store mock call JBRuleset memory returnedRuleset = JBRuleset({ cycleNumber: 1,