From 8f0262810593cd8d3bda5998684f30b1ce566414 Mon Sep 17 00:00:00 2001 From: mejango Date: Mon, 6 Jan 2025 11:05:03 -0300 Subject: [PATCH 01/10] cantina/109 --- src/JBPermissions.sol | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/JBPermissions.sol b/src/JBPermissions.sol index 46f02fd0..e7f1fe6b 100644 --- a/src/JBPermissions.sol +++ b/src/JBPermissions.sol @@ -13,6 +13,7 @@ contract JBPermissions is IJBPermissions { // --------------------------- custom errors ------------------------- // //*********************************************************************// + error JBPermissions_CantSetRootPermissionForWildcardProject(); error JBPermissions_PermissionIdOutOfBounds(uint256 permissionId); error JBPermissions_Unauthorized(); @@ -225,7 +226,17 @@ contract JBPermissions is IJBPermissions { includeWildcardProjectId: true }) ) - ) revert JBPermissions_Unauthorized(); + ) { + revert JBPermissions_Unauthorized(); + } + + // ROOT permission cannot be set for a wildcard project ID. + if ( + permissionsData.projectId == WILDCARD_PROJECT_ID + && _includesPermission({permissions: packed, permissionId: JBPermissionIds.ROOT}) + ) { + revert JBPermissions_CantSetRootPermissionForWildcardProject(); + } // Store the new value. permissionsOf[permissionsData.operator][account][permissionsData.projectId] = packed; From 374dc8e09ff7d3fcd95408021fb3a64a01de830c Mon Sep 17 00:00:00 2001 From: mejango Date: Mon, 6 Jan 2025 11:05:41 -0300 Subject: [PATCH 02/10] spacing --- src/JBPermissions.sol | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/JBPermissions.sol b/src/JBPermissions.sol index e7f1fe6b..1a30b68a 100644 --- a/src/JBPermissions.sol +++ b/src/JBPermissions.sol @@ -226,17 +226,13 @@ contract JBPermissions is IJBPermissions { includeWildcardProjectId: true }) ) - ) { - revert JBPermissions_Unauthorized(); - } + ) revert JBPermissions_Unauthorized(); // ROOT permission cannot be set for a wildcard project ID. if ( permissionsData.projectId == WILDCARD_PROJECT_ID && _includesPermission({permissions: packed, permissionId: JBPermissionIds.ROOT}) - ) { - revert JBPermissions_CantSetRootPermissionForWildcardProject(); - } + ) revert JBPermissions_CantSetRootPermissionForWildcardProject(); // Store the new value. permissionsOf[permissionsData.operator][account][permissionsData.projectId] = packed; From 3bba08eb77eb79c740166f0d6c2dafe168122aea Mon Sep 17 00:00:00 2001 From: mejango Date: Mon, 6 Jan 2025 12:23:38 -0300 Subject: [PATCH 03/10] cantina/63 --- src/JBPermissions.sol | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/JBPermissions.sol b/src/JBPermissions.sol index 46f02fd0..5b7364d0 100644 --- a/src/JBPermissions.sol +++ b/src/JBPermissions.sol @@ -13,6 +13,7 @@ contract JBPermissions is IJBPermissions { // --------------------------- custom errors ------------------------- // //*********************************************************************// + error JBPermissions_NoZeroPermission(); error JBPermissions_PermissionIdOutOfBounds(uint256 permissionId); error JBPermissions_Unauthorized(); @@ -210,6 +211,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 ( From 13f5ac0dd2b73518beba60c29f2235547191551c Mon Sep 17 00:00:00 2001 From: simplemachine92 Date: Tue, 7 Jan 2025 17:36:57 -0600 Subject: [PATCH 04/10] wildcard tests --- test/TestPermissions.sol | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/TestPermissions.sol b/test/TestPermissions.sol index 1033bd85..4987059a 100644 --- a/test/TestPermissions.sol +++ b/test/TestPermissions.sol @@ -158,12 +158,17 @@ contract TestPermissions_Local is TestBaseWorkflow, JBTest { // Check if all the items in `check_permissions` also exist in `set_permissions`. bool _shouldHavePermissions = true; + bool _containsRoot; 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]; + + // Update if we find root value. + if (_u8_set_permissions[_j] == 1) _containsRoot = true; + // If we find this item we break and mark the flag. if (_u8_check_permissions[_i] == _u8_set_permissions[_j]) { _exists = true; @@ -178,6 +183,10 @@ contract TestPermissions_Local is TestBaseWorkflow, JBTest { } } + if (_containsRoot && _projectId == 0) { + vm.expectRevert(JBPermissions.JBPermissions_CantSetRootPermissionForWildcardProject.selector); + } + // Set the permissions. vm.prank(_account); _permissions.setPermissionsFor( @@ -190,6 +199,20 @@ contract TestPermissions_Local is TestBaseWorkflow, JBTest { ); } + function testSetRootWildcardProjectId(address _account, address _operator) public { + uint8[] memory _set_permissions = new uint8[](1); + _set_permissions[0] = JBPermissionIds.ROOT; + + // Set the permissions. + vm.prank(_account); + + vm.expectRevert(JBPermissions.JBPermissions_CantSetRootPermissionForWildcardProject.selector); + _permissions.setPermissionsFor( + _account, + JBPermissionsData({operator: _operator, projectId: 0, /* wildcard */ permissionIds: _set_permissions}) + ); + } + function testBasicAccessSetup() public { address zeroOwner = makeAddr("zeroOwner"); address token = address(usdcToken()); From 541d5cac16202cfb0c4e1b4338409450463d4050 Mon Sep 17 00:00:00 2001 From: simplemachine92 Date: Wed, 8 Jan 2025 16:55:37 -0600 Subject: [PATCH 05/10] testPermissions update --- test/TestPermissions.sol | 86 ++++++++++++---------------------------- 1 file changed, 25 insertions(+), 61 deletions(-) diff --git a/test/TestPermissions.sol b/test/TestPermissions.sol index 4987059a..f14af947 100644 --- a/test/TestPermissions.sol +++ b/test/TestPermissions.sol @@ -122,81 +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, + address _op, uint56 _projectId, - uint8[] memory _u8_check_permissions, - uint8[] memory _u8_set_permissions + 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; - bool _containsRoot; - 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]; - - // Update if we find root value. - if (_u8_set_permissions[_j] == 1) _containsRoot = true; - - // 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); - if (_containsRoot && _projectId == 0) { - vm.expectRevert(JBPermissions.JBPermissions_CantSetRootPermissionForWildcardProject.selector); + 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 testSetRootWildcardProjectId(address _account, address _operator) public { From b113995497b177d2fca6199da330b69c860423d3 Mon Sep 17 00:00:00 2001 From: mejango Date: Thu, 9 Jan 2025 19:43:14 -0300 Subject: [PATCH 06/10] rm --- src/JBPermissions.sol | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/JBPermissions.sol b/src/JBPermissions.sol index 74087cae..aa8efbe6 100644 --- a/src/JBPermissions.sol +++ b/src/JBPermissions.sol @@ -232,12 +232,6 @@ contract JBPermissions is IJBPermissions { ) ) revert JBPermissions_Unauthorized(); - // ROOT permission cannot be set for a wildcard project ID. - if ( - permissionsData.projectId == WILDCARD_PROJECT_ID - && _includesPermission({permissions: packed, permissionId: JBPermissionIds.ROOT}) - ) revert JBPermissions_CantSetRootPermissionForWildcardProject(); - // Store the new value. permissionsOf[permissionsData.operator][account][permissionsData.projectId] = packed; From e9d842da54b01c6193207908a9c0256aa34bed3a Mon Sep 17 00:00:00 2001 From: mejango Date: Mon, 13 Jan 2025 12:40:20 -0300 Subject: [PATCH 07/10] run --- test/units/static/JBMultiTerminal/TestSendPayoutsOf.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/units/static/JBMultiTerminal/TestSendPayoutsOf.sol b/test/units/static/JBMultiTerminal/TestSendPayoutsOf.sol index 8837456b..9ab6d6e3 100644 --- a/test/units/static/JBMultiTerminal/TestSendPayoutsOf.sol +++ b/test/units/static/JBMultiTerminal/TestSendPayoutsOf.sol @@ -170,8 +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({ From 7c0c84897cf813eff2106caa54c6756e56211bfb Mon Sep 17 00:00:00 2001 From: mejango Date: Mon, 13 Jan 2025 12:42:45 -0300 Subject: [PATCH 08/10] fmt --- test/units/static/JBMultiTerminal/TestSendPayoutsOf.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/test/units/static/JBMultiTerminal/TestSendPayoutsOf.sol b/test/units/static/JBMultiTerminal/TestSendPayoutsOf.sol index 9ab6d6e3..28204fe9 100644 --- a/test/units/static/JBMultiTerminal/TestSendPayoutsOf.sol +++ b/test/units/static/JBMultiTerminal/TestSendPayoutsOf.sol @@ -172,7 +172,6 @@ contract TestSendPayoutsOf_Local is JBMultiTerminalSetup { // it will revert UNDER_MIN_TOKENS_PAID_OUT function test_WhenPayoutFailsDoNotTakeFee() external { - // needed for terminal store mock call JBRuleset memory returnedRuleset = JBRuleset({ cycleNumber: 1, From a8a39a8a58c04d7603b313fd08dcb8764020453a Mon Sep 17 00:00:00 2001 From: mejango Date: Mon, 13 Jan 2025 13:50:58 -0300 Subject: [PATCH 09/10] ci test pass --- lib/forge-std | 2 +- test/units/static/JBMultiTerminal/TestSendPayoutsOf.sol | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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/test/units/static/JBMultiTerminal/TestSendPayoutsOf.sol b/test/units/static/JBMultiTerminal/TestSendPayoutsOf.sol index 28204fe9..3705ba5f 100644 --- a/test/units/static/JBMultiTerminal/TestSendPayoutsOf.sol +++ b/test/units/static/JBMultiTerminal/TestSendPayoutsOf.sol @@ -234,7 +234,7 @@ contract TestSendPayoutsOf_Local is JBMultiTerminalSetup { address(0), 97, // Amount that would have been transferred after fee. 3, // fee amount - bytes(hex"9996b3150000000000000000000000000000000000000000000000000000000000000000"), + bytes(hex"5274afe70000000000000000000000000000000000000000000000000000000000000000"), address(this) ); From de0e5ebf31a4e709f0b17e7ec34936938f7efc6f Mon Sep 17 00:00:00 2001 From: mejango Date: Mon, 13 Jan 2025 13:56:58 -0300 Subject: [PATCH 10/10] ci test pass --- .../static/JBController/TestSendReservedTokensToSplitsOf.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/units/static/JBController/TestSendReservedTokensToSplitsOf.sol b/test/units/static/JBController/TestSendReservedTokensToSplitsOf.sol index 24d4c3c4..c6244d28 100644 --- a/test/units/static/JBController/TestSendReservedTokensToSplitsOf.sol +++ b/test/units/static/JBController/TestSendReservedTokensToSplitsOf.sol @@ -597,7 +597,7 @@ contract TestSendReservedTokensToSplitsOf_Local is JBControllerSetup { abi.encode(address(_token)) ); - vm.expectRevert(abi.encodeWithSignature(("AddressEmptyCode(address)"), address(_token))); + vm.expectRevert(abi.encodeWithSignature(("SafeERC20FailedOperation(address)"), address(_token))); _controller.sendReservedTokensToSplitsOf(_projectId); } }