Use the bank module to keep track of scope value owners

.changelog/unreleased/api-breaking/2137-bank-scope-value-owners.md

@@ -0,0 +1,10 @@
+* The `Ownership` query in the `x/metadata` module now only returns scopes that have the provided address in the `owners` list [#2137](https://github.com/provenance-io/provenance/issues/2137).
+  Previously, if an address was the value owner of a scope, but not in the `owners` list, the scope would be returned
+  by the `Ownership` query when given that address.  That is no longer the case.
+  The `ValueOwnership` query can be to identify scopes with a specific value owner (like before).
+  If a scope has a value owner that is also in its `owners` list, it will still be returned by both queries.
+* The `WriteScope` endpoint now uses the `scope.value_owner_address` differently [#2137](https://github.com/provenance-io/provenance/issues/2137).
+  If it is empty, it indicates that there is no change to the value owner of the scope and the releated lookups and validation
+  are skipped. If it isn't empty, the current value owner will be looked up and the coin for the scope will be transferred to
+  the provided address (assuming signer validation passed).
+* An authz grant on `MsgWriteScope` no longer also applies to the `UpdateValueOwners` or `MigrateValueOwner` endpoints [#2137](https://github.com/provenance-io/provenance/issues/2137).

.changelog/unreleased/features/2137-bank-scope-value-owners.md

@@ -0,0 +1 @@
+* Create the `viridian` upgrade. [#2137](https://github.com/provenance-io/provenance/issues/2137).

.changelog/unreleased/improvements/2137-bank-scope-value-owners.md

@@ -0,0 +1 @@
+* Use the bank module to keep track of the value owner of scopes. [#2137](https://github.com/provenance-io/provenance/issues/2137).

.golangci.yml

@@ -85,6 +85,7 @@ linters-settings:
 
           - cosmossdk.io/api
           - cosmossdk.io/client/v2/autocli
+          - cosmossdk.io/collections
           - cosmossdk.io/core
           - cosmossdk.io/errors
           - cosmossdk.io/log

app/app.go

@@ -208,6 +208,7 @@ var (
 		wasmtypes.ModuleName:      {authtypes.Burner},
 		triggertypes.ModuleName:   nil,
 		oracletypes.ModuleName:    nil,
+		metadatatypes.ModuleName:  {authtypes.Minter, authtypes.Burner},
 	}
 )
 
@@ -580,7 +581,7 @@ func New(
 	)
 
 	app.MetadataKeeper = metadatakeeper.NewKeeper(
-		appCodec, keys[metadatatypes.StoreKey], app.AccountKeeper, app.AuthzKeeper, app.AttributeKeeper, app.MarkerKeeper,
+		appCodec, keys[metadatatypes.StoreKey], app.AccountKeeper, app.AuthzKeeper, app.AttributeKeeper, app.MarkerKeeper, app.BankKeeper,
 	)
 
 	app.HoldKeeper = holdkeeper.NewKeeper(
@@ -590,6 +591,7 @@ func New(
 	app.ExchangeKeeper = exchangekeeper.NewKeeper(
 		appCodec, keys[exchange.StoreKey], authtypes.FeeCollectorName,
 		app.AccountKeeper, app.AttributeKeeper, app.BankKeeper, app.HoldKeeper, app.MarkerKeeper,
+		app.MetadataKeeper,
 	)
 
 	pioMessageRouter := MessageRouterFunc(func(msg sdk.Msg) baseapp.MsgServiceHandler {
@@ -1312,15 +1314,6 @@ func RegisterSwaggerAPI(_ client.Context, rtr *mux.Router, swaggerEnabled bool)
 	return nil
 }
 
-// GetMaccPerms returns a copy of the module account permissions
-func GetMaccPerms() map[string][]string {
-	dupMaccPerms := make(map[string][]string)
-	for k, v := range maccPerms {
-		dupMaccPerms[k] = v
-	}
-	return dupMaccPerms
-}
-
 // initParamsKeeper init params keeper and its subspaces
 func initParamsKeeper(appCodec codec.BinaryCodec, legacyAmino *codec.LegacyAmino, key, tkey storetypes.StoreKey) paramskeeper.Keeper {
 	paramsKeeper := paramskeeper.NewKeeper(appCodec, legacyAmino, key, tkey)

app/app_test.go

@@ -70,11 +70,6 @@ func TestSimAppExportAndBlockedAddrs(t *testing.T) {
 	require.NoError(t, err, "ExportAppStateAndValidators at zero height")
 }
 
-func TestGetMaccPerms(t *testing.T) {
-	dup := GetMaccPerms()
-	require.Equal(t, maccPerms, dup, "duplicated module account permissions differed from actual module account permissions")
-}
-
 func TestExportAppStateAndValidators(t *testing.T) {
 	opts := SetupOptions{
 		Logger:  log.NewTestLogger(t),

app/upgrades.go

@@ -161,6 +161,32 @@ var upgrades = map[string]appUpgrade{
 			return vm, nil
 		},
 	},
+	"viridian-rc1": { // upgrade for v1.20.0-rc1
+		Handler: func(ctx sdk.Context, app *App, vm module.VersionMap) (module.VersionMap, error) {
+			var err error
+			if err = pruneIBCExpiredConsensusStates(ctx, app); err != nil {
+				return nil, err
+			}
+			if vm, err = runModuleMigrations(ctx, app, vm); err != nil {
+				return nil, err
+			}
+			removeInactiveValidatorDelegations(ctx, app)
+			return vm, nil
+		},
+	},
+	"viridian": { // upgrade for v1.20.0
+		Handler: func(ctx sdk.Context, app *App, vm module.VersionMap) (module.VersionMap, error) {
+			var err error
+			if err = pruneIBCExpiredConsensusStates(ctx, app); err != nil {
+				return nil, err
+			}
+			if vm, err = runModuleMigrations(ctx, app, vm); err != nil {
+				return nil, err
+			}
+			removeInactiveValidatorDelegations(ctx, app)
+			return vm, nil
+		},
+	},
 	// TODO - Add new upgrade definitions here.
 }
 

app/upgrades_test.go

@@ -29,6 +29,7 @@ import (
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
 
 	internalsdk "github.com/provenance-io/provenance/internal/sdk"
+	metadatakeeper "github.com/provenance-io/provenance/x/metadata/keeper"
 	metadatatypes "github.com/provenance-io/provenance/x/metadata/types"
 )
 
@@ -1022,3 +1023,236 @@ func (s *UpgradeTestSuite) TestExecuteStoreCodeMsg() {
 		})
 	}
 }
+
+func (s *UpgradeTestSuite) TestViridianRC1() {
+	expInLog := []string{
+		"INF Pruning expired consensus states for IBC.",
+		"INF Starting module migrations. This may take a significant amount of time to complete. Do not restart node.",
+		"INF Removing inactive validator delegations.",
+	}
+	s.AssertUpgradeHandlerLogs("viridian-rc1", expInLog, nil)
+}
+
+func (s *UpgradeTestSuite) TestViridian() {
+	expInLog := []string{
+		"INF Pruning expired consensus states for IBC.",
+		"INF Starting module migrations. This may take a significant amount of time to complete. Do not restart node.",
+		"INF Removing inactive validator delegations.",
+	}
+	s.AssertUpgradeHandlerLogs("viridian", expInLog, nil)
+}
+
+func (s *UpgradeTestSuite) TestMetadataMigration() {
+	// TODO: Delete this test with the rest of the viridian stuff.
+	newAddr := func(name string) sdk.AccAddress {
+		switch {
+		case len(name) < 20:
+			// If it's less than 19 bytes long, pad it to 20 chars.
+			return sdk.AccAddress(name + strings.Repeat("_", 20-len(name)))
+		case len(name) > 20 && len(name) < 32:
+			// If it's 21 to 31 bytes long, pad it to 32 chars.
+			return sdk.AccAddress(name + strings.Repeat("_", 32-len(name)))
+		}
+		// If the name is exactly 20 long already, or longer than 32, don't include any padding.
+		return sdk.AccAddress(name)
+	}
+	addrs := make([]string, 1000)
+	for i := range addrs {
+		addrs[i] = newAddr(fmt.Sprintf("%03d", i)).String()
+	}
+
+	newUUID := func(i int) uuid.UUID {
+		// Sixteen 9's is the largest number we can handle; one more and it's 17 digits.
+		s.Require().LessOrEqual(i, 9999999999999999, "value provided to newScopeID")
+		str := fmt.Sprintf("________________%d", i)
+		str = str[len(str)-16:]
+		rv, err := uuid.FromBytes([]byte(str))
+		s.Require().NoError(err, "uuid.FromBytes([]byte(%q))", str)
+		return rv
+	}
+	newScopeID := func(i int) metadatatypes.MetadataAddress {
+		return metadatatypes.ScopeMetadataAddress(newUUID(i))
+	}
+	newSpecID := func(i int) metadatatypes.MetadataAddress {
+		// The spec id shouldn't really matter in here, but I want it different from a scope's index.
+		// So I do some math to make it seem kind of random, but is still deterministic.
+		// 7, 39, and 79 were picked randomly and have no special meaning.
+		// 4,999 was chosen so that there's 3,333 possible results.
+		// This will overflow at 2,097,112, but it shouldn't get bigger than 300,000 in here, so we ignore that.
+		j := ((i + 7) * (i + 39) * (i + 79)) % 49_999
+		return metadatatypes.ScopeSpecMetadataAddress(newUUID(j))
+	}
+	newScope := func(i int) metadatatypes.Scope {
+		rv := metadatatypes.Scope{
+			ScopeId:            newScopeID(i),
+			SpecificationId:    newSpecID(i),
+			RequirePartyRollup: i%6 > 2, // 50% chance, three false, then three true, repeated.
+		}
+
+		// 1 in 7 does not have a value owner.
+		incVO := i%7 != 0
+		if incVO {
+			rv.ValueOwnerAddress = addrs[i%len(addrs)]
+		}
+
+		// Include 1 to 5 owners and make one of them the value owner in just under 1 in 11 scopes.
+		ownerCount := (i % 5) + 1
+		incVOInOwners := incVO && i%11 == 0
+		incVOAt := i % ownerCount
+		rv.Owners = make([]metadatatypes.Party, ownerCount)
+		for o := range rv.Owners {
+			a := i + (o+1)*300
+			if incVOInOwners && o == incVOAt {
+				a = i
+			}
+			rv.Owners[o].Address = addrs[a%len(addrs)]
+			rv.Owners[o].Role = metadatatypes.PartyType(1 + (i+o)%11) // 11 different roles, 1 to 11.
+		}
+
+		daCount := (i % 3) // 0 to 2.
+		for d := 0; d < daCount; d++ {
+			a := i + (d+1)*33
+			rv.DataAccess = append(rv.DataAccess, addrs[a%len(addrs)])
+		}
+
+		return rv
+	}
+	voInOwners := func(scope metadatatypes.Scope) bool {
+		if len(scope.ValueOwnerAddress) == 0 {
+			return false
+		}
+		for _, owner := range scope.Owners {
+			if owner.Address == scope.ValueOwnerAddress {
+				return true
+			}
+		}
+		return false
+	}
+
+	// Create 300,005 scopes and write them to state.
+	// 6 of 7 have a value owner = 257_147 = 300_005 * 6 / 7 (truncated).
+	// 1 of 7 do not = 42_858 = 300_005 - 257_147 .
+	// There's 23,377 scopes that have the value owner in owners.
+	// So we expect 490,917 keys to delete = 257,147 * 2 - 23,377.
+	expCoin := make([]metadatatypes.MetadataAddress, 0, 257_147)
+	expNoCoin := make([]metadatatypes.MetadataAddress, 0, 42_858)
+	expDelInds := make([][]byte, 0, 490_917)
+	expBals := make(map[string]sdk.Coins)
+	t1 := time.Now()
+	for i := 0; i < 300_005; i++ {
+		scope := newScope(i)
+		s.Require().NoError(s.app.MetadataKeeper.V3WriteNewScope(s.ctx, scope), "[%d]: V3WriteNewScope", i)
+
+		if len(scope.ValueOwnerAddress) == 0 {
+			expNoCoin = append(expNoCoin, scope.ScopeId)
+			continue
+		}
+
+		expCoin = append(expCoin, scope.ScopeId)
+
+		vo := scope.ValueOwnerAddress
+		voAddr := sdk.MustAccAddressFromBech32(vo)
+		expDelInds = append(expDelInds, metadatakeeper.GetValueOwnerScopeCacheKey(voAddr, scope.ScopeId))
+		if !voInOwners(scope) {
+			expDelInds = append(expDelInds, metadatatypes.GetAddressScopeCacheKey(voAddr, scope.ScopeId))
+		}
+
+		expBals[vo] = expBals[vo].Add(scope.ScopeId.Coin())
+	}
+	t2 := time.Now()
+	s.T().Logf("setup took %s", t2.Sub(t1))
+	s.T().Logf("len(expDelInds) = %d", len(expDelInds))
+	s.T().Logf("len(expCoin) = %d", len(expCoin))
+	s.T().Logf("len(expNoCoin) = %d", len(expNoCoin))
+
+	mdStore := s.ctx.KVStore(s.app.GetKey(metadatatypes.ModuleName))
+	for _, ind := range expDelInds {
+		has := mdStore.Has(ind)
+		s.Assert().True(has, "mdStore.Has(%v) before running the migrations", ind)
+	}
+	mdStore = nil
+
+	expLogs := []string{
+		"INF Starting module migrations. This may take a significant amount of time to complete. Do not restart node.",
+		"INF Starting migration of x/metadata from 3 to 4. module=x/metadata",
+		"INF Moving scope value owner data into x/bank ledger. module=x/metadata",
+		"INF Progress update: module=x/metadata scopes=10000 value owners=8571",
+		"INF Progress update: module=x/metadata scopes=20000 value owners=17143",
+		"INF Progress update: module=x/metadata scopes=30000 value owners=25714",
+		"INF Progress update: module=x/metadata scopes=40000 value owners=34286",
+		"INF Progress update: module=x/metadata scopes=50000 value owners=42857",
+		"INF Progress update: module=x/metadata scopes=60000 value owners=51428",
+		"INF Progress update: module=x/metadata scopes=70000 value owners=60000",
+		"INF Progress update: module=x/metadata scopes=80000 value owners=68571",
+		"INF Progress update: module=x/metadata scopes=90000 value owners=77143",
+		"INF Progress update: module=x/metadata scopes=100000 value owners=85714",
+		"INF Progress update: module=x/metadata scopes=110000 value owners=94286",
+		"INF Progress update: module=x/metadata scopes=120000 value owners=102857",
+		"INF Progress update: module=x/metadata scopes=130000 value owners=111428",
+		"INF Progress update: module=x/metadata scopes=140000 value owners=120000",
+		"INF Progress update: module=x/metadata scopes=150000 value owners=128571",
+		"INF Progress update: module=x/metadata scopes=160000 value owners=137143",
+		"INF Progress update: module=x/metadata scopes=170000 value owners=145714",
+		"INF Progress update: module=x/metadata scopes=180000 value owners=154286",
+		"INF Progress update: module=x/metadata scopes=190000 value owners=162857",
+		"INF Progress update: module=x/metadata scopes=200000 value owners=171428",
+		"INF Progress update: module=x/metadata scopes=210000 value owners=180000",
+		"INF Progress update: module=x/metadata scopes=220000 value owners=188572",
+		"INF Progress update: module=x/metadata scopes=230000 value owners=197143",
+		"INF Progress update: module=x/metadata scopes=240000 value owners=205714",
+		"INF Progress update: module=x/metadata scopes=250000 value owners=214286",
+		"INF Progress update: module=x/metadata scopes=260000 value owners=222857",
+		"INF Progress update: module=x/metadata scopes=270000 value owners=231429",
+		"INF Progress update: module=x/metadata scopes=280000 value owners=240000",
+		"INF Progress update: module=x/metadata scopes=290000 value owners=248572",
+		"INF Progress update: module=x/metadata scopes=300000 value owners=257143",
+		"INF Done moving scope value owners into bank module. module=x/metadata scopes=300005 value owners=257147",
+		"INF Done migrating x/metadata from 3 to 4. module=x/metadata",
+		"INF Module migrations completed.",
+	}
+
+	vm, err := s.app.UpgradeKeeper.GetModuleVersionMap(s.ctx)
+	s.Require().NoError(err, "GetModuleVersionMap")
+	s.Require().Equal(4, int(vm[metadatatypes.ModuleName]), "%s module version", metadatatypes.ModuleName)
+	// Drop it back to 3 so the migration runs.
+	vm[metadatatypes.ModuleName] = 3
+
+	runner := func() {
+		t1 = time.Now()
+		vm, err = runModuleMigrations(s.ctx, s.app, vm)
+		t2 = time.Now()
+	}
+	s.ExecuteAndAssertLogs(runner, expLogs, nil, true, "runModuleMigrations")
+	s.Assert().NoError(err, "error from runModuleMigrations")
+	s.Assert().Equal(4, int(vm[metadatatypes.ModuleName]), "vm[metadatatypes.ModuleName]")
+	s.T().Logf("runModuleMigrations took %s", t2.Sub(t1))
+
+	for _, scopeID := range expCoin {
+		denom := scopeID.Denom()
+		supply := s.app.BankKeeper.GetSupply(s.ctx, denom)
+		s.Assert().Equal("1"+denom, supply.String(), "GetSupply(%q)", denom)
+	}
+
+	for _, scopeID := range expNoCoin {
+		denom := scopeID.Denom()
+		supply := s.app.BankKeeper.GetSupply(s.ctx, denom)
+		s.Assert().Equal("0"+denom, supply.String(), "GetSupply(%q)", denom)
+	}
+
+	for i, addr := range addrs {
+		accAddr := sdk.MustAccAddressFromBech32(addr)
+		actBal := s.app.BankKeeper.GetAllBalances(s.ctx, accAddr)
+		for _, expCoin := range expBals[addr] {
+			found, actCoin := actBal.Find(expCoin.Denom)
+			if s.Assert().True(found, "[%d]%q: found bool from actBal.Find(%q)", i, addr, expCoin.Denom) {
+				s.Assert().Equal(expCoin.String(), actCoin.String(), "[%d]%q: balance coin for %q", i, addr, expCoin.Denom)
+			}
+		}
+	}
+
+	mdStore = s.ctx.KVStore(s.app.GetKey(metadatatypes.ModuleName))
+	for _, ind := range expDelInds {
+		has := mdStore.Has(ind)
+		s.Assert().False(has, "mdStore.Has(%v) after running the migrations", ind)
+	}
+}