harmony-one / harmony

The core protocol of harmony
https://harmony.one
GNU Lesser General Public License v3.0
1.47k stars 289 forks source link

Add RPC for Boot Nodes #4735

Closed GheisMohammadi closed 1 month ago

GheisMohammadi commented 2 months ago

Description

This PR enhances the visibility and monitoring capabilities of boot nodes by adding an RPC server. Previously, boot nodes lacked any RPCs and did not expose any meta data, limiting our ability to monitor them effectively. By adding RPC for boot nodes, this PR introduces several APIs that allow for better and more efficient monitoring, enabling us to observe and track the connectivity and peers connected to the boot node.

The PR would add these RPC methods for both hmyboot and hmybootv2 domains:

echo "=======[hmyboot_getNodeMetadata]================="

curl -d '{
  "jsonrpc":"2.0",
  "method":"hmyboot_getNodeMetadata",
  "params":[],
  "id":1
 }' -H 'Content-Type:application/json' -X POST '0.0.0.0:8888'

 #{"jsonrpc":"2.0","id":1,"result":{"node-unix-start-time":1724318966,"p2p-connectivity":{"connected":24,"not-connected":1,"total-known-peers":25},"peerid":"", "network":"localnet", "version":"Harmony (C) 2023. bootnode, version v8421-v2024.0.0-83-g1cb52f7f0-dirty (root@ 2024-08-22T17:29:12+0800)"}}

 echo "=======[hmyboot_getPeerInfo]================="

curl -d '{
  "jsonrpc":"2.0",
  "method":"hmyboot_getPeerInfo",
  "params":[],
  "id":1
 }' -H 'Content-Type:application/json' -X POST '0.0.0.0:8888'

 #{"jsonrpc":"2.0","id":1,"result":{"peerid":"","known-peers":[],"connected-peers":[],"C":{}}}

 echo "=======[hmyboot_protocolVersion]================="

curl -d '{
  "jsonrpc":"2.0",
  "method":"hmyboot_protocolVersion",
  "params":[],
  "id":1
 }' -H 'Content-Type:application/json' -X POST '0.0.0.0:8888'

 #{"jsonrpc":"2.0","id":1,"result":"0x1"}

Additionally, this PR includes a redefinition of the node and RPC folder structure, making the codebase more organized and well structured to add boot nodes functions as well.

Importantly, the changes made in this PR do not alter any logic for Harmony nodes, ensuring that the merge is safe and will not affect existing functionality. This improvement aims to facilitate better network management and monitoring of boot nodes.

Frozen commented 2 months ago

Pls fix conflicts

sophoah commented 2 months ago

We don't need to update the PR and this is just a remark :

    httpPort := flag.Int("rpc_http_port", 9500, "port of the rpc http")
    wsPort := flag.Int("rpc_ws_port", 9800, "port of the rpc ws")

those default port are the same as the harmony node. We'll have to make sure node running boot and harmony node have distinct port cc @mur-me

mur-me commented 2 months ago

We don't need to update the PR and this is just a remark :

  httpPort := flag.Int("rpc_http_port", 9500, "port of the rpc http")
  wsPort := flag.Int("rpc_ws_port", 9800, "port of the rpc ws")

those default port are the same as the harmony node. We'll have to make sure node running boot and harmony node have distinct port cc @mur-me

Agreed, here could be a conflict if run it on one machine together, but this isn't a huge problem

GheisMohammadi commented 2 months ago

We don't need to update the PR and this is just a remark :

  httpPort := flag.Int("rpc_http_port", 9500, "port of the rpc http")
  wsPort := flag.Int("rpc_ws_port", 9800, "port of the rpc ws")

those default port are the same as the harmony node. We'll have to make sure node running boot and harmony node have distinct port cc @mur-me

I noticed this when I was adjusting the ports. I intentionally chose to standardize and unify the RPC ports across nodes. This way, users don't need to seek for a new port every time they want to access Boot RPCs—keeping it consistent with the Harmony node makes it more user-friendly. I think in terms of safety having less ports open would make more sense. Additionally, I included a flag that allows us to change the Boot node RPC ports if both the boot and Harmony nodes are running on the same server, ensuring flexibility. I've already updated the localnet scripts to prevent any conflicts there.

That said, I'm open to changing the ports if we think it's necessary. Let me know your thoughts!

sophoah commented 1 month ago

Thanks @GheisMohammadi for all the removal. I am now looking the results returned by 3 APIs.

curl -d '{
  "jsonrpc":"2.0",
  "method":"hmyboot_getNodeMetadata",
  "params":[],
  "id":1
 }' -H 'Content-Type:application/json' -X POST '0.0.0.0:8888'

returns

 {"jsonrpc":"2.0","id":1,"result":{"blskey":null,"chain-config":{"AllowlistEpoch":null,"FeeCollectEpoch":null,"chain-id":null,"eth-compatible-chain-id":null,"eth-compatible-shard-0-chain-id":null},"consensus":{"blocknum":0,"finality":0,"mode":"","phase":"","viewChangeId":0,"viewId":0},"current-block-number":0,"current-epoch":0,"dns-zone":"","is-archival":false,"is-backup":false,"is-leader":false,"network":"","node-unix-start-time":1725851267,"p2p-connectivity":{"connected":24,"not-connected":1,"total-known-peers":25},"peerid":"","role":"","shard-id":0,"version":"Harmony (C) 2023. bootnode, version v8448-v2024.2.0-52-g6d3b3fa7c (soph@ 2024-09-09T10:07:28+0700)"}}

I don’t believe the fields blskey, consensus, current-block-number, current-epoch, is-archival, is-backup, is-leader, shard-id provide much value here. What are your thoughts? Additionally, the fields network, peer-id, role are not returning any results either.

curl -d '{
  "jsonrpc":"2.0",
  "method":"hmyboot_getPeerInfo",
  "params":[],
  "id":1
 }' -H 'Content-Type:application/json' -X POST '0.0.0.0:8888'

returns

 {"jsonrpc":"2.0","id":1,"result":{"blocked-peers":[],"connected-peers":[],"peerid":""}}

Is it normal for the connected-peers field to be empty? Similarly, the peerid field also is not working.

hmyboot_protocolVersion looks ok.

GheisMohammadi commented 1 month ago
hmyboot_getNodeMetadata

You're right. The current structure uses the Harmony node metadata format, which includes fields that are not particularly relevant to boot nodes. Since the boot node only utilizes specific fields related to its functionality, I agree it would make more sense to separate the boot node metadata from the Harmony node metadata. I will go ahead and refactor this to create a more tailored structure for boot nodes. Let me take care of that now.

sophoah commented 1 month ago

@GheisMohammadi really nice, here is the latest test result : hmyboot_getNodeMetadata:

curl -d '{
  "jsonrpc":"2.0",
  "method":"hmyboot_getNodeMetadata",
  "params":[],
  "id":1
 }' -H 'Content-Type:application/json' -X POST '0.0.0.0:8888'
 {"jsonrpc":"2.0","id":1,"result":{"network":"localnet","node-unix-start-time":1725940221,"p2p-connectivity":{"connected":24,"not-connected":1,"total-known-peers":25},"peerid":"Qmc1V6W7BwX8Ugb42Ti8RnXF1rY5PF7nnZ6bKBryCgi6cv","version":"Harmony (C) 2023. bootnode, version v8452-v2024.2.0-56-g8e7a164d5 (soph@ 2024-09-10T10:50:07+0700)"}}

hmyboot_getPeerInfo:

curl -d '{
  "jsonrpc":"2.0",
  "method":"hmyboot_getPeerInfo",
  "params":[],
  "id":1
 }' -H 'Content-Type:application/json' -X POST '0.0.0.0:8888'
{"jsonrpc":"2.0","id":1,"result":{"c":{"connected":24,"not-connected":1,"total-known-peers":25},"connected-peers":["QmTnxxzCBD22quNagCFoDqvH6tQrMBAKqjNzx6Hbhog9ms","QmX48ysgKnWBNrXKpFQ1nFVAynCqYBWYjJ4QaHwkf4g4Y5","QmVyGzpU2muKMQ2KXG2WTrKBJazZ77WZmG4dqdpsaTzy8K","QmeLsMmiNdbzPiD8iRenkVKxXronhCxUQ6KAoxmjWqBNcU","QmP8tBd8KGEbB3xcW3ajFfLF4y8HVXpvdRwRDptuAQbJ74","QmQDCFekk8PuomyPSdAuiF4GCUBaCJFw6Uz3k35CrXAMRD","QmUBXaTCyH9fpzqLmpvgkbT3FvUqywJbfNNWoiKnrL2Cb5","QmXRi4nKeyNaXeiVR3MDwBbkZCvDAKavkyMD7aoPwUBw2Q","QmPsY3ndVWrhHW3M3nSVEocmCZtxHJm3NjXUpiHmX5wAph","QmeY1spswbWY7yMBCEc367aWZnH12zS2MWQgYQcLYEQFQJ","QmTwG4pCtE1gaY7gm5nZg1HtHK2AR2oL1HMGXPucDDFHWt","QmUJLE7wytFaKHRP25run8xRbGdHqTvyeFrDHLywzFkNGY","QmV7oduzPb1urLyaCSC942VHDLT4w6QMCZgXCWLxJcduWf","QmXJSA31NzmzKWmjGatsBCkBCFoYWHgeKV3E2cbeXqQyGe","QmZ17k6iWv2Dj4RHHhdX1VZ8H5UKze4Yue3RNRRS9skTKq","QmdhkDwWbgiqu6V7U3eD5j95Cvjf54NwgaHv9w6vBnVbGY","QmQ82zJZrtEANUkp2DJDg9VGixbSn6oDL9AD1mKPMY74yG","QmTgEFsc7tVUJ4CnCgmkuG5ANm96SEHrNkgJgA1PCw2maz","QmcCxKtPuLhTenq6J4DoFKEeEcRZJW74e9SvhFkGibThPr","QmWnNpwnUJf2mhtwkr7AZbMxF4CDHFfoe7AaPbX7zF3J7a","QmYYDjzMgBnxQrYUrqTAYcSFpJ2rT6XCZjnFreYBwSdiuZ","QmZLQ45S2aEnJohFuuTc37x7LkMgTUUGN8zFjJ7K7wqFAy","QmXhm56KuvZ9idXERpp12gmJAmESxb6nYki1zEqGRzckfv","QmdGkuubG1wHgMEsLXEhm2jPDj3s2pZ18tKZ5T94F2gqtb"],"known-peers":["QmUBXaTCyH9fpzqLmpvgkbT3FvUqywJbfNNWoiKnrL2Cb5","QmWnNpwnUJf2mhtwkr7AZbMxF4CDHFfoe7AaPbX7zF3J7a","QmZ17k6iWv2Dj4RHHhdX1VZ8H5UKze4Yue3RNRRS9skTKq","QmdhkDwWbgiqu6V7U3eD5j95Cvjf54NwgaHv9w6vBnVbGY","QmTgEFsc7tVUJ4CnCgmkuG5ANm96SEHrNkgJgA1PCw2maz","QmeLsMmiNdbzPiD8iRenkVKxXronhCxUQ6KAoxmjWqBNcU","QmXRi4nKeyNaXeiVR3MDwBbkZCvDAKavkyMD7aoPwUBw2Q","QmTnxxzCBD22quNagCFoDqvH6tQrMBAKqjNzx6Hbhog9ms","QmVyGzpU2muKMQ2KXG2WTrKBJazZ77WZmG4dqdpsaTzy8K","QmQ82zJZrtEANUkp2DJDg9VGixbSn6oDL9AD1mKPMY74yG","QmdGkuubG1wHgMEsLXEhm2jPDj3s2pZ18tKZ5T94F2gqtb","QmXJSA31NzmzKWmjGatsBCkBCFoYWHgeKV3E2cbeXqQyGe","QmcCxKtPuLhTenq6J4DoFKEeEcRZJW74e9SvhFkGibThPr","QmYYDjzMgBnxQrYUrqTAYcSFpJ2rT6XCZjnFreYBwSdiuZ","QmTwG4pCtE1gaY7gm5nZg1HtHK2AR2oL1HMGXPucDDFHWt","QmeY1spswbWY7yMBCEc367aWZnH12zS2MWQgYQcLYEQFQJ","Qmc1V6W7BwX8Ugb42Ti8RnXF1rY5PF7nnZ6bKBryCgi6cv","QmZLQ45S2aEnJohFuuTc37x7LkMgTUUGN8zFjJ7K7wqFAy","QmV7oduzPb1urLyaCSC942VHDLT4w6QMCZgXCWLxJcduWf","QmXhm56KuvZ9idXERpp12gmJAmESxb6nYki1zEqGRzckfv","QmPsY3ndVWrhHW3M3nSVEocmCZtxHJm3NjXUpiHmX5wAph","QmX48ysgKnWBNrXKpFQ1nFVAynCqYBWYjJ4QaHwkf4g4Y5","QmP8tBd8KGEbB3xcW3ajFfLF4y8HVXpvdRwRDptuAQbJ74","QmQDCFekk8PuomyPSdAuiF4GCUBaCJFw6Uz3k35CrXAMRD","QmUJLE7wytFaKHRP25run8xRbGdHqTvyeFrDHLywzFkNGY"],"peerid":"Qmc1V6W7BwX8Ugb42Ti8RnXF1rY5PF7nnZ6bKBryCgi6cv"}}

hmyboot_protocolVersion:

curl -d '{
  "jsonrpc":"2.0",
  "method":"hmyboot_protocolVersion",
  "params":[],
  "id":1
 }' -H 'Content-Type:application/json' -X POST '0.0.0.0:8888'
{"jsonrpc":"2.0","id":1,"result":"0x1"}
sophoah commented 1 month ago

@mur-me let's update our devops to add the network flag accordingly when we deploy that PR to our bootnode -network

sophoah commented 1 month ago

Note that the flag is just informative/cosmetic, it doesn't play any role in the bootnode logic behavior here.

GheisMohammadi commented 1 month ago

@sophoah @Frozen @mur-me Thanks for the reviews and great comments