-
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
Update TxBuilder to use SignatureV2 #6443
Conversation
Codecov Report
@@ Coverage Diff @@
## master #6443 +/- ##
==========================================
+ Coverage 55.74% 55.83% +0.09%
==========================================
Files 466 466
Lines 27890 27906 +16
==========================================
+ Hits 15546 15582 +36
+ Misses 11231 11210 -21
- Partials 1113 1114 +1 |
…ronc/6213-sig-v2-builder
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.
@aaronc looks good, added a few comments
// StdTxBuilder wraps StdTx to implement to the context.TxBuilder interface | ||
// StdTxBuilder wraps StdTx to implement to the context.TxBuilder interface. | ||
// Note that this type just exists for backwards compatibility with amino StdTx | ||
// and will not work for protobuf transactions. |
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.
This file has low coverage and we should prob add unit tests on another PR
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 agree we should cover it better. @anilcse let's coordinate on a PR to test TxBuilder
implementations generically.
This is one of several bite-size PR's that are being pulled out of #6216 to implement #6213.
This PR:
client.TxBuilder
to useSignatureV2
introduced in Add SignatureV2 infrastructure #6373sign-mode
command-line flagFeeTx
andTxWithMemo
fromx/auth/ante
totypes
to deal with cyclic dependency issuesLegacyAminoJSONHandler
directly intox/auth/types
to avoid cyclic dependenciesBefore 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 explorer