Clean up the Marker's expected BankKeeper interface.

CHANGELOG.md

@@ -71,12 +71,13 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * Switch to InputOutputCoinsProv for exchange transfers [#1930](https://github.com/provenance-io/provenance/pull/1930).
 * Use fields of the SimulationState for the encoders needed for simulations [#1931](https://github.com/provenance-io/provenance/pull/1931).
 * Removes sync-info code for sdk v0.50 [#1760](https://github.com/provenance-io/provenance/issues/1760).
-* Fix most of the failing unit tests [#1943](https://github.com/provenance-io/provenance/pull/1943)
+* Fix most of the failing unit tests [#1943](https://github.com/provenance-io/provenance/pull/1943).
 * Clean up ReadFromClient [#1760](https://github.com/provenance-io/provenance/issues/1760).
 * Remove all `GetSigners()` methods [#1957](https://github.com/provenance-io/provenance/pull/1957).
 * Ensure all `Msg`s have correctly identified `signer` fields [#1957](https://github.com/provenance-io/provenance/pull/1957).
 * Clean up all the module codecs [#1957](https://github.com/provenance-io/provenance/pull/1957).
 * Switch to auto-generated `String` and `Equal` methods for most proto messages [#1957](https://github.com/provenance-io/provenance/pull/1957).
+* Clean up the marker module's expected BankKeeper interface [#1954](https://github.com/provenance-io/provenance/pull/1954).
 
 ### Dependencies
 

x/marker/keeper/keeper_test.go

@@ -11,13 +11,14 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	abci "github.com/cometbft/cometbft/abci/types"
+
 	sdkmath "cosmossdk.io/math"
 	"cosmossdk.io/x/feegrant"
 
-	abci "github.com/cometbft/cometbft/abci/types"
-
 	"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/types/query"
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	"github.com/cosmos/cosmos-sdk/x/bank/testutil"
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
@@ -41,6 +42,25 @@ func setNewAccount(app *simapp.App, ctx sdk.Context, acc sdk.AccountI) sdk.Accou
 	return newAcc
 }
 
+// getAllMarkerHolders gets all the accounts holding a given denom, and the amount they each hold.
+func getAllMarkerHolders(t *testing.T, ctx context.Context, app *simapp.App, denom string) []types.Balance {
+	req := &banktypes.QueryDenomOwnersRequest{
+		Denom:      denom,
+		Pagination: &query.PageRequest{Limit: 10000},
+	}
+	resp, err := app.BankKeeper.DenomOwners(ctx, req)
+	require.NoError(t, err, "BankKeeper.DenomOwners(%q)", denom)
+	if len(resp.DenomOwners) == 0 {
+		return nil
+	}
+	rv := make([]types.Balance, len(resp.DenomOwners))
+	for i, owner := range resp.DenomOwners {
+		rv[i].Address = owner.Address
+		rv[i].Coins = sdk.Coins{owner.Balance}
+	}
+	return rv
+}
+
 func TestAccountMapperGetSet(t *testing.T) {
 	app := simapp.Setup(t)
 	ctx := app.BaseApp.NewContext(false)
@@ -87,7 +107,7 @@ func TestAccountMapperGetSet(t *testing.T) {
 	acc = app.AccountKeeper.GetAccount(ctx, addr)
 	require.Nil(t, acc)
 
-	require.Empty(t, app.MarkerKeeper.GetAllMarkerHolders(ctx, "testcoin"))
+	require.Empty(t, getAllMarkerHolders(t, ctx, app, "testcoin"))
 
 	// check for error on invaid marker denom
 	_, err := app.MarkerKeeper.GetMarkerByDenom(ctx, "doesntexist")
@@ -415,13 +435,13 @@ func TestMintBurnCoins(t *testing.T) {
 	require.Error(t, app.MarkerKeeper.CancelMarker(ctx, user, "testcoin"))
 
 	// two a user and the marker
-	require.Equal(t, 2, len(app.MarkerKeeper.GetAllMarkerHolders(ctx, "testcoin")))
+	require.Equal(t, 2, len(getAllMarkerHolders(t, ctx, app, "testcoin")))
 
 	// put the coins back in the types.
 	require.NoError(t, app.BankKeeper.SendCoins(ctx, user, addr, sdk.NewCoins(sdk.NewInt64Coin("testcoin", 50))))
 
 	// one, only the marker
-	require.Equal(t, 1, len(app.MarkerKeeper.GetAllMarkerHolders(ctx, "testcoin")))
+	require.Equal(t, 1, len(getAllMarkerHolders(t, ctx, app, "testcoin")))
 
 	// succeeds because marker has all its supply
 	require.NoError(t, app.MarkerKeeper.CancelMarker(ctx, user, "testcoin"))
@@ -446,7 +466,7 @@ func TestMintBurnCoins(t *testing.T) {
 	require.NoError(t, app.MarkerKeeper.DeleteMarker(ctx, user, "testcoin"))
 
 	// none, marker has been deleted
-	require.Equal(t, 0, len(app.MarkerKeeper.GetAllMarkerHolders(ctx, "testcoin")))
+	require.Equal(t, 0, len(getAllMarkerHolders(t, ctx, app, "testcoin")))
 
 	// verify status is destroyed and supply is zero.
 	m, err = app.MarkerKeeper.GetMarker(ctx, addr)

x/marker/keeper/marker.go

@@ -16,24 +16,6 @@ import (
 	"github.com/provenance-io/provenance/x/marker/types"
 )
 
-// GetAllMarkerHolders returns an array of all account addresses holding the given denom (and the amount)
-func (k Keeper) GetAllMarkerHolders(ctx sdk.Context, denom string) []types.Balance {
-	defer telemetry.MeasureSince(time.Now(), types.ModuleName, "get_all_marker_holders")
-
-	var results []types.Balance
-	k.bankKeeper.IterateAllBalances(ctx, func(addr sdk.AccAddress, coin sdk.Coin) (stop bool) {
-		if coin.Denom == denom && !coin.Amount.IsZero() {
-			results = append(results,
-				types.Balance{
-					Address: addr.String(),
-					Coins:   sdk.NewCoins(coin),
-				})
-		}
-		return false // do not stop iterating
-	})
-	return results
-}
-
 // GetMarkerByDenom looks up marker with the given denom
 func (k Keeper) GetMarkerByDenom(ctx sdk.Context, denom string) (types.MarkerAccountI, error) {
 	defer telemetry.MeasureSince(time.Now(), types.ModuleName, "get_marker_by_denom")

x/marker/types/expected_keepers.go

@@ -49,14 +49,6 @@ type BankKeeper interface {
 
 	GetDenomMetaData(context context.Context, denom string) (banktypes.Metadata, bool)
 	SetDenomMetaData(context context.Context, denomMetaData banktypes.Metadata)
-
-	// TODO[1760]: bank: Delete the below entries when no longer needed (or change this back to a regular TODO).
-
-	// IterateAllBalances only used in GetAllMarkerHolders used by the unneeded querier.
-	// The Holding query just uses the DenomOwners query endpoint.
-	IterateAllBalances(context context.Context, cb func(address sdk.AccAddress, coin sdk.Coin) (stop bool))
-	// GetAllSendEnabledEntries only needed by RemoveIsSendEnabledEntries in the quicksilver upgrade.
-	GetAllSendEnabledEntries(context context.Context) []banktypes.SendEnabled
 }
 
 // FeeGrantKeeper defines the fee-grant functionality needed by the marker module.

x/marker/types/marker_test.go

@@ -81,13 +81,6 @@ func TestNewEmptyMarkerValidate(t *testing.T) {
 		m.SetSupply(sdk.NewInt64Coin("other", 1000)), "expected failure setting supply to invalid denom of coin")
 	require.EqualValues(t, m.GetSupply(), sdk.NewInt64Coin("test", 1), "supply should be still be one")
 
-	// TODO[1760]: yaml: Decide if we still want this yaml convertion tested.
-	/*
-		yaml, merr := m.MarshalYAML()
-		require.NoError(t, merr, "marshall of yaml should succeed without error")
-		require.Equal(t, yaml, m.String(), "should use yaml for string() view")
-	*/
-
 	if err != nil {
 		t.Fatalf("expect no errors from marker validation: %s", err)
 	}