-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Migrate x/genutil to use TxConfig #6734
Conversation
Codecov Report
@@ Coverage Diff @@
## master #6734 +/- ##
==========================================
+ Coverage 60.22% 61.55% +1.33%
==========================================
Files 389 509 +120
Lines 25361 31525 +6164
==========================================
+ Hits 15274 19406 +4132
- Misses 8919 10610 +1691
- Partials 1168 1509 +341 |
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, but I have a question why we're passing explicitly txJSONDecoder
and txBinaryEncoder
(and not just txGenerator)
I explained this to @blushi. The reason why is that we may get amino json and need to convert to protobuf binary. TxConfig isn't really intended to be used for the purpose. |
…rie/5917-x-genutil
…ronc/6734-updates
Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>
…os-sdk into aaronc/6734-updates
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.
Mostly LGTM. Caught one piece that isn't migrated.
…rie/5917-x-genutil
* Update genutil collect and gentx to use TxGenerator * Remove print statement * Use Tx in genutil DeliverGenTxs * Use Tx in genutil genesis_state * Use Tx in ValidateGenesis * Use amino txJSONDecoder and txBinaryEncoder in genutil InitGenesis * Use TxConfig in place of TxGenerator * Add gentx tests * Remove commented line * Test fixes * Apply suggestions from code review Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com> * Fixes * Fixes * Fixes * Fixes * Remove unneeded test case (doesn't apply to proto marshaling) * linting * Refactor to use new TxEncodingConfig interface in genutil module * Replace golang/protobuf with gogo/protobuf package * Use TxEncodingConfig in InitTestnet * Remove old amino.go file * Use TxJSONDecoder in genutil ValidateGenesis * Add parameter to ValidateGenesis to resolve the tx JSON decoder issue * Address review feedback Co-authored-by: Jack Zampolin <jack.zampolin@gmail.com> Co-authored-by: Aaron Craelius <aaronc@users.noreply.github.com> Co-authored-by: Aaron Craelius <aaron@regen.network> Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Description
ref: #6213 , #5917
x/genutil/client/cli
covered as part of #6717Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/
) or specification (x/<module>/spec/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerCodecov Report
in the comment section below once CI passes