Skip to content
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

Multiple assigns to TagAction for Msg #3966

Closed
rigelrozanski opened this issue Mar 23, 2019 · 5 comments
Closed

Multiple assigns to TagAction for Msg #3966

rigelrozanski opened this issue Mar 23, 2019 · 5 comments
Assignees
Labels
good first issue T:Bug T:Docs Changes and features related to documentation.
Milestone

Comments

@rigelrozanski
Copy link
Contributor

rigelrozanski commented Mar 23, 2019

Just noticed that many of the modules use the action tag, but also the action tag is appended for each message within baseapp (here https://github.com/cosmos/cosmos-sdk/blob/develop/baseapp/baseapp.go#L690)

We should consolidate so either modules don't use the action tag for each message, or baseapp doesn't append an action tag

Additionally, the spec tag sections must be updated if we go with the latter choice.

CC @jackzampolin @cwgoes @alexanderbez

Relevant search:

types/tags.go:73:	TagAction       = "action"
baseapp/baseapp.go:690:		tags = append(tags, sdk.MakeTag(sdk.TagAction, msg.Type()))
client/config.go:54:	getAction := viper.GetBool(flagGet)
client/config.go:55:	if getAction && len(args) != 1 {
client/config.go:77:	if getAction {
x/bank/tags.go:5:	TagActionUndelegateCoins = []byte("undelegateCoins")
x/bank/tags.go:6:	TagActionDelegateCoins   = []byte("delegateCoins")
x/bank/keeper.go:384:		sdk.TagAction, TagActionDelegateCoins,
x/bank/keeper.go:409:		sdk.TagAction, TagActionUndelegateCoins,
x/staking/alias.go:171:	ActionCompleteUnbonding    = tags.ActionCompleteUnbonding
x/staking/alias.go:172:	ActionCompleteRedelegation = tags.ActionCompleteRedelegation
x/staking/alias.go:174:	TagAction       = tags.Action
x/staking/handler.go:63:			tags.Action, ActionCompleteUnbonding,
x/staking/handler.go:79:			tags.Action, tags.ActionCompleteRedelegation,
x/staking/tags/tags.go:9:	ActionCompleteUnbonding    = "complete-unbonding"
x/staking/tags/tags.go:10:	ActionCompleteRedelegation = "complete-redelegation"
x/staking/tags/tags.go:12:	Action       = sdk.TagAction
x/staking/client/rest/query.go:154:			actions = append(actions, tags.ActionCompleteUnbonding)
x/staking/client/rest/query.go:157:			actions = append(actions, tags.ActionCompleteRedelegation)
x/staking/client/rest/query.go:161:			actions = append(actions, tags.ActionCompleteUnbonding)
x/staking/client/rest/query.go:163:			actions = append(actions, tags.ActionCompleteRedelegation)
x/staking/client/rest/utils.go:35:	query := fmt.Sprintf("%s='%s' AND %s='%s'", tags.Action, tag, tags.Delegator, delegatorAddr)
x/slashing/handler.go:64:		tags.Action, tags.ActionValidatorUnjailed,
x/slashing/tags/tags.go:9:	ActionValidatorUnjailed = "validator-unjailed"
x/slashing/tags/tags.go:11:	Action    = sdk.TagAction
x/gov/tags/tags.go:9:	ActionProposalDropped  = "proposal-dropped"
x/gov/tags/tags.go:10:	ActionProposalPassed   = "proposal-passed"
x/gov/tags/tags.go:11:	ActionProposalRejected = "proposal-rejected"
x/gov/tags/tags.go:13:	Action            = sdk.TagAction
x/gov/endblocker.go:30:		resTags = resTags.AppendTag(tags.ProposalResult, tags.ActionProposalDropped)
x/gov/endblocker.go:59:			tagValue = tags.ActionProposalPassed
x/gov/endblocker.go:63:			tagValue = tags.ActionProposalRejected
x/gov/client/utils/query.go:45:		fmt.Sprintf("%s='%s'", tags.Action, gov.MsgDeposit{}.Type()),
x/gov/client/utils/query.go:90:		fmt.Sprintf("%s='%s'", tags.Action, gov.MsgVote{}.Type()),
x/gov/client/utils/query.go:130:		fmt.Sprintf("%s='%s'", tags.Action, gov.MsgVote{}.Type()),
x/gov/client/utils/query.go:173:		fmt.Sprintf("%s='%s'", tags.Action, gov.MsgDeposit{}.Type()),
x/gov/client/utils/query.go:216:		fmt.Sprintf("%s='%s'", tags.Action, gov.MsgSubmitProposal{}.Type()),
@rigelrozanski rigelrozanski changed the title Multiple TagAction Multiple assigns to TagAction for Msg Mar 23, 2019
@rigelrozanski rigelrozanski added the T:Docs Changes and features related to documentation. label Mar 23, 2019
@cwgoes
Copy link
Contributor

cwgoes commented Mar 24, 2019

Yes, we should. A sanity check for duplicate tags (at least logging a warning) might be prudent.

@alexanderbez
Copy link
Contributor

alexanderbez commented Mar 25, 2019

Yeah totally agree here. My gut tells me the baseapp should continue to perform its role in adding the action tag -- this provides less things to worry about in each handler and requires less docs update.

@fedekunze fedekunze self-assigned this Apr 4, 2019
@fedekunze fedekunze added this to the v0.35.0 milestone Apr 4, 2019
@fedekunze
Copy link
Collaborator

fedekunze commented Apr 4, 2019

how are this tags useful if they don't involve any transaction (i.e they are not queryable)? Should we get rid of them ?

x/staking/tags/tags.go:9:	ActionCompleteUnbonding    = "complete-unbonding"
x/staking/tags/tags.go:10:	ActionCompleteRedelegation = "complete-redelegation"
x/gov/tags/tags.go:9:	ActionProposalDropped  = "proposal-dropped"
x/gov/tags/tags.go:10:	ActionProposalPassed   = "proposal-passed"
x/gov/tags/tags.go:11:	ActionProposalRejected = "proposal-rejected"

@fedekunze
Copy link
Collaborator

also what's a delegateCoins and undelegateCoins action ?

@alexanderbez
Copy link
Contributor

@fedekunze we should do a sweep through and see what tags are not used anymore and why they're not used. Remove the ones that you deem useless.

delegateCoins and undelegateCoins get called from the bank keeper when bonding or unbonding tokens.

alessio pushed a commit that referenced this issue Apr 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue T:Bug T:Docs Changes and features related to documentation.
Projects
None yet
Development

No branches or pull requests

4 participants