Collection of changes for new node management (#195)

- Add new `get-all-nodes` command.
- decrease verbosity of some logging
- mark payer key as required
- some dirty protos changes
- fix sol formatting

cmd/cli/main.go

@@ -26,6 +26,7 @@ type CLI struct {
 	GetPubKey    config.GetPubKeyOptions
 	GenerateKey  config.GenerateKeyOptions
 	RegisterNode config.RegisterNodeOptions
+	GetAllNodes  config.GetAllNodesOptions
 }
 
 /*
@@ -42,6 +43,7 @@ func parseOptions(args []string) (*CLI, error) {
 	var generateKeyOptions config.GenerateKeyOptions
 	var registerNodeOptions config.RegisterNodeOptions
 	var getPubKeyOptions config.GetPubKeyOptions
+	var getAllNodesOptions config.GetAllNodesOptions
 
 	parser := flags.NewParser(&options, flags.Default)
 	if _, err := parser.AddCommand("generate-key", "Generate a public/private keypair", "", &generateKeyOptions); err != nil {
@@ -53,6 +55,9 @@ func parseOptions(args []string) (*CLI, error) {
 	if _, err := parser.AddCommand("get-pub-key", "Get the public key for a private key", "", &getPubKeyOptions); err != nil {
 		return nil, fmt.Errorf("Could not add get-pub-key command: %s", err)
 	}
+	if _, err := parser.AddCommand("get-all-nodes", "Get all nodes from the registry", "", &getAllNodesOptions); err != nil {
+		return nil, fmt.Errorf("Could not add get-all-nodes command: %s", err)
+	}
 	if _, err := parser.ParseArgs(args); err != nil {
 		if err, ok := err.(*flags.Error); !ok || err.Type != flags.ErrHelp {
 			return nil, fmt.Errorf("Could not parse options: %s", err)
@@ -70,6 +75,7 @@ func parseOptions(args []string) (*CLI, error) {
 		getPubKeyOptions,
 		generateKeyOptions,
 		registerNodeOptions,
+		getAllNodesOptions,
 	}, nil
 }
 
@@ -125,7 +131,7 @@ func registerNode(logger *zap.Logger, options *CLI) {
 	}
 	logger.Info(
 		"successfully added node",
-		zap.String("node-address", options.RegisterNode.OwnerAddress),
+		zap.String("owner-address", options.RegisterNode.OwnerAddress),
 		zap.String("node-http-address", options.RegisterNode.HttpAddress),
 		zap.String("node-signing-key-pub", utils.EcdsaPublicKeyToString(signingKeyPub)),
 	)
@@ -143,6 +149,43 @@ func generateKey(logger *zap.Logger) {
 	)
 }
 
+func getAllNodes(logger *zap.Logger, options *CLI) {
+	ctx := context.Background()
+	chainClient, err := blockchain.NewClient(ctx, options.Contracts.RpcUrl)
+	if err != nil {
+		logger.Fatal("could not create chain client", zap.Error(err))
+	}
+
+	signer, err := blockchain.NewPrivateKeySigner(
+		options.GetAllNodes.AdminPrivateKey,
+		options.Contracts.ChainID,
+	)
+
+	if err != nil {
+		logger.Fatal("could not create signer", zap.Error(err))
+	}
+
+	registryAdmin, err := blockchain.NewNodeRegistryAdmin(
+		logger,
+		chainClient,
+		signer,
+		options.Contracts,
+	)
+	if err != nil {
+		logger.Fatal("could not create registry admin", zap.Error(err))
+	}
+
+	nodes, err := registryAdmin.GetAllNodes(ctx)
+	if err != nil {
+		logger.Fatal("could not retrieve nodes from registry", zap.Error(err))
+	}
+	logger.Info(
+		"got nodes",
+		zap.Int("size", len(nodes)),
+		zap.Any("nodes", nodes),
+	)
+}
+
 func main() {
 	options, err := parseOptions(os.Args[1:])
 	if err != nil {
@@ -166,6 +209,9 @@ func main() {
 	case "register-node":
 		registerNode(logger, options)
 		return
+	case "get-all-nodes":
+		getAllNodes(logger, options)
+		return
 	}
 
 }

contracts/src/Nodes.sol

@@ -5,14 +5,15 @@ import "@openzeppelin/contracts/token/ERC721/ERC721.sol";
 import "@openzeppelin/contracts/access/Ownable.sol";
 
 /**
-A NFT contract for XMTP Node Operators.
-
-The deployer of this contract is responsible for minting NFTs and assinging them to node operators.
-
-All nodes on the network periodically check this contract to determine which nodes they should connect to.
+ * A NFT contract for XMTP Node Operators.
+ *
+ * The deployer of this contract is responsible for minting NFTs and assinging them to node operators.
+ *
+ * All nodes on the network periodically check this contract to determine which nodes they should connect to.
  */
 contract Nodes is ERC721, Ownable {
     constructor() ERC721("XMTP Node Operator", "XMTP") Ownable(msg.sender) {}
+
     uint32 private _nodeIncrement = 100;
     // uint32 counter so that we cannot create more than max IDs
     // The ERC721 standard expects the tokenID to be uint256 for standard methods unfortunately
@@ -36,59 +37,46 @@ contract Nodes is ERC721, Ownable {
     mapping(uint256 => Node) private _nodes;
 
     /**
-    Mint a new node NFT and store the metadata in the smart contract
+     * Mint a new node NFT and store the metadata in the smart contract
      */
-    function addNode(
-        address to,
-        bytes calldata signingKeyPub,
-        string calldata httpAddress
-    ) public onlyOwner returns (uint32) {
+    function addNode(address to, bytes calldata signingKeyPub, string calldata httpAddress)
+        public
+        onlyOwner
+        returns (uint32)
+    {
         // the first node starts with 100
         _nodeCounter++;
-        uint32 nodeId = _nodeCounter*_nodeIncrement;
+        uint32 nodeId = _nodeCounter * _nodeIncrement;
         _mint(to, nodeId);
         _nodes[nodeId] = Node(signingKeyPub, httpAddress, true);
         _emitNodeUpdate(nodeId);
         return nodeId;
     }
 
     /**
-    Override the built in transferFrom function to block NFT owners from transferring
-    node ownership.
-
-    NFT owners are only allowed to update their HTTP address and MTLS cert.
+     * Override the built in transferFrom function to block NFT owners from transferring
+     * node ownership.
+     *
+     * NFT owners are only allowed to update their HTTP address and MTLS cert.
      */
-    function transferFrom(
-        address from,
-        address to,
-        uint256 tokenId
-    ) public override {
-        require(
-            _msgSender() == owner(),
-            "Only the contract owner can transfer Node ownership"
-        );
+    function transferFrom(address from, address to, uint256 tokenId) public override {
+        require(_msgSender() == owner(), "Only the contract owner can transfer Node ownership");
         super.transferFrom(from, to, tokenId);
     }
 
     /**
-    Allow a NFT holder to update the HTTP address of their node
+     * Allow a NFT holder to update the HTTP address of their node
      */
-    function updateHttpAddress(
-        uint256 tokenId,
-        string calldata httpAddress
-    ) public {
-        require(
-            _msgSender() == ownerOf(tokenId),
-            "Only the owner of the Node NFT can update its http address"
-        );
+    function updateHttpAddress(uint256 tokenId, string calldata httpAddress) public {
+        require(_msgSender() == ownerOf(tokenId), "Only the owner of the Node NFT can update its http address");
         _nodes[tokenId].httpAddress = httpAddress;
         _emitNodeUpdate(tokenId);
     }
 
     /**
-    The contract owner may update the health status of the node.
-
-    No one else is allowed to call this function.
+     * The contract owner may update the health status of the node.
+     *
+     * No one else is allowed to call this function.
      */
     function updateHealth(uint256 tokenId, bool isHealthy) public onlyOwner {
         // Make sure that the token exists
@@ -98,7 +86,7 @@ contract Nodes is ERC721, Ownable {
     }
 
     /**
-    Get a list of healthy nodes with their ID and metadata
+     * Get a list of healthy nodes with their ID and metadata
      */
     function healthyNodes() public view returns (NodeWithId[] memory) {
         uint256 healthyCount = 0;
@@ -119,10 +107,7 @@ contract Nodes is ERC721, Ownable {
         for (uint32 i = 0; i < _nodeCounter; i++) {
             uint32 nodeId = _nodeIncrement * (i + 1);
             if (_nodeExists(nodeId) && _nodes[nodeId].isHealthy) {
-                healthyNodesList[currentIndex] = NodeWithId({
-                    nodeId: nodeId,
-                    node: _nodes[nodeId]
-                });
+                healthyNodesList[currentIndex] = NodeWithId({nodeId: nodeId, node: _nodes[nodeId]});
                 currentIndex++;
             }
         }
@@ -131,26 +116,23 @@ contract Nodes is ERC721, Ownable {
     }
 
     /**
-    Get all nodes regardless of their health status
+     * Get all nodes regardless of their health status
      */
     function allNodes() public view returns (NodeWithId[] memory) {
         NodeWithId[] memory allNodesList = new NodeWithId[](_nodeCounter);
 
         for (uint32 i = 0; i < _nodeCounter; i++) {
             uint32 nodeId = _nodeIncrement * (i + 1);
             if (_nodeExists(nodeId)) {
-                allNodesList[i] = NodeWithId({
-                    nodeId: nodeId,
-                    node: _nodes[nodeId]
-                });
+                allNodesList[i] = NodeWithId({nodeId: nodeId, node: _nodes[nodeId]});
             }
         }
 
         return allNodesList;
     }
 
     /**
-    Get a node's metadata by ID
+     * Get a node's metadata by ID
      */
     function getNode(uint256 tokenId) public view returns (Node memory) {
         _requireOwned(tokenId);

dev/get-all-nodes

@@ -0,0 +1,8 @@
+#!/bin/bash
+
+set -eu
+
+. dev/local.env
+
+dev/cli get-all-nodes \
+    --admin-private-key=$PRIVATE_KEY

pkg/blockchain/registryAdmin.go

@@ -81,3 +81,11 @@ func (n *NodeRegistryAdmin) AddNode(
 		tx.Hash(),
 	)
 }
+func (n *NodeRegistryAdmin) GetAllNodes(
+	ctx context.Context,
+) ([]abis.NodesNodeWithId, error) {
+
+	return n.contract.AllNodes(&bind.CallOpts{
+		Context: ctx,
+	})
+}

pkg/blockchain/rpcLogStreamer.go

@@ -114,7 +114,7 @@ func (r *RpcLogStreamer) watchContract(watcher contractConfig) {
 				continue
 			}
 
-			logger.Info("Got logs", zap.Int("numLogs", len(logs)), zap.Int("fromBlock", fromBlock))
+			logger.Debug("Got logs", zap.Int("numLogs", len(logs)), zap.Int("fromBlock", fromBlock))
 			if len(logs) == 0 {
 				time.Sleep(NO_LOGS_SLEEP_TIME)
 			}

pkg/config/options.go

@@ -34,7 +34,7 @@ type MetricsOptions struct {
 }
 
 type PayerOptions struct {
-	PrivateKey string `long:"private-key" env:"XMTPD_PAYER_PRIVATE_KEY" description:"Private key used to sign blockchain transactions"`
+	PrivateKey string `long:"private-key" env:"XMTPD_PAYER_PRIVATE_KEY" description:"Private key used to sign blockchain transactions" required:"true"`
 }
 
 type MlsValidationOptions struct {
@@ -62,6 +62,10 @@ type SignerOptions struct {
 
 type GenerateKeyOptions struct{}
 
+type GetAllNodesOptions struct {
+	AdminPrivateKey string `long:"admin-private-key" description:"Private key of the admin to register the node"`
+}
+
 type GetPubKeyOptions struct {
 	PrivateKey string `long:"private-key" description:"Private key you want the public key for" required:"true"`
 }