refactor: Update GetSigners to return []string
#9418
Conversation
github-actions
bot
added
C:x/auth
C:x/authz
C:x/bank
C:x/crisis
C:x/distribution
atheeshp
changed the title
GetSigners to return []stringGetSigners to return []string
Codecov Report
@@ Coverage Diff @@
## master #9418 +/- ##
==========================================
- Coverage 63.54% 63.46% -0.08%
==========================================
Files 570 571 +1
Lines 37474 37524 +50
==========================================
+ Hits 23812 23815 +3
- Misses 11803 11851 +48
+ Partials 1859 1858 -1
|
atheeshp
removed
C:CLI
C:x/auth
C:x/authz
C:x/bank
C:x/crisis
C:x/distribution
github-actions
bot
added
C:x/crisis
C:x/distribution
client/tx/tx.go
Outdated
| @@ -23,6 +23,23 @@ import ( | |||
| authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" | |||
| ) | |||
|
|
|||
| func ValidateMsg(msg sdk.Msg) error { | |||
There was a problem hiding this comment.
Can we have a godoc for ValidateMsg?
baseapp/baseapp.go
Outdated
| @@ -13,6 +13,7 @@ import ( | |||
| tmproto "github.com/tendermint/tendermint/proto/tendermint/types" | |||
| dbm "github.com/tendermint/tm-db" | |||
|
|
|||
| txvalidate "github.com/cosmos/cosmos-sdk/client/tx" | |||
There was a problem hiding this comment.
baseapp should not import anything from client, it creates messy code. Is there somewhere else we can put ValidateMsg?
Edit: Maybe types/tx?
There was a problem hiding this comment.
how about types/utils.go ?
There was a problem hiding this comment.
I believe we're moving away of putting anything in the types root package, it's too large already.
Edit: or put it in types/bech32, and call it bech32.ValidateMsgSigners
We can also create a new types/msg package. @robert-zaremba @aaronc @ryanchristo Do you have any opinions on this?
There was a problem hiding this comment.
or put it in types/bech32, and call it bech32.ValidateMsgSigners
I don't think it is fine to put it in types/bech32, since it is calling msg.ValidateBasic() inside. For now I kept it in the types/tx.
There was a problem hiding this comment.
I would like to avoid adding these as new public functions if possible and that means we don't need to think about packages as much if they're private. Since we are deprecating global bech32's soon that will avoid another API breaking change.
amaury1093
added
the
A:automerge
Description
closes: #9239
Before 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/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerCodecov Reportin the comment section below once CI passes