From d37c623b78a9c2b5e488d4d0eba009842d9a1f68 Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Thu, 31 Aug 2023 12:51:48 +0900 Subject: [PATCH 01/14] Add coins --- x/collection/keeper/supply.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/collection/keeper/supply.go b/x/collection/keeper/supply.go index 8fb9e5a474..0a09943742 100644 --- a/x/collection/keeper/supply.go +++ b/x/collection/keeper/supply.go @@ -183,8 +183,8 @@ func (k Keeper) MintFT(ctx sdk.Context, contractID string, to sdk.AccAddress, am } func (k Keeper) mintFT(ctx sdk.Context, contractID string, to sdk.AccAddress, classID string, amount sdk.Int) { - tokenID := collection.NewFTID(classID) - k.setBalance(ctx, contractID, to, tokenID, amount) + coins := collection.NewCoins(collection.NewFTCoin(classID, amount)) + k.addCoins(ctx, contractID, to, coins) // update statistics supply := k.GetSupply(ctx, contractID, classID) From 3cb86e4634198c9eb096a91971813cb308a201ae Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Thu, 31 Aug 2023 12:52:11 +0900 Subject: [PATCH 02/14] Add the corresponding unit test --- x/collection/keeper/supply_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/x/collection/keeper/supply_test.go b/x/collection/keeper/supply_test.go index b79c53be65..49a1617d45 100644 --- a/x/collection/keeper/supply_test.go +++ b/x/collection/keeper/supply_test.go @@ -101,6 +101,16 @@ func (s *KeeperTestSuite) TestMintFT() { } }) } + + // accumulation test + s.Run("accumulation test", func() { + ctx, _ := s.ctx.CacheContext() + numMints := int64(10) + for i := int64(1); i <= numMints; i++ { + s.keeper.MintFT(ctx, s.contractID, s.stranger, collection.NewCoins(collection.NewFTCoin(s.ftClassID, sdk.OneInt()))) + s.Require().Equal(sdk.NewInt(i), s.keeper.GetBalance(ctx, s.contractID, s.stranger, collection.NewFTID(s.ftClassID))) + } + }) } func (s *KeeperTestSuite) TestMintNFT() { From 924261c19cafeaf0c8a60099bc185bda8026735a Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Thu, 31 Aug 2023 13:23:28 +0900 Subject: [PATCH 03/14] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f0ab927712..5b2ee4088d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/bank) [#1093](https://github.com/Finschia/finschia-sdk/pull/1093) Remove message events including `sender` attribute whose information is already present in the relevant events (backport #1066) * (ostracon) [\#1099](https://github.com/Finschia/finschia-sdk/pull/1099) feat!: remove libsodium vrf library. * (x/collection) [\#1102](https://github.com/finschia/finschia-sdk/pull/1102) Reject modifying NFT class with token index filled in MsgModify +* (x/collection) [\#1105](https://github.com/finschia/finschia-sdk/pull/1105) Add minted coins to balance in x/collection MsgMintFT ### Build, CI * (build,ci) [\#1043](https://github.com/Finschia/finschia-sdk/pull/1043) Update golang version to 1.20 From 3a3e42368a3d74c756b6a368e27b727f52ee6380 Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Thu, 31 Aug 2023 18:33:33 +0900 Subject: [PATCH 04/14] Add total-supply invariants --- x/collection/keeper/invariants.go | 76 +++++++++++++++++++++++++++++++ x/collection/module/module.go | 4 +- 2 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 x/collection/keeper/invariants.go diff --git a/x/collection/keeper/invariants.go b/x/collection/keeper/invariants.go new file mode 100644 index 0000000000..7d000bea1d --- /dev/null +++ b/x/collection/keeper/invariants.go @@ -0,0 +1,76 @@ +package keeper + +import ( + "strings" + + sdk "github.com/Finschia/finschia-sdk/types" + "github.com/Finschia/finschia-sdk/x/collection" +) + +const ( + totalSupplyInvariant = "total-supply" +) + +func RegisterInvariants(ir sdk.InvariantRegistry, k Keeper) { + for name, invariant := range map[string]func(k Keeper) sdk.Invariant{ + totalSupplyInvariant: TotalSupplyInvariant, + } { + ir.RegisterRoute(collection.ModuleName, name, invariant(k)) + } +} + +func TotalSupplyInvariant(k Keeper) sdk.Invariant { + return func(ctx sdk.Context) (string, bool) { + // cache, we don't want to write changes + ctx, _ = ctx.CacheContext() + + invalidClassIDs := map[string][]string{} + k.iterateContracts(ctx, func(contract collection.Contract) (stop bool) { + supplies := map[string]sdk.Int{} + k.iterateContractSupplies(ctx, contract.Id, func(classID string, amount sdk.Int) (stop bool) { + supplies[classID] = amount + return false + }) + + k.iterateContractBalances(ctx, contract.Id, func(address sdk.AccAddress, balance collection.Coin) (stop bool) { + classID := collection.SplitTokenID(balance.TokenId) + amount, ok := supplies[classID] + if !ok { + amount = sdk.ZeroInt() + } + + supplies[classID] = amount.Sub(balance.Amount) + return false + }) + + invalidClassIDsCandidate := []string{} + for classID, supply := range supplies { + if !supply.IsZero() { + invalidClassIDsCandidate = append(invalidClassIDsCandidate, classID) + } + } + + if len(invalidClassIDsCandidate) != 0 { + invalidClassIDs[contract.Id] = invalidClassIDsCandidate + } + + return false + }) + + broken := len(invalidClassIDs) != 0 + msg := "no violation found" + if broken { + concatenated := []string{} + delimiter := ":" + for contractID, classIDs := range invalidClassIDs { + for _, classID := range classIDs { + concatenated = append(concatenated, contractID+delimiter+classID) + } + } + + msg = "violation found on following classIDs: " + strings.Join(concatenated, ", ") + } + + return sdk.FormatInvariant(collection.ModuleName, totalSupplyInvariant, msg), broken + } +} diff --git a/x/collection/module/module.go b/x/collection/module/module.go index 1c9a323cd5..51c0ae6d12 100644 --- a/x/collection/module/module.go +++ b/x/collection/module/module.go @@ -89,7 +89,9 @@ func NewAppModule(cdc codec.Codec, keeper keeper.Keeper) AppModule { } // RegisterInvariants does nothing, there are no invariants to enforce -func (AppModule) RegisterInvariants(_ sdk.InvariantRegistry) {} +func (am AppModule) RegisterInvariants(ir sdk.InvariantRegistry) { + keeper.RegisterInvariants(ir, am.keeper) +} // Route returns the message routing key for the collection module. func (am AppModule) Route() sdk.Route { return sdk.Route{} } From 8cdd61c748fd731970d64c6484f1b651c5dca1b7 Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Thu, 31 Aug 2023 23:11:57 +0900 Subject: [PATCH 05/14] Add migration logic --- x/collection/keeper/migrations.go | 32 +++++ x/collection/keeper/migrations/v2/keys.go | 92 ++++++++++++++ x/collection/keeper/migrations/v2/store.go | 134 +++++++++++++++++++++ x/collection/module/module.go | 1 + 4 files changed, 259 insertions(+) create mode 100644 x/collection/keeper/migrations.go create mode 100644 x/collection/keeper/migrations/v2/keys.go create mode 100644 x/collection/keeper/migrations/v2/store.go diff --git a/x/collection/keeper/migrations.go b/x/collection/keeper/migrations.go new file mode 100644 index 0000000000..b403d656fb --- /dev/null +++ b/x/collection/keeper/migrations.go @@ -0,0 +1,32 @@ +package keeper + +import ( + sdk "github.com/Finschia/finschia-sdk/types" + "github.com/Finschia/finschia-sdk/types/module" + "github.com/Finschia/finschia-sdk/x/collection" + v2 "github.com/Finschia/finschia-sdk/x/collection/keeper/migrations/v2" +) + +// Migrator is a struct for handling in-place store migrations. +type Migrator struct { + keeper Keeper +} + +// NewMigrator returns a new Migrator. +func NewMigrator(keeper Keeper) Migrator { + return Migrator{keeper: keeper} +} + +func (m Migrator) Register(register func(moduleName string, fromVersion uint64, handler module.MigrationHandler) error) error { + for fromVersion, handler := range map[uint64]module.MigrationHandler{ + 1: func(ctx sdk.Context) error { + return v2.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc) + }, + } { + if err := register(collection.ModuleName, fromVersion, handler); err != nil { + return err + } + } + + return nil +} diff --git a/x/collection/keeper/migrations/v2/keys.go b/x/collection/keeper/migrations/v2/keys.go new file mode 100644 index 0000000000..440e540b4e --- /dev/null +++ b/x/collection/keeper/migrations/v2/keys.go @@ -0,0 +1,92 @@ +package v2 + +import ( + sdk "github.com/Finschia/finschia-sdk/types" +) + +var ( + contractKeyPrefix = []byte{0x10} + classKeyPrefix = []byte{0x11} + nextClassIDKeyPrefix = []byte{0x12} + + balanceKeyPrefix = []byte{0x20} + + supplyKeyPrefix = []byte{0x40} + mintedKeyPrefix = []byte{0x41} + burntKeyPrefix = []byte{0x42} +) + +func balanceKeyPrefixByContractID(contractID string) []byte { + key := make([]byte, len(balanceKeyPrefix)+1+len(contractID)) + + begin := 0 + copy(key, balanceKeyPrefix) + + begin += len(balanceKeyPrefix) + key[begin] = byte(len(contractID)) + + begin++ + copy(key[begin:], contractID) + + return key +} + +func splitBalanceKey(key []byte) (contractID string, address sdk.AccAddress, tokenID string) { + begin := len(balanceKeyPrefix) + 1 + end := begin + int(key[begin-1]) + contractID = string(key[begin:end]) + + begin = end + 1 + end = begin + int(key[begin-1]) + address = sdk.AccAddress(key[begin:end]) + + begin = end + tokenID = string(key[begin:]) + + return +} + +func statisticKey(keyPrefix []byte, contractID string, classID string) []byte { + prefix := statisticKeyPrefixByContractID(keyPrefix, contractID) + key := make([]byte, len(prefix)+len(classID)) + + copy(key, prefix) + copy(key[len(prefix):], classID) + + return key +} + +func statisticKeyPrefixByContractID(keyPrefix []byte, contractID string) []byte { + key := make([]byte, len(keyPrefix)+1+len(contractID)) + + begin := 0 + copy(key, keyPrefix) + + begin += len(keyPrefix) + key[begin] = byte(len(contractID)) + + begin++ + copy(key[begin:], contractID) + + return key +} + +func splitStatisticKey(keyPrefix, key []byte) (contractID string, classID string) { + begin := len(keyPrefix) + 1 + end := begin + int(key[begin-1]) + contractID = string(key[begin:end]) + + begin = end + classID = string(key[begin:]) + + return +} + +func nextClassIDKey(contractID string) []byte { + key := make([]byte, len(nextClassIDKeyPrefix)+len(contractID)) + + copy(key, nextClassIDKeyPrefix) + copy(key[len(nextClassIDKeyPrefix):], contractID) + + return key +} diff --git a/x/collection/keeper/migrations/v2/store.go b/x/collection/keeper/migrations/v2/store.go new file mode 100644 index 0000000000..08d943d288 --- /dev/null +++ b/x/collection/keeper/migrations/v2/store.go @@ -0,0 +1,134 @@ +package v2 + +import ( + "fmt" + + "github.com/Finschia/finschia-sdk/codec" + storetypes "github.com/Finschia/finschia-sdk/store/types" + sdk "github.com/Finschia/finschia-sdk/types" + "github.com/Finschia/finschia-sdk/x/collection" +) + +// MigrateStore performs in-place store migrations from v1 to v2. +func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.BinaryCodec) error { + store := ctx.KVStore(storeKey) + + // fix ft statistics + if err := fixFTStatistics(store, cdc); err != nil { + return err + } + + return nil +} + +func fixFTStatistics(store storetypes.KVStore, cdc codec.BinaryCodec) error { + iterator := sdk.KVStorePrefixIterator(store, contractKeyPrefix) + defer iterator.Close() + + for ; iterator.Valid(); iterator.Next() { + var contract collection.Contract + if err := cdc.Unmarshal(iterator.Value(), &contract); err != nil { + return err + } + + if err := fixContractFTStatistics(store, cdc, contract.Id); err != nil { + return err + } + } + + return nil +} + +func fixContractFTStatistics(store storetypes.KVStore, cdc codec.BinaryCodec, contractID string) error { + supplies, err := evalContractFTSupplies(store, contractID) + if err != nil { + return err + } + + if err := updateContractFTStatistics(store, contractID, supplies); err != nil { + return err + } + + return nil +} + +func evalContractFTSupplies(store storetypes.KVStore, contractID string) (map[string]sdk.Int, error) { + prefix := balanceKeyPrefixByContractID(contractID) + iterator := sdk.KVStorePrefixIterator(store, prefix) + defer iterator.Close() + + supplies := map[string]sdk.Int{} + for ; iterator.Valid(); iterator.Next() { + _, _, tokenID := splitBalanceKey(iterator.Key()) + if err := collection.ValidateFTID(tokenID); err != nil { + continue + } + + var amount sdk.Int + if err := amount.Unmarshal(iterator.Value()); err != nil { + return nil, err + } + + classID := collection.SplitTokenID(tokenID) + if supply, ok := supplies[classID]; ok { + supplies[classID] = supply.Add(amount) + } else { + supplies[classID] = amount + } + } + + return supplies, nil +} + +func updateContractFTStatistics(store storetypes.KVStore, contractID string, supplies map[string]sdk.Int) error { + bz := store.Get(nextClassIDKey(contractID)) + if bz == nil { + return fmt.Errorf("no next class ids of contract %s", contractID) + } + + var nextClassIDs collection.NextClassIDs + if err := nextClassIDs.Unmarshal(bz); err != nil { + return err + } + + for intClassID := uint64(1); intClassID < nextClassIDs.Fungible.Uint64(); intClassID++ { + classID := fmt.Sprintf("%08x", intClassID) + + // update supply + supplyKey := statisticKey(supplyKeyPrefix, contractID, classID) + supply, ok := supplies[classID] + if ok { + bz, err := supply.Marshal() + if err != nil { + return err + } + store.Set(supplyKey, bz) + } else { + store.Delete(supplyKey) + } + + // get burnt + burntKey := statisticKey(burntKeyPrefix, contractID, classID) + burnt := sdk.ZeroInt() + if bz := store.Get(burntKey); bz != nil { + if err := burnt.Unmarshal(bz); err != nil { + return err + } + } + + // update minted + minted := supply.Add(burnt) + mintedKey := statisticKey(mintedKeyPrefix, contractID, classID) + if !minted.IsZero() { + bz, err := minted.Marshal() + if err != nil { + return err + } + store.Set(mintedKey, bz) + } else { + store.Delete(mintedKey) + } + } + + return nil +} diff --git a/x/collection/module/module.go b/x/collection/module/module.go index 51c0ae6d12..ab713a9d81 100644 --- a/x/collection/module/module.go +++ b/x/collection/module/module.go @@ -109,6 +109,7 @@ func (am AppModule) LegacyQuerierHandler(legacyQuerierCdc *codec.LegacyAmino) sd func (am AppModule) RegisterServices(cfg module.Configurator) { collection.RegisterMsgServer(cfg.MsgServer(), keeper.NewMsgServer(am.keeper)) collection.RegisterQueryServer(cfg.QueryServer(), keeper.NewQueryServer(am.keeper)) + keeper.NewMigrator(am.keeper).Register(cfg.RegisterMigration) } // InitGenesis performs genesis initialization for the collection module. It returns From eee7b936dc1800dc1340b5a6979363b476592ae5 Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Fri, 1 Sep 2023 00:25:42 +0900 Subject: [PATCH 06/14] Add unit tests on migration --- x/collection/keeper/migrations/v2/keys.go | 56 +++++-- x/collection/keeper/migrations/v2/store.go | 8 +- .../keeper/migrations/v2/store_test.go | 152 ++++++++++++++++++ 3 files changed, 196 insertions(+), 20 deletions(-) create mode 100644 x/collection/keeper/migrations/v2/store_test.go diff --git a/x/collection/keeper/migrations/v2/keys.go b/x/collection/keeper/migrations/v2/keys.go index 440e540b4e..1dff25e6ef 100644 --- a/x/collection/keeper/migrations/v2/keys.go +++ b/x/collection/keeper/migrations/v2/keys.go @@ -11,11 +11,46 @@ var ( balanceKeyPrefix = []byte{0x20} - supplyKeyPrefix = []byte{0x40} - mintedKeyPrefix = []byte{0x41} - burntKeyPrefix = []byte{0x42} + SupplyKeyPrefix = []byte{0x40} + MintedKeyPrefix = []byte{0x41} + BurntKeyPrefix = []byte{0x42} ) +func ContractKey(contractID string) []byte { + key := make([]byte, len(contractKeyPrefix)+len(contractID)) + + copy(key, contractKeyPrefix) + copy(key[len(contractKeyPrefix):], contractID) + + return key +} + +func BalanceKey(contractID string, address sdk.AccAddress, tokenID string) []byte { + prefix := balanceKeyPrefixByAddress(contractID, address) + key := make([]byte, len(prefix)+len(tokenID)) + + copy(key, prefix) + copy(key[len(prefix):], tokenID) + + return key +} + +func balanceKeyPrefixByAddress(contractID string, address sdk.AccAddress) []byte { + prefix := balanceKeyPrefixByContractID(contractID) + key := make([]byte, len(prefix)+1+len(address)) + + begin := 0 + copy(key, prefix) + + begin += len(prefix) + key[begin] = byte(len(address)) + + begin++ + copy(key[begin:], address) + + return key +} + func balanceKeyPrefixByContractID(contractID string) []byte { key := make([]byte, len(balanceKeyPrefix)+1+len(contractID)) @@ -46,7 +81,7 @@ func splitBalanceKey(key []byte) (contractID string, address sdk.AccAddress, tok return } -func statisticKey(keyPrefix []byte, contractID string, classID string) []byte { +func StatisticKey(keyPrefix []byte, contractID string, classID string) []byte { prefix := statisticKeyPrefixByContractID(keyPrefix, contractID) key := make([]byte, len(prefix)+len(classID)) @@ -71,18 +106,7 @@ func statisticKeyPrefixByContractID(keyPrefix []byte, contractID string) []byte return key } -func splitStatisticKey(keyPrefix, key []byte) (contractID string, classID string) { - begin := len(keyPrefix) + 1 - end := begin + int(key[begin-1]) - contractID = string(key[begin:end]) - - begin = end - classID = string(key[begin:]) - - return -} - -func nextClassIDKey(contractID string) []byte { +func NextClassIDKey(contractID string) []byte { key := make([]byte, len(nextClassIDKeyPrefix)+len(contractID)) copy(key, nextClassIDKeyPrefix) diff --git a/x/collection/keeper/migrations/v2/store.go b/x/collection/keeper/migrations/v2/store.go index 08d943d288..4fbe32a2f7 100644 --- a/x/collection/keeper/migrations/v2/store.go +++ b/x/collection/keeper/migrations/v2/store.go @@ -81,7 +81,7 @@ func evalContractFTSupplies(store storetypes.KVStore, contractID string) (map[st } func updateContractFTStatistics(store storetypes.KVStore, contractID string, supplies map[string]sdk.Int) error { - bz := store.Get(nextClassIDKey(contractID)) + bz := store.Get(NextClassIDKey(contractID)) if bz == nil { return fmt.Errorf("no next class ids of contract %s", contractID) } @@ -95,7 +95,7 @@ func updateContractFTStatistics(store storetypes.KVStore, contractID string, sup classID := fmt.Sprintf("%08x", intClassID) // update supply - supplyKey := statisticKey(supplyKeyPrefix, contractID, classID) + supplyKey := StatisticKey(SupplyKeyPrefix, contractID, classID) supply, ok := supplies[classID] if ok { bz, err := supply.Marshal() @@ -108,7 +108,7 @@ func updateContractFTStatistics(store storetypes.KVStore, contractID string, sup } // get burnt - burntKey := statisticKey(burntKeyPrefix, contractID, classID) + burntKey := StatisticKey(BurntKeyPrefix, contractID, classID) burnt := sdk.ZeroInt() if bz := store.Get(burntKey); bz != nil { if err := burnt.Unmarshal(bz); err != nil { @@ -118,7 +118,7 @@ func updateContractFTStatistics(store storetypes.KVStore, contractID string, sup // update minted minted := supply.Add(burnt) - mintedKey := statisticKey(mintedKeyPrefix, contractID, classID) + mintedKey := StatisticKey(MintedKeyPrefix, contractID, classID) if !minted.IsZero() { bz, err := minted.Marshal() if err != nil { diff --git a/x/collection/keeper/migrations/v2/store_test.go b/x/collection/keeper/migrations/v2/store_test.go new file mode 100644 index 0000000000..cbd97d77c6 --- /dev/null +++ b/x/collection/keeper/migrations/v2/store_test.go @@ -0,0 +1,152 @@ +package v2_test + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" + + simappparams "github.com/Finschia/finschia-sdk/simapp/params" + "github.com/Finschia/finschia-sdk/testutil" + sdk "github.com/Finschia/finschia-sdk/types" + "github.com/Finschia/finschia-sdk/x/collection" + + "github.com/Finschia/finschia-sdk/x/collection/keeper/migrations/v2" +) + +func TestMigrateStore(t *testing.T) { + collectionKey := sdk.NewKVStoreKey(collection.StoreKey) + newKey := sdk.NewTransientStoreKey("transient_test") + encCfg := simappparams.MakeTestEncodingConfig() + ctx := testutil.DefaultContext(collectionKey, newKey) + + // set state + store := ctx.KVStore(collectionKey) + + contractID := "deadbeef" + store.Set(v2.ContractKey(contractID), encCfg.Marshaler.MustMarshal(&collection.Contract{Id: contractID})) + nextClassIDs := collection.DefaultNextClassIDs(contractID) + classID := fmt.Sprintf("%08x", nextClassIDs.Fungible.Uint64()) + nextClassIDs.Fungible = nextClassIDs.Fungible.Incr() + store.Set(v2.NextClassIDKey(contractID), encCfg.Marshaler.MustMarshal(&nextClassIDs)) + + tokenID := collection.NewFTID(classID) + oneIntBz, err := sdk.OneInt().Marshal() + require.NoError(t, err) + addresses := []sdk.AccAddress{ + sdk.AccAddress("fennec"), + sdk.AccAddress("penguin"), + sdk.AccAddress("cheetah"), + } + for _, addr := range addresses { + store.Set(v2.BalanceKey(contractID, addr, tokenID), oneIntBz) + } + store.Set(v2.StatisticKey(v2.SupplyKeyPrefix, contractID, classID), oneIntBz) + store.Set(v2.StatisticKey(v2.MintedKeyPrefix, contractID, classID), oneIntBz) + store.Set(v2.StatisticKey(v2.BurntKeyPrefix, contractID, classID), oneIntBz) + + for name, tc := range map[string]struct { + malleate func(ctx sdk.Context) + valid bool + supply int + minted int + }{ + "valid": { + valid: true, + supply: len(addresses), + minted: len(addresses) + 1, + }, + "valid (nil supply)": { + malleate: func(ctx sdk.Context) { + store := ctx.KVStore(collectionKey) + store.Delete(v2.StatisticKey(v2.SupplyKeyPrefix, contractID, classID)) + }, + valid: true, + supply: len(addresses), + minted: len(addresses) + 1, + }, + "valid (nil minted)": { + malleate: func(ctx sdk.Context) { + store := ctx.KVStore(collectionKey) + store.Delete(v2.StatisticKey(v2.MintedKeyPrefix, contractID, classID)) + }, + valid: true, + supply: len(addresses), + minted: len(addresses) + 1, + }, + "valid (nil burnt)": { + malleate: func(ctx sdk.Context) { + store := ctx.KVStore(collectionKey) + store.Delete(v2.StatisticKey(v2.BurntKeyPrefix, contractID, classID)) + }, + valid: true, + supply: len(addresses), + minted: len(addresses), + }, + "contract unmarshal failed": { + malleate: func(ctx sdk.Context) { + store := ctx.KVStore(collectionKey) + store.Set(v2.ContractKey(contractID), encCfg.Marshaler.MustMarshal(&collection.GenesisState{})) + }, + }, + "balance unmarshal failed": { + malleate: func(ctx sdk.Context) { + store := ctx.KVStore(collectionKey) + store.Set(v2.BalanceKey(contractID, sdk.AccAddress("hyena"), tokenID), encCfg.Marshaler.MustMarshal(&collection.GenesisState{})) + }, + }, + "no next class id": { + malleate: func(ctx sdk.Context) { + store := ctx.KVStore(collectionKey) + store.Delete(v2.NextClassIDKey(contractID)) + }, + }, + "next class id unmarshal failed": { + malleate: func(ctx sdk.Context) { + store := ctx.KVStore(collectionKey) + store.Set(v2.NextClassIDKey(contractID), []byte("invalid")) + }, + }, + "burnt unmarshal failed": { + malleate: func(ctx sdk.Context) { + store := ctx.KVStore(collectionKey) + store.Set(v2.StatisticKey(v2.BurntKeyPrefix, contractID, classID), encCfg.Marshaler.MustMarshal(&collection.GenesisState{})) + }, + }, + } { + t.Run(name, func(t *testing.T) { + ctx, _ := ctx.CacheContext() + if tc.malleate != nil { + tc.malleate(ctx) + } + + // migrate + err := v2.MigrateStore(ctx, collectionKey, encCfg.Marshaler) + if !tc.valid { + require.Error(t, err) + return + } + require.NoError(t, err) + + store := ctx.KVStore(collectionKey) + + // supply + supplyKey := v2.StatisticKey(v2.SupplyKeyPrefix, contractID, classID) + supply := sdk.ZeroInt() + if bz := store.Get(supplyKey); bz != nil { + err := supply.Unmarshal(bz) + require.NoError(t, err) + } + require.Equal(t, int64(tc.supply), supply.Int64()) + + // minted + mintedKey := v2.StatisticKey(v2.MintedKeyPrefix, contractID, classID) + minted := sdk.ZeroInt() + if bz := store.Get(mintedKey); bz != nil { + err := minted.Unmarshal(bz) + require.NoError(t, err) + } + require.Equal(t, int64(tc.minted), minted.Int64()) + }) + } +} From e85305f69508f21753368d57cbd61fa9b4ab745c Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Fri, 1 Sep 2023 13:53:36 +0900 Subject: [PATCH 07/14] Do not verify nfts --- x/collection/keeper/invariants.go | 33 ++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/x/collection/keeper/invariants.go b/x/collection/keeper/invariants.go index 7d000bea1d..44cbccece3 100644 --- a/x/collection/keeper/invariants.go +++ b/x/collection/keeper/invariants.go @@ -8,32 +8,41 @@ import ( ) const ( - totalSupplyInvariant = "total-supply" + totalFTSupplyInvariant = "total-ft-supply" ) func RegisterInvariants(ir sdk.InvariantRegistry, k Keeper) { for name, invariant := range map[string]func(k Keeper) sdk.Invariant{ - totalSupplyInvariant: TotalSupplyInvariant, + totalFTSupplyInvariant: TotalFTSupplyInvariant, } { ir.RegisterRoute(collection.ModuleName, name, invariant(k)) } } -func TotalSupplyInvariant(k Keeper) sdk.Invariant { +func TotalFTSupplyInvariant(k Keeper) sdk.Invariant { return func(ctx sdk.Context) (string, bool) { // cache, we don't want to write changes ctx, _ = ctx.CacheContext() - invalidClassIDs := map[string][]string{} + invalidFTClassIDs := map[string][]string{} k.iterateContracts(ctx, func(contract collection.Contract) (stop bool) { supplies := map[string]sdk.Int{} k.iterateContractSupplies(ctx, contract.Id, func(classID string, amount sdk.Int) (stop bool) { + if err := collection.ValidateLegacyFTClassID(classID); err != nil { + return false + } + supplies[classID] = amount + return false }) k.iterateContractBalances(ctx, contract.Id, func(address sdk.AccAddress, balance collection.Coin) (stop bool) { classID := collection.SplitTokenID(balance.TokenId) + if err := collection.ValidateLegacyFTClassID(classID); err != nil { + return false + } + amount, ok := supplies[classID] if !ok { amount = sdk.ZeroInt() @@ -43,34 +52,34 @@ func TotalSupplyInvariant(k Keeper) sdk.Invariant { return false }) - invalidClassIDsCandidate := []string{} + invalidFTClassIDsCandidate := []string{} for classID, supply := range supplies { if !supply.IsZero() { - invalidClassIDsCandidate = append(invalidClassIDsCandidate, classID) + invalidFTClassIDsCandidate = append(invalidFTClassIDsCandidate, classID) } } - if len(invalidClassIDsCandidate) != 0 { - invalidClassIDs[contract.Id] = invalidClassIDsCandidate + if len(invalidFTClassIDsCandidate) != 0 { + invalidFTClassIDs[contract.Id] = invalidFTClassIDsCandidate } return false }) - broken := len(invalidClassIDs) != 0 + broken := len(invalidFTClassIDs) != 0 msg := "no violation found" if broken { concatenated := []string{} delimiter := ":" - for contractID, classIDs := range invalidClassIDs { + for contractID, classIDs := range invalidFTClassIDs { for _, classID := range classIDs { concatenated = append(concatenated, contractID+delimiter+classID) } } - msg = "violation found on following classIDs: " + strings.Join(concatenated, ", ") + msg = "violation found on following ft classIDs: " + strings.Join(concatenated, ", ") } - return sdk.FormatInvariant(collection.ModuleName, totalSupplyInvariant, msg), broken + return sdk.FormatInvariant(collection.ModuleName, totalFTSupplyInvariant, msg), broken } } From a77937926cd5cc5d45fdd8ace48d3ea16074b219 Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Fri, 1 Sep 2023 13:53:49 +0900 Subject: [PATCH 08/14] Add a unit test on the invariant --- x/collection/keeper/invariants_test.go | 30 ++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 x/collection/keeper/invariants_test.go diff --git a/x/collection/keeper/invariants_test.go b/x/collection/keeper/invariants_test.go new file mode 100644 index 0000000000..2b73430f93 --- /dev/null +++ b/x/collection/keeper/invariants_test.go @@ -0,0 +1,30 @@ +package keeper_test + +import ( + sdk "github.com/Finschia/finschia-sdk/types" + "github.com/Finschia/finschia-sdk/x/collection/keeper" +) + +func (s *KeeperTestSuite) TestTotalSupplyInvariant() { + testCases := map[string]struct { + malleate func(ctx sdk.Context) + valid bool + }{ + "invariant not broken": { + valid: true, + }, + } + + for name, tc := range testCases { + s.Run(name, func() { + ctx, _ := s.ctx.CacheContext() + if tc.malleate != nil { + tc.malleate(ctx) + } + + invariant := keeper.TotalFTSupplyInvariant(s.keeper) + _, broken := invariant(ctx) + s.Require().Equal(!tc.valid, broken) + }) + } +} From b9306e25742c2ecb9b2116bcd842af82bb6ac0de Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Fri, 1 Sep 2023 13:55:44 +0900 Subject: [PATCH 09/14] Lint --- x/collection/keeper/migrations/v2/keys.go | 1 - x/collection/keeper/migrations/v2/store.go | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/x/collection/keeper/migrations/v2/keys.go b/x/collection/keeper/migrations/v2/keys.go index 1dff25e6ef..0f5d68c89c 100644 --- a/x/collection/keeper/migrations/v2/keys.go +++ b/x/collection/keeper/migrations/v2/keys.go @@ -6,7 +6,6 @@ import ( var ( contractKeyPrefix = []byte{0x10} - classKeyPrefix = []byte{0x11} nextClassIDKeyPrefix = []byte{0x12} balanceKeyPrefix = []byte{0x20} diff --git a/x/collection/keeper/migrations/v2/store.go b/x/collection/keeper/migrations/v2/store.go index 4fbe32a2f7..2ed0bb456b 100644 --- a/x/collection/keeper/migrations/v2/store.go +++ b/x/collection/keeper/migrations/v2/store.go @@ -31,7 +31,7 @@ func fixFTStatistics(store storetypes.KVStore, cdc codec.BinaryCodec) error { return err } - if err := fixContractFTStatistics(store, cdc, contract.Id); err != nil { + if err := fixContractFTStatistics(store, contract.Id); err != nil { return err } } @@ -39,7 +39,7 @@ func fixFTStatistics(store storetypes.KVStore, cdc codec.BinaryCodec) error { return nil } -func fixContractFTStatistics(store storetypes.KVStore, cdc codec.BinaryCodec, contractID string) error { +func fixContractFTStatistics(store storetypes.KVStore, contractID string) error { supplies, err := evalContractFTSupplies(store, contractID) if err != nil { return err From 621073d871f9acacec1a8122cd871516cd42f90c Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Fri, 1 Sep 2023 17:21:13 +0900 Subject: [PATCH 10/14] Update consensus version --- x/collection/module/module.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/collection/module/module.go b/x/collection/module/module.go index ab713a9d81..66d819d9b1 100644 --- a/x/collection/module/module.go +++ b/x/collection/module/module.go @@ -129,4 +129,4 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw } // ConsensusVersion implements AppModule/ConsensusVersion. -func (AppModule) ConsensusVersion() uint64 { return 1 } +func (AppModule) ConsensusVersion() uint64 { return 2 } From a4ec5e6dc507ab8958442df1891c56cd6a676159 Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Fri, 1 Sep 2023 18:13:42 +0900 Subject: [PATCH 11/14] Revert "Add a unit test on the invariant" This reverts commit a77937926cd5cc5d45fdd8ace48d3ea16074b219. --- x/collection/keeper/invariants_test.go | 30 -------------------------- 1 file changed, 30 deletions(-) delete mode 100644 x/collection/keeper/invariants_test.go diff --git a/x/collection/keeper/invariants_test.go b/x/collection/keeper/invariants_test.go deleted file mode 100644 index 2b73430f93..0000000000 --- a/x/collection/keeper/invariants_test.go +++ /dev/null @@ -1,30 +0,0 @@ -package keeper_test - -import ( - sdk "github.com/Finschia/finschia-sdk/types" - "github.com/Finschia/finschia-sdk/x/collection/keeper" -) - -func (s *KeeperTestSuite) TestTotalSupplyInvariant() { - testCases := map[string]struct { - malleate func(ctx sdk.Context) - valid bool - }{ - "invariant not broken": { - valid: true, - }, - } - - for name, tc := range testCases { - s.Run(name, func() { - ctx, _ := s.ctx.CacheContext() - if tc.malleate != nil { - tc.malleate(ctx) - } - - invariant := keeper.TotalFTSupplyInvariant(s.keeper) - _, broken := invariant(ctx) - s.Require().Equal(!tc.valid, broken) - }) - } -} From ee0af49f30696f530d253b1b859eda06882c8f0d Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Fri, 1 Sep 2023 18:13:53 +0900 Subject: [PATCH 12/14] Revert "Do not verify nfts" This reverts commit e85305f69508f21753368d57cbd61fa9b4ab745c. --- x/collection/keeper/invariants.go | 33 +++++++++++-------------------- 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/x/collection/keeper/invariants.go b/x/collection/keeper/invariants.go index 44cbccece3..7d000bea1d 100644 --- a/x/collection/keeper/invariants.go +++ b/x/collection/keeper/invariants.go @@ -8,41 +8,32 @@ import ( ) const ( - totalFTSupplyInvariant = "total-ft-supply" + totalSupplyInvariant = "total-supply" ) func RegisterInvariants(ir sdk.InvariantRegistry, k Keeper) { for name, invariant := range map[string]func(k Keeper) sdk.Invariant{ - totalFTSupplyInvariant: TotalFTSupplyInvariant, + totalSupplyInvariant: TotalSupplyInvariant, } { ir.RegisterRoute(collection.ModuleName, name, invariant(k)) } } -func TotalFTSupplyInvariant(k Keeper) sdk.Invariant { +func TotalSupplyInvariant(k Keeper) sdk.Invariant { return func(ctx sdk.Context) (string, bool) { // cache, we don't want to write changes ctx, _ = ctx.CacheContext() - invalidFTClassIDs := map[string][]string{} + invalidClassIDs := map[string][]string{} k.iterateContracts(ctx, func(contract collection.Contract) (stop bool) { supplies := map[string]sdk.Int{} k.iterateContractSupplies(ctx, contract.Id, func(classID string, amount sdk.Int) (stop bool) { - if err := collection.ValidateLegacyFTClassID(classID); err != nil { - return false - } - supplies[classID] = amount - return false }) k.iterateContractBalances(ctx, contract.Id, func(address sdk.AccAddress, balance collection.Coin) (stop bool) { classID := collection.SplitTokenID(balance.TokenId) - if err := collection.ValidateLegacyFTClassID(classID); err != nil { - return false - } - amount, ok := supplies[classID] if !ok { amount = sdk.ZeroInt() @@ -52,34 +43,34 @@ func TotalFTSupplyInvariant(k Keeper) sdk.Invariant { return false }) - invalidFTClassIDsCandidate := []string{} + invalidClassIDsCandidate := []string{} for classID, supply := range supplies { if !supply.IsZero() { - invalidFTClassIDsCandidate = append(invalidFTClassIDsCandidate, classID) + invalidClassIDsCandidate = append(invalidClassIDsCandidate, classID) } } - if len(invalidFTClassIDsCandidate) != 0 { - invalidFTClassIDs[contract.Id] = invalidFTClassIDsCandidate + if len(invalidClassIDsCandidate) != 0 { + invalidClassIDs[contract.Id] = invalidClassIDsCandidate } return false }) - broken := len(invalidFTClassIDs) != 0 + broken := len(invalidClassIDs) != 0 msg := "no violation found" if broken { concatenated := []string{} delimiter := ":" - for contractID, classIDs := range invalidFTClassIDs { + for contractID, classIDs := range invalidClassIDs { for _, classID := range classIDs { concatenated = append(concatenated, contractID+delimiter+classID) } } - msg = "violation found on following ft classIDs: " + strings.Join(concatenated, ", ") + msg = "violation found on following classIDs: " + strings.Join(concatenated, ", ") } - return sdk.FormatInvariant(collection.ModuleName, totalFTSupplyInvariant, msg), broken + return sdk.FormatInvariant(collection.ModuleName, totalSupplyInvariant, msg), broken } } From fe6883fc79d723f13bc1d54de938edd7c5ae08a3 Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Fri, 1 Sep 2023 18:13:59 +0900 Subject: [PATCH 13/14] Revert "Add total-supply invariants" This reverts commit 3a3e42368a3d74c756b6a368e27b727f52ee6380. --- x/collection/keeper/invariants.go | 76 ------------------------------- x/collection/module/module.go | 4 +- 2 files changed, 1 insertion(+), 79 deletions(-) delete mode 100644 x/collection/keeper/invariants.go diff --git a/x/collection/keeper/invariants.go b/x/collection/keeper/invariants.go deleted file mode 100644 index 7d000bea1d..0000000000 --- a/x/collection/keeper/invariants.go +++ /dev/null @@ -1,76 +0,0 @@ -package keeper - -import ( - "strings" - - sdk "github.com/Finschia/finschia-sdk/types" - "github.com/Finschia/finschia-sdk/x/collection" -) - -const ( - totalSupplyInvariant = "total-supply" -) - -func RegisterInvariants(ir sdk.InvariantRegistry, k Keeper) { - for name, invariant := range map[string]func(k Keeper) sdk.Invariant{ - totalSupplyInvariant: TotalSupplyInvariant, - } { - ir.RegisterRoute(collection.ModuleName, name, invariant(k)) - } -} - -func TotalSupplyInvariant(k Keeper) sdk.Invariant { - return func(ctx sdk.Context) (string, bool) { - // cache, we don't want to write changes - ctx, _ = ctx.CacheContext() - - invalidClassIDs := map[string][]string{} - k.iterateContracts(ctx, func(contract collection.Contract) (stop bool) { - supplies := map[string]sdk.Int{} - k.iterateContractSupplies(ctx, contract.Id, func(classID string, amount sdk.Int) (stop bool) { - supplies[classID] = amount - return false - }) - - k.iterateContractBalances(ctx, contract.Id, func(address sdk.AccAddress, balance collection.Coin) (stop bool) { - classID := collection.SplitTokenID(balance.TokenId) - amount, ok := supplies[classID] - if !ok { - amount = sdk.ZeroInt() - } - - supplies[classID] = amount.Sub(balance.Amount) - return false - }) - - invalidClassIDsCandidate := []string{} - for classID, supply := range supplies { - if !supply.IsZero() { - invalidClassIDsCandidate = append(invalidClassIDsCandidate, classID) - } - } - - if len(invalidClassIDsCandidate) != 0 { - invalidClassIDs[contract.Id] = invalidClassIDsCandidate - } - - return false - }) - - broken := len(invalidClassIDs) != 0 - msg := "no violation found" - if broken { - concatenated := []string{} - delimiter := ":" - for contractID, classIDs := range invalidClassIDs { - for _, classID := range classIDs { - concatenated = append(concatenated, contractID+delimiter+classID) - } - } - - msg = "violation found on following classIDs: " + strings.Join(concatenated, ", ") - } - - return sdk.FormatInvariant(collection.ModuleName, totalSupplyInvariant, msg), broken - } -} diff --git a/x/collection/module/module.go b/x/collection/module/module.go index 66d819d9b1..d53bb9b703 100644 --- a/x/collection/module/module.go +++ b/x/collection/module/module.go @@ -89,9 +89,7 @@ func NewAppModule(cdc codec.Codec, keeper keeper.Keeper) AppModule { } // RegisterInvariants does nothing, there are no invariants to enforce -func (am AppModule) RegisterInvariants(ir sdk.InvariantRegistry) { - keeper.RegisterInvariants(ir, am.keeper) -} +func (AppModule) RegisterInvariants(_ sdk.InvariantRegistry) {} // Route returns the message routing key for the collection module. func (am AppModule) Route() sdk.Route { return sdk.Route{} } From 33e364482814ec2c912800456ebdd15cf51b1b3e Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Fri, 1 Sep 2023 19:52:28 +0900 Subject: [PATCH 14/14] Apply suggestions from code review --- CHANGELOG.md | 2 +- x/collection/module/module.go | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b2ee4088d..0cc28f74e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,7 +64,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/bank) [#1093](https://github.com/Finschia/finschia-sdk/pull/1093) Remove message events including `sender` attribute whose information is already present in the relevant events (backport #1066) * (ostracon) [\#1099](https://github.com/Finschia/finschia-sdk/pull/1099) feat!: remove libsodium vrf library. * (x/collection) [\#1102](https://github.com/finschia/finschia-sdk/pull/1102) Reject modifying NFT class with token index filled in MsgModify -* (x/collection) [\#1105](https://github.com/finschia/finschia-sdk/pull/1105) Add minted coins to balance in x/collection MsgMintFT +* (x/collection) [\#1105](https://github.com/Finschia/finschia-sdk/pull/1105) Add minted coins to balance in x/collection MsgMintFT ### Build, CI * (build,ci) [\#1043](https://github.com/Finschia/finschia-sdk/pull/1043) Update golang version to 1.20 diff --git a/x/collection/module/module.go b/x/collection/module/module.go index d53bb9b703..6c0cf64e30 100644 --- a/x/collection/module/module.go +++ b/x/collection/module/module.go @@ -107,7 +107,9 @@ func (am AppModule) LegacyQuerierHandler(legacyQuerierCdc *codec.LegacyAmino) sd func (am AppModule) RegisterServices(cfg module.Configurator) { collection.RegisterMsgServer(cfg.MsgServer(), keeper.NewMsgServer(am.keeper)) collection.RegisterQueryServer(cfg.QueryServer(), keeper.NewQueryServer(am.keeper)) - keeper.NewMigrator(am.keeper).Register(cfg.RegisterMigration) + if err := keeper.NewMigrator(am.keeper).Register(cfg.RegisterMigration); err != nil { + panic(err) + } } // InitGenesis performs genesis initialization for the collection module. It returns