Ensure IBC Proto Registration #7210
Conversation
jackzampolin
requested review from
aaronc,
alessio and
alexanderbez
as code owners
Codecov Report
@@ Coverage Diff @@
## master #7210 +/- ##
==========================================
+ Coverage 54.91% 54.98% +0.07%
==========================================
Files 562 562
Lines 38623 38680 +57
==========================================
+ Hits 21210 21270 +60
+ Misses 15666 15663 -3
Partials 1747 1747 |
|
Did these cause errors when not being registered? I was under the impression only types being used as an or maybe there is another use case? Only the tendermint msgs and header should have been necessary to register (but this is already fixed on master) |
|
@jackzampolin can you check if your issue is resolved on master? |
fedekunze
added
the
A:automerge
| @@ -256,7 +256,7 @@ func BuildSimTx(txf Factory, msgs ...sdk.Msg) ([]byte, error) { | |||
| // sentinel pubkey. | |||
| sig := signing.SignatureV2{ | |||
| Data: &signing.SingleSignatureData{ | |||
| SignMode: txf.signMode, | |||
| SignMode: txf.WithSignMode(signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON).signMode, | |||
There was a problem hiding this comment.
I feel like this is suspicious to set SIGN_MODE_LEGACY_AMINO. I thought simulation was switched over to SIGN_MODE_DIRECT
There was a problem hiding this comment.
This is specifically for the gas simulation. I don't think it makes a huge difference but not 100% sure
There was a problem hiding this comment.
Yeah, signatures aren't verified in simulation mode, so I believe it doesn't make a difference.
Anyways, it's reverted here, to make it cleaner: https://github.com/cosmos/cosmos-sdk/pull/7207/files#diff-df9cc9b2c6d08e86dbf0554ea3adb00aR259
There was a problem hiding this comment.
This was causing a test to fail but maybe we fixed it another way 🤷♂️ fine with reverting and thank you @amaurymartiny
There are a number of missing proto registrations in the IBC module currently. This PR fixes that issue.