Light client "header from the future" with unsynchronized hermes clock

[ancazamfir]

Summary of Bug

(thanks @ljoss17 @romac @josef-widder for the help with the investigation)
@ljoss17 reported a hermes operator seeing these errors:

Mar 04 11:24:16 stride hermes[3403174]: 2023-03-04T10:24:16.855322Z  WARN ThreadId(76) worker.batch{chain=osmosis-1}:supervisor.handle_batch{chain=osmosis-1}:supervisor.process_batch{chain=osmosis-1}:worker.packet.cmd{src_chain=osmosis-1 src_port=transfer src_channel=channel-204 dst_chain=evmos_9001-2}: task encountered ignorable error: link error: failed during a client operation: error raised while updating client on chain evmos_9001-2: failed building header with error: light client verification error for chain id osmosis-1: invalid light block: header from the future: header_time=2023-03-04T10:24:43.824950138Z now=2023-03-04T10:24:16.84190695Z

After debugging the issue it turns out the relayer has a delayed clock, delay that is higher than the client clock drift.

When hermes needs to update the client for B on A it calls the light client verification that performs this check:

if (header_from_B.time > now + clock_drift) {
  return Err(HeaderFromTheFuture)
}

(see original code here)
Where clock_drift = B.clock_drift + A.clock_drift + A.max_block_time. In this case the checks fails.
Note: A similar check is done in hermes later here

hermes/crates/relayer/src/foreign_client.rs

Lines 971 to 978 in 6452553

	let ts_adjusted = (status.timestamp + client_state.max_clock_drift()).map_err(|e| {
	ForeignClientError::client_update_timing(
	self.dst_chain.id(),
	client_state.max_clock_drift(),
	"failed to adjust timestamp of destination chain with clock drift".to_string(),
	e,
	)
	})?;

.

Version

all

Steps to Reproduce

Run a relayer with unsynchronized clock, with negative drift.

Acceptance Criteria

We have two options:

• Make mandatory that hermes should sync the clock and clearly document it, OR
• Make hermes work with unsynchronized clock. (fixed in Detect light client attacks and report them to both the chain and the RPC node #3224)

In addition:

• Document and explain why the final client "clock_drift" is derived from the reference and target chain clock_drifts, and the target chain max_block_time.

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate milestone (priority) applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[ancazamfir]

Hermes should be able relay correctly with unsynchronized system clock. We should also have a single check for the "header in the future" case.
In light of this imo the second option is preferable.

[josef-widder]

I think it is important to note that while the system clock need not be synchronized, there still is the requirement that the times in the blocks/headers of the two chains need to be roughly synchronized.

[ancazamfir]

I think it is important to note that while the system clock need not be synchronized, there still is the requirement that the times in the blocks/headers of the two chains need to be roughly synchronized.

True. The requirement is encoded in hermes but the documentation lacks and needs to improve.

[seanchen1991]

Is this issue able to be closed at this point?

[romac]

I think it is important to note that while the system clock need not be synchronized, there still is the requirement that the times in the blocks/headers of the two chains need to be roughly synchronized.

True. The requirement is encoded in hermes but the documentation lacks and needs to improve.

Let's document this somewhere in the guide and then we can close this, as we are not relying on the system time anymore.