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

Serialization of timestamp field for CommitSig::BlockIdFlagAbsent does not match Go counterpart #1352

Open
romac opened this issue Sep 14, 2023 · 0 comments
Assignees
Labels
bug Something isn't working

Comments

@romac
Copy link
Member

romac commented Sep 14, 2023

What went wrong?

I noticed the following error when testing the light client attack evidence submission to CometBFT 0.34 and 0.37 via the /broacast_evidence RPC endpoint from Hermes:

evidence.ValidateBasic failed: invalid conflicting light block: invalid signed header: invalid commit: wrong CommitSig #1: time is present

My investigation led me to the following problem:


CommitSig::BlockIdFlagAbsent currently serializes to JSON as

{
  "block_id_flag": 1,
  "validator_address": "",
  "timestamp": "1970-01-01T00:00:00Z",
  "signature": ""
}

which fails the CometBFT check CommitSig.ValidateBasic() with time is present

This check is performed as

switch cs.BlockIDFlag {
  case BlockIDFlagAbsent:
    if len(cs.ValidatorAddress) != 0 {
      return errors.New("validator address is present")
    }
    if !cs.Timestamp.IsZero() {
      return errors.New("time is present")
}

IsZero() is defined as

// IsZero reports whether t represents the zero time instant, // January 1, year 1, 00:00:00 UTC.
func (t Time) IsZero() bool {
  return t.sec() == 0 && t.nsec() == 0
}

From the documentation of the Time struct, we see that it expects a
zero time to be 0001-01-01T00:00:00Z and not 1970-01-01T00:00:00Z:

// The zero value for a Time is defined to be
//	January 1, year 1, 00:00:00.000000000 UTC
// which (1) looks like a zero, or as close as you can get in a date // (1-1-1 00:00:00 UTC), (2) is unlikely enough to arise in practice to // be a suitable "not set" sentinel, unlike Jan 1 1970, and (3) has a // non-negative year even in time zones west of UTC, unlike 1-1-0 // 00:00:00 UTC, which would be 12-31-(-1) 19:00:00 in New York.

There may be other instances where the timestamp is formatted
incorrectly, but I don't have time to look into it at the moment.

Steps to reproduce

  1. Download this evidence serialized as JSON: https://gist.github.com/romac/58548ff5b4f2ad880bcb53364b1b7ffa

  2. Create a new Rust project, add tendermint-rpc v0.31.1 as a dependency:

$ cargo new --bin evidence
$ cd evidence/
$ cargo add [email protected]
$ mv ~/Downloads/evidence.json ./
  1. Paste the following in src/main.rs:
use std::fs::File;

use tendermint_rpc::dialect::v0_34::Evidence;

fn main() {
    let file = File::open("evidence.json").unwrap();
    let evidence: Evidence = serde_json::from_reader(file).unwrap();
    println!("{}", serde_json::to_string_pretty(&evidence).unwrap());
}
  1. Run the executable
$ cargo run -q | jq '.value.ConflictingBlock.signed_header.commit.signatures[1].timestamp'

Expected

"0001-01-01T00:00:00Z"

Result

"1970-01-01T00:00:00Z"

Definition of "done"

The result should match the expected behaviour and tendermint-rs should be able to submit a valid evidence to CometBFT 0.34 and 0.37.

Related issues

Looks like this was already popped up in past issues, but either the fix was lost or overwritten or it was never correct in the first place:

@romac romac added the bug Something isn't working label Sep 14, 2023
@romac romac self-assigned this Sep 14, 2023
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

No branches or pull requests

1 participant