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

Time values are not always formatted correctly, causing the light client to sometimes return malformed light blocks #774

Closed
romac opened this issue Jan 7, 2021 · 1 comment · Fixed by #775
Assignees
Labels
bug Something isn't working
Milestone

Comments

@romac
Copy link
Member

romac commented Jan 7, 2021

@ancazamfir recently discovered that the Rust relayer would sometimes send malformed light blocks to the chain, eg. in a ClientUpdate message. The chain would then fail with the following error:

header failed basic validation:
  commit signs block A195DA45ACDC71387E33B7FAF7756425483FAB5DA21D8B0D02A4D5A5772A95F8
  header is block E1372844E223FDBB7209B08AA1231ED9C9D85AFBCF72FF9278ABE02A12B549EC

It appears that light blocks sometimes get corrupted when stored and retrieved from the LightStore, specifically the signed_header.header.time field:

         chain_id: chain::Id(ibc-0),
         height: block::Height(3962),
         time: Time(
-            2021-01-06T21:50:58.067816Z,
+            2021-01-06T21:50:58.678160Z,
         ),
         last_block_id: Some(
             Id {

Because this field is part of the hash of the header, the chain would then compute a different hash than the commit's last_block_id_hash, yielding the error above.

The problem stems from the custom serde serializer for Time values, which is used by serde_cbor to encode the light blocks as a byte array before storing them in the sled-backed light store.

I have tracked this to the custom RFC3339 formatter which we use to format tendermint::Time via tendermint_proto::serializers::timestamp::Timestamp.

This function incorrectly trims the leading zeros of the nanoseconds digits, which would break the Time -> String -> Time roundtrip.

For example, the date "2021-01-06T21:50:58.067816Z" is serialized as "2021-01-06T21:50:58.67816Z", which then decodes to "2021-01-06T21:50:58.678160Z", shifting the leading zero to the end.

This explains why this error pops up non-deterministically, as it requires a light block whose time would feature one or more zeros in its subseconds digits for the formatting to break.

@romac romac added the bug Something isn't working label Jan 7, 2021
@romac romac added this to the v0.17.1 milestone Jan 7, 2021
@romac romac self-assigned this Jan 7, 2021
@romac romac changed the title Time values are not formatted properly, causing the light client to return malformed light blocks Time values are not always formatted correctly, causing the light client to someties return malformed light blocks Jan 7, 2021
@romac romac changed the title Time values are not always formatted correctly, causing the light client to someties return malformed light blocks Time values are not always formatted correctly, causing the light client to sometimes return malformed light blocks Jan 7, 2021
@romac
Copy link
Member Author

romac commented Jan 7, 2021

This glitch was not caught by the Rust light client because light blocks are verified before they are stored in the light store (at which point they are serialized and the glitch occurs). On the other hand, the light client always fetches a block from the light store before returning it upon successful verification.

thanethomson added a commit that referenced this issue Jan 9, 2021
Mainly focuses on fixing #774.

Signed-off-by: Thane Thomson <connect@thanethomson.com>
@thanethomson thanethomson mentioned this issue Jan 9, 2021
5 tasks
thanethomson added a commit that referenced this issue Jan 11, 2021
* Release v0.17.1

Mainly focuses on fixing #774.

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Update release date in CHANGELOG

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Fix year

It is 2021, even though it may not feel like it.

Signed-off-by: Thane Thomson <connect@thanethomson.com>

* Update release date for v0.17.1

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant