-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
Conversation
@@ -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 comment
The 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.
func MockStore() (store.KVStoreService, context.Context) { | ||
// They can be used to test collections. The StoreService.NewStoreContext | ||
// can be used to instantiate a new empty KVStore. | ||
func MockStore() (*StoreService, context.Context) { |
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 the KVStoreService
.
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.
utACK. not sure if we need a changelog, did we do it for params
x/auth/module.go
Outdated
@@ -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 { | |||
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 comment
The 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))
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.
lgtm
Co-authored-by: testinginprod <testinginprod@somewhere.idk>
Description
Closes: #15831
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!
in the type prefix if API or client breaking change