-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
refactor(auth): use collections for GlobalAccountNumber #15830
Changes from 8 commits
29e6e18
6399c36
b81cd0e
e2dab71
23e9211
54785e0
c857b38
43363fd
6f82bb1
2a43a0c
2951a36
5ded2ec
1b90ad4
40dd227
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,7 @@ import ( | |
) | ||
|
||
// DefaultSequenceStart defines the default starting number of a sequence. | ||
const DefaultSequenceStart uint64 = 1 | ||
const DefaultSequenceStart uint64 = 0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. modified this to make it match the account number semantics, which start from zero. |
||
|
||
// Sequence builds on top of an Item, and represents a monotonically increasing number. | ||
type Sequence Item[uint64] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
package v6 | ||
julienrbrt marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
import ( | ||
"context" | ||
|
||
"cosmossdk.io/collections" | ||
storetypes "cosmossdk.io/core/store" | ||
"github.com/cosmos/gogoproto/types" | ||
) | ||
|
||
var LegacyGlobalAccountNumberKey = []byte("globalAccountNumber") | ||
|
||
func Migrate(ctx context.Context, storeService storetypes.KVStoreService, sequence collections.Sequence) error { | ||
store := storeService.OpenKVStore(ctx) | ||
b, err := store.Get(LegacyGlobalAccountNumberKey) | ||
if err != nil { | ||
return err | ||
} | ||
if b == nil { | ||
// this would mean no account was ever created in this chain which is being migrated? | ||
// we're doing nothing as the collections.Sequence already handles the non-existing value. | ||
return nil | ||
} | ||
|
||
// get old value | ||
v := new(types.UInt64Value) | ||
err = v.Unmarshal(b) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
// set the old value in the collection | ||
err = sequence.Set(ctx, v.Value) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
// remove the value from the old prefix. | ||
err = store.Delete(LegacyGlobalAccountNumberKey) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
return nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
package v6 | ||
|
||
import ( | ||
"testing" | ||
|
||
"cosmossdk.io/collections" | ||
"cosmossdk.io/collections/colltest" | ||
"github.com/cosmos/gogoproto/types" | ||
"github.com/stretchr/testify/require" | ||
) | ||
|
||
func TestMigrate(t *testing.T) { | ||
kv, ctx := colltest.MockStore() | ||
sb := collections.NewSchemaBuilder(kv) | ||
seq := collections.NewSequence(sb, collections.NewPrefix(0), "seq") | ||
|
||
wantValue := uint64(100) | ||
|
||
// set old sequence to wanted value | ||
legacySeqBytes, err := (&types.UInt64Value{Value: wantValue}).Marshal() | ||
require.NoError(t, err) | ||
|
||
err = kv.OpenKVStore(ctx).Set(LegacyGlobalAccountNumberKey, legacySeqBytes) | ||
require.NoError(t, err) | ||
|
||
err = Migrate(ctx, kv, seq) | ||
require.NoError(t, err) | ||
|
||
// check that after migration the sequence is what we want it to be | ||
gotValue, err := seq.Peek(ctx) | ||
require.NoError(t, err) | ||
require.Equal(t, wantValue, gotValue) | ||
|
||
// case the global account number was not set | ||
ctx = kv.NewStoreContext() // this resets the store to zero | ||
wantValue = collections.DefaultSequenceStart | ||
|
||
err = Migrate(ctx, kv, seq) | ||
require.NoError(t, err) | ||
|
||
gotValue, err = seq.Next(ctx) | ||
require.NoError(t, err) | ||
require.Equal(t, wantValue, gotValue) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,7 +33,7 @@ import ( | |
) | ||
|
||
// ConsensusVersion defines the current x/auth module consensus version. | ||
const ConsensusVersion = 4 | ||
const ConsensusVersion = 5 | ||
|
||
var ( | ||
_ module.AppModule = AppModule{} | ||
|
@@ -151,6 +151,10 @@ func (am AppModule) RegisterServices(cfg module.Configurator) { | |
if err := cfg.RegisterMigration(types.ModuleName, 4, func(ctx sdk.Context) error { return nil }); err != nil { | ||
julienrbrt marked this conversation as resolved.
Show resolved
Hide resolved
|
||
panic(fmt.Sprintf("failed to migrate x/%s from version 4 to 5: %v", types.ModuleName, err)) | ||
} | ||
|
||
if err := cfg.RegisterMigration(types.ModuleName, 5, m.Migrate5to6); err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is actually migration 4 to 5, as migration 4 was for v0.47. You can as well delete the placeholder for migration to 5, which wasn't necessary (#14483 (comment)) |
||
panic(fmt.Sprintf("failed to migrate x/%s from version 5 to 6", types.ModuleName)) | ||
} | ||
} | ||
|
||
// InitGenesis performs genesis initialization for the auth module. It returns | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changes here allow the
MockStore
to properly match the design of theKVStoreService
.