From c802dd4e6cdbbea105ed372d83ff19e5171d5138 Mon Sep 17 00:00:00 2001 From: Hussein Ait Lahcen Date: Tue, 24 Dec 2024 11:37:02 +0100 Subject: [PATCH 1/5] feat(evm): avoid overflow on expiry if trustingperiod is set to max(uint64) --- evm/contracts/clients/CometblsClient.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/evm/contracts/clients/CometblsClient.sol b/evm/contracts/clients/CometblsClient.sol index 9855e76fd4..0ec8fdbda2 100644 --- a/evm/contracts/clients/CometblsClient.sol +++ b/evm/contracts/clients/CometblsClient.sol @@ -68,7 +68,7 @@ library CometblsClientLib { uint64 trustingPeriod, uint64 currentTime ) internal pure returns (bool) { - return currentTime > (headerTime + trustingPeriod); + return uint256(currentTime) > (uint256(headerTime) + uint256(trustingPeriod)); } function encodeMemory( From afd3e02bac2eaeedd8eec203705ae0bc71716a1e Mon Sep 17 00:00:00 2001 From: Hussein Ait Lahcen Date: Tue, 24 Dec 2024 11:37:33 +0100 Subject: [PATCH 2/5] fix(evm): cometbls: incorrect height on consensus update event --- evm/contracts/clients/CometblsClient.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/evm/contracts/clients/CometblsClient.sol b/evm/contracts/clients/CometblsClient.sol index 0ec8fdbda2..0dbfd63796 100644 --- a/evm/contracts/clients/CometblsClient.sol +++ b/evm/contracts/clients/CometblsClient.sol @@ -386,7 +386,7 @@ contract CometblsClient is return ConsensusStateUpdate({ clientStateCommitment: clientState.commit(), consensusStateCommitment: consensusState.commit(), - height: header.trustedHeight + height: untrustedHeightNumber }); } From 11e3b6981f8c2d104fee59c0b1475822d136f6ed Mon Sep 17 00:00:00 2001 From: Hussein Ait Lahcen Date: Tue, 24 Dec 2024 11:38:20 +0100 Subject: [PATCH 3/5] fix(evm): cometbls: processed moment no longer used --- evm/contracts/clients/CometblsClient.sol | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/evm/contracts/clients/CometblsClient.sol b/evm/contracts/clients/CometblsClient.sol index 0dbfd63796..654af8056a 100644 --- a/evm/contracts/clients/CometblsClient.sol +++ b/evm/contracts/clients/CometblsClient.sol @@ -216,11 +216,6 @@ contract CometblsClient is } clientStates[clientId] = clientState; consensusStates[clientId][clientState.latestHeight] = consensusState; - // Normalize to nanosecond because ibc-go recvPacket expects nanos... - processedMoments[clientId][clientState.latestHeight] = ProcessedMoment({ - timestamp: block.timestamp * 1e9, - height: block.number - }); return ConsensusStateUpdate({ clientStateCommitment: clientState.commit(), consensusStateCommitment: consensusState.commit(), @@ -378,11 +373,6 @@ contract CometblsClient is consensusState.nextValidatorsHash = header.signedHeader.nextValidatorsHash; - ProcessedMoment storage processed = - processedMoments[clientId][header.trustedHeight]; - processed.timestamp = block.timestamp * 1e9; - processed.height = block.number; - return ConsensusStateUpdate({ clientStateCommitment: clientState.commit(), consensusStateCommitment: consensusState.commit(), From 164862e1d467b3ca86816da86edb3800ebbaee0d Mon Sep 17 00:00:00 2001 From: Hussein Ait Lahcen Date: Tue, 24 Dec 2024 11:57:15 +0100 Subject: [PATCH 4/5] fix(voyager): clamp unbonding period for decent trusting period --- evm/contracts/clients/CometblsClient.sol | 1 - voyager/modules/consensus/cometbls/src/main.rs | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/evm/contracts/clients/CometblsClient.sol b/evm/contracts/clients/CometblsClient.sol index 654af8056a..1a1df2b3b1 100644 --- a/evm/contracts/clients/CometblsClient.sol +++ b/evm/contracts/clients/CometblsClient.sol @@ -311,7 +311,6 @@ contract CometblsClient is } uint64 maxClockDrift = currentTime + clientState.maxClockDrift; - if (untrustedTimestamp >= maxClockDrift) { revert CometblsClientLib.ErrMaxClockDriftExceeded(); } diff --git a/voyager/modules/consensus/cometbls/src/main.rs b/voyager/modules/consensus/cometbls/src/main.rs index 638f71c2ad..17cf83738d 100644 --- a/voyager/modules/consensus/cometbls/src/main.rs +++ b/voyager/modules/consensus/cometbls/src/main.rs @@ -206,6 +206,9 @@ impl ConsensusModuleServer for Module { let unbonding_period = u64::try_from(params.unbonding_time.clone().unwrap().seconds).unwrap() * 1_000_000_000; + // Avoid low unbonding period preventing relayer from submitting slightly old headers + let unbonding_period = unbonding_period.max(3 * 24 * 3600 * 1_000_000_000); + Ok(serde_json::to_value(ClientState { chain_id: cometbls_light_client_types::ChainId::from_string(self.chain_id.to_string()) .unwrap(), From dac7adaa6d9cbeccc9b5f9e07d0ed318aa3519bf Mon Sep 17 00:00:00 2001 From: Hussein Ait Lahcen Date: Tue, 24 Dec 2024 12:00:16 +0100 Subject: [PATCH 5/5] fix(evm): cometbls: updateclient test must check untrusted height --- evm/contracts/clients/CometblsClient.sol | 3 ++- evm/tests/src/02-client/CometblsClient.t.sol | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/evm/contracts/clients/CometblsClient.sol b/evm/contracts/clients/CometblsClient.sol index 1a1df2b3b1..564b7bb53a 100644 --- a/evm/contracts/clients/CometblsClient.sol +++ b/evm/contracts/clients/CometblsClient.sol @@ -68,7 +68,8 @@ library CometblsClientLib { uint64 trustingPeriod, uint64 currentTime ) internal pure returns (bool) { - return uint256(currentTime) > (uint256(headerTime) + uint256(trustingPeriod)); + return uint256(currentTime) + > (uint256(headerTime) + uint256(trustingPeriod)); } function encodeMemory( diff --git a/evm/tests/src/02-client/CometblsClient.t.sol b/evm/tests/src/02-client/CometblsClient.t.sol index 4bf9c939e0..70b11d1950 100644 --- a/evm/tests/src/02-client/CometblsClient.t.sol +++ b/evm/tests/src/02-client/CometblsClient.t.sol @@ -714,7 +714,7 @@ contract CometblsClientTest is Test { expectedConsensusStateCommitment, "Consensus state commitment mismatch" ); - assertEq(update.height, 100, "Height mismatch"); + assertEq(update.height, 101, "Height mismatch"); } function verifyMembership(