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

Commit signatures may be empty/missing #704

Closed
ethanfrey opened this issue Mar 11, 2021 · 3 comments · Fixed by #705
Closed

Commit signatures may be empty/missing #704

ethanfrey opened this issue Mar 11, 2021 · 3 comments · Fixed by #705

Comments

@ethanfrey
Copy link
Contributor

Tendermint rpc can return an empty field here: https://github.com/cosmos/cosmjs/blob/main/packages/tendermint-rpc/src/tendermint34/adaptors/v0-34/responses.ts#L421

I got the following error querying musselnet:

(node:66990) UnhandledPromiseRejectionWarning: Error: Value must not be null
    at assertSet (/home/ethan/js/ts-relayer/node_modules/@cosmjs/tendermint-rpc/build/tendermint34/encodings.js:16:15)
    at Object.assertNotEmpty (/home/ethan/js/ts-relayer/node_modules/@cosmjs/tendermint-rpc/build/tendermint34/encodings.js:107:5)
    at decodeCommitSignature (/home/ethan/js/ts-relayer/node_modules/@cosmjs/tendermint-rpc/build/tendermint34/adaptors/v0-34/responses.js:182:54)
    at Array.map (<anonymous>)
    at decodeCommit (/home/ethan/js/ts-relayer/node_modules/@cosmjs/tendermint-rpc/build/tendermint34/adaptors/v0-34/responses.js:190:62)
    at decodeCommitResponse (/home/ethan/js/ts-relayer/node_modules/@cosmjs/tendermint-rpc/build/tendermint34/adaptors/v0-34/responses.js:197:17)
    at decodeCommit (/home/ethan/js/ts-relayer/node_modules/@cosmjs/tendermint-rpc/build/tendermint34/adaptors/v0-34/responses.js:354:16)
    at Tendermint34Client.doCall (/home/ethan/js/ts-relayer/node_modules/@cosmjs/tendermint-rpc/build/tendermint34/tendermint34client.js:263:16)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at async IbcClient.getSignedHeader (/home/ethan/js/ts-relayer/build/main/lib/ibcclient.js:170:58)

Please make it optional (and maybe investigate a bit more first if you wish, there may be other aspects)

@ethanfrey
Copy link
Contributor Author

Digging in more, it seems this happens when we get latest commit (non-canonical, not all sigs may have arrives). It will often have some "absent" fields. If block_flag_id is 1 (absent), all other fields should not be present when making protobuf. If block_flag_id is 2 (signed), all other fields are required.

Note that timestamp is not empty or "", but rather the "0" time: 0001-01-01T00:00:00Z. But we must omit the field when making protobuf, not end up with some super low time... and this causes a few issues parsing from this ancient timestamp to a js Date object to a protobuf Timestamp.

I will allow empty arrays and add special handling for this timestamp. Hopefully they have a better api soon.

curl https://rpc.musselnet.cosmwasm.com/commit | jq .result.signed_header.commit.signatures

[
  {
    "block_id_flag": 1,
    "validator_address": "",
    "timestamp": "0001-01-01T00:00:00Z",
    "signature": null
  },
  {
    "block_id_flag": 2,
    "validator_address": "341CEE7B1BA35C23217A972885249913E7BB9FB3",
    "timestamp": "2021-03-11T23:32:15.528027253Z",
    "signature": "/ePTRC4jngJEUpVLcKx+Nr2r866qC25WCgOCXHQDHYFarC/tMkOtbVEQpjpikiTLZFaxmWv9hIXRUkQ9mY7BCA=="
  },
  {
    "block_id_flag": 2,
    "validator_address": "51341FF26563FC48BE4CC288BCC9A7EE545FA640",
    "timestamp": "2021-03-11T23:32:15.5472272Z",
    "signature": "zp7anVoXoPKJ17L/1jGUgJqySNmlK673vcGe7zvWqCkfgERKIL4PzDvmwtRi0BYADb9rH/U7N/H6vHuOUI1/DQ=="
  },
  {
    "block_id_flag": 2,
    "validator_address": "87E6508A07E5F8BEA157162857C1DC3FA8AFC41B",
    "timestamp": "2021-03-11T23:32:15.543324576Z",
    "signature": "rZ6bqWE9IobtYEVeIZEs+szZBpJo/DJXFqNl/73FLynSywjWscgtpkyugrZBojiL0+apQ8i58A5NKu5J4bPhDQ=="
  },
  {
    "block_id_flag": 2,
    "validator_address": "9830095087C6FD7E15DDCDB2CB7E544B86C81BD3",
    "timestamp": "2021-03-11T23:32:15.542127538Z",
    "signature": "LBbWFHRAiog2uAmUVeoaWZwNCagZoTIgUMFIpKNAR2IckPBHfcfmTKQ3oxUwvaH2Assy/lNdptulY04SSmuEDg=="
  },
  {
    "block_id_flag": 1,
    "validator_address": "",
    "timestamp": "0001-01-01T00:00:00Z",
    "signature": null
  },
  {
    "block_id_flag": 2,
    "validator_address": "E2E5431899A94672CC9850157C14BC67A6A5EAF7",
    "timestamp": "2021-03-11T23:32:15.550503887Z",
    "signature": "YeXxk7KsHKi9X1lMLjfuXldDL8MOVIfnekS2tCnn85yPAovgQmgFoC+AogZ1E4MKZ50AMgHHSDPu2avPFEA9Cg=="
  }
]

@webmaster128
Copy link
Member

Done in #705 and released as 0.24.1

@clockworkgr
Copy link
Contributor

clockworkgr commented Nov 30, 2023

@ethanfrey @webmaster128

and this causes a few issues parsing from this ancient timestamp to a js Date object to a protobuf Timestamp.

At least some of the problem was due to a bug in the fromRfc3339 method which parsed year 0001 as 1901 (due to how Date.UTC() treats years 1-99).

The solution in #705 combined with [email protected] breaks https://github.com/confio/ts-relayer like so:

  • Tendermint light client in the relayer reads the CommitSigs in order to include them in a MsgUpdateClient.
  • Properly handle missing commit signatures #705 solution sets the timestamps to undefined
  • cosmjs-types encoding encodes undefined as google.protobuf.Timestamp { seconds:0n, nanos: 0 }
  • that is Unix epoch zero (i.e. 1970-01-01 00:00:00.000) rather than time.Time zero
  • Attempting to submit MsgUpdateClient fails with rpc error: code = Unknown desc = header is not a tendermint header: time is present

This is due to the following check: https://github.com/cometbft/cometbft/blob/main/types/block.go#L662
since '1970-01-01 00:00:00.000' != '0001-01-01 00:00:00.000'

I suggest no special treatment as implemented here:
#1516

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 a pull request may close this issue.

3 participants