Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add modifier to EntitlementChecker to only allow registered operator to be checkers #80

Merged
merged 13 commits into from
Jun 5, 2024
Merged

add modifier to EntitlementChecker to only allow registered operator to be checkers #80

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
552d136
add modifier
giuseppecrj May 28, 2024
a58d5eb
setup entitlement checker to check for registered operators
giuseppecrj May 31, 2024
83577e9
fix nodeOperator rename mistake
giuseppecrj Jun 3, 2024
353ddb0
register xchain with operator key from contracts env
clemire Jun 3, 2024
48d35ce
Clip 0x
clemire Jun 3, 2024
241705e
Merge branch 'main' into g/gate-entitlementchecker-register
giuseppecrj Jun 4, 2024
c0da3d1
Revert "Clip 0x"
giuseppecrj Jun 4, 2024
aa9a0c0
Revert "register xchain with operator key from contracts env"
giuseppecrj Jun 4, 2024
7695e7f
xchain: register node before starting
bas-vk Jun 5, 2024
9c2e4da
Merge branch 'main' into g/gate-entitlementchecker-register
giuseppecrj Jun 3, 2024
38bc3e6
Merge branch 'main' into g/gate-entitlementchecker-register
giuseppecrj Jun 5, 2024
9be5d1a
fix launch conditions
texuf Jun 5, 2024
69cbeca
Merge branch 'main' into g/gate-entitlementchecker-register
giuseppecrj Jun 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions contracts/src/base/registry/facets/checker/EntitlementChecker.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ contract EntitlementChecker is IEntitlementChecker, Facet {
* @param node The address of the node to register
* @dev Only valid operators can register a node
*/
function registerNode(address node) external {
function registerNode(address node) external onlyRegisteredOperator {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll need to change files like core/node/crypto/chain_txpool_test.go to work with this modifier. Specifically lines like:

return tc.NodeRegistry.RegisterNode(opts, nodeWallet.Address, url, 2)

need to use the operator wallet instead of the deployer wallet:

txPool, err := crypto.NewTransactionPoolWithPolicies(
ctx, tc.Client(), tc.DeployerBlockchain.Wallet, resubmitPolicy, repricePolicy, tc.ChainMonitor)
require.NoError(err, "unable to construct transaction pool")

EntitlementCheckerStorage.Layout storage layout = EntitlementCheckerStorage
.layout();

Expand All @@ -71,7 +71,9 @@ contract EntitlementChecker is IEntitlementChecker, Facet {
* @param node The address of the node to unregister
* @dev Only the operator of the node can unregister it
*/
function unregisterNode(address node) external {
function unregisterNode(
address node
) external onlyNodeOperator(node, msg.sender) {
EntitlementCheckerStorage.Layout storage layout = EntitlementCheckerStorage
.layout();

Expand Down
103 changes: 62 additions & 41 deletions contracts/test/base/registry/NodeOperator.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {IOwnableBase} from "contracts/src/diamond/facets/ownable/IERC173.sol";
import {IERC721ABase} from "contracts/src/diamond/facets/token/ERC721A/IERC721A.sol";
import {INodeOperator, INodeOperatorBase} from "contracts/src/base/registry/facets/operator/INodeOperator.sol";
import {ISpaceDelegationBase} from "contracts/src/base/registry/facets/delegation/ISpaceDelegation.sol";
import {INodeOperatorBase} from "contracts/src/base/registry/facets/operator/INodeOperator.sol";

// libraries

Expand Down Expand Up @@ -45,7 +44,6 @@ contract NodeOperatorFacetTest is
introspection = IntrospectionFacet(address(baseRegistry));
erc721 = ERC721A(address(baseRegistry));
riverFacet = River(riverToken);
operator = INodeOperator(address(baseRegistry));
}

function test_initialization() public {
Expand All @@ -59,11 +57,13 @@ contract NodeOperatorFacetTest is
// =============================================================
modifier givenOperatorIsRegistered(address _operator) {
vm.assume(address(_operator) != address(0));
vm.assume(!nodeOperator.isOperator(_operator));

vm.expectEmit();
emit OperatorRegistered(_operator);
emit OperatorRegistered(_operator);
vm.prank(_operator);
operator.registerOperator(_operator);
nodeOperator.registerOperator(_operator);
_;
}

Expand All @@ -72,14 +72,15 @@ contract NodeOperatorFacetTest is
) public givenOperatorIsRegistered(randomOperator) {
vm.expectRevert(NodeOperator__AlreadyRegistered.selector);
vm.prank(randomOperator);
operator.registerOperator(randomOperator);
nodeOperator.registerOperator(randomOperator);
}

function test_registerOperatorWithValidAddress(
address randomOperator
) public givenOperatorIsRegistered(randomOperator) {
assertTrue(
operator.getOperatorStatus(randomOperator) == NodeOperatorStatus.Standby
nodeOperator.getOperatorStatus(randomOperator) ==
NodeOperatorStatus.Standby
);
}

Expand All @@ -89,13 +90,15 @@ contract NodeOperatorFacetTest is
function test_revertWhen_isOperatorWithInvalidOperator(
address randomOperator
) external {
assertFalse(operator.isOperator(randomOperator));
vm.assume(randomOperator != address(0));
vm.assume(nodeOperator.isOperator(randomOperator) == false);
assertFalse(nodeOperator.isOperator(randomOperator));
}

function test_isOperatorWithValidOperator(
address randomOperator
) public givenOperatorIsRegistered(randomOperator) {
assertTrue(operator.isOperator(randomOperator));
assertTrue(nodeOperator.isOperator(randomOperator));
}

// =============================================================
Expand All @@ -111,7 +114,7 @@ contract NodeOperatorFacetTest is
vm.expectRevert(
abi.encodeWithSelector(Ownable__NotOwner.selector, randomOwner)
);
operator.setOperatorStatus(randomOperator, NodeOperatorStatus.Approved);
nodeOperator.setOperatorStatus(randomOperator, NodeOperatorStatus.Approved);
}

modifier whenCalledByDeployer() {
Expand All @@ -124,15 +127,15 @@ contract NodeOperatorFacetTest is
whenCalledByDeployer
{
vm.expectRevert(NodeOperator__InvalidAddress.selector);
operator.setOperatorStatus(address(0), NodeOperatorStatus.Approved);
nodeOperator.setOperatorStatus(address(0), NodeOperatorStatus.Approved);
}

function test_revert_setOperatorStatus_withNotRegistered(
address notRegisteredOperator
) public whenCalledByDeployer {
vm.assume(notRegisteredOperator != address(0));
vm.expectRevert(NodeOperator__NotRegistered.selector);
operator.setOperatorStatus(
nodeOperator.setOperatorStatus(
notRegisteredOperator,
NodeOperatorStatus.Approved
);
Expand All @@ -142,21 +145,21 @@ contract NodeOperatorFacetTest is
address randomOperator
) public givenOperatorIsRegistered(randomOperator) whenCalledByDeployer {
vm.expectRevert(NodeOperator__StatusNotChanged.selector);
operator.setOperatorStatus(randomOperator, NodeOperatorStatus.Standby);
nodeOperator.setOperatorStatus(randomOperator, NodeOperatorStatus.Standby);
}

function test_revertWhen_setOperatorStatusFromStandbyToExiting(
address randomOperator
) public givenOperatorIsRegistered(randomOperator) whenCalledByDeployer {
vm.expectRevert(NodeOperator__InvalidStatusTransition.selector);
operator.setOperatorStatus(randomOperator, NodeOperatorStatus.Exiting);
nodeOperator.setOperatorStatus(randomOperator, NodeOperatorStatus.Exiting);
}

// function test_revertWhen_setOperatorStatusFromStandbyToApprovedWithNoStake(
// address randomOperator
// ) public givenOperatorIsRegistered(randomOperator) whenCalledByDeployer {
// vm.expectRevert(NodeOperator__NotEnoughStake.selector);
// nodeOperator.setOperatorStatus(randomOperator, NodeOperatorStatus.Approved);
// nodenodeOperator.setOperatorStatus(randomOperator, NodeOperatorStatus.Approved);
// }

modifier whenSetOperatorStatusIsCalledByTheOwner(
Expand All @@ -166,7 +169,7 @@ contract NodeOperatorFacetTest is
vm.prank(deployer);
vm.expectEmit();
emit OperatorStatusChanged(_operator, _newStatus);
operator.setOperatorStatus(_operator, _newStatus);
nodeOperator.setOperatorStatus(_operator, _newStatus);
_;
}

Expand All @@ -192,7 +195,7 @@ contract NodeOperatorFacetTest is
vm.assume(_operator != address(0));
vm.assume(_operator != _claimAddress);
vm.prank(_operator);
operator.setClaimAddressForOperator(_claimAddress, _operator);
nodeOperator.setClaimAddressForOperator(_claimAddress, _operator);
_;
}

Expand All @@ -213,7 +216,7 @@ contract NodeOperatorFacetTest is
// stakeRequirement
// );
// assertTrue(
// nodeOperator.getOperatorStatus(randomOperator) == NodeOperatorStatus.Approved
// nodenodeOperator.getOperatorStatus(randomOperator) == NodeOperatorStatus.Approved
// );
// }

Expand All @@ -232,7 +235,8 @@ contract NodeOperatorFacetTest is
)
{
assertTrue(
operator.getOperatorStatus(randomOperator) == NodeOperatorStatus.Approved
nodeOperator.getOperatorStatus(randomOperator) ==
NodeOperatorStatus.Approved
);
}

Expand All @@ -252,7 +256,7 @@ contract NodeOperatorFacetTest is
{
vm.prank(deployer);
vm.expectRevert(NodeOperator__InvalidStatusTransition.selector);
operator.setOperatorStatus(randomOperator, NodeOperatorStatus.Standby);
nodeOperator.setOperatorStatus(randomOperator, NodeOperatorStatus.Standby);
}

function test_revertWhen_setOperatorStatusIsCalledFromExitingToApproved(
Expand All @@ -275,7 +279,7 @@ contract NodeOperatorFacetTest is
{
vm.prank(deployer);
vm.expectRevert(NodeOperator__InvalidStatusTransition.selector);
operator.setOperatorStatus(randomOperator, NodeOperatorStatus.Approved);
nodeOperator.setOperatorStatus(randomOperator, NodeOperatorStatus.Approved);
}

function test_setOperatorStatus_toExiting(
Expand All @@ -297,7 +301,8 @@ contract NodeOperatorFacetTest is
)
{
assertTrue(
operator.getOperatorStatus(randomOperator) == NodeOperatorStatus.Exiting
nodeOperator.getOperatorStatus(randomOperator) ==
NodeOperatorStatus.Exiting
);

// assertEq(totalApprovedOperators, 0);
Expand All @@ -310,16 +315,19 @@ contract NodeOperatorFacetTest is
function test_getOperatorStatus_operatorNotRegistered(
address randomOperator
) public {
vm.assume(!nodeOperator.isOperator(randomOperator));
assertTrue(
operator.getOperatorStatus(randomOperator) == NodeOperatorStatus.Exiting
nodeOperator.getOperatorStatus(randomOperator) ==
NodeOperatorStatus.Exiting
);
}

function test_getOperatorStatus_registeredOperator(
address randomOperator
) public givenOperatorIsRegistered(randomOperator) {
assertTrue(
operator.getOperatorStatus(randomOperator) == NodeOperatorStatus.Standby
nodeOperator.getOperatorStatus(randomOperator) ==
NodeOperatorStatus.Standby
);
}

Expand All @@ -338,7 +346,8 @@ contract NodeOperatorFacetTest is
)
{
assertTrue(
operator.getOperatorStatus(randomOperator) == NodeOperatorStatus.Approved
nodeOperator.getOperatorStatus(randomOperator) ==
NodeOperatorStatus.Approved
);
}

Expand All @@ -361,7 +370,8 @@ contract NodeOperatorFacetTest is
)
{
assertTrue(
operator.getOperatorStatus(randomOperator) == NodeOperatorStatus.Exiting
nodeOperator.getOperatorStatus(randomOperator) ==
NodeOperatorStatus.Exiting
);
}

Expand All @@ -375,7 +385,7 @@ contract NodeOperatorFacetTest is
) public {
vm.expectRevert(NodeOperator__NotClaimer.selector);
vm.prank(randomClaimer);
operator.setClaimAddressForOperator(randomClaimer, randomOperator);
nodeOperator.setClaimAddressForOperator(randomClaimer, randomOperator);
}

function test_setClaimAddress(
Expand All @@ -387,7 +397,7 @@ contract NodeOperatorFacetTest is
givenOperatorHasSetClaimAddress(randomOperator, randomClaimer)
{
assertEq(
operator.getClaimAddressForOperator(randomOperator),
nodeOperator.getClaimAddressForOperator(randomOperator),
randomClaimer
);
}
Expand All @@ -399,22 +409,27 @@ contract NodeOperatorFacetTest is
address randomOperator,
uint256 rate
) external givenOperatorIsRegistered(randomOperator) {
vm.assume(rate > 0 && rate < 100);

vm.prank(randomOperator);
vm.expectEmit(address(operator));
vm.assume(rate <= 100);
vm.expectEmit(address(nodeOperator));
emit OperatorCommissionChanged(randomOperator, rate);
operator.setCommissionRate(rate);
nodeOperator.setCommissionRate(rate);

assertEq(operator.getCommissionRate(randomOperator), rate);
assertEq(nodeOperator.getCommissionRate(randomOperator), rate);
}

function test_revertWhen_setCommissionRateIsCalledByInvalidOperator(
address randomOperator,
uint256 rate
) external {
vm.assume(randomOperator != address(0));
vm.assume(rate > 0 && rate < 100);
vm.assume(!nodeOperator.isOperator(randomOperator));

vm.expectRevert(NodeOperator__NotRegistered.selector);
vm.prank(randomOperator);
operator.setCommissionRate(rate);
nodeOperator.setCommissionRate(rate);
}

// =============================================================
Expand All @@ -424,14 +439,14 @@ contract NodeOperatorFacetTest is
// address randomOperator
// ) public givenOperatorIsRegistered(randomOperator) {
// vm.expectRevert(NodeOperator__InvalidAddress.selector);
// nodeOperator.addSpaceDelegation(address(0), randomOperator);
// nodenodeOperator.addSpaceDelegation(address(0), randomOperator);
giuseppecrj marked this conversation as resolved.
Show resolved Hide resolved
// }

// function test_revertWhen_addSpaceDelegationIsCalledWithZeroOperatorAddress()
// public
// {
// vm.expectRevert(NodeOperator__InvalidAddress.selector);
// nodeOperator.addSpaceDelegation(space, address(0));
// nodenodeOperator.addSpaceDelegation(space, address(0));
// }

// function test_revertWhen_addSpaceDelegationIsCalledByInvalidSpaceOwner(
Expand All @@ -442,22 +457,22 @@ contract NodeOperatorFacetTest is

// vm.prank(randomUser);
// vm.expectRevert(NodeOperator__InvalidSpace.selector);
// nodeOperator.addSpaceDelegation(space, randomOperator);
// nodenodeOperator.addSpaceDelegation(space, randomOperator);
// }

// function test_revertWhen_addSpaceDelegationIsCalledWithInvalidOperator(
// address randomOperator
// ) public {
// vm.assume(randomOperator != address(0));
// vm.expectRevert(NodeOperator__NotRegistered.selector);
// nodeOperator.addSpaceDelegation(space, randomOperator);
// nodenodeOperator.addSpaceDelegation(space, randomOperator);
// }

// modifier givenSpaceHasDelegatedToOperator(address _operator) {
// vm.prank(founder);
// vm.expectEmit();
// emit SpaceDelegatedToOperator(space, _operator);
// nodeOperator.addSpaceDelegation(space, _operator);
// nodenodeOperator.addSpaceDelegation(space, _operator);
// _;
// }

Expand All @@ -475,7 +490,7 @@ contract NodeOperatorFacetTest is
// randomOperator
// )
// );
// nodeOperator.addSpaceDelegation(space, randomOperator);
// nodenodeOperator.addSpaceDelegation(space, randomOperator);
// }

// function test_addSpaceDelegation(
Expand All @@ -485,7 +500,7 @@ contract NodeOperatorFacetTest is
// givenOperatorIsRegistered(randomOperator)
// givenSpaceHasDelegatedToOperator(randomOperator)
// {
// assertEq(nodeOperator.getSpaceDelegation(space), randomOperator);
// assertEq(nodenodeOperator.getSpaceDelegation(space), randomOperator);
// }

// =============================================================
Expand All @@ -502,11 +517,17 @@ contract NodeOperatorFacetTest is
}

function test_revertWhen_transferIsCalledNotRegistered(
address notRegisteredOperator
address notRegisteredOperator,
address someAddress
) public {
vm.assume(notRegisteredOperator != address(0));
vm.assume(erc721.balanceOf(notRegisteredOperator) == 0);

uint256 tokenId = erc721.totalSupply() + 1;

vm.prank(notRegisteredOperator);
vm.expectRevert(OwnerQueryForNonexistentToken.selector);
erc721.transferFrom(notRegisteredOperator, _randomAddress(), 0);
erc721.transferFrom(notRegisteredOperator, someAddress, tokenId);
}

// =============================================================
Expand All @@ -523,7 +544,7 @@ contract NodeOperatorFacetTest is
for (uint256 i = 0; i < totalOperators; i++) {
address operatorAddress = erc721.ownerOf(i);

NodeOperatorStatus currentStatus = operator.getOperatorStatus(
NodeOperatorStatus currentStatus = nodeOperator.getOperatorStatus(
operatorAddress
);

Expand Down
Loading
Loading