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

[Feature Request] node.peers api return node_name #3439

Closed
yourmoonlight opened this issue May 31, 2022 · 3 comments · Fixed by #3471
Closed

[Feature Request] node.peers api return node_name #3439

yourmoonlight opened this issue May 31, 2022 · 3 comments · Fixed by #3471
Assignees
Labels
enhancement Enhancement help wanted Extra attention is needed

Comments

@yourmoonlight
Copy link
Collaborator

Feature Request

Describe the Feature Request

Could the node.peers api return field node_name as the same level as the field peer_id?

Additional Context

Currently, the peer_id is not related to the wallet address, but if we run the node with node_name argument, and set it to wallet address,then it is solved in an easy way. It makes the later possible airdrop process easier.

@yourmoonlight yourmoonlight added enhancement Enhancement help wanted Extra attention is needed labels May 31, 2022
@jolestar jolestar changed the title [Feature Request] [Feature Request] node.peers api return node_name May 31, 2022
@coldnight
Copy link
Contributor

Hi, I do a little research about this, and found that we need to add a node_name field to network_p2p::protocol::message::generic::Status while handshaking, and currently it encoded/decoded by bcs which lacks backwards compatibility. The changes that I made could refer to coldnight@4da7e2b.

@jolestar
Copy link
Member

@coldnight I think we do not need to change the handshake message.

node network state
{
  "ok": {
    "connectedPeers": {
      "12D3KooW9quK2EEjeyTs3csNRWPnfMw4M3afGE1SHm1dCZDRWSAj": {
        "endpoint": {
          "dialing": "/dns4/main5.seed.starcoin.org/tcp/9840/p2p/12D3KooW9quK2EEjeyTs3csNRWPnfMw4M3afGE1SHm1dCZDRWSAj"
        },
        "knownAddresses": [
          "/dns4/main5.seed.starcoin.org/tcp/9840",
          "/ip4/18.182.8.131/tcp/9840",
          "/dns4/main5.seed.starcoin.org/tcp/9840/p2p/12D3KooW9quK2EEjeyTs3csNRWPnfMw4M3afGE1SHm1dCZDRWSAj"
        ],
        "latestPingTime": {
          "nanos": 248017299,
          "secs": 0
        },
        "versionString": "starcoin/1.11.7-rc (build:1.11.7-rc) (berserk-seashore-0643)"
      },

network state versionString field contains the node name (berserk-seashore-0643).

We can just add the versionString field to node.peers's response. Maybe version_string is more suitable for the filed name.

coldnight added a commit to coldnight/starcoin that referenced this issue Jun 16, 2022
So the RPC API `node.peers` could show the `node_name` in it.

Related starcoinorg#3439
@coldnight
Copy link
Contributor

@jolestar Thanks for your help, a PR has been made.

jolestar pushed a commit that referenced this issue Jun 16, 2022
* [network] add `version_string` field to PeerInfo

Related #3439
@jolestar jolestar linked a pull request Jun 17, 2022 that will close this issue
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants