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 - pt2 (Stage 3.2 of Node API Overhaul) #3312

Merged
merged 39 commits into from
Aug 25, 2020

Conversation

Daniel-VDM
Copy link
Contributor

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

Stage 3.2 of Node API Overhaul

This PR implements Rosetta's /network & /block node endpoint services.

This PR also adds unit tests to the rosetta package to ensure correct formatting behavior.

Basic functionality was tested using a fork of Rosetta's CLI (to check transactions). Edge case testing with a testnet node is currently being done (8/22).

Here is the unit test coverage:

PASS
coverage: 56.0% of statements
ok  	github.com/harmony-one/harmony/rosetta/services	0.413s

As we don't currently have an easy way to create a mock harmony object, this is probably the best we can do without investing a bunch of time to create a testing package (which I think we should do at some point).

@Daniel-VDM Daniel-VDM added the rpc RPC or API label Aug 22, 2020
@Daniel-VDM Daniel-VDM self-assigned this Aug 22, 2020
Copy link
Contributor

@rlan35 rlan35 left a comment

Choose a reason for hiding this comment

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

generally looks good.

node/api.go Outdated Show resolved Hide resolved
rosetta/services/block.go Show resolved Hide resolved
Copy link
Contributor

@LeoHChen LeoHChen left a comment

Choose a reason for hiding this comment

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

LGTM. Is rosetta api test part of regression now? also, can we add it to nightly?

rosetta/rosetta.go Outdated Show resolved Hide resolved
@Daniel-VDM
Copy link
Contributor Author

LGTM. Is rosetta api test part of regression now? also, can we add it to nightly?

We don't have all the endpoints implemented yet for the rosetta integration test. Will add it as part of the PR testing process after implementation, and can add it as a stage to the nighly check afterward.

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>
* Implement /network/list

Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
* Rename *_service.go files to remove the suffix
* Update StartServers to use new operation types

Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
* Fix import structure for rosetta.go

Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
* Force errors to remain the same with unit tests
* Force operations to remain the same with unit tests
* Ensure network checking works for all cases with unit tests

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>
* Make names consistent

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>
* Add Error creator

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>
* Updated todo comments & function formatting

Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
…m metadata

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>
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>
…erKey

Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
* Add unit tests for supporting helper functions

Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
* Print stack trace on panic recovery

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

rlan35 commented Aug 25, 2020

looks good.

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