-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: multi zone - plugin & reflection #8
Conversation
* added plugins feature and reflection features * fixed minor sdkTx variables issue * Roll back some changes on config.go * fixed makefile build command * improvements over lint * Added nolint ant changed variable name * fix .gitworkflow build * fix .gitworkflow build * Added makefile build plugin steps * added nolint on staticcheck on load.go * added nolint on staticcheck on load.go * added nolint on staticcheck on load.go * added nolint on staticcheck on load.go * gofumpt on project * skiped a test due to reliance on an experimental feature of cosmos v0.46.0-beta * build plugin before test * build plugin before test * fixed failing test * fix make test * added docs * small refactor on variable names
CC: @julienrbrt who should we add as reviewers of this code? |
Hum, I don't know. Are you (Zondax) not the sole maintainer of this now 😬? |
Thx for the response @julienrbrt ! We are the main mainteiners, thought it would be nice to check it with other devs who were involved on it, lets sync up with @tac0turtle |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
my only concern is the plugin path is not configurable right now but we can address that in other pr
@@ -6,10 +6,13 @@ rosetta: | |||
go build -mod=readonly ./cmd/rosetta | |||
|
|||
build: | |||
go build ./cmd/rosetta.go | |||
go build -mod=readonly ./cmd/rosetta | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whats the difference between this two? should we compile inside a build dir??
README.md
Outdated
go install cosmossdk.io/rosetta/cmd/rosetta | ||
``` | ||
|
||
Alternatively, for building from source, simply run `make rosetta`. The binary will be located in `tools/rosetta`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is not correct right now as make rosetta
compiles the binary in the root project dir
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think this has been yet updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK.
Unrelated question: are you going to submit a PR to remove it from the SDK main?
README.md
Outdated
go install cosmossdk.io/rosetta/cmd/rosetta | ||
``` | ||
|
||
Alternatively, for building from source, simply run `make rosetta`. The binary will be located in `tools/rosetta`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think this has been yet updated.
github.com/cosmos/cosmos-sdk v0.46.0-beta2.0.20230614103911-b3da8bb4e801 | ||
github.com/cosmos/gogoproto v1.4.10 | ||
github.com/cometbft/cometbft v0.37.2 | ||
github.com/cosmos/cosmos-sdk v0.47.3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that we already have a Rosetta version released for v0.47, doesn't it makes more sense to integrate with 0.50 already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yaah it makes sense, i would wait until v0.50 is out of alpha tbh.
Once merged this I will move on to clean up rosetta from cosmos-sdk
thx for the review :)
* added plugins feature and reflection features * fixed minor sdkTx variables issue * Roll back some changes on config.go * fixed makefile build command * improvements over lint * Added nolint ant changed variable name * fix .gitworkflow build * fix .gitworkflow build * Added makefile build plugin steps * added nolint on staticcheck on load.go * added nolint on staticcheck on load.go * added nolint on staticcheck on load.go * added nolint on staticcheck on load.go * gofumpt on project * skiped a test due to reliance on an experimental feature of cosmos v0.46.0-beta * build plugin before test * build plugin before test * fixed failing test * fix make test * added docs * small refactor on variable names * Updated documentation and added --plugin flag * updated install step on README * lint fix * updated docs
Any clue why we cannot merge this? |
For some reason, the approval didn't work. Re-approved and now it seems you can merge. |
* feat: multi zone - plugin & reflection (#5) * added plugins feature and reflection features * fixed minor sdkTx variables issue * Roll back some changes on config.go * fixed makefile build command * improvements over lint * Added nolint ant changed variable name * fix .gitworkflow build * fix .gitworkflow build * Added makefile build plugin steps * added nolint on staticcheck on load.go * added nolint on staticcheck on load.go * added nolint on staticcheck on load.go * added nolint on staticcheck on load.go * gofumpt on project * skiped a test due to reliance on an experimental feature of cosmos v0.46.0-beta * build plugin before test * build plugin before test * fixed failing test * fix make test * added docs * small refactor on variable names * Zondax/feature/multi zone fix * added plugins feature and reflection features * fixed minor sdkTx variables issue * Roll back some changes on config.go * fixed makefile build command * improvements over lint * Added nolint ant changed variable name * fix .gitworkflow build * fix .gitworkflow build * Added makefile build plugin steps * added nolint on staticcheck on load.go * added nolint on staticcheck on load.go * added nolint on staticcheck on load.go * added nolint on staticcheck on load.go * gofumpt on project * skiped a test due to reliance on an experimental feature of cosmos v0.46.0-beta * build plugin before test * build plugin before test * fixed failing test * fix make test * added docs * small refactor on variable names * Updated documentation and added --plugin flag * updated install step on README * lint fix * updated docs * updated go.sum (#8) * refactor: Upgrade rosetta to cosmos-sdk v0.50.0 (#7) * changed go.mod name, renamed dependencies on rosetta, adapted methods and uses of cosmos-sdk, upgraded dependencies * fixed all code errors * improvements over parsing * fees and gas parsing * remmoved validation on message due to deprecation (it should be validated on the server side) * solved issue on missing tx tipper * added codec types * fixed interface registre addr issue * improvements * fixed converter tests * commented plugin_test.go due to some modules not being available at v0.50 yet * improved code, moved parsing functions to utils.go * solved comments, fixed lint and added utils.go for parsing * fixed comment on cast variables * lint comments * Commented ibc-go dependdency until upgrade * added type assertion error handlinhg * updated go.sum * rename module for vanity url * update go.mod and go.sum
* feat: multi zone - plugin & reflection (#5) * added plugins feature and reflection features * fixed minor sdkTx variables issue * Roll back some changes on config.go * fixed makefile build command * improvements over lint * Added nolint ant changed variable name * fix .gitworkflow build * fix .gitworkflow build * Added makefile build plugin steps * added nolint on staticcheck on load.go * added nolint on staticcheck on load.go * added nolint on staticcheck on load.go * added nolint on staticcheck on load.go * gofumpt on project * skiped a test due to reliance on an experimental feature of cosmos v0.46.0-beta * build plugin before test * build plugin before test * fixed failing test * fix make test * added docs * small refactor on variable names * Zondax/feature/multi zone fix * added plugins feature and reflection features * fixed minor sdkTx variables issue * Roll back some changes on config.go * fixed makefile build command * improvements over lint * Added nolint ant changed variable name * fix .gitworkflow build * fix .gitworkflow build * Added makefile build plugin steps * added nolint on staticcheck on load.go * added nolint on staticcheck on load.go * added nolint on staticcheck on load.go * added nolint on staticcheck on load.go * gofumpt on project * skiped a test due to reliance on an experimental feature of cosmos v0.46.0-beta * build plugin before test * build plugin before test * fixed failing test * fix make test * added docs * small refactor on variable names * Updated documentation and added --plugin flag * updated install step on README * lint fix * updated docs * updated go.sum (#8) * refactor: Upgrade rosetta to cosmos-sdk v0.50.0 (#7) * changed go.mod name, renamed dependencies on rosetta, adapted methods and uses of cosmos-sdk, upgraded dependencies * fixed all code errors * improvements over parsing * fees and gas parsing * remmoved validation on message due to deprecation (it should be validated on the server side) * solved issue on missing tx tipper * added codec types * fixed interface registre addr issue * improvements * fixed converter tests * commented plugin_test.go due to some modules not being available at v0.50 yet * improved code, moved parsing functions to utils.go * solved comments, fixed lint and added utils.go for parsing * fixed comment on cast variables * lint comments * Commented ibc-go dependdency until upgrade * added type assertion error handlinhg * updated go.sum * rename module for vanity url * update go.mod and go.sum * refactor: error handling improvements (#9) * issue on load.go * fixed tests * added client offline * client_online.go * added error handling on utils.go and unified both files * added plugins.go * added config, converter and libraries * linted and fixed issues * fixed some issues
Description
Main feature: Enable Rosetta multi zone compatibility
Closes:
Changes on this PR
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!
in the type prefix if API or client breaking change