Remove all marker entries from send deny store when deleted (#1796)

* Add prefix key for getting entries by marker. Add ClearSendDeny and GetSendDenyList to easily obtain and remove from list. * Add DenySendMarkerPrefix. * Add tests for previously existing functions add and remove. * Fix empty list bug in GetSendDenyList, and add tests. * Add test for ClearSendDeny. * Add checks for send deny list in tests that verify removal. * Add changelog entry.
provenance-io · Jan 9, 2024 · 604269a · 604269a
1 parent 2df3542
commit 604269a
Show file tree

Hide file tree

Showing 5 changed files with 316 additions and 0 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -52,6 +52,10 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * Remove the rust upgrade handlers [PR 1774](https://github.com/provenance-io/provenance/pull/1774).
 * Add StoreLoader wrapper to check configuration settings [#1792](https://github.com/provenance-io/provenance/pull/1792).
 
+### Bug Fixes
+
+* Remove deleted marker send deny entries [#1666](https://github.com/provenance-io/provenance/issues/1666).
+
 ### Dependencies
 
 - Bump `bufbuild/buf-setup-action` from 1.27.1 to 1.28.1 ([#1724](https://github.com/provenance-io/provenance/pull/1724), [#1744](https://github.com/provenance-io/provenance/pull/1744), [#1750](https://github.com/provenance-io/provenance/pull/1750))

diff --git a/x/marker/keeper/keeper.go b/x/marker/keeper/keeper.go
@@ -174,6 +174,7 @@ func (k Keeper) RemoveMarker(ctx sdk.Context, marker types.MarkerAccountI) {
 	k.authKeeper.RemoveAccount(ctx, marker)
 
 	k.RemoveNetAssetValues(ctx, marker.GetAddress())
+	k.ClearSendDeny(ctx, marker.GetAddress())
 	store.Delete(types.MarkerStoreKey(marker.GetAddress()))
 }
 
@@ -223,6 +224,14 @@ func (k Keeper) RemoveSendDeny(ctx sdk.Context, markerAddr, senderAddr sdk.AccAd
 	store.Delete(types.DenySendKey(markerAddr, senderAddr))
 }
 
+// ClearSendDeny removes all entries of a marker from a send deny list
+func (k Keeper) ClearSendDeny(ctx sdk.Context, markerAddr sdk.AccAddress) {
+	list := k.GetSendDenyList(ctx, markerAddr)
+	for _, sender := range list {
+		k.RemoveSendDeny(ctx, markerAddr, sender)
+	}
+}
+
 // IterateMarkers  iterates all markers with the given handler function.
 func (k Keeper) IterateSendDeny(ctx sdk.Context, handler func(key []byte) (stop bool)) {
 	store := ctx.KVStore(k.storeKey)
@@ -236,6 +245,21 @@ func (k Keeper) IterateSendDeny(ctx sdk.Context, handler func(key []byte) (stop
 	}
 }
 
+// GetSendDenyList gets the list of sender addresses from the marker's deny list
+func (k Keeper) GetSendDenyList(ctx sdk.Context, markerAddr sdk.AccAddress) []sdk.AccAddress {
+	store := ctx.KVStore(k.storeKey)
+	iterator := sdk.KVStorePrefixIterator(store, types.DenySendMarkerPrefix(markerAddr))
+	list := []sdk.AccAddress{}
+
+	defer iterator.Close()
+	for ; iterator.Valid(); iterator.Next() {
+		_, denied := types.GetDenySendAddresses(iterator.Key())
+		list = append(list, denied)
+	}
+
+	return list
+}
+
 // AddSetNetAssetValues adds a set of net asset values to a marker
 func (k Keeper) AddSetNetAssetValues(ctx sdk.Context, marker types.MarkerAccountI, netAssetValues []types.NetAssetValue, source string) error {
 	for _, nav := range netAssetValues {

diff --git a/x/marker/keeper/keeper_test.go b/x/marker/keeper/keeper_test.go
@@ -64,8 +64,14 @@ func TestAccountMapperGetSet(t *testing.T) {
 	require.True(t, ok)
 	require.True(t, mac.AddressHasAccess(user, types.Access_Admin))
 
+	// add something to the send deny list just to verify removal
+	app.MarkerKeeper.AddSendDeny(ctx, addr, addr)
+
 	app.MarkerKeeper.RemoveMarker(ctx, mac)
 
+	// marker should not exist in send deny list
+	require.Empty(t, app.MarkerKeeper.GetSendDenyList(ctx, addr), "should not have entries in send deny list")
+
 	// getting account after delete should be nil
 	acc = app.AccountKeeper.GetAccount(ctx, addr)
 	require.Nil(t, acc)
@@ -1316,6 +1322,272 @@ func TestGetAuthority(t *testing.T) {
 	require.Equal(t, "cosmos10d07y265gmmuvt4z0w9aw880jnsr700j6zn9kn", app.MarkerKeeper.GetAuthority())
 }
 
+func TestClearSendDeny(t *testing.T) {
+	app := simapp.Setup(t)
+	ctx := app.BaseApp.NewContext(false, tmproto.Header{})
+
+	type SendDenyPair struct {
+		marker   sdk.AccAddress
+		sendDeny sdk.AccAddress
+	}
+
+	tests := []struct {
+		name   string
+		pairs  []SendDenyPair
+		marker sdk.AccAddress
+	}{
+		{
+			name:   "non existant marker",
+			pairs:  []SendDenyPair{},
+			marker: sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+		},
+		{
+			name: "single entry for marker",
+			pairs: []SendDenyPair{
+				{
+					marker:   sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+					sendDeny: sdk.AccAddress("cosmos1w6t0l7z0yerj49ehnqwqaayxqpe3u7e23edgma"),
+				},
+			},
+			marker: sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+		},
+		{
+			name: "multiple entry for marker",
+			pairs: []SendDenyPair{
+				{
+					marker:   sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+					sendDeny: sdk.AccAddress("cosmos1w6t0l7z0yerj49ehnqwqaayxqpe3u7e23edgma"),
+				},
+				{
+					marker:   sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+					sendDeny: sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+				},
+			},
+			marker: sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+		},
+		{
+			name: "multiple markers with multiple entries",
+			pairs: []SendDenyPair{
+				{
+					marker:   sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+					sendDeny: sdk.AccAddress("cosmos1w6t0l7z0yerj49ehnqwqaayxqpe3u7e23edgma"),
+				},
+				{
+					marker:   sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+					sendDeny: sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+				},
+				{
+					marker:   sdk.AccAddress("cosmos1w6t0l7z0yerj49ehnqwqaayxqpe3u7e23edgma"),
+					sendDeny: sdk.AccAddress("cosmos1w6t0l7z0yerj49ehnqwqaayxqpe3u7e23edgma"),
+				},
+				{
+					marker:   sdk.AccAddress("cosmos1w6t0l7z0yerj49ehnqwqaayxqpe3u7e23edgma"),
+					sendDeny: sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+				},
+			},
+			marker: sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			for _, pair := range tc.pairs {
+				app.MarkerKeeper.AddSendDeny(ctx, pair.marker, pair.sendDeny)
+			}
+
+			app.MarkerKeeper.ClearSendDeny(ctx, tc.marker)
+			list := app.MarkerKeeper.GetSendDenyList(ctx, tc.marker)
+			assert.Empty(t, list, "should remove all entries from send deny list")
+
+			for _, pair := range tc.pairs {
+				app.MarkerKeeper.RemoveSendDeny(ctx, pair.marker, pair.sendDeny)
+			}
+		})
+	}
+}
+
+func TestGetSendDenyList(t *testing.T) {
+	app := simapp.Setup(t)
+	ctx := app.BaseApp.NewContext(false, tmproto.Header{})
+
+	type SendDenyPair struct {
+		marker   sdk.AccAddress
+		sendDeny sdk.AccAddress
+	}
+
+	tests := []struct {
+		name     string
+		pairs    []SendDenyPair
+		marker   sdk.AccAddress
+		expected []sdk.AccAddress
+	}{
+		{
+			name:     "non existant marker",
+			pairs:    []SendDenyPair{},
+			marker:   sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+			expected: []sdk.AccAddress{},
+		},
+		{
+			name: "single entry for marker",
+			pairs: []SendDenyPair{
+				{
+					marker:   sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+					sendDeny: sdk.AccAddress("cosmos1w6t0l7z0yerj49ehnqwqaayxqpe3u7e23edgma"),
+				},
+			},
+			marker: sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+			expected: []sdk.AccAddress{
+				sdk.AccAddress("cosmos1w6t0l7z0yerj49ehnqwqaayxqpe3u7e23edgma"),
+			},
+		},
+		{
+			name: "multiple entry for marker",
+			pairs: []SendDenyPair{
+				{
+					marker:   sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+					sendDeny: sdk.AccAddress("cosmos1w6t0l7z0yerj49ehnqwqaayxqpe3u7e23edgma"),
+				},
+				{
+					marker:   sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+					sendDeny: sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+				},
+			},
+			marker: sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+			expected: []sdk.AccAddress{
+				sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+				sdk.AccAddress("cosmos1w6t0l7z0yerj49ehnqwqaayxqpe3u7e23edgma"),
+			},
+		},
+		{
+			name: "multiple markers with multiple entries",
+			pairs: []SendDenyPair{
+				{
+					marker:   sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+					sendDeny: sdk.AccAddress("cosmos1w6t0l7z0yerj49ehnqwqaayxqpe3u7e23edgma"),
+				},
+				{
+					marker:   sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+					sendDeny: sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+				},
+				{
+					marker:   sdk.AccAddress("cosmos1w6t0l7z0yerj49ehnqwqaayxqpe3u7e23edgma"),
+					sendDeny: sdk.AccAddress("cosmos1w6t0l7z0yerj49ehnqwqaayxqpe3u7e23edgma"),
+				},
+				{
+					marker:   sdk.AccAddress("cosmos1w6t0l7z0yerj49ehnqwqaayxqpe3u7e23edgma"),
+					sendDeny: sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+				},
+			},
+			marker: sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+			expected: []sdk.AccAddress{
+				sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+				sdk.AccAddress("cosmos1w6t0l7z0yerj49ehnqwqaayxqpe3u7e23edgma"),
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			for _, pair := range tc.pairs {
+				app.MarkerKeeper.AddSendDeny(ctx, pair.marker, pair.sendDeny)
+			}
+
+			list := app.MarkerKeeper.GetSendDenyList(ctx, tc.marker)
+			assert.Equal(t, tc.expected, list, "should return the correct send deny entries for a marker")
+
+			for _, pair := range tc.pairs {
+				app.MarkerKeeper.RemoveSendDeny(ctx, pair.marker, pair.sendDeny)
+			}
+		})
+	}
+}
+
+func TestAddRemoveSendDeny(t *testing.T) {
+	app := simapp.Setup(t)
+	ctx := app.BaseApp.NewContext(false, tmproto.Header{})
+
+	type SendDenyPair struct {
+		marker   sdk.AccAddress
+		sendDeny sdk.AccAddress
+	}
+
+	tests := []struct {
+		name  string
+		pairs []SendDenyPair
+	}{
+		{
+			name: "new marker and send deny pair",
+			pairs: []SendDenyPair{
+				{
+					marker:   sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+					sendDeny: sdk.AccAddress("cosmos1w6t0l7z0yerj49ehnqwqaayxqpe3u7e23edgma"),
+				},
+			},
+		},
+		{
+			name: "duplicate marker and send deny pair",
+			pairs: []SendDenyPair{
+				{
+					marker:   sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+					sendDeny: sdk.AccAddress("cosmos1w6t0l7z0yerj49ehnqwqaayxqpe3u7e23edgma"),
+				},
+				{
+					marker:   sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+					sendDeny: sdk.AccAddress("cosmos1w6t0l7z0yerj49ehnqwqaayxqpe3u7e23edgma"),
+				},
+			},
+		},
+		{
+			name: "multiple senders for marker",
+			pairs: []SendDenyPair{
+				{
+					marker:   sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+					sendDeny: sdk.AccAddress("cosmos1w6t0l7z0yerj49ehnqwqaayxqpe3u7e23edgma"),
+				},
+				{
+					marker:   sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+					sendDeny: sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+				},
+			},
+		},
+		{
+			name: "multiple markers",
+			pairs: []SendDenyPair{
+				{
+					marker:   sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+					sendDeny: sdk.AccAddress("cosmos1w6t0l7z0yerj49ehnqwqaayxqpe3u7e23edgma"),
+				},
+				{
+					marker:   sdk.AccAddress("cosmos1w6t0l7z0yerj49ehnqwqaayxqpe3u7e23edgma"),
+					sendDeny: sdk.AccAddress("cosmos1v57fx2l2rt6ehujuu99u2fw05779m5e2ux4z2h"),
+				},
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			for _, pair := range tc.pairs {
+				app.MarkerKeeper.AddSendDeny(ctx, pair.marker, pair.sendDeny)
+			}
+
+			for _, pair := range tc.pairs {
+				isSendDeny := app.MarkerKeeper.IsSendDeny(ctx, pair.marker, pair.sendDeny)
+				require.True(t, isSendDeny, "should have entry for added pair")
+			}
+
+			for _, pair := range tc.pairs {
+				app.MarkerKeeper.RemoveSendDeny(ctx, pair.marker, pair.sendDeny)
+			}
+
+			for _, pair := range tc.pairs {
+				isSendDeny := app.MarkerKeeper.IsSendDeny(ctx, pair.marker, pair.sendDeny)
+				require.False(t, isSendDeny, "should not have entry for removed pair")
+			}
+		})
+	}
+}
+
 func TestReqAttrBypassAddrs(t *testing.T) {
 	// Tests both GetReqAttrBypassAddrs and IsReqAttrBypassAddr.
 	expectedNames := []string{

diff --git a/x/marker/types/key.go b/x/marker/types/key.go
@@ -83,6 +83,13 @@ func GetDenySendAddresses(key []byte) (markerAddr sdk.AccAddress, denyAddr sdk.A
 	return
 }
 
+// DenySendMarkerPrefix returns an extended prefix [prefix][denom addr] for send deny list for restricted markers
+func DenySendMarkerPrefix(markerAddr sdk.AccAddress) []byte {
+	key := DenySendKeyPrefix
+	key = append(key, address.MustLengthPrefix(markerAddr.Bytes())...)
+	return key
+}
+
 // NetAssetValueKey returns key [prefix][marker address] for marker net asset values
 func NetAssetValueKeyPrefix(markerAddr sdk.AccAddress) []byte {
 	return append(NetAssetValuePrefix, address.MustLengthPrefix(markerAddr.Bytes())...)

diff --git a/x/marker/types/key_test.go b/x/marker/types/key_test.go
@@ -54,3 +54,12 @@ func TestNetAssetValueKey(t *testing.T) {
 	assert.Equal(t, addr.Bytes(), navKey[2:denomArrLen+2], "should match denom key")
 	assert.Equal(t, "nhash", string(navKey[denomArrLen+2:]))
 }
+
+func TestDenySendMarkerPrefix(t *testing.T) {
+	addr, err := MarkerAddress("nhash")
+	require.NoError(t, err, "MarkerAddress(nhash)")
+	denyKey := DenySendMarkerPrefix(addr)
+
+	assert.Equal(t, uint8(3), denyKey[0], "should have correct prefix for send deny")
+	assert.Equal(t, denyKey[2:], addr.Bytes(), "should have marker address in iterable prefix")
+}