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.

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 RPC for Boot Nodes #4735

Merged
merged 50 commits into from
Sep 11, 2024
Merged

Add RPC for Boot Nodes #4735

merged 50 commits into from
Sep 11, 2024

Conversation

GheisMohammadi
Copy link
Contributor

@GheisMohammadi GheisMohammadi commented Aug 13, 2024

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.

@GheisMohammadi GheisMohammadi self-assigned this Aug 13, 2024
@GheisMohammadi GheisMohammadi marked this pull request as draft August 13, 2024 14:51
@GheisMohammadi GheisMohammadi added the WIP Work in progress don't merge yet! label Aug 14, 2024
@GheisMohammadi GheisMohammadi marked this pull request as ready for review August 22, 2024 09:51
@Frozen
Copy link
Contributor

Frozen commented Aug 28, 2024

Pls fix conflicts

@sophoah
Copy link
Contributor

sophoah commented Aug 30, 2024

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

cmd/bootnode/main.go Outdated Show resolved Hide resolved
@mur-me
Copy link
Collaborator

mur-me commented Aug 30, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor

sophoah commented Sep 9, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor

sophoah commented Sep 10, 2024

@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
Copy link
Contributor

sophoah commented Sep 10, 2024

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

@sophoah
Copy link
Contributor

sophoah commented Sep 10, 2024

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

@GheisMohammadi
Copy link
Contributor Author

GheisMohammadi commented Sep 10, 2024

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

@sophoah sophoah merged commit 6d9b09d into dev Sep 11, 2024
4 checks passed
@sophoah sophoah mentioned this pull request Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP Work in progress don't merge yet!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants