feat(bank): Create message for Setting SendEnabled settings #11981

dwedul-figure · 2022-05-17T21:37:50Z

Description

This PR is related to #11977. It creates and implements MsgSetSendEnabled, a message for use as a governance proposal to update the SendEnabled information.

Note that this PR is currently set to merge into dwedul/11859-send-disabled-change instead of main. This PR is dependent on functionality added by that branch. Once that PR has merged into main, I'll update this to also merge into main.

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...

Reviewers 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...

confirmed the correct type prefix in the PR title
confirmed ! in the type prefix if API or client breaking change
confirmed all author checklist items have been addressed
reviewed state machine logic
reviewed API design and naming
reviewed documentation is accurate
reviewed tests and test coverage
manually tested (if applicable)

…or getting, setting, and deleting them.

…cluded." This reverts commit 8b7acf8.

…roto." This reverts commit 0bd904c.

…oto." This reverts commit 33d4652.

…endEnabledCoins. Store creation has some overhead.

…ple TODO notes. Create a way to iterate over the SendEnabled entries and get all of them.

…ed entries similar to when they were params.

… function.

…unction significantlly simpler.

# Conflicts: # api/cosmos/bank/v1beta1/bank.pulsar.go # api/cosmos/bank/v1beta1/genesis.pulsar.go

…o longer returns {} when there aren't any SendEnabled entries and the default is false. SendEnabled is back to outputting a yaml format.

…ries at once.

… set SendEnabled.

…or MsgSetSendEnabled.

dwedul-figure · 2022-08-15T14:47:15Z

@dwedul-figure are you waiting on a review from the sdk team here?

Yup. Sorry, I had let it get a bit behind, but it's all good now.

There's just the question of the SetSendEnabled endpoint vs UpdateParams. As things are right now (in this branch), the SetSendEnabled can do everything that UpdateParams does. That suggests we might want to get rid of the UpdateParams endpoint. However, UpdateParams is a standard for modules that have params, so it might be worth keeping it around (for familiarity). Along that line, if a param were ever added to the bank module that isn't associated with the send-enabled stuff, we'd want an UpdateParams endpoint for sure.

My personal gut feeling is to have both; the overlapping responsibilities are small enough that it shouldn't be much of an extra burden. But I also don't mind getting rid of UpdateParams. Another alternative is to remove the default stuff from SetSendEnabled, but I'm not a fan of that option. I feel like there will be times when it is desirable to set some flags and change the default, all at the same time and make them part of the same governance proposal.

Just let me know which direction I should go on that.

# Conflicts: # CHANGELOG.md

…mos/cosmos-sdk into dwedul/11859-send-enabled-w-gov-prop

# Conflicts: # CHANGELOG.md

…changes).

# Conflicts: # CHANGELOG.md

… fields from MsgSetSendEnabled.

…nabled and default_send_enabled fields.

# Conflicts: # CHANGELOG.md

tac0turtle

utACK

addressed

# Conflicts: # CHANGELOG.md # x/bank/spec/03_messages.md # x/bank/spec/README.md

…mos/cosmos-sdk into dwedul/11859-send-enabled-w-gov-prop # Conflicts: # CHANGELOG.md

…mos-sdk/simapp) in x/bank/app_test.go and update other go mod stuff associated with that.

…1981) * go mod tidy everything. * Add some third_party proto files that are imported but not included. * [11859]: Add a new key for the SendEnabled flags and keeper methods for getting, setting, and deleting them. * [11859]: Remove the send_enabled field from the bank Params proto. * Revert "Add some third_party proto files that are imported but not included." This reverts commit 8b7acf8. * [11859]: Regenerate the bank params stuff from the changed proto. * [11859]: Add a send_enabled field to the bank genesis proto. * Revert "[11859]: Remove the send_enabled field from the bank Params proto." This reverts commit 0bd904c. * Revert "[11859]: Regenerate the bank params stuff from the changed proto." This reverts commit 33d4652. * [11859]: Deprecate the bank Params send_enabled field. * [11859]: Regenerate the bank go code from the updated protos. * [11859]: Reduce the number of times the store is recreated during IsSendEnabledCoins. Store creation has some overhead. * [11859]: Add the SendEnabled stuff to the genesis methods. Make a couple TODO notes. Create a way to iterate over the SendEnabled entries and get all of them. * [11859]: Update the bank sim genesis stuff to create random SendEnabled entries similar to when they were params. * Remove some of the bank params methods that are no longer meaningful. * Add a comment about why we're calling a mutation method in a Validate function. * [11859]: Add some more TODO notes and make the SendEnabled.String() function significantlly simpler. * [11859]: Get rid of the SendEnabledParams type. * Fix up a few comments. * [11859]: Update the bank keeper test due to recent changes. * [11859]: Tweak the bank Params and SendEnabled String funcs. Params no longer returns {} when there aren't any SendEnabled entries and the default is false. SendEnabled is back to outputting a yaml format. * [11859]: Fix the params tests and add some new ones to it and key_test. * [11859]: Create a 1-store method for updating several SendEnabled entries at once. * [11859]: Create a migration for both the module and genesis state. * [11859]: Create a new MsgSetSendEnabled for governanance proposals to set SendEnabled. * [11859]: Add SetAllSendEnabled to the SendKeeper interface. * [11859]: Add an authority to the bank keeper and create the handler for MsgSetSendEnabled. * [11859]: Add an rpc endpoint for querying SendEnabled. * [11859]: Implement the SendEnabled query. * [11859]: Add a function for decoding a --page-key base64 value so that pagination can work as expected. * [11859]: Implement a CLI command for querying SendEnabled. * [11859]: Move the v047 store migration stuff into Migrate3to4 directly to prevent a circular dependency between 047 and the keeper. Not using the keeper for that would be a significant pain in the butt. * [11869]: Implement the Msg interface for MsgSetSendEnabled. * [11859]: Fix some unit tests that I broke along the way. * [11859]: Reorg the funcs added to the SendKeeper interface. * [11859]: Fix the return values of a couple of the MsgSetSendEnabled LegacyMsg funcs. * [11859]: Tweak MigrateSendEnabled to add stuff to the existing slice (if there's anything to add). And then use that in the MigrateGenState function. * [11859]: Don't set the Pagination field when looking up specific entries. * [11859]: Put validateSendEnabledParams back to the way it was to allow reading the old Params without error. * [11859]: Write up a bunch of unit tests. * [11859]: Update the MsgSetSendEnabled.ValidateBasic() function with some extra failure points. Write up some tests. * Update a test I fixed then broke. * [11859]: Have the run-tests make target exit with a non-zero status if any of the tests fail. * [11859]: Add changelog entries. * [11859]: Add a missing func comment. * [11859]: Only do a couple assertions if the elements exist to do so. * [11859]: Add some more missing function comments. * [11859]: Update the bank spec documentation. * [11859]: Change name of WithPageKeyDecoded to FlagSetWithPageKeyDecoded, have it return an error and make MustFlagSetWithPageKeyDecoded for the one-liner. * [11859]: Update the documentation on the SendEnabled query. * [11859]: Add final newline to query.proto. * [11859]: Upddate the SetSendEnabled endpoint to allow deleting entries and update the default value. * [11859]: Regen code from protos to include recent changes. * [11859]: Implement the functionality newly added to MsgSetSendEnabled. * [11859]: Remove the SetSendEnabled msg and endpoint. * [11859]: Add new fields to NewMsgSetSendEnabled. * [11859]: Tweak the incorrect authority error a little to make it clearer. * [11859]: Add some unit tests on handling of the MsgSetSendEnabled message. * [11859]: Change the use_default field to use_default_for to make it less ambiguous. * [11859]: Tweak a test to check that DeleteSendEnabled works with multiple denoms. * [11859]: Add a test about a bad denom in the UseDefaultFor list. * [11859]: Use nil instead of an empty slice of SendEnabled for defaults and where called for. * [11859]: Update SetParams to migrate entries too. * [11859]: Remove the spec doc info about the MsgSetSendEnabled that's part of another PR. * [11859]: Update the bank spec docs again since that last merge undid it. * [11859]: Update the changelog. * Revert "[11859]: Update the changelog." This reverts commit 85052b8. * [11859]: Update the changelog. * [11859]: Rename the QuerySendEnabled message to QuerySendEnabledRequest to match the other messages in that proto. * [11859]: Remove the authority field that is only needed for governance stuff (in the other PR). * Revert "[11859]: Remove the authority field that is only needed for governance stuff (in the other PR)." This reverts commit 93f4148. * [11859]: Add a version to the deprecation message. * [11859]: Add unit test on MsgSetSendEnabled with the correct authority, but not signed by that address. * [11859]: Update the comment on the now-deprecated SendEnabled params proto field to reference 0.46 instead of 0.47. * Add some spacing to GetCmdQuerySendEnabled -> RunE. * [11859]: Create banktypes.GenesisState.GetAllSendEnabled() to house the combination logic of the SendEnabled field and Params.SendEnabled. Have MigrateSendEnabled() use that. Remove some calls to MigrateSendEnabled and use GetAllSendEnabled in those cases. * [11859]: Update Bank's ConsensusVersion to 4. * [11859]: Add 'Since' comments to the new proto stuff. * Add the authority to the bankInputs so it can be provided to NewBaseKeeper. I've no clue how that will get set though. * [11859]: Fix a unit test that broke because it assumed the bank module's version was 3. * [11859]: Remove an empty line. Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> * [11859]: Remove movement of SendEnabled from the `ExportGenesis` function too. Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> * [11859]: Hard-code the bank authority value with a TODO to allow injection of that value. * [11859]: Inject the authority string for the bank module provider. * [11859]: inject the bank authority value for module unit tests that need it. * [11859]: Add a unit test for MsgSendEnabled.ValidateBasic that has an empty authority. * [11859]: Don't require a MsgSetSendEnabled.Authority value in ValidateBasic. Maybe that field is supposed to be filed in by the governance module before passing on the message. * [11859]: Update the capability testutil app_config to include the new bank authority value. * [11859]: Undo the banktypes.Authority type thing. * [11859]: Fix bad merge piece. * Fix a couple unit tests that broke with the merge. * [11859]: Remove the Route and Type LegacyMsg functions from the new MsgSetSendEnabled. * [11859]: Add 'Since: cosmos-sdk 0.47' to the new proto stuff. * [11859]: Get rid fo the BaseKeeper.GetAuthority since it was added to the SendKeeper. * [11859]: emit an event when processing a SetSendEnabled. * [11859]: Update MsgSetSendEnabled.ValidateBasic to return wrapped sdk errors. * Add a link in the bank spec readme to the MsgUpdateParams. * rename govtypesv1 import to govv1 since that's what's used elsewhere. * [11859]: Update changelog link to point at the PR instead of the issue number. * [11859]: Add some extra test cases for GetAuthority. * Move changelog entry to unreleased. * [11859]: Change a call to moduletestutil.MakeTestEncodingConfig (from simapp.MakeTestEncodingConfig). * [11859]: Add SetSendEnabled to the MockBankKeeper. * [11859]: make mocks. * [11859]: Fix the bank app tests (had compilation issues due to merge changes). * [11859]: Remove the set_default_send_enabled and default_send_enabled fields from MsgSetSendEnabled. * [11859]: Remove any uses/references to the removed set_default_send_enabled and default_send_enabled fields. Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> Co-authored-by: Daniel Wedul <github@wedul.com> Co-authored-by: Marko <marbar3778@yahoo.com>

dwedul-figure added 30 commits May 11, 2022 10:14

go mod tidy everything.

6ba7d5e

Add some third_party proto files that are imported but not included.

8b7acf8

[11859]: Add a new key for the SendEnabled flags and keeper methods f…

bc7fe5e

…or getting, setting, and deleting them.

[11859]: Remove the send_enabled field from the bank Params proto.

0bd904c

Revert "Add some third_party proto files that are imported but not in…

80de698

…cluded." This reverts commit 8b7acf8.

[11859]: Regenerate the bank params stuff from the changed proto.

33d4652

[11859]: Add a send_enabled field to the bank genesis proto.

505c5b6

Revert "[11859]: Remove the send_enabled field from the bank Params p…

174170a

…roto." This reverts commit 0bd904c.

Revert "[11859]: Regenerate the bank params stuff from the changed pr…

0cca158

…oto." This reverts commit 33d4652.

[11859]: Deprecate the bank Params send_enabled field.

0c70071

[11859]: Regenerate the bank go code from the updated protos.

26aa8ac

[11859]: Reduce the number of times the store is recreated during IsS…

e8992e2

…endEnabledCoins. Store creation has some overhead.

[11859]: Add the SendEnabled stuff to the genesis methods. Make a cou…

6701d63

…ple TODO notes. Create a way to iterate over the SendEnabled entries and get all of them.

[11859]: Update the bank sim genesis stuff to create random SendEnabl…

ccdc316

…ed entries similar to when they were params.

Remove some of the bank params methods that are no longer meaningful.

ebeda16

Add a comment about why we're calling a mutation method in a Validate…

4d3a0b4

… function.

[11859]: Add some more TODO notes and make the SendEnabled.String() f…

e418780

…unction significantlly simpler.

Merge branch 'main' into dwedul/11859-send-disabled-change

5327c95

# Conflicts: # api/cosmos/bank/v1beta1/bank.pulsar.go # api/cosmos/bank/v1beta1/genesis.pulsar.go

[11859]: Get rid of the SendEnabledParams type.

3c76823

Fix up a few comments.

1e86801

[11859]: Update the bank keeper test due to recent changes.

4390747

[11859]: Tweak the bank Params and SendEnabled String funcs. Params n…

bcdaae3

…o longer returns {} when there aren't any SendEnabled entries and the default is false. SendEnabled is back to outputting a yaml format.

[11859]: Fix the params tests and add some new ones to it and key_test.

b1dac5f

[11859]: Create a 1-store method for updating several SendEnabled ent…

9a70f78

…ries at once.

[11859]: Create a migration for both the module and genesis state.

d6df3fa

[11859]: Create a new MsgSetSendEnabled for governanance proposals to…

e3c3875

… set SendEnabled.

[11859]: Add SetAllSendEnabled to the SendKeeper interface.

8b594de

[11859]: Add an authority to the bank keeper and create the handler f…

6222bac

…or MsgSetSendEnabled.

[11859]: Add an rpc endpoint for querying SendEnabled.

88014ac

[11859]: Implement the SendEnabled query.

004fd1e

SpicyLemon and others added 7 commits August 18, 2022 12:29

Merge branch 'main' into dwedul/11859-send-enabled-w-gov-prop

f265136

Merge branch 'main' into dwedul/11859-send-enabled-w-gov-prop

e54b079

# Conflicts: # CHANGELOG.md

Merge branch 'main' into dwedul/11859-send-enabled-w-gov-prop

ff1831b

Merge branch 'main' into dwedul/11859-send-enabled-w-gov-prop

61b968b

Merge branch 'dwedul/11859-send-enabled-w-gov-prop' of github.com:cos…

b5c020c

…mos/cosmos-sdk into dwedul/11859-send-enabled-w-gov-prop

Merge branch 'main' into dwedul/11859-send-enabled-w-gov-prop

ae5a5b3

# Conflicts: # CHANGELOG.md

[11859]: Add SetSendEnabled to the MockBankKeeper.

4f0662b

github-actions bot added the C:x/authz label Aug 29, 2022

SpicyLemon added 2 commits August 29, 2022 11:17

Merge branch 'main' into dwedul/11859-send-enabled-w-gov-prop

db8a13b

[11859]: make mocks.

7cb385c

github-actions bot added C:x/gov C:x/group labels Aug 29, 2022

SpicyLemon added 5 commits August 29, 2022 11:48

[11859]: Fix the bank app tests (had compilation issues due to merge …

559a1c0

…changes).

Merge branch 'main' into dwedul/11859-send-enabled-w-gov-prop

9901da5

# Conflicts: # CHANGELOG.md

[11859]: Remove the set_default_send_enabled and default_send_enabled…

9846317

… fields from MsgSetSendEnabled.

[11859]: Remove any uses/references to the removed set_default_send_e…

f033cbc

…nabled and default_send_enabled fields.

Merge branch 'main' into dwedul/11859-send-enabled-w-gov-prop

b6d2e6c

# Conflicts: # CHANGELOG.md

tac0turtle approved these changes Sep 5, 2022

View reviewed changes

Merge branch 'main' into dwedul/11859-send-enabled-w-gov-prop

1bb627e

tac0turtle enabled auto-merge (squash) September 5, 2022 08:03

SpicyLemon added 4 commits September 8, 2022 17:27

Merge branch 'main' into dwedul/11859-send-enabled-w-gov-prop

84a0346

# Conflicts: # CHANGELOG.md # x/bank/spec/03_messages.md # x/bank/spec/README.md

[11859]: Update the bank readme with the new MsgSetSendEnabled message.

c72ac77

Merge branch 'dwedul/11859-send-enabled-w-gov-prop' of github.com:cos…

f90c563

…mos/cosmos-sdk into dwedul/11859-send-enabled-w-gov-prop # Conflicts: # CHANGELOG.md

[11859]: Fix import of cosmossdk.io/simapp (was github.com/cosmos/cos…

720f8f7

…mos-sdk/simapp) in x/bank/app_test.go and update other go mod stuff associated with that.

tac0turtle merged commit 6a6e6c7 into main Sep 9, 2022

tac0turtle deleted the dwedul/11859-send-enabled-w-gov-prop branch September 9, 2022 00:21

julienrbrt mentioned this pull request Sep 9, 2022

fix: remove newly introduced simapp dependency #13219

Merged

19 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(bank): Create message for Setting SendEnabled settings #11981

feat(bank): Create message for Setting SendEnabled settings #11981

dwedul-figure commented May 17, 2022 •

edited

Loading

dwedul-figure commented Aug 15, 2022

tac0turtle left a comment

feat(bank): Create message for Setting SendEnabled settings #11981

feat(bank): Create message for Setting SendEnabled settings #11981

Conversation

dwedul-figure commented May 17, 2022 • edited Loading

Description

Author Checklist

Reviewers Checklist

dwedul-figure commented Aug 15, 2022

tac0turtle left a comment

Choose a reason for hiding this comment

dwedul-figure commented May 17, 2022 •

edited

Loading