Allow marker's transfer authority to prevent transfer of restricted c…

…oin with deny list on send (#1604)

* add change log entry

* add send deny list field to marker proto, add getter/setter for send deny list

* add send deny list check in validateSendDenom

* remove deny list in marker proto in order to move it to store as key

* add new key for deny list

* add is send deny check in send restrictions method

* add key test

* add setter for deny list, add send restriction test

* add proto rpc endpoint to add/remove send deny list addresses, no impl yet

* add MsgUpdateSendDenyListRequest sdk.Msg type

* add msg server impl

* fix lint

* add a few refactors, start msg server test

* complete msg server tests, check that both lists are not empty

* add validate basic tests

* add update deny list command

* add spec doc

* add msg server event

* add validate basic test case

* add get signers test

* remove space in link

* update marker tx proto docs

* name test correctly

* update docs

* fix test name

* update docs

* change to allow user with transfer authority to add/remove from deny list

* change changelog wording

* change spec and proto docs to reflect transfer authority change

* fix prefix, remove unneeded check, fix change log

* fix test

* implement pr suggestions

* update test to not use constructor

CHANGELOG.md

@@ -37,6 +37,10 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## Unreleased
 
+### Features
+
+* Allow marker's transfer authority to prevent transfer of restricted coin with deny list on send [#1518](https://github.com/provenance-io/provenance/issues/1518).
+
 ### Improvements
 
 * Update ibcnet ports so they don't conflict with host machine. [#1622](https://github.com/provenance-io/provenance/issues/1622)

client/docs/swagger-ui/swagger.yaml

@@ -82145,6 +82145,11 @@ definitions:
     title: >-
       MsgUpdateRequiredAttributesResponse defines the
       Msg/UpdateRequiredAttributes response type
+  provenance.marker.v1.MsgUpdateSendDenyListResponse:
+    type: object
+    title: >-
+      MsgUpdateSendDenyListResponse defines the Msg/UpdateSendDenyList response
+      type
   provenance.marker.v1.MsgWithdrawResponse:
     type: object
     title: MsgWithdrawResponse defines the Msg/Withdraw response type

docs/proto-docs.md

@@ -157,6 +157,8 @@
     - [MsgUpdateForcedTransferResponse](#provenance.marker.v1.MsgUpdateForcedTransferResponse)
     - [MsgUpdateRequiredAttributesRequest](#provenance.marker.v1.MsgUpdateRequiredAttributesRequest)
     - [MsgUpdateRequiredAttributesResponse](#provenance.marker.v1.MsgUpdateRequiredAttributesResponse)
+    - [MsgUpdateSendDenyListRequest](#provenance.marker.v1.MsgUpdateSendDenyListRequest)
+    - [MsgUpdateSendDenyListResponse](#provenance.marker.v1.MsgUpdateSendDenyListResponse)
     - [MsgWithdrawRequest](#provenance.marker.v1.MsgWithdrawRequest)
     - [MsgWithdrawResponse](#provenance.marker.v1.MsgWithdrawResponse)
 
@@ -2640,10 +2642,10 @@ add list
 
 | Field | Type | Label | Description |
 | ----- | ---- | ----- | ----------- |
-| `denom` | [string](#string) |  |  |
-| `remove_required_attributes` | [string](#string) | repeated |  |
-| `add_required_attributes` | [string](#string) | repeated |  |
-| `transfer_authority` | [string](#string) |  | signer of the proposal |
+| `denom` | [string](#string) |  | The denomination of the marker to update. |
+| `remove_required_attributes` | [string](#string) | repeated | List of required attributes to remove from marker. |
+| `add_required_attributes` | [string](#string) | repeated | List of required attributes to add to marker. |
+| `transfer_authority` | [string](#string) |  | The signer of the message. Must have transfer authority to marker or be governance module account address. |
 
 
 
@@ -2660,6 +2662,35 @@ MsgUpdateRequiredAttributesResponse defines the Msg/UpdateRequiredAttributes res
 
 
 
+<a name="provenance.marker.v1.MsgUpdateSendDenyListRequest"></a>
+
+### MsgUpdateSendDenyListRequest
+MsgUpdateSendDenyListRequest defines a msg to add/remove addresses to send deny list for a resticted marker
+signer must have transfer authority
+
+
+| Field | Type | Label | Description |
+| ----- | ---- | ----- | ----------- |
+| `denom` | [string](#string) |  | The denomination of the marker to update. |
+| `remove_denied_addresses` | [string](#string) | repeated | List of bech32 addresses to remove from the deny send list. |
+| `add_denied_addresses` | [string](#string) | repeated | List of bech32 addresses to add to the deny send list. |
+| `authority` | [string](#string) |  | The signer of the message. Must have admin authority to marker or be governance module account address. |
+
+
+
+
+
+
+<a name="provenance.marker.v1.MsgUpdateSendDenyListResponse"></a>
+
+### MsgUpdateSendDenyListResponse
+MsgUpdateSendDenyListResponse defines the Msg/UpdateSendDenyList response type
+
+
+
+
+
+
 <a name="provenance.marker.v1.MsgWithdrawRequest"></a>
 
 ### MsgWithdrawRequest
@@ -2720,6 +2751,7 @@ Msg defines the Marker Msg service.
 | `UpdateRequiredAttributes` | [MsgUpdateRequiredAttributesRequest](#provenance.marker.v1.MsgUpdateRequiredAttributesRequest) | [MsgUpdateRequiredAttributesResponse](#provenance.marker.v1.MsgUpdateRequiredAttributesResponse) | UpdateRequiredAttributes will only succeed if signer has transfer authority | |
 | `UpdateForcedTransfer` | [MsgUpdateForcedTransferRequest](#provenance.marker.v1.MsgUpdateForcedTransferRequest) | [MsgUpdateForcedTransferResponse](#provenance.marker.v1.MsgUpdateForcedTransferResponse) | UpdateForcedTransfer updates the allow_forced_transfer field of a marker via governance proposal. | |
 | `SetAccountData` | [MsgSetAccountDataRequest](#provenance.marker.v1.MsgSetAccountDataRequest) | [MsgSetAccountDataResponse](#provenance.marker.v1.MsgSetAccountDataResponse) | SetAccountData sets the accountdata for a denom. Signer must have deposit authority. | |
+| `UpdateSendDenyList` | [MsgUpdateSendDenyListRequest](#provenance.marker.v1.MsgUpdateSendDenyListRequest) | [MsgUpdateSendDenyListResponse](#provenance.marker.v1.MsgUpdateSendDenyListResponse) | UpdateSendDenyList will only succeed if signer has admin authority | |
 
  <!-- end services -->
 

proto/provenance/marker/v1/tx.proto

@@ -58,6 +58,8 @@ service Msg {
   rpc UpdateForcedTransfer(MsgUpdateForcedTransferRequest) returns (MsgUpdateForcedTransferResponse);
   // SetAccountData sets the accountdata for a denom. Signer must have deposit authority.
   rpc SetAccountData(MsgSetAccountDataRequest) returns (MsgSetAccountDataResponse);
+  // UpdateSendDenyList will only succeed if signer has admin authority
+  rpc UpdateSendDenyList(MsgUpdateSendDenyListRequest) returns (MsgUpdateSendDenyListResponse);
 }
 
 // MsgGrantAllowanceRequest validates permission to create a fee grant based on marker admin access. If
@@ -250,10 +252,14 @@ message MsgUpdateRequiredAttributesRequest {
   option (gogoproto.equal)      = true;
   option (cosmos.msg.v1.signer) = "transfer_authority";
 
-  string          denom                      = 1;
+  // The denomination of the marker to update.
+  string denom = 1;
+  // List of required attributes to remove from marker.
   repeated string remove_required_attributes = 2;
-  repeated string add_required_attributes    = 3;
-  string          transfer_authority = 4 [(cosmos_proto.scalar) = "cosmos.AddressString"]; // signer of the proposal
+  // List of required attributes to add to marker.
+  repeated string add_required_attributes = 3;
+  // The signer of the message.  Must have transfer authority to marker or be governance module account address.
+  string transfer_authority = 4 [(cosmos_proto.scalar) = "cosmos.AddressString"];
 }
 
 // MsgUpdateRequiredAttributesResponse defines the Msg/UpdateRequiredAttributes response type
@@ -291,4 +297,23 @@ message MsgSetAccountDataRequest {
 }
 
 // MsgSetAccountDataResponse defines the Msg/SetAccountData response type
-message MsgSetAccountDataResponse {}
+message MsgSetAccountDataResponse {}
+
+// MsgUpdateSendDenyListRequest defines a msg to add/remove addresses to send deny list for a resticted marker
+// signer must have transfer authority
+message MsgUpdateSendDenyListRequest {
+  option (gogoproto.equal)      = true;
+  option (cosmos.msg.v1.signer) = "authority";
+
+  // The denomination of the marker to update.
+  string denom = 1;
+  // List of bech32 addresses to remove from the deny send list.
+  repeated string remove_denied_addresses = 2;
+  // List of bech32 addresses to add to the deny send list.
+  repeated string add_denied_addresses = 3;
+  // The signer of the message.  Must have admin authority to marker or be governance module account address.
+  string authority = 4 [(cosmos_proto.scalar) = "cosmos.AddressString"];
+}
+
+// MsgUpdateSendDenyListResponse defines the Msg/UpdateSendDenyList response type
+message MsgUpdateSendDenyListResponse {}

x/marker/client/cli/tx.go

@@ -84,6 +84,7 @@ func NewTxCmd() *cobra.Command {
 		GetCmdUpdateRequiredAttributes(),
 		GetCmdUpdateForcedTransfer(),
 		GetCmdSetAccountData(),
+		GetCmdUpdateSendDenyListRequest(),
 	)
 	return txCmd
 }
@@ -1050,6 +1051,53 @@ func GetCmdUpdateForcedTransfer() *cobra.Command {
 	return cmd
 }
 
+// GetCmdUpdateSendDenyListRequest implements the update deny list command
+func GetCmdUpdateSendDenyListRequest() *cobra.Command {
+	cmd := &cobra.Command{
+		Use:     "update-deny-list <denom>",
+		Aliases: []string{"udl", "deny-list", "deny"},
+		Args:    cobra.ExactArgs(1),
+		Short:   "Update list of addresses for a restricted marker that are allowed to execute transfers",
+		Long: strings.TrimSpace(`Update list of addresses for a restricted marker that are allowed to execute transfers.
+`),
+		Example: fmt.Sprintf(`$ %s tx marker update-deny-list hotdogcoin --%s=bech32addr1,bech32addrs2,... --%s=bech32addr1,bech32addrs2,...`,
+			version.AppName,
+			FlagAdd,
+			FlagRemove,
+		),
+		RunE: func(cmd *cobra.Command, args []string) error {
+			clientCtx, err := client.GetClientTxContext(cmd)
+			if err != nil {
+				return err
+			}
+			flagSet := cmd.Flags()
+
+			msg := &types.MsgUpdateSendDenyListRequest{Denom: args[0]}
+
+			msg.AddDeniedAddresses, err = flagSet.GetStringSlice(FlagAdd)
+			if err != nil {
+				return fmt.Errorf("incorrect value for %s flag.  Accepted: comma delimited list of bech32 addresses Error: %w", FlagAdd, err)
+			}
+
+			msg.RemoveDeniedAddresses, err = flagSet.GetStringSlice(FlagRemove)
+			if err != nil {
+				return fmt.Errorf("incorrect value for %s flag.  Accepted: comma delimited list of bech32 addresses Error: %w", FlagRemove, err)
+			}
+
+			authSetter := func(authority string) {
+				msg.Authority = authority
+			}
+
+			return generateOrBroadcastOptGovProp(clientCtx, flagSet, authSetter, msg)
+		},
+	}
+	cmd.Flags().StringSlice(FlagAdd, []string{}, "comma delimited list of bech32 addresses to be added to restricted marker transfer deny list")
+	cmd.Flags().StringSlice(FlagRemove, []string{}, "comma delimited list of bech32 addresses to be removed removed from restricted marker deny list")
+	addOptGovPropFlags(cmd)
+	flags.AddTxFlagsToCmd(cmd)
+	return cmd
+}
+
 // GetCmdSetAccountData returns a CLI command for setting a marker's account data.
 func GetCmdSetAccountData() *cobra.Command {
 	cmd := &cobra.Command{

x/marker/keeper/keeper.go

@@ -192,3 +192,18 @@ func (k Keeper) GetEscrow(ctx sdk.Context, marker types.MarkerAccountI) sdk.Coin
 func (k Keeper) GetAuthority() string {
 	return k.authority
 }
+
+func (k Keeper) IsSendDeny(ctx sdk.Context, markerAddr, senderAddr sdk.AccAddress) bool {
+	store := ctx.KVStore(k.storeKey)
+	return store.Has(types.DenySendKey(markerAddr, senderAddr))
+}
+
+func (k Keeper) AddSendDeny(ctx sdk.Context, markerAddr, senderAddr sdk.AccAddress) {
+	store := ctx.KVStore(k.storeKey)
+	store.Set(types.DenySendKey(markerAddr, senderAddr), []byte{})
+}
+
+func (k Keeper) RemoveSendDeny(ctx sdk.Context, markerAddr, senderAddr sdk.AccAddress) {
+	store := ctx.KVStore(k.storeKey)
+	store.Delete(types.DenySendKey(markerAddr, senderAddr))
+}

x/marker/keeper/keeper_test.go

@@ -1415,3 +1415,113 @@ func TestRemoveIsSendEnabledEntries(t *testing.T) {
 	assert.True(t, app.BankKeeper.IsSendEnabledDenom(ctx, "testcoin"), "should not exist in table therefore default to true")
 
 }
+
+func TestUpdateSendDenyList(t *testing.T) {
+	app := simapp.Setup(t)
+	ctx := app.BaseApp.NewContext(false, tmproto.Header{})
+	server := markerkeeper.NewMsgServerImpl(app.MarkerKeeper)
+
+	authority := authtypes.NewModuleAddress(govtypes.ModuleName)
+
+	authUser := testUserAddress("test")
+	notAuthUser := testUserAddress("test1")
+
+	notRestrictedMarker := types.NewEmptyMarkerAccount(
+		"not-restricted-marker",
+		authUser.String(),
+		[]types.AccessGrant{})
+
+	err := app.MarkerKeeper.AddMarkerAccount(ctx, notRestrictedMarker)
+	require.NoError(t, err)
+
+	rMarkerDenom := "restricted-marker"
+	rMarkerAcct := authtypes.NewBaseAccount(types.MustGetMarkerAddress(rMarkerDenom), nil, 0, 0)
+	app.MarkerKeeper.SetMarker(ctx, types.NewMarkerAccount(rMarkerAcct, sdk.NewInt64Coin(rMarkerDenom, 1000), authUser, []types.AccessGrant{{Address: authUser.String(), Permissions: []types.Access{types.Access_Transfer}}}, types.StatusFinalized, types.MarkerType_RestrictedCoin, true, false, false, []string{}))
+
+	rMarkerGovDenom := "restricted-marker-gov"
+	rMarkerGovAcct := authtypes.NewBaseAccount(types.MustGetMarkerAddress(rMarkerGovDenom), nil, 0, 0)
+	app.MarkerKeeper.SetMarker(ctx, types.NewMarkerAccount(rMarkerGovAcct, sdk.NewInt64Coin(rMarkerGovDenom, 1000), authUser, []types.AccessGrant{{Address: authUser.String(), Permissions: []types.Access{}}}, types.StatusFinalized, types.MarkerType_RestrictedCoin, true, true, false, []string{}))
+
+	denyAddrToRemove := testUserAddress("denyAddrToRemove")
+	app.MarkerKeeper.AddSendDeny(ctx, rMarkerAcct.GetAddress(), denyAddrToRemove)
+	require.True(t, app.MarkerKeeper.IsSendDeny(ctx, rMarkerAcct.GetAddress(), denyAddrToRemove), rMarkerDenom+" should have added address to deny list "+denyAddrToRemove.String())
+
+	denyAddrToAdd := testUserAddress("denyAddrToAdd")
+
+	denyAddrToAddGov := testUserAddress("denyAddrToAddGov")
+
+	testCases := []struct {
+		name   string
+		msg    types.MsgUpdateSendDenyListRequest
+		expErr string
+	}{
+		{
+			name:   "should fail, cannot find marker",
+			msg:    types.MsgUpdateSendDenyListRequest{Denom: "blah", Authority: authUser.String(), RemoveDeniedAddresses: []string{}, AddDeniedAddresses: []string{}},
+			expErr: "marker not found for blah: marker blah not found for address: cosmos1psw3a97ywtr595qa4295lw07cz9665hynnfpee",
+		},
+		{
+			name:   "should fail, not a restricted marker",
+			msg:    types.MsgUpdateSendDenyListRequest{Denom: notRestrictedMarker.Denom, Authority: authUser.String(), RemoveDeniedAddresses: []string{}, AddDeniedAddresses: []string{}},
+			expErr: "marker not-restricted-marker is not a restricted marker",
+		},
+		{
+			name:   "should fail, signer does not have admin access",
+			msg:    types.MsgUpdateSendDenyListRequest{Denom: rMarkerDenom, Authority: notAuthUser.String(), RemoveDeniedAddresses: []string{}, AddDeniedAddresses: []string{}},
+			expErr: "cosmos1ku2jzvpkt4ffxxaajyk2r88axk9cr5jqlthcm4 does not have transfer authority for restricted-marker marker",
+		},
+		{
+			name:   "should fail, gov not enabled for restricted marker",
+			msg:    types.MsgUpdateSendDenyListRequest{Denom: rMarkerDenom, Authority: authority.String(), RemoveDeniedAddresses: []string{}, AddDeniedAddresses: []string{}},
+			expErr: "restricted-marker marker does not allow governance control",
+		},
+		{
+			name:   "should fail, address is already on deny list",
+			msg:    types.MsgUpdateSendDenyListRequest{Denom: rMarkerDenom, Authority: authUser.String(), RemoveDeniedAddresses: []string{}, AddDeniedAddresses: []string{denyAddrToRemove.String()}},
+			expErr: denyAddrToRemove.String() + " is already on deny list cannot add address",
+		},
+		{
+			name:   "should fail, address can not be removed not in deny list",
+			msg:    types.MsgUpdateSendDenyListRequest{Denom: rMarkerDenom, Authority: authUser.String(), RemoveDeniedAddresses: []string{denyAddrToAdd.String()}, AddDeniedAddresses: []string{}},
+			expErr: denyAddrToAdd.String() + " is not on deny list cannot remove address",
+		},
+		{
+			name:   "should fail, invalid address on add list",
+			msg:    types.MsgUpdateSendDenyListRequest{Denom: rMarkerDenom, Authority: authUser.String(), RemoveDeniedAddresses: []string{}, AddDeniedAddresses: []string{"invalid-add-address"}},
+			expErr: "decoding bech32 failed: invalid separator index -1",
+		},
+		{
+			name:   "should fail, invalid address on remove list",
+			msg:    types.MsgUpdateSendDenyListRequest{Denom: rMarkerDenom, Authority: authUser.String(), RemoveDeniedAddresses: []string{"invalid-remove-address"}, AddDeniedAddresses: []string{}},
+			expErr: "decoding bech32 failed: invalid separator index -1",
+		},
+		{
+			name: "should succeed to add to deny list",
+			msg:  types.MsgUpdateSendDenyListRequest{Denom: rMarkerDenom, Authority: authUser.String(), RemoveDeniedAddresses: []string{}, AddDeniedAddresses: []string{denyAddrToAdd.String()}},
+		},
+		{
+			name: "should succeed to remove from deny list",
+			msg:  types.MsgUpdateSendDenyListRequest{Denom: rMarkerDenom, Authority: authUser.String(), RemoveDeniedAddresses: []string{denyAddrToRemove.String()}, AddDeniedAddresses: []string{}},
+		},
+		{
+			name: "should succeed gov allowed for marker",
+			msg:  types.MsgUpdateSendDenyListRequest{Denom: rMarkerGovDenom, Authority: authority.String(), RemoveDeniedAddresses: []string{}, AddDeniedAddresses: []string{denyAddrToAddGov.String()}},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			res, err := server.UpdateSendDenyList(sdk.WrapSDKContext(ctx),
+				&tc.msg)
+
+			if len(tc.expErr) > 0 {
+				assert.Nil(t, res)
+				assert.EqualError(t, err, tc.expErr)
+
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, res, &types.MsgUpdateSendDenyListResponse{})
+			}
+		})
+	}
+}

x/marker/keeper/msg_server.go

@@ -752,3 +752,59 @@ func (k msgServer) SetAccountData(goCtx context.Context, msg *types.MsgSetAccoun
 
 	return &types.MsgSetAccountDataResponse{}, nil
 }
+
+// UpdateSendDenyList updates the deny send list for restricted marker. Signer must be admin or gov proposal.
+func (k msgServer) UpdateSendDenyList(goCtx context.Context, msg *types.MsgUpdateSendDenyListRequest) (*types.MsgUpdateSendDenyListResponse, error) {
+	ctx := sdk.UnwrapSDKContext(goCtx)
+
+	marker, err := k.GetMarkerByDenom(ctx, msg.Denom)
+	if err != nil {
+		return nil, fmt.Errorf("marker not found for %s: %w", msg.Denom, err)
+	}
+
+	if marker.GetMarkerType() != types.MarkerType_RestrictedCoin {
+		return nil, fmt.Errorf("marker %s is not a restricted marker", msg.Denom)
+	}
+
+	if msg.Authority == k.GetAuthority() {
+		if !marker.HasGovernanceEnabled() {
+			return nil, fmt.Errorf("%s marker does not allow governance control", msg.Denom)
+		}
+	} else {
+		if !marker.HasAccess(msg.Authority, types.Access_Transfer) {
+			return nil, fmt.Errorf("%s does not have transfer authority for %s marker", msg.Authority, msg.Denom)
+		}
+	}
+
+	markerAddr := marker.GetAddress()
+	for _, addr := range msg.RemoveDeniedAddresses {
+		denyAddr, err := sdk.AccAddressFromBech32(addr)
+		if err != nil {
+			return nil, err
+		}
+		if !k.IsSendDeny(ctx, markerAddr, denyAddr) {
+			return nil, fmt.Errorf("%s is not on deny list cannot remove address", addr)
+		}
+		k.RemoveSendDeny(ctx, markerAddr, denyAddr)
+	}
+
+	for _, addr := range msg.AddDeniedAddresses {
+		denyAddr, err := sdk.AccAddressFromBech32(addr)
+		if err != nil {
+			return nil, err
+		}
+		if k.IsSendDeny(ctx, markerAddr, denyAddr) {
+			return nil, fmt.Errorf("%s is already on deny list cannot add address", addr)
+		}
+		k.AddSendDeny(ctx, markerAddr, denyAddr)
+	}
+
+	ctx.EventManager().EmitEvent(
+		sdk.NewEvent(
+			sdk.EventTypeMessage,
+			sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName),
+		),
+	)
+
+	return &types.MsgUpdateSendDenyListResponse{}, nil
+}