Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Supply by denom Migrations #8780

Merged
merged 12 commits into from
Mar 5, 2021
3 changes: 2 additions & 1 deletion docs/core/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -8178,7 +8178,7 @@ upgrade.
| `title` | [string](#string) | | |
| `description` | [string](#string) | | |
| `plan` | [cosmos.upgrade.v1beta1.Plan](#cosmos.upgrade.v1beta1.Plan) | | |
| `upgraded_client_state` | [google.protobuf.Any](#google.protobuf.Any) | | An UpgradedClientState must be provided to perform an IBC breaking upgrade. This will make the chain commit to the correct upgraded (self) client state before the upgrade occurs, so that connected chains can verify that the new upgraded client is valid by verifying a proof of the intended new client state on the previous version of the chain. This will allow IBC connections to persist smoothly across planned chain upgrades. |
| `upgraded_client_state` | [google.protobuf.Any](#google.protobuf.Any) | | An UpgradedClientState must be provided to perform an IBC breaking upgrade. This will make the chain commit to the correct upgraded (self) client state before the upgrade occurs, so that connecting chains can verify that the new upgraded client is valid by verifying a proof on the previous version of the chain. This will allow IBC connections to persist smoothly across planned chain upgrades |



Expand Down Expand Up @@ -10895,3 +10895,4 @@ that implements Misbehaviour interface expected by ICS-02
| <a name="bool" /> bool | | bool | boolean | boolean | bool | bool | boolean | TrueClass/FalseClass |
| <a name="string" /> string | A string must always contain UTF-8 encoded or 7-bit ASCII text. | string | String | str/unicode | string | string | string | String (UTF-8) |
| <a name="bytes" /> bytes | May contain any arbitrary sequence of bytes. | string | ByteString | str | []byte | ByteString | string | String (ASCII-8BIT) |

15 changes: 15 additions & 0 deletions proto/cosmos/bank/v1beta1/bank.proto
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,21 @@ message Output {
[(gogoproto.nullable) = false, (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins"];
}

// Supply represents a struct that passively keeps track of the total supply
// amounts in the network.
// This message is deprecated now that supply is indexed by denom.
message Supply {
option deprecated = true;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it documented somewhere how we want to deal with changes in proto. As I understand it, we can make changes as it's still labellled beta. wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, https://github.com/cosmos/cosmos-sdk/blob/master/docs/architecture/adr-023-protobuf-naming.md. From what I understand, if buf is shouting, then something's wrong and the pkg version needs to be bumped.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to require proto-breaking CI checks...


option (gogoproto.equal) = true;
option (gogoproto.goproto_getters) = false;

option (cosmos_proto.implements_interface) = "*github.com/cosmos/cosmos-sdk/x/bank/exported.SupplyI";

repeated cosmos.base.v1beta1.Coin total = 1
[(gogoproto.nullable) = false, (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins"];
}

// DenomUnit represents a struct that describes a given
// denomination unit of the basic token.
message DenomUnit {
Expand Down
2 changes: 1 addition & 1 deletion x/bank/keeper/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@ func NewMigrator(keeper BaseKeeper) Migrator {

// Migrate1to2 migrates from version 1 to 2.
func (m Migrator) Migrate1to2(ctx sdk.Context) error {
return v042.MigrateStore(ctx, m.keeper.storeKey)
return v042.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc)
}
3 changes: 2 additions & 1 deletion x/bank/keeper/querier.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ func queryTotalSupply(ctx sdk.Context, req abci.RequestQuery, k Keeper, legacyQu
return nil, sdkerrors.Wrap(sdkerrors.ErrJSONUnmarshal, err.Error())
}

//TODO: pagenate
// TODO: paginate
// https://github.com/cosmos/cosmos-sdk/issues/8761
totalSupply := k.GetTotalSupply(ctx)

start, end := client.Paginate(len(totalSupply), params.Page, params.Limit, 100)
Expand Down
51 changes: 43 additions & 8 deletions x/bank/legacy/v042/store.go
Original file line number Diff line number Diff line change
@@ -1,21 +1,45 @@
package v042

import (
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/store/prefix"
sdk "github.com/cosmos/cosmos-sdk/types"
v040auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v040"
v040bank "github.com/cosmos/cosmos-sdk/x/bank/legacy/v040"
"github.com/cosmos/cosmos-sdk/x/bank/types"
)

// MigrateStore performs in-place store migrations from v0.40 to v0.42. The
// migration includes:
//
// - Change addresses to be length-prefixed.
// - Change balances prefix to 1 byte
func MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey) error {
store := ctx.KVStore(storeKey)
// migrateSupply migrates the supply to be stored by denom key instead in a
// single blob.
// ref: https://github.com/cosmos/cosmos-sdk/issues/7092
func migrateSupply(store sdk.KVStore, cdc codec.BinaryMarshaler) error {
// Old supply was stored as a single blob under the SupplyKey.
var oldSupply types.Supply
err := cdc.UnmarshalBinaryBare(store.Get(v040bank.SupplyKey), &oldSupply)
if err != nil {
return err
}

// We delete the single key holding the whole blob.
store.Delete(v040bank.SupplyKey)

// We add a new key for each denom
supplyStore := prefix.NewStore(store, types.SupplyKey)
for _, coin := range oldSupply.Total {
coinBz, err := cdc.MarshalBinaryBare(&coin)
if err != nil {
return err
}

supplyStore.Set([]byte(coin.Denom), coinBz)
}

return nil
}

// migrateBalanceKeys migrate the balances keys to cater for variable-length
// addresses.
func migrateBalanceKeys(store sdk.KVStore) {
// old key is of format:
// prefix ("balances") || addrBytes (20 bytes) || denomBytes
// new key is of format
Expand All @@ -34,6 +58,17 @@ func MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey) error {
store.Set(newStoreKey, oldStoreIter.Value())
oldStore.Delete(oldStoreIter.Key())
}
}

return nil
// MigrateStore performs in-place store migrations from v0.40 to v0.42. The
// migration includes:
//
// - Change addresses to be length-prefixed.
// - Change balances prefix to 1 byte
// - Change supply to be indexed by denom
func MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, cdc codec.BinaryMarshaler) error {
store := ctx.KVStore(storeKey)

migrateBalanceKeys(store)
return migrateSupply(store, cdc)
}
33 changes: 31 additions & 2 deletions x/bank/legacy/v042/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (

"github.com/stretchr/testify/require"

"github.com/cosmos/cosmos-sdk/simapp"
"github.com/cosmos/cosmos-sdk/store/prefix"
"github.com/cosmos/cosmos-sdk/testutil"
"github.com/cosmos/cosmos-sdk/testutil/testdata"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand All @@ -13,7 +15,34 @@ import (
"github.com/cosmos/cosmos-sdk/x/bank/types"
)

func TestStoreMigration(t *testing.T) {
func TestSupplyMigration(t *testing.T) {
encCfg := simapp.MakeTestEncodingConfig()
bankKey := sdk.NewKVStoreKey("bank")
ctx := testutil.DefaultContext(bankKey, sdk.NewTransientStoreKey("transient_test"))
store := ctx.KVStore(bankKey)

oldFooCoin := sdk.NewCoin("foo", sdk.NewInt(100))
oldBarCoin := sdk.NewCoin("bar", sdk.NewInt(200))

// Old supply was stored as a single blob under the `SupplyKey`.
oldSupply := types.Supply{Total: sdk.NewCoins(oldFooCoin, oldBarCoin)}
store.Set(v040bank.SupplyKey, encCfg.Marshaler.MustMarshalBinaryBare(&oldSupply))

// Run migration.
err := v042bank.MigrateStore(ctx, bankKey, encCfg.Marshaler)
require.NoError(t, err)

// New supply is indexed by denom.
var newFooCoin, newBarCoin sdk.Coin
supplyStore := prefix.NewStore(store, types.SupplyKey)
encCfg.Marshaler.MustUnmarshalBinaryBare(supplyStore.Get([]byte("foo")), &newFooCoin)
encCfg.Marshaler.MustUnmarshalBinaryBare(supplyStore.Get([]byte("bar")), &newBarCoin)

require.Equal(t, oldFooCoin, newFooCoin)
require.Equal(t, oldBarCoin, newBarCoin)
}

func TestBalanceKeysMigration(t *testing.T) {
bankKey := sdk.NewKVStoreKey("bank")
ctx := testutil.DefaultContext(bankKey, sdk.NewTransientStoreKey("transient_test"))
store := ctx.KVStore(bankKey)
Expand All @@ -25,7 +54,7 @@ func TestStoreMigration(t *testing.T) {
oldKey := append(append(v040bank.BalancesPrefix, addr...), denom...)
store.Set(oldKey, value)

err := v042bank.MigrateStore(ctx, bankKey)
err := v042bank.MigrateStore(ctx, bankKey, nil)
require.NoError(t, err)

newKey := append(types.CreateAccountBalancesPrefix(addr), denom...)
Expand Down
5 changes: 3 additions & 2 deletions x/bank/spec/01_state.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ order: 1

# State

The `x/bank` module keeps state of two primary objects, account balances and the
The `x/bank` module keeps state of three primary objects, account balances, denom metadata and the
total supply of all balances.

- Supply: `0x0 -> ProtocolBuffer(Supply)`
- Supply: `0x0 | byte(denom) -> ProtocolBuffer(coins)`
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
- Denom Metadata: `0x0 | byte(denom) -> ProtocolBuffer(Metadata)`
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
- Balances: `0x2 | byte(address length) | []byte(address) | []byte(balance.Denom) -> ProtocolBuffer(balance)`
Loading