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

fix multisig account pubkeys migration #8794

Merged
merged 2 commits into from
Mar 5, 2021
Merged

Conversation

akhilkumarpilli
Copy link
Contributor

@akhilkumarpilli akhilkumarpilli commented Mar 5, 2021

ref: #8776 (comment)

When migrating genesis to v0.40, set pub_key to null if account is multisig as a quick fix

closes: #8776


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer
  • Review Codecov Report in the comment section below once CI passes

@codecov
Copy link

codecov bot commented Mar 5, 2021

Codecov Report

Merging #8794 (04dae6a) into master (72fb8b3) will decrease coverage by 0.00%.
The diff coverage is 57.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8794      +/-   ##
==========================================
- Coverage   59.37%   59.36%   -0.01%     
==========================================
  Files         563      563              
  Lines       31251    31257       +6     
==========================================
+ Hits        18554    18557       +3     
- Misses      10543    10544       +1     
- Partials     2154     2156       +2     
Impacted Files Coverage Δ
x/auth/types/params.go 79.68% <0.00%> (-1.27%) ⬇️
x/auth/legacy/v040/migrate.go 87.30% <33.33%> (-2.87%) ⬇️
x/auth/ante/sigverify.go 74.85% <100.00%> (+0.43%) ⬆️

Copy link
Contributor

@amaury1093 amaury1093 left a comment

Choose a reason for hiding this comment

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

I'm okay to merge this if this is urgent.

But if this can wait 1 or 2 weeks, then I can take a look at fixing it correctly, so that 0.39->0.40 upgrades don't lose the pubkey field.

x/auth/legacy/v040/migrate.go Show resolved Hide resolved
@alessio alessio merged commit 67964b1 into master Mar 5, 2021
@alessio alessio deleted the akhil/fix-multisig-migrate branch March 5, 2021 12:33
hydrogen18 pushed a commit to akash-network/cosmos-sdk that referenced this pull request Mar 5, 2021
charleenfei pushed a commit to charleenfei/cosmos-sdk that referenced this pull request Mar 10, 2021
mergify bot added a commit that referenced this pull request Mar 10, 2021
* fixed broken links, typos

* Update docs/ibc/overview.md

Co-authored-by: Amaury <[email protected]>

* Update docs/intro/sdk-app-architecture.md

Co-authored-by: Marko <[email protected]>

* Update docs/building-modules/simulator.md

Co-authored-by: Marko <[email protected]>

* build(deps): bump JamesIves/github-pages-deploy-action from 4.0.0 to 4.1.0 (#8792)

Bumps [JamesIves/github-pages-deploy-action](https://github.com/JamesIves/github-pages-deploy-action) from 4.0.0 to 4.1.0.
- [Release notes](https://github.com/JamesIves/github-pages-deploy-action/releases)
- [Commits](JamesIves/github-pages-deploy-action@4.0.0...3dbacc7)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix multisig account pubkeys migration (#8794)

closes: #8776

* Update mergify (#8784)

* Update mergify

Prep for the v0.42 release series.

* retire v0.41, the hub can upgrade to v0.42 smoothly

* perf change (#8796)

Co-authored-by: Alessio Treglia <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* Supply by denom Migrations (#8780)

* Add back supply proto

* Add migration for supply

* Fix lint

* Update x/bank/spec/01_state.md

* Fix test

* Proto gen

* Update x/bank/spec/01_state.md

* Make proto gen

Co-authored-by: Jonathan Gimeno <[email protected]>

* fix make protoc error (#8799)

* reduce gas costs by 10x for transient store operations (#8790)

* reduce gas costs by 10x for transient store operations

* fix TestTransientGasConfig for ReadCostFlat

* added changelog entry

* fix changelog

* fix changelog

Co-authored-by: Alessio Treglia <[email protected]>

* x/gov: fix NormalizeProposalType() return values (#8808)

Closes: #8806

* store/cachekv: use typed types/kv.List instead of container/list.List (#8811)

Reduces CPU burn by using a typed List to avoid the expensive type
assertions from using an interface. This is the only option for now
until Go makes generics generally available.

The change brings time spent on the time assertion cummulatively to:
    580ms down from 6.88s

Explanation:
The type assertions were showing up heavily in profiles:
* Before this commit
```shell
Total: 42.18s
ROUTINE ======================== github.com/cosmos/cosmos-sdk/store/cachekv.newMemIterator
in /Users/emmanuelodeke/go/src/github.com/cosmos/cosmos-sdk/store/cachekv/memiterator.go
    14.01s     18.87s (flat, cum) 44.74% of Total
         .          .     17:	items      []*kv.Pair
         .          .     18:	ascending  bool
         .          .     19:}
         .          .     20:
         .          .     21:func newMemIterator(start, end []byte, items *list.List, ascending bool) *memIterator {
         .      620ms     22:	itemsInDomain := make([]*kv.Pair, 0, items.Len())
         .          .     23:
         .          .     24:	var entered bool
         .          .     25:
     510ms      870ms     26:	for e := items.Front(); e != nil; e = e.Next() {
     6.85s      6.88s     27:		item := e.Value.(*kv.Pair)
     5.71s      8.19s     28:		if !dbm.IsKeyInDomain(item.Key, start, end) {
     120ms      120ms     29:			if entered {
         .          .     30:				break
         .          .     31:			}
         .          .     32:
         .          .     33:			continue
         .          .     34:		}
         .          .     35:
     820ms      980ms     36:		itemsInDomain = append(itemsInDomain, item)
         .          .     37:		entered = true
         .          .     38:	}
         .          .     39:
         .      1.21s     40:	return &memIterator{
         .          .     41:		start:     start,
         .          .     42:		end:       end,
         .          .     43:		items:     itemsInDomain,
         .          .     44:		ascending: ascending,
         .          .     45:	}
```

and given that the list only uses that type, it is only right to lift the
code from container/list.List, and only modify Element.Value's type.

For emphasis, the code is basically just a retrofit of
container/list/list.go but with a typing, and we'll keep it as is
until perhaps Go1.17 or Go1.18 or when everyone uses Go1.17+ after
generics have landed.

* After this commit
```shell
Total: 45.25s
ROUTINE ======================== github.com/cosmos/cosmos-sdk/store/cachekv.newMemIterator
in /Users/emmanuelodeke/go/src/github.com/cosmos/cosmos-sdk/store/cachekv/memiterator.go
     4.84s      6.77s (flat, cum) 14.96% of Total
         .          .     16:	items      []*kv.Pair
         .          .     17:	ascending  bool
         .          .     18:}
         .          .     19:
         .          .     20:func newMemIterator(start, end []byte, items *kv.List, ascending bool) *memIterator {
         .      330ms     21:	itemsInDomain := make([]*kv.Pair, 0, items.Len())
         .          .     22:
         .          .     23:	var entered bool
         .          .     24:
      60ms      160ms     25:	for e := items.Front(); e != nil; e = e.Next() {
     580ms      580ms     26:		item := e.Value
     3.68s      4.78s     27:		if !dbm.IsKeyInDomain(item.Key, start, end) {
      80ms       80ms     28:			if entered {
         .          .     29:				break
         .          .     30:			}
         .          .     31:
         .          .     32:			continue
         .          .     33:		}
         .          .     34:
     440ms      580ms     35:		itemsInDomain = append(itemsInDomain, item)
         .          .     36:		entered = true
         .          .     37:	}
         .          .     38:
         .      260ms     39:	return &memIterator{
         .          .     40:		start:     start,
         .          .     41:		end:       end,
         .          .     42:		items:     itemsInDomain,
         .          .     43:		ascending: ascending,
         .          .     44:	}
```

Fixes #8810

* Move all migration scripts to v043 (#8814)

* Move all migration scripts to v043

* Fix permaling

* Fix test

* Fix test again

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* permalinks

Co-authored-by: chrly <[email protected]>
Co-authored-by: Amaury <[email protected]>
Co-authored-by: Jonathan Gimeno <[email protected]>
Co-authored-by: Marko <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Akhil Kumar P <[email protected]>
Co-authored-by: Alessio Treglia <[email protected]>
Co-authored-by: Marko <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Albert Chon <[email protected]>
Co-authored-by: Emmanuel T Odeke <[email protected]>
mergify bot pushed a commit that referenced this pull request Mar 10, 2021
* fixed broken links, typos

* Update docs/ibc/overview.md

Co-authored-by: Amaury <[email protected]>

* Update docs/intro/sdk-app-architecture.md

Co-authored-by: Marko <[email protected]>

* Update docs/building-modules/simulator.md

Co-authored-by: Marko <[email protected]>

* build(deps): bump JamesIves/github-pages-deploy-action from 4.0.0 to 4.1.0 (#8792)

Bumps [JamesIves/github-pages-deploy-action](https://github.com/JamesIves/github-pages-deploy-action) from 4.0.0 to 4.1.0.
- [Release notes](https://github.com/JamesIves/github-pages-deploy-action/releases)
- [Commits](JamesIves/github-pages-deploy-action@4.0.0...3dbacc7)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix multisig account pubkeys migration (#8794)

closes: #8776

* Update mergify (#8784)

* Update mergify

Prep for the v0.42 release series.

* retire v0.41, the hub can upgrade to v0.42 smoothly

* perf change (#8796)

Co-authored-by: Alessio Treglia <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* Supply by denom Migrations (#8780)

* Add back supply proto

* Add migration for supply

* Fix lint

* Update x/bank/spec/01_state.md

* Fix test

* Proto gen

* Update x/bank/spec/01_state.md

* Make proto gen

Co-authored-by: Jonathan Gimeno <[email protected]>

* fix make protoc error (#8799)

* reduce gas costs by 10x for transient store operations (#8790)

* reduce gas costs by 10x for transient store operations

* fix TestTransientGasConfig for ReadCostFlat

* added changelog entry

* fix changelog

* fix changelog

Co-authored-by: Alessio Treglia <[email protected]>

* x/gov: fix NormalizeProposalType() return values (#8808)

Closes: #8806

* store/cachekv: use typed types/kv.List instead of container/list.List (#8811)

Reduces CPU burn by using a typed List to avoid the expensive type
assertions from using an interface. This is the only option for now
until Go makes generics generally available.

The change brings time spent on the time assertion cummulatively to:
    580ms down from 6.88s

Explanation:
The type assertions were showing up heavily in profiles:
* Before this commit
```shell
Total: 42.18s
ROUTINE ======================== github.com/cosmos/cosmos-sdk/store/cachekv.newMemIterator
in /Users/emmanuelodeke/go/src/github.com/cosmos/cosmos-sdk/store/cachekv/memiterator.go
    14.01s     18.87s (flat, cum) 44.74% of Total
         .          .     17:	items      []*kv.Pair
         .          .     18:	ascending  bool
         .          .     19:}
         .          .     20:
         .          .     21:func newMemIterator(start, end []byte, items *list.List, ascending bool) *memIterator {
         .      620ms     22:	itemsInDomain := make([]*kv.Pair, 0, items.Len())
         .          .     23:
         .          .     24:	var entered bool
         .          .     25:
     510ms      870ms     26:	for e := items.Front(); e != nil; e = e.Next() {
     6.85s      6.88s     27:		item := e.Value.(*kv.Pair)
     5.71s      8.19s     28:		if !dbm.IsKeyInDomain(item.Key, start, end) {
     120ms      120ms     29:			if entered {
         .          .     30:				break
         .          .     31:			}
         .          .     32:
         .          .     33:			continue
         .          .     34:		}
         .          .     35:
     820ms      980ms     36:		itemsInDomain = append(itemsInDomain, item)
         .          .     37:		entered = true
         .          .     38:	}
         .          .     39:
         .      1.21s     40:	return &memIterator{
         .          .     41:		start:     start,
         .          .     42:		end:       end,
         .          .     43:		items:     itemsInDomain,
         .          .     44:		ascending: ascending,
         .          .     45:	}
```

and given that the list only uses that type, it is only right to lift the
code from container/list.List, and only modify Element.Value's type.

For emphasis, the code is basically just a retrofit of
container/list/list.go but with a typing, and we'll keep it as is
until perhaps Go1.17 or Go1.18 or when everyone uses Go1.17+ after
generics have landed.

* After this commit
```shell
Total: 45.25s
ROUTINE ======================== github.com/cosmos/cosmos-sdk/store/cachekv.newMemIterator
in /Users/emmanuelodeke/go/src/github.com/cosmos/cosmos-sdk/store/cachekv/memiterator.go
     4.84s      6.77s (flat, cum) 14.96% of Total
         .          .     16:	items      []*kv.Pair
         .          .     17:	ascending  bool
         .          .     18:}
         .          .     19:
         .          .     20:func newMemIterator(start, end []byte, items *kv.List, ascending bool) *memIterator {
         .      330ms     21:	itemsInDomain := make([]*kv.Pair, 0, items.Len())
         .          .     22:
         .          .     23:	var entered bool
         .          .     24:
      60ms      160ms     25:	for e := items.Front(); e != nil; e = e.Next() {
     580ms      580ms     26:		item := e.Value
     3.68s      4.78s     27:		if !dbm.IsKeyInDomain(item.Key, start, end) {
      80ms       80ms     28:			if entered {
         .          .     29:				break
         .          .     30:			}
         .          .     31:
         .          .     32:			continue
         .          .     33:		}
         .          .     34:
     440ms      580ms     35:		itemsInDomain = append(itemsInDomain, item)
         .          .     36:		entered = true
         .          .     37:	}
         .          .     38:
         .      260ms     39:	return &memIterator{
         .          .     40:		start:     start,
         .          .     41:		end:       end,
         .          .     42:		items:     itemsInDomain,
         .          .     43:		ascending: ascending,
         .          .     44:	}
```

Fixes #8810

* Move all migration scripts to v043 (#8814)

* Move all migration scripts to v043

* Fix permaling

* Fix test

* Fix test again

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* permalinks

Co-authored-by: chrly <[email protected]>
Co-authored-by: Amaury <[email protected]>
Co-authored-by: Jonathan Gimeno <[email protected]>
Co-authored-by: Marko <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Akhil Kumar P <[email protected]>
Co-authored-by: Alessio Treglia <[email protected]>
Co-authored-by: Marko <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Albert Chon <[email protected]>
Co-authored-by: Emmanuel T Odeke <[email protected]>
(cherry picked from commit 55fc465)
alessio pushed a commit that referenced this pull request Mar 10, 2021
* fixed broken links, typos

* Update docs/ibc/overview.md

Co-authored-by: Amaury <[email protected]>

* Update docs/intro/sdk-app-architecture.md

Co-authored-by: Marko <[email protected]>

* Update docs/building-modules/simulator.md

Co-authored-by: Marko <[email protected]>

* build(deps): bump JamesIves/github-pages-deploy-action from 4.0.0 to 4.1.0 (#8792)

Bumps [JamesIves/github-pages-deploy-action](https://github.com/JamesIves/github-pages-deploy-action) from 4.0.0 to 4.1.0.
- [Release notes](https://github.com/JamesIves/github-pages-deploy-action/releases)
- [Commits](JamesIves/github-pages-deploy-action@4.0.0...3dbacc7)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix multisig account pubkeys migration (#8794)

closes: #8776

* Update mergify (#8784)

* Update mergify

Prep for the v0.42 release series.

* retire v0.41, the hub can upgrade to v0.42 smoothly

* perf change (#8796)

Co-authored-by: Alessio Treglia <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* Supply by denom Migrations (#8780)

* Add back supply proto

* Add migration for supply

* Fix lint

* Update x/bank/spec/01_state.md

* Fix test

* Proto gen

* Update x/bank/spec/01_state.md

* Make proto gen

Co-authored-by: Jonathan Gimeno <[email protected]>

* fix make protoc error (#8799)

* reduce gas costs by 10x for transient store operations (#8790)

* reduce gas costs by 10x for transient store operations

* fix TestTransientGasConfig for ReadCostFlat

* added changelog entry

* fix changelog

* fix changelog

Co-authored-by: Alessio Treglia <[email protected]>

* x/gov: fix NormalizeProposalType() return values (#8808)

Closes: #8806

* store/cachekv: use typed types/kv.List instead of container/list.List (#8811)

Reduces CPU burn by using a typed List to avoid the expensive type
assertions from using an interface. This is the only option for now
until Go makes generics generally available.

The change brings time spent on the time assertion cummulatively to:
    580ms down from 6.88s

Explanation:
The type assertions were showing up heavily in profiles:
* Before this commit
```shell
Total: 42.18s
ROUTINE ======================== github.com/cosmos/cosmos-sdk/store/cachekv.newMemIterator
in /Users/emmanuelodeke/go/src/github.com/cosmos/cosmos-sdk/store/cachekv/memiterator.go
    14.01s     18.87s (flat, cum) 44.74% of Total
         .          .     17:	items      []*kv.Pair
         .          .     18:	ascending  bool
         .          .     19:}
         .          .     20:
         .          .     21:func newMemIterator(start, end []byte, items *list.List, ascending bool) *memIterator {
         .      620ms     22:	itemsInDomain := make([]*kv.Pair, 0, items.Len())
         .          .     23:
         .          .     24:	var entered bool
         .          .     25:
     510ms      870ms     26:	for e := items.Front(); e != nil; e = e.Next() {
     6.85s      6.88s     27:		item := e.Value.(*kv.Pair)
     5.71s      8.19s     28:		if !dbm.IsKeyInDomain(item.Key, start, end) {
     120ms      120ms     29:			if entered {
         .          .     30:				break
         .          .     31:			}
         .          .     32:
         .          .     33:			continue
         .          .     34:		}
         .          .     35:
     820ms      980ms     36:		itemsInDomain = append(itemsInDomain, item)
         .          .     37:		entered = true
         .          .     38:	}
         .          .     39:
         .      1.21s     40:	return &memIterator{
         .          .     41:		start:     start,
         .          .     42:		end:       end,
         .          .     43:		items:     itemsInDomain,
         .          .     44:		ascending: ascending,
         .          .     45:	}
```

and given that the list only uses that type, it is only right to lift the
code from container/list.List, and only modify Element.Value's type.

For emphasis, the code is basically just a retrofit of
container/list/list.go but with a typing, and we'll keep it as is
until perhaps Go1.17 or Go1.18 or when everyone uses Go1.17+ after
generics have landed.

* After this commit
```shell
Total: 45.25s
ROUTINE ======================== github.com/cosmos/cosmos-sdk/store/cachekv.newMemIterator
in /Users/emmanuelodeke/go/src/github.com/cosmos/cosmos-sdk/store/cachekv/memiterator.go
     4.84s      6.77s (flat, cum) 14.96% of Total
         .          .     16:	items      []*kv.Pair
         .          .     17:	ascending  bool
         .          .     18:}
         .          .     19:
         .          .     20:func newMemIterator(start, end []byte, items *kv.List, ascending bool) *memIterator {
         .      330ms     21:	itemsInDomain := make([]*kv.Pair, 0, items.Len())
         .          .     22:
         .          .     23:	var entered bool
         .          .     24:
      60ms      160ms     25:	for e := items.Front(); e != nil; e = e.Next() {
     580ms      580ms     26:		item := e.Value
     3.68s      4.78s     27:		if !dbm.IsKeyInDomain(item.Key, start, end) {
      80ms       80ms     28:			if entered {
         .          .     29:				break
         .          .     30:			}
         .          .     31:
         .          .     32:			continue
         .          .     33:		}
         .          .     34:
     440ms      580ms     35:		itemsInDomain = append(itemsInDomain, item)
         .          .     36:		entered = true
         .          .     37:	}
         .          .     38:
         .      260ms     39:	return &memIterator{
         .          .     40:		start:     start,
         .          .     41:		end:       end,
         .          .     42:		items:     itemsInDomain,
         .          .     43:		ascending: ascending,
         .          .     44:	}
```

Fixes #8810

* Move all migration scripts to v043 (#8814)

* Move all migration scripts to v043

* Fix permaling

* Fix test

* Fix test again

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* permalinks

Co-authored-by: chrly <[email protected]>
Co-authored-by: Amaury <[email protected]>
Co-authored-by: Jonathan Gimeno <[email protected]>
Co-authored-by: Marko <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Akhil Kumar P <[email protected]>
Co-authored-by: Alessio Treglia <[email protected]>
Co-authored-by: Marko <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Albert Chon <[email protected]>
Co-authored-by: Emmanuel T Odeke <[email protected]>
(cherry picked from commit 55fc465)

Co-authored-by: Charly <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

migrate command failed with error cannot unmarshal string into Go value of type uint32
4 participants