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

Status command #119

Merged
merged 13 commits into from
Nov 21, 2023
Merged

Status command #119

merged 13 commits into from
Nov 21, 2023

Conversation

mariacarmina
Copy link
Member

@mariacarmina mariacarmina commented Nov 17, 2023

Fixes #32 .

Changes proposed in this PR:

  • implemented core function for status

  • modified OceanNodeKeys to include ethAddress

  • generated address from private key in getPeerIdFromPrivateKey

  • expose HTTP interface

  • expose P2P interface

@mariacarmina mariacarmina self-assigned this Nov 17, 2023
@mariacarmina mariacarmina changed the title Status command. Status command Nov 17, 2023
src/utils/constants.ts Outdated Show resolved Hide resolved
@mariacarmina mariacarmina marked this pull request as ready for review November 20, 2023 11:57
@mariacarmina
Copy link
Member Author

This is the response from Postman calling /directCommand with status:

Screenshot 2023-11-20 at 13 59 33

Extended version:

{
    "id": "16Uiu2HAmJKKnHoDrc5xcxPfVJUWLrv8iNnhqZ3YdnUfRRsAiJvAJ",
    "publicKey": {
        "0": 8,
        "1": 2,
        "2": 18,
        "3": 33,
        "4": 3,
        "5": 84,
        "6": 28,
        "7": 121,
        "8": 227,
        "9": 133,
        "10": 2,
        "11": 175,
        "12": 121,
        "13": 26,
        "14": 211,
        "15": 103,
        "16": 80,
        "17": 5,
        "18": 222,
        "19": 172,
        "20": 81,
        "21": 16,
        "22": 222,
        "23": 176,
        "24": 207,
        "25": 4,
        "26": 178,
        "27": 204,
        "28": 198,
        "29": 186,
        "30": 188,
        "31": 60,
        "32": 16,
        "33": 205,
        "34": 204,
        "35": 13,
        "36": 231
    },
    "address": "0x02354A1F160A3fd7ac8b02ee91F04104440B28E7",
    "version": "1.0.0",
    "http": true,
    "p2p": true,
    "provider": [
        {
            "chainId": "1",
            "network": "mainnet"
        },
        {
            "chainId": "137",
            "network": "polygon"
        },
        {
            "chainId": "80001",
            "network": "mumbai"
        }
    ],
    "indexer": [
        {
            "chainId": "1",
            "network": "mainnet",
            "block": "0"
        },
        {
            "chainId": "137",
            "network": "polygon",
            "block": "0"
        },
        {
            "chainId": "80001",
            "network": "mumbai",
            "block": "0"
        }
    ]
}

For testing, I suppose this issue needs to be solved first in order to create an Ocean node class and call this commands when the node is up.
cc @alexcos20

@jamiehewitt15
Copy link
Member

It would be good to see some tests added here

@mariacarmina
Copy link
Member Author

It would be good to see some tests added here

Yes, that's right but for this type of functionality for the moment we cannot create unit tests. When #117 is done, we'll be able to instantiate a OceanNode obj (to start the node directly) and create tests based on that class. For the moment, we can start the node only from command line.

@jamiehewitt15
Copy link
Member

It would be good to see some tests added here

Yes, that's right but for this type of functionality for the moment we cannot create unit tests. When #117 is done, we'll be able to instantiate a OceanNode obj (to start the node directly) and create tests based on that class. For the moment, we can start the node only from command line.

Ok right, yeah that makes sense

Copy link
Contributor

@paulo-ocean paulo-ocean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @mariacarmina
overall, lgtm , just wondering if we should do the public key more "user friendly" to read.. instead of returning the byte array we could do something like
Buffer.from(config.keys.publicKey).toString('hex') for instance
Like we have on the node getPeerDetails()
Just cosmetic anyway, but imo, it might look better & is more aligned with the other fields.
thanks

@mariacarmina
Copy link
Member Author

hi @mariacarmina

overall, lgtm , just wondering if we should do the public key more "user friendly" to read.. instead of returning the byte array we could do something like

Buffer.from(config.keys.publicKey).toString('hex') for instance

Like we have on the node getPeerDetails()

Just cosmetic anyway, but imo, it might look better & is more aligned with the other fields.

thanks

Sure thing, I'll do that to look easier to read. Thanks!

@mariacarmina
Copy link
Member Author

Updated response

Screenshot 2023-11-20 at 22 25 01

@mariacarmina mariacarmina merged commit 807a8d7 into develop Nov 21, 2023
@mariacarmina mariacarmina deleted the feature/status-command branch November 21, 2023 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants