feat(x/tx): legacy amino json sign mode handler #15515
Conversation
…ign-legacy-amino-json
…ign-legacy-amino-json
…ign-legacy-amino-json
…ign-legacy-amino-json
…ign-legacy-amino-json
kocubinski
force-pushed
the
kocubinski/sign-legacy-amino-json
branch
from
51d823b
to
b960845
Compare
…ign-legacy-amino-json
…/cosmos-sdk into kocubinski/sign-legacy-amino-json
…ign-legacy-amino-json
x/tx/signing/testutil/util.go
Outdated
| }, | ||
| } | ||
|
|
||
| //fee := &txv1beta1.Fee{ |
There was a problem hiding this comment.
I guess it should be deleted?
| } | ||
|
|
||
| if signerData.Address == "" { | ||
| return nil, fmt.Errorf("got empty address in %s handler: invalid request", h.Mode()) |
There was a problem hiding this comment.
Is there a reason we do not wrap the errors anymore like in the legacy_amino_json.go file?
There was a problem hiding this comment.
I'm not sure what would be wrapping what, is returning a simple error not enough?
There was a problem hiding this comment.
I don't know, I wondered because I was comparing the two files
| } | ||
| isTipper := tip != nil && tip.Tipper == signerData.Address | ||
|
|
||
| var fee *aminojsonpb.AminoSignFee |
There was a problem hiding this comment.
Can we add the same comment as in the other file legacy_amino_json file, about the no fee and no gas?
…ign-legacy-amino-json
…/cosmos-sdk into kocubinski/sign-legacy-amino-json
| // Textual are options for SIGN_MODE_TEXTUAL | ||
| Textual textual.SignModeOptions | ||
| // DirectAux are options for SIGN_MODE_DIRECT_AUX | ||
| DirectAux direct_aux.SignModeHandlerOptions | ||
| // AminoJSON are options for SIGN_MODE_LEGACY_AMINO_JSON | ||
| AminoJSON aminojson.SignModeHandlerOptions |
There was a problem hiding this comment.
Can we refactor these options to not duplicate shared options? For instance I imagine that a lot of these will require type resolvers and file resolvers. Let's just have individual fields for each type of option here. Okay to do in a separate PR if that's easier
Description
Closes: #15170
Also includes a minor cleanup to #15557, removal of gogoproto dependency mistakenly introduced in the unknown proto testpb files.
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.mdReviewers 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