Skip to content

Commit

Permalink
standardize updateIsApiEnabled API
Browse files Browse the repository at this point in the history
  • Loading branch information
fbac committed Feb 25, 2025
1 parent c9c8544 commit d3e5be4
Show file tree
Hide file tree
Showing 11 changed files with 46 additions and 37 deletions.
4 changes: 4 additions & 0 deletions cmd/cli/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ func updateActive(logger *zap.Logger, options *CLI) {
logger.Info(
"successfully updated node active",
zap.Uint32("node-id", uint32(options.UpdateActive.NodeId)),
zap.Bool("is-active", options.UpdateActive.IsActive),
)
}

Expand Down Expand Up @@ -252,13 +253,15 @@ func updateApiEnabled(logger *zap.Logger, options *CLI) {
err = registryAdmin.UpdateIsApiEnabled(
ctx,
uint32(options.UpdateApiEnabled.NodeId),
options.UpdateApiEnabled.IsApiEnabled,
)
if err != nil {
logger.Fatal("could not update node api enabled", zap.Error(err))
}
logger.Info(
"successfully updated node api enabled",
zap.Uint32("node-id", uint32(options.UpdateApiEnabled.NodeId)),
zap.Bool("is-api-enabled", options.UpdateApiEnabled.IsApiEnabled),
)
}

Expand Down Expand Up @@ -299,6 +302,7 @@ func updateReplicationEnabled(logger *zap.Logger, options *CLI) {
logger.Info(
"successfully updated node replication enabled",
zap.Uint32("node-id", uint32(options.UpdateReplicationEnabled.NodeId)),
zap.Bool("is-replication-enabled", options.UpdateReplicationEnabled.IsReplicationEnabled),
)
}

Expand Down
28 changes: 14 additions & 14 deletions contracts/pkg/nodes/Nodes.go

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions contracts/src/Nodes.sol
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,11 @@ contract Nodes is AccessControlDefaultAdminRules, ERC721, INodes {
}

/// @inheritdoc INodes
function updateIsApiEnabled(uint256 nodeId) external {
function updateIsApiEnabled(uint256 nodeId, bool isApiEnabled) external {
require(_ownerOf(nodeId) == msg.sender, Unauthorized());
_nodes[nodeId].isApiEnabled = !_nodes[nodeId].isApiEnabled;
_nodes[nodeId].isApiEnabled = isApiEnabled;

if (!_nodes[nodeId].isApiEnabled && _nodes[nodeId].isActive) {
if (!isApiEnabled && _nodes[nodeId].isActive) {
_deactivateNode(nodeId);
}

Expand Down
3 changes: 2 additions & 1 deletion contracts/src/interfaces/INodes.sol
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,8 @@ interface INodes is IERC721 {
/// @notice Toggles the API enabled flag for the node owned by the caller.
/// @dev Only the owner of the node NFT may call this.
/// @param nodeId The unique identifier of the node.
function updateIsApiEnabled(uint256 nodeId) external;
/// @param isApiEnabled The new API enabled flag.
function updateIsApiEnabled(uint256 nodeId, bool isApiEnabled) external;

// ***************************************************************
// * GETTER FUNCTIONS *
Expand Down
19 changes: 9 additions & 10 deletions contracts/test/Nodes.sol
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ contract NodesTest is Test, Utils {
_addNode();
nodes.updateIsReplicationEnabled(nodeId, true);
vm.prank(nodeOperator);
nodes.updateIsApiEnabled(nodeId);
nodes.updateIsApiEnabled(nodeId, true);
vm.expectEmit(address(nodes));
emit INodes.NodeActivateUpdated(nodeId, true);
nodes.updateActive(nodeId, true);
Expand Down Expand Up @@ -468,7 +468,7 @@ contract NodesTest is Test, Utils {
_addNode();
nodes.updateIsReplicationEnabled(nodeId, true);
vm.prank(nodeOperator);
nodes.updateIsApiEnabled(nodeId);
nodes.updateIsApiEnabled(nodeId, true);

nodes.updateActive(nodeId, true);
vm.recordLogs();
Expand Down Expand Up @@ -585,7 +585,7 @@ contract NodesTest is Test, Utils {
_addNode();
nodes.updateIsReplicationEnabled(nodeId, true);
vm.prank(nodeOperator);
nodes.updateIsApiEnabled(nodeId);
nodes.updateIsApiEnabled(nodeId, true);

nodes.updateMaxActiveNodes(1);
nodes.updateActive(nodeId, true);
Expand Down Expand Up @@ -676,11 +676,10 @@ contract NodesTest is Test, Utils {
vm.expectEmit(address(nodes));
emit INodes.ApiEnabledUpdated(nodeId, true);
vm.startPrank(nodeOperator);
nodes.updateIsApiEnabled(nodeId);
nodes.updateIsApiEnabled(nodeId, true);
assertEq(nodes.getNode(nodeId).isApiEnabled, true);

/// @dev Toggle again
nodes.updateIsApiEnabled(nodeId);
nodes.updateIsApiEnabled(nodeId, false);
assertEq(nodes.getNode(nodeId).isApiEnabled, false);
vm.stopPrank();
}
Expand All @@ -690,7 +689,7 @@ contract NodesTest is Test, Utils {
vm.recordLogs();
vm.expectRevert(INodes.Unauthorized.selector);
/// @dev Default user is admin
nodes.updateIsApiEnabled(nodeId);
nodes.updateIsApiEnabled(nodeId, true);
_checkNoLogsEmitted();
}

Expand Down Expand Up @@ -770,7 +769,7 @@ contract NodesTest is Test, Utils {
_addNode();
nodes.updateIsReplicationEnabled(nodeId, true);
vm.prank(nodeOperator);
nodes.updateIsApiEnabled(nodeId);
nodes.updateIsApiEnabled(nodeId, true);
nodes.updateActive(nodeId, true);
vm.assertEq(nodes.getActiveNodesCount(), 1);

Expand Down Expand Up @@ -800,7 +799,7 @@ contract NodesTest is Test, Utils {
_addNode();
nodes.updateIsReplicationEnabled(nodeId, true);
vm.prank(nodeOperator);
nodes.updateIsApiEnabled(nodeId);
nodes.updateIsApiEnabled(nodeId, true);

nodes.updateActive(nodeId, true);
vm.assertEq(nodes.getNodeIsActive(nodeId), true);
Expand Down Expand Up @@ -866,7 +865,7 @@ contract NodesTest is Test, Utils {
for (uint256 i = 0; i < nodeIds.length; i++) {
nodes.updateIsReplicationEnabled(nodeIds[i], true);
vm.prank(operators[i]);
nodes.updateIsApiEnabled(nodeIds[i]);
nodes.updateIsApiEnabled(nodeIds[i], true);
}
}

Expand Down
5 changes: 3 additions & 2 deletions dev/register-local-node
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ NODE_ID=$(echo $ADD_NODE_RESPONSE | jq -r '."node-id"')

dev/cli update-api-enabled \
--node-id=$NODE_ID \
--operator-private-key=$OPERATOR_PRIVATE_KEY
--operator-private-key=$OPERATOR_PRIVATE_KEY \
--enable

dev/cli update-replication-enabled \
--node-id=$NODE_ID \
Expand All @@ -31,4 +32,4 @@ dev/cli update-active \
--admin-private-key=$PRIVATE_KEY \
--activate

echo -e "\033[32m✔\033[0m Node $NODE_ID registered and activated"
echo -e "\033[32m✔\033[0m Node $NODE_ID registered and activated\n"
5 changes: 3 additions & 2 deletions dev/register-local-node-2
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ NODE_ID=$(echo $ADD_NODE_RESPONSE | jq -r '."node-id"')

dev/cli update-api-enabled \
--node-id=$NODE_ID \
--operator-private-key=$OPERATOR_PRIVATE_KEY
--operator-private-key=$OPERATOR_PRIVATE_KEY \
--enable

dev/cli update-replication-enabled \
--node-id=$NODE_ID \
Expand All @@ -32,4 +33,4 @@ dev/cli update-active \
--admin-private-key=$PRIVATE_KEY \
--activate

echo -e "\033[32m✔\033[0m Node $NODE_ID registered and activated"
echo -e "\033[32m✔\033[0m Node $NODE_ID registered and activated\n"
1 change: 1 addition & 0 deletions pkg/blockchain/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ type NodeRegistry interface {
UpdateIsApiEnabled(
ctx context.Context,
nodeId uint32,
isApiEnabled bool,
) error
UpdateIsReplicationEnabled(
ctx context.Context,
Expand Down
3 changes: 2 additions & 1 deletion pkg/blockchain/registryAdmin.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ func (n *NodeRegistryAdmin) UpdateActive(
func (n *NodeRegistryAdmin) UpdateIsApiEnabled(
ctx context.Context,
nodeId uint32,
isApiEnabled bool,
) error {
if n.signer == nil {
return fmt.Errorf("no signer provided")
Expand All @@ -150,7 +151,7 @@ func (n *NodeRegistryAdmin) UpdateIsApiEnabled(
Context: ctx,
From: n.signer.FromAddress(),
Signer: n.signer.SignerFunc(),
}, big.NewInt(int64(nodeId)))
}, big.NewInt(int64(nodeId)), isApiEnabled)

if err != nil {
return err
Expand Down
8 changes: 4 additions & 4 deletions pkg/blockchain/registryAdmin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func TestUpdateIsApiEnabled(t *testing.T) {
)
require.NoError(t, err)

err = registry.UpdateIsApiEnabled(ctx, nodeId)
err = registry.UpdateIsApiEnabled(ctx, nodeId, true)
require.NoError(t, err)
}

Expand All @@ -150,7 +150,7 @@ func TestUpdateIsApiEnabledUnauthorized(t *testing.T) {
nodeId, _, err := addRandomNode(t, ctx, registry)
require.NoError(t, err)

err = registry.UpdateIsApiEnabled(ctx, nodeId)
err = registry.UpdateIsApiEnabled(ctx, nodeId, true)

// 0x82b42900 is the signature of Unauthorized() error
require.Equal(t, err.Error(), "execution reverted: custom error 0x82b42900")
Expand All @@ -160,7 +160,7 @@ func TestUpdateIsApiEnabledBadNodeId(t *testing.T) {
registry, _, ctx, cleanup := buildRegistry(t)
defer cleanup()

err := registry.UpdateIsApiEnabled(ctx, 1)
err := registry.UpdateIsApiEnabled(ctx, 1, true)
require.Equal(t, err.Error(), "execution reverted: custom error 0x82b42900")
}

Expand Down Expand Up @@ -190,7 +190,7 @@ func TestUpdateActive(t *testing.T) {
)
require.NoError(t, err)

err = registry.UpdateIsApiEnabled(ctx, nodeId)
err = registry.UpdateIsApiEnabled(ctx, nodeId, true)
require.NoError(t, err)

err = registry.UpdateIsReplicationEnabled(ctx, nodeId, true)
Expand Down
1 change: 1 addition & 0 deletions pkg/config/cliOptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ type UpdateActiveOptions struct {
type UpdateApiEnabledOptions struct {
NodeId int64 `long:"node-id" description:"NodeId to update"`
OperatorPrivateKey string `long:"operator-private-key" description:"Private key of the operator to update the node" required:"true"`
IsApiEnabled bool `long:"enable" description:"Whether the node is api enabled"`
}

type UpdateReplicationEnabledOptions struct {
Expand Down

0 comments on commit d3e5be4

Please sign in to comment.