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

migrate command failed with error cannot unmarshal string into Go value of type uint32 #8776

Closed
4 tasks
akhilkumarpilli opened this issue Mar 4, 2021 · 11 comments · Fixed by #8794 or #8841
Closed
4 tasks
Assignees
Labels
C:x/genutil genutil module issues T:Bug
Milestone

Comments

@akhilkumarpilli
Copy link
Contributor

Summary of Bug

We took a state dump of akash mainnet at height 2155115. Migrate command for akash on v0.10.0 (with SDK version 0.41.3) failed with error json: cannot unmarshal string into Go value of type uint32.

Later, we got to know issue is from multisig accounts in auth genesis state. Previously, threshold field in pubkey of multisig accounts was string and in v0.40, it is converted to uint32.

Version

v0.41.3

Steps to Reproduce

This is the state dump file: https://github.com/akhilkumarpilli/akash-mainnet-export/blob/main/old_genesis_export.json

akash migrate v0.40 old_genesis_export.json --chain-id=akashnet-2 > 040_migrated_genesis.json
panic: json: cannot unmarshal string into Go value of type uint32goroutine 1 [running]:
github.com/cosmos/cosmos-sdk/codec.(*LegacyAmino).MustUnmarshalJSON(...)
        github.com/cosmos/cosmos-sdk@v0.41.3/codec/amino.go:171
github.com/cosmos/cosmos-sdk/x/genutil/legacy/v040.Migrate(0xc001051da0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x7ffdbe85c1d8, 0xa, 0x2ed93a0, 0xc000f108d0, ...)
        github.com/cosmos/cosmos-sdk@v0.41.3/x/genutil/legacy/v040/migrate.go:56 +0x1e93
github.com/cosmos/cosmos-sdk/x/genutil/client/cli.MigrateGenesisCmd.func1(0xc001039600, 0xc001015ec0, 0x2, 0x3, 0x0, 0x0)
        github.com/cosmos/cosmos-sdk@v0.41.3/x/genutil/client/cli/migrate.go:100 +0x1f8
github.com/spf13/cobra.(*Command).execute(0xc001039600, 0xc001015e60, 0x3, 0x3, 0xc001039600, 0xc001015e60)
        github.com/spf13/cobra@v1.1.1/command.go:850 +0x47c
github.com/spf13/cobra.(*Command).ExecuteC(0xc000ddbb80, 0x291570f, 0x5, 0xc000d52a50)
        github.com/spf13/cobra@v1.1.1/command.go:958 +0x375
github.com/spf13/cobra.(*Command).Execute(...)
        github.com/spf13/cobra@v1.1.1/command.go:895
github.com/spf13/cobra.(*Command).ExecuteContext(...)
        github.com/spf13/cobra@v1.1.1/command.go:888
github.com/ovrclk/akash/cmd/akash/cmd.Execute(0xc000ddbb80, 0x2edf3a0, 0xc000f0dc80)
        github.com/ovrclk/akash/cmd/akash/cmd/root.go:116 +0x26d
main.main()
        github.com/ovrclk/akash/cmd/akash/main.go:14 +0x2a

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@amaury1093 amaury1093 added T:Bug C:x/genutil genutil module issues labels Mar 4, 2021
@akhilkumarpilli
Copy link
Contributor Author

We resolved above issue using jq for now. But, tried to test migrate command in local devnet, we found new issue. Multisig account public_keys are empty in migrated genesis.json i.e., v0.40 genesis.json. Multisig account is in below format in new genesis:

{
          "@type": "/cosmos.auth.v1beta1.BaseAccount",
          "account_number": "1",
          "address": "akash1prf6hf6frg39k9n7f0kn7cpvm0g8k3dz573p7x",
          "pub_key": {
            "@type": "/cosmos.crypto.multisig.LegacyAminoPubKey",
            "public_keys": [],
            "threshold": 2
          },
          "sequence": "1"
        },

Multisig transactions are failing because of empty public_keys array. We pasted public_keys manually in genesis and reset chain, multisig transactions worked fine.

@boz
Copy link
Contributor

boz commented Mar 4, 2021

It also seems to work okay with "pub_key": null,, though I only tried a 2/2 multisig.

@aaronc
Copy link
Member

aaronc commented Mar 4, 2021

Is this an issue on the hub right now?

@anilcse
Copy link
Collaborator

anilcse commented Mar 4, 2021

Is this an issue on the hub right now?

No, hub has this pubkey as nil. Nothing changed afterwards but not sure if there's any edgecase on Akash

@aaronc
Copy link
Member

aaronc commented Mar 4, 2021

Why were we setting it to nil on the hub?

@anilcse
Copy link
Collaborator

anilcse commented Mar 4, 2021

Why were we setting it to nil on the hub?

@zmanian might have better context

@amaury1093
Copy link
Contributor

Why were we setting it to nil on the hub?

It happens here

baseAccount := NewBaseAccount(acc.Address, acc.Coins.Sort(), nil, acc.AccountNumber, acc.Sequence)

I believe it's a bug: the migrate command goes from 0.34 (where Accounts don't have pubkeys) to 0.38 (where BaseAccounts have pubkeys).

But the hub goes from 0.37->0.38(then ->until 0.40) using the same migrate command, which loses the "pubkey" information. I'm sure there are some intermediary migrate commands missing.

All BaseAccounts in the hub have a pubkey=nil field right now. As Zaki pointed out, it's okay, until we add rekeying one day.

Anyways, this issue still needs to be fixed.

alessio pushed a commit that referenced this issue Mar 5, 2021
@amaury1093
Copy link
Contributor

@alessio This issue is not fixed. As I pointed out, #8794 is just a temporary workaround.

@amaury1093 amaury1093 reopened this Mar 5, 2021
@anilcse
Copy link
Collaborator

anilcse commented Mar 5, 2021

@alessio This issue is not fixed. As I pointed out, #8794 is just a temporary workaround.

Also it's just a partial fix. 0.39 export should also needs a change. multisig threshold need to be type casted to uint32

hydrogen18 pushed a commit to akash-network/cosmos-sdk that referenced this issue Mar 5, 2021
@alessio
Copy link
Contributor

alessio commented Mar 5, 2021

Also it's just a partial fix. 0.39 export should also needs a change. multisig threshold need to be type casted to uint32

I see, thanks.

charleenfei pushed a commit to charleenfei/cosmos-sdk that referenced this issue Mar 10, 2021
@mergify mergify bot closed this as completed in 55fc465 Mar 10, 2021
@aaronc aaronc removed the backlog label Mar 10, 2021
mergify bot pushed a commit that referenced this issue Mar 10, 2021
* fixed broken links, typos

* Update docs/ibc/overview.md

Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>

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

Co-authored-by: Marko <markobaricevic3778@gmail.com>

* Update docs/building-modules/simulator.md

Co-authored-by: Marko <markobaricevic3778@gmail.com>

* 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] <support@github.com>

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 <alessio@tendermint.com>
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 <jgimeno@gmail.com>

* 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 <alessio@tendermint.com>

* 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 <chrly@chrlys-MacBook-Pro.local>
Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>
Co-authored-by: Jonathan Gimeno <jgimeno@gmail.com>
Co-authored-by: Marko <markobaricevic3778@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Akhil Kumar P <36399231+akhilkumarpilli@users.noreply.github.com>
Co-authored-by: Alessio Treglia <alessio@tendermint.com>
Co-authored-by: Marko <marbar3778@yahoo.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Albert Chon <albert@injectiveprotocol.com>
Co-authored-by: Emmanuel T Odeke <emmanuel@orijtech.com>
(cherry picked from commit 55fc465)
alessio pushed a commit that referenced this issue Mar 10, 2021
* fixed broken links, typos

* Update docs/ibc/overview.md

Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>

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

Co-authored-by: Marko <markobaricevic3778@gmail.com>

* Update docs/building-modules/simulator.md

Co-authored-by: Marko <markobaricevic3778@gmail.com>

* 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] <support@github.com>

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 <alessio@tendermint.com>
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 <jgimeno@gmail.com>

* 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 <alessio@tendermint.com>

* 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 <chrly@chrlys-MacBook-Pro.local>
Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>
Co-authored-by: Jonathan Gimeno <jgimeno@gmail.com>
Co-authored-by: Marko <markobaricevic3778@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Akhil Kumar P <36399231+akhilkumarpilli@users.noreply.github.com>
Co-authored-by: Alessio Treglia <alessio@tendermint.com>
Co-authored-by: Marko <marbar3778@yahoo.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Albert Chon <albert@injectiveprotocol.com>
Co-authored-by: Emmanuel T Odeke <emmanuel@orijtech.com>
(cherry picked from commit 55fc465)

Co-authored-by: Charly <charly@interchain.berlin>
@amaury1093 amaury1093 reopened this Mar 12, 2021
@amaury1093
Copy link
Contributor

Still not closed. Will be fixed by #8841.

@mergify mergify bot closed this as completed in #8841 Mar 15, 2021
alessio pushed a commit that referenced this issue Mar 15, 2021
(cherry picked from commit d4d27e1)

closes: #8776

Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>
Co-authored-by: Alessio Treglia <alessio@tendermint.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Anil Kumar Kammari <anil@vitwit.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:x/genutil genutil module issues T:Bug
Projects
None yet
7 participants