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

0.47 -> 0.50 regression: Amino JSON signing not working for MsgVoteWeighted, MsgCreateValidator #18546

Open
webmaster128 opened this issue Nov 22, 2023 · 7 comments
Labels

Comments

@webmaster128
Copy link
Member

When testing CosmJS against a Cosmos SDK simapp 0.50.1 chain, I see a few messages that cannot be signed anmore (e.g. Error sdk/4 signature verification failed; please verify account number (31), sequence (0) and chain-id (simd-testing): unauthorized during CheckTx). All of the cases above work with 0.46 and 0.47 chains:

message Sign mode direct Sign mode Amino JSON
/cosmos.bank.v1beta1.MsgSend
/cosmos.staking.v1beta1.MsgDelegate
/cosmos.vesting.v1beta1.MsgCreateVestingAccount
/cosmos.staking.v1beta1.MsgCreateValidator
/cosmos.staking.v1beta1.MsgEditValidator1
/cosmos.gov.v1beta1.MsgSubmitProposal
/cosmos.gov.v1beta1.MsgVote
/cosmos.gov.v1beta1.MsgVoteWeighted

Ping @julienrbrt

Footnotes

  1. I thought this is broken but actually I don't know because the test fails due to MsgCreateValidator

@github-actions github-actions bot added the needs-triage Issue that needs to be triaged label Nov 22, 2023
@github-project-automation github-project-automation bot moved this to 👀 To Do in Cosmos-SDK Nov 22, 2023
@julienrbrt julienrbrt removed the needs-triage Issue that needs to be triaged label Nov 22, 2023
@julienrbrt julienrbrt self-assigned this Nov 22, 2023
@julienrbrt julienrbrt moved this from 👀 To Do to ✍ In Progress in Cosmos-SDK Nov 27, 2023
@julienrbrt
Copy link
Member

I am not able to reproduce:

$ simd tx staking create-validator tx.json --sign-mode amino-json --from bob
auth_info:
  fee:
    amount: []
    gas_limit: "200000"
    granter: ""
    payer: ""
  signer_infos: []
  tip: null
body:
  extension_options: []
  memo: ""
  messages:
  - '@type': /cosmos.staking.v1beta1.MsgCreateValidator
    commission:
      max_change_rate: "0.010000000000000000"
      max_rate: "0.200000000000000000"
      rate: "0.100000000000000000"
    delegator_address: ""
    description:
      details: validator's (optional) details
      identity: optional identity signature (ex. UPort or Keybase)
      moniker: myvalidator
      security_contact: validator's (optional) security contact email
      website: validator's (optional) website
    min_self_delegation: "1"
    pubkey:
      '@type': /cosmos.crypto.ed25519.PubKey
      key: oWg2ISpLF405Jcm2vXV+2v4fnjodh6aafuIdeoW+rUw=
    validator_address: cosmosvaloper1xdegzwlas52szgf7qzvv6l2fcxj34z7z0tuxmp
    value:
      amount: "1000000"
      denom: stake
  non_critical_extension_options: []
  timeout_height: "0"
signatures: []
confirm transaction before signing and broadcasting [y/N]: y
code: 0
codespace: ""
data: ""
events: []
gas_used: "0"
gas_wanted: "0"
height: "0"
info: ""
logs: []
raw_log: ""
timestamp: ""
tx: null
txhash: F01C08B96343435DB736FBABFE301A3FC7C68843CB56CCF6AE08E9FE10FB67CB
$ simd q tx F01C08B96343435DB736FBABFE301A3FC7C68843CB56CCF6AE08E9FE10FB67CB
code: 0
codespace: ""
data: 12340A322F636F736D6F732E7374616B696E672E763162657461312E4D736743726561746556616C696461746F72526573706F6E7365
events:
- attributes:
  - index: true
    key: fee
    value: ""
  - index: true
    key: fee_payer
    value: cosmos1xdegzwlas52szgf7qzvv6l2fcxj34z7z2lgnhj
  type: tx
- attributes:
  - index: true
    key: acc_seq
    value: cosmos1xdegzwlas52szgf7qzvv6l2fcxj34z7z2lgnhj/0
  type: tx
- attributes:
  - index: true
    key: signature
    value: CbKTD+qSR4IN0/4wUm4y9hWaxLyU/P9xjpesYeI41w9Kz236WSG0C1H6XY2kovlC9ScZELp0tiCjt9ErKx7laA==
  type: tx
- attributes:
  - index: true
    key: action
    value: /cosmos.staking.v1beta1.MsgCreateValidator
  - index: true
    key: sender
    value: cosmos1xdegzwlas52szgf7qzvv6l2fcxj34z7z2lgnhj
  - index: true
    key: module
    value: staking
  - index: true
    key: msg_index
    value: "0"
  type: message
- attributes:
  - index: true
    key: spender
    value: cosmos1xdegzwlas52szgf7qzvv6l2fcxj34z7z2lgnhj
  - index: true
    key: amount
    value: 1000000stake
  - index: true
    key: msg_index
    value: "0"
  type: coin_spent
- attributes:
  - index: true
    key: receiver
    value: cosmos1tygms3xhhs3yv487phx3dw4a95jn7t7lpm470r
  - index: true
    key: amount
    value: 1000000stake
  - index: true
    key: msg_index
    value: "0"
  type: coin_received
- attributes:
  - index: true
    key: validator
    value: cosmosvaloper1xdegzwlas52szgf7qzvv6l2fcxj34z7z0tuxmp
  - index: true
    key: amount
    value: 1000000stake
  - index: true
    key: msg_index
    value: "0"
  type: create_validator
gas_used: "174884"
gas_wanted: "200000"
height: "4"
info: ""
logs: []
raw_log: ""
timestamp: "2023-11-27T10:18:18Z"
tx:
  '@type': /cosmos.tx.v1beta1.Tx
  auth_info:
    fee:
      amount: []
      gas_limit: "200000"
      granter: ""
      payer: ""
    signer_infos:
    - mode_info:
        single:
          mode: SIGN_MODE_LEGACY_AMINO_JSON
      public_key:
        '@type': /cosmos.crypto.secp256k1.PubKey
        key: A+wQvRughJ6w8DQxjKdkyOnyf2pGiRxgRUfiZ52s+fRx
      sequence: "0"
    tip: null
  body:
    extension_options: []
    memo: ""
    messages:
    - '@type': /cosmos.staking.v1beta1.MsgCreateValidator
      commission:
        max_change_rate: "0.010000000000000000"
        max_rate: "0.200000000000000000"
        rate: "0.100000000000000000"
      delegator_address: ""
      description:
        details: validator's (optional) details
        identity: optional identity signature (ex. UPort or Keybase)
        moniker: myvalidator
        security_contact: validator's (optional) security contact email
        website: validator's (optional) website
      min_self_delegation: "1"
      pubkey:
        '@type': /cosmos.crypto.ed25519.PubKey
        key: oWg2ISpLF405Jcm2vXV+2v4fnjodh6aafuIdeoW+rUw=
      validator_address: cosmosvaloper1xdegzwlas52szgf7qzvv6l2fcxj34z7z0tuxmp
      value:
        amount: "1000000"
        denom: stake
    non_critical_extension_options: []
    timeout_height: "0"
  signatures:
  - CbKTD+qSR4IN0/4wUm4y9hWaxLyU/P9xjpesYeI41w9Kz236WSG0C1H6XY2kovlC9ScZELp0tiCjt9ErKx7laA==
txhash: F01C08B96343435DB736FBABFE301A3FC7C68843CB56CCF6AE08E9FE10FB67CB

@webmaster128
Copy link
Member Author

webmaster128 commented Nov 27, 2023

We had the issue multiple times in the past that bugs remained hidden when both the signer and the verifier uses the same Go implementation. To verify this, the Amino JSON signdoc bytes need to be compared across different versions of the SDK.

@webmaster128
Copy link
Member Author

Here you find JSON sign docs for the cases that don't work. Same sign docs are valid for SDK 0.47.

{"account_number":"2","chain_id":"simd-testing","fee":{"amount":[{"amount":"25000","denom":"ucosm"}],"gas":"1500000"},"memo":"","msgs":[{"type":"cosmos-sdk/MsgVoteWeighted","value":{"options":[{"option":1,"weight":"0.700000000000000000"},{"option":3,"weight":"0.200000000000000000"},{"option":2,"weight":"0.100000000000000000"}],"proposal_id":"1","voter":"cosmos10dyr9899g6t0pelew4nvf4j5c3jcgv0r73qga5"}}],"sequence":"2"}
{"account_number":"27","chain_id":"simd-testing","fee":{"amount":[{"amount":"5000","denom":"ucosm"}],"gas":"200000"},"memo":"","msgs":[{"type":"cosmos-sdk/MsgCreateValidator","value":{"commission":{"max_change_rate":"0.010000000000000000","max_rate":"0.200000000000000000","rate":"0.100000000000000000"},"delegator_address":"cosmos1f3unlztnx4hyp5cv7fky4dl3tcmfs5dqvgjxt7","description":{"details":"What should I write?","identity":"AABB1234","moniker":"That's me","security_contact":"DM on Twitter","website":"http://example.com/me"},"min_self_delegation":"1","pubkey":{"type":"tendermint/PubKeyEd25519","value":"FdK9O4wabOyE9knkolEfTwokoRf7jaDsCYk9/yKNjW8="},"validator_address":"cosmosvaloper1f3unlztnx4hyp5cv7fky4dl3tcmfs5dqfuxn8d","value":{"amount":"1","denom":"ustake"}}}],"sequence":"0"}
{"account_number":"28","chain_id":"simd-testing","fee":{"amount":[{"amount":"5000","denom":"ucosm"}],"gas":"200000"},"memo":"","msgs":[{"type":"cosmos-sdk/MsgCreateValidator","value":{"commission":{"max_change_rate":"0.010000000000000000","max_rate":"0.200000000000000000","rate":"0.100000000000000000"},"delegator_address":"cosmos1km0m05c7z72m0ugzmv9fzl9tduhce2qsdxfka8","description":{"details":"What should I write?","identity":"AABB1234","moniker":"That's me","security_contact":"DM on Twitter","website":"http://example.com/me"},"min_self_delegation":"1","pubkey":{"type":"tendermint/PubKeyEd25519","value":"lJgb0gLRPWzaD8gZqcOOc3UWgZbhPY9BNWXet6OZgRs="},"validator_address":"cosmosvaloper1km0m05c7z72m0ugzmv9fzl9tduhce2qsgjar35","value":{"amount":"3","denom":"ustake"}}}],"sequence":"0"}
{"account_number":"28","chain_id":"simd-testing","fee":{"amount":[{"amount":"5000","denom":"ucosm"}],"gas":"200000"},"memo":"","msgs":[{"type":"cosmos-sdk/MsgEditValidator","value":{"description":{"details":"Still no idea","identity":"ZZZZ","moniker":"new name","security_contact":"DM on Discord","website":"http://example.com/new-site"},"validator_address":"cosmosvaloper1km0m05c7z72m0ugzmv9fzl9tduhce2qsgjar35"}}],"sequence":"1"}
{"account_number":"28","chain_id":"simd-testing","fee":{"amount":[{"amount":"5000","denom":"ucosm"}],"gas":"200000"},"memo":"","msgs":[{"type":"cosmos-sdk/MsgEditValidator","value":{"description":{"details":"Still no idea","identity":"ZZZZ","moniker":"new name","security_contact":"DM on Discord","website":"http://example.com/new-site"},"min_self_delegation":"3","validator_address":"cosmosvaloper1km0m05c7z72m0ugzmv9fzl9tduhce2qsgjar35"}}],"sequence":"2"}

Given the types that are failing, I'd suspect something around decimals. But this is just a gut feeling.

@tac0turtle tac0turtle moved this from 🤸‍♂️ In Progress to 📋 Backlog in Cosmos-SDK Jun 3, 2024
@alpe alpe self-assigned this Jun 19, 2024
@alpe alpe removed their assignment Jul 17, 2024
@kocubinski
Copy link
Member

#20803 is a potential fix but kind of stuck in limbo at the moment.

@webmaster128
Copy link
Member Author

@yihuang was this close intentional? Looks a bit weird to me

@yihuang
Copy link
Collaborator

yihuang commented Dec 17, 2024

@yihuang was this close intentional? Looks a bit weird to me

not intentional, seems triggered by commit msg accidentally.

@yihuang
Copy link
Collaborator

yihuang commented Dec 17, 2024

The merge of the PR in our repo don't trigger the close, but the push of the commit in some fork of our repo triggered it, looks indeed a bit weird.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: 📋 Backlog
Development

No branches or pull requests

5 participants