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

Rosetta Implementation - pt1 (Stage 3.1 of Node API Overhaul) #3297

Merged
merged 19 commits into from
Aug 14, 2020

Conversation

Daniel-VDM
Copy link
Contributor

@Daniel-VDM Daniel-VDM commented Aug 13, 2020

Stage 3.1 of Node API Overhaul

This PR adds the rosetta server to the node program. The rosetta server is hosted on port 9700 by default on an explorer node. Other nodes can enable rosetta it if they wish, but by default, it is off for all validator nodes.

This PR also outlines the structure of the rosetta package to follow the example provided here.

The server was tested using rosetta's example client here and passes.

In order to use rosetta's SDK, we must use go-ethereum v1.9.18. As this is not compatible with our current code, I've added a replace in the go.mod file to support go-ethereum v1.8.27 as well. I've also updated/cleaned up the relative packages needed for rosetta's SDK.

* Update go.mod for rosetta SDK

Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
@Daniel-VDM Daniel-VDM added the rpc RPC or API label Aug 13, 2020
@Daniel-VDM Daniel-VDM self-assigned this Aug 13, 2020
Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
@Daniel-VDM Daniel-VDM changed the title Node API Refactor - pt3 (Stage 3.1 of Node API Overhaul) [WIP] Node API Refactor - pt3 (Stage 3.1 of Node API Overhaul) Aug 13, 2020
Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
* Make go.mod changes minimal

Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
@Daniel-VDM Daniel-VDM force-pushed the rosetta-server-impl branch from 28b386b to e046938 Compare August 13, 2020 17:02
Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
@Daniel-VDM Daniel-VDM changed the title [WIP] Node API Refactor - pt3 (Stage 3.1 of Node API Overhaul) Node API Refactor - pt3 (Stage 3.1 of Node API Overhaul) Aug 13, 2020
Copy link
Contributor

@JackyWYX JackyWYX left a comment

Choose a reason for hiding this comment

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

  1. Since we are adding new ports, we should do our best try not to collide our port with well-known services. 10000 is binded with NDMP (https://www.snia.org/ndmp). Can we use another one?

  2. You need to consider the backward compatibility of the harmonyConfig structure (It's dumped to the config file along with a version). My suggestions are:

    • Increase the version to v1.1.0. (Preferred to import the version module from libp2p)
    • If user are still using old version , recommend user to upgrade to config v1.1.0 with command ./harmony dumpconfig harmony.conf
    • Add a unit test case to make sure the data loaded from v1.0.0 config file is expected.

Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
* Included update message if old config is loaded

Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
@Daniel-VDM
Copy link
Contributor Author

  1. Since we are adding new ports, we should do our best try not to collide our port with well-known services. 10000 is binded with NDMP (https://www.snia.org/ndmp). Can we use another one?

  2. You need to consider the backward compatibility of the harmonyConfig structure (It's dumped to the config file along with a version). My suggestions are:

    • Increase the version to v1.1.0. (Preferred to import the version module from libp2p)
    • If user are still using old version , recommend user to upgrade to config v1.1.0 with command ./harmony dumpconfig harmony.conf
    • Add a unit test case to make sure the data loaded from v1.0.0 config file is expected.
  1. Changed the port to 9700
  2. I bumped the config version and tested running the node with the old config, it works. Also added a unit test and warning/suggestion message if loading an old config.

Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
@Daniel-VDM Daniel-VDM force-pushed the rosetta-server-impl branch from 05d69ae to 8707e7e Compare August 14, 2020 06:47
@Daniel-VDM Daniel-VDM merged commit 8068d80 into harmony-one:main Aug 14, 2020
@Daniel-VDM Daniel-VDM changed the title Node API Refactor - pt3 (Stage 3.1 of Node API Overhaul) Rosetta Implementation - pt1 (Stage 3.1 of Node API Overhaul) Aug 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rpc RPC or API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants