Skip to content

Commit

Permalink
Merge pull request #285 from hyperledger-labs/fix-04-upgrade-timeout-…
Browse files Browse the repository at this point in the history 
…unit

Fix to use nanosecond unit as 04-upgrade timeout

Signed-off-by: Jun Kimura <[email protected]>
  • Loading branch information
@bluele
bluele authored Jul 13, 2024
2 parents d60289f + b254c91 commit 64ca8f4
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 38 deletions.
24 changes: 12 additions & 12 deletions .gas-snapshot
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ IBCTest:testBenchmarkRecvPacket() (gas: 158870)
IBCTest:testBenchmarkSendPacket() (gas: 128432)
IBCTest:testBenchmarkUpdateMockClient() (gas: 160229)
IBCTest:testToUint128((uint64,uint64)) (runs: 256, μ: 947, ~: 947)
TestICS02:testCreateClient() (gas: 36496843)
TestICS02:testInvalidCreateClient() (gas: 36394062)
TestICS02:testInvalidUpdateClient() (gas: 36392970)
TestICS02:testRegisterClient() (gas: 36048636)
TestICS02:testRegisterClientDuplicatedClientType() (gas: 36033945)
TestICS02:testRegisterClientInvalidClientType() (gas: 36063406)
TestICS02:testUpdateClient() (gas: 36561170)
TestICS02:testCreateClient() (gas: 36540928)
TestICS02:testInvalidCreateClient() (gas: 36438147)
TestICS02:testInvalidUpdateClient() (gas: 36437055)
TestICS02:testRegisterClient() (gas: 36092721)
TestICS02:testRegisterClientDuplicatedClientType() (gas: 36078030)
TestICS02:testRegisterClientInvalidClientType() (gas: 36107491)
TestICS02:testUpdateClient() (gas: 36605255)
TestICS03Handshake:testConnOpenAck() (gas: 1858230)
TestICS03Handshake:testConnOpenConfirm() (gas: 2054143)
TestICS03Handshake:testConnOpenInit() (gas: 1429838)
Expand All @@ -41,9 +41,9 @@ TestICS04Handshake:testInvalidChanOpenInit() (gas: 1760558)
TestICS04Handshake:testInvalidChanOpenTry() (gas: 1775528)
TestICS04Packet:testAcknowledgementPacket() (gas: 3351116)
TestICS04Packet:testInvalidSendPacket() (gas: 3551583)
TestICS04Packet:testRecvPacket() (gas: 10996130)
TestICS04Packet:testRecvPacket() (gas: 10996942)
TestICS04Packet:testRecvPacketTimeoutHeight() (gas: 3259727)
TestICS04Packet:testRecvPacketTimeoutTimestamp() (gas: 3278877)
TestICS04Packet:testRecvPacketTimeoutTimestamp() (gas: 3279103)
TestICS04Packet:testSendPacket() (gas: 6412442)
TestICS04Packet:testTimeoutOnClose() (gas: 3553289)
TestICS04Upgrade:testUpgradeAuthorityCancel() (gas: 46742335)
Expand All @@ -58,9 +58,9 @@ TestICS04Upgrade:testUpgradeOutOfSync() (gas: 3903220)
TestICS04Upgrade:testUpgradeRelaySuccessAtCounterpartyFlushComplete() (gas: 5237770)
TestICS04Upgrade:testUpgradeRelaySuccessAtFlushing() (gas: 5610957)
TestICS04Upgrade:testUpgradeSendPacketFailAtFlushingOrFlushComplete() (gas: 4071025)
TestICS04Upgrade:testUpgradeTimeoutAbortAck() (gas: 17681581)
TestICS04Upgrade:testUpgradeTimeoutAbortConfirm() (gas: 21317741)
TestICS04Upgrade:testUpgradeTimeoutUpgrade() (gas: 44264077)
TestICS04Upgrade:testUpgradeTimeoutAbortAck() (gas: 17694261)
TestICS04Upgrade:testUpgradeTimeoutAbortConfirm() (gas: 21333518)
TestICS04Upgrade:testUpgradeTimeoutUpgrade() (gas: 71049040)
TestICS04Upgrade:testUpgradeToOrdered() (gas: 56509529)
TestICS04Upgrade:testUpgradeToUnordered() (gas: 45113829)
TestICS04UpgradeApp:testUpgradeAuthorizationChanneNotFound() (gas: 61690)
Expand Down
2 changes: 1 addition & 1 deletion contracts/clients/09-localhost/LocalhostClient.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ contract LocalhostClient is ILightClient, ILightClientErrors {
} else if (height.revision_height > block.number) {
revert InvalidHeightRevisionHeight();
}
return uint64(block.timestamp);
return uint64(block.timestamp) * 1e9;
}

/**
Expand Down
4 changes: 2 additions & 2 deletions contracts/core/04-channel/IBCChannelPacketSendRecv.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ contract IBCChannelPacketSendRecv is
{
revert IBCChannelTimeoutPacketHeight(block.number, msg_.packet.timeoutHeight.revision_height);
}
if (msg_.packet.timeoutTimestamp != 0 && block.timestamp * 1e9 >= msg_.packet.timeoutTimestamp) {
revert IBCChannelTimeoutPacketTimestamp(block.timestamp * 1e9, msg_.packet.timeoutTimestamp);
if (msg_.packet.timeoutTimestamp != 0 && hostTimestamp() >= msg_.packet.timeoutTimestamp) {
revert IBCChannelTimeoutPacketTimestamp(hostTimestamp(), msg_.packet.timeoutTimestamp);
}

verifyPacketCommitment(
Expand Down
4 changes: 2 additions & 2 deletions contracts/core/04-channel/IBCChannelUpgrade.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,7 @@ contract IBCChannelUpgradeInitTryAck is IBCChannelUpgradeBase, IIBCChannelUpgrad
Timeout.Data calldata timeout = msg_.counterpartyUpgrade.timeout;
if (
(timeout.height.revision_height != 0 && block.number >= timeout.height.revision_height)
|| (timeout.timestamp != 0 && block.timestamp >= timeout.timestamp)
|| (timeout.timestamp != 0 && hostTimestamp() >= timeout.timestamp)
) {
restoreChannel(msg_.portId, msg_.channelId, UpgradeHandshakeError.Timeout);
return false;
Expand Down Expand Up @@ -620,7 +620,7 @@ contract IBCChannelUpgradeConfirmTimeoutCancel is IBCChannelUpgradeBase, IIBCCha
Timeout.Data calldata timeout = msg_.counterpartyUpgrade.timeout;
if (
(timeout.height.revision_height != 0 && block.number >= timeout.height.revision_height)
|| (timeout.timestamp != 0 && block.timestamp >= timeout.timestamp)
|| (timeout.timestamp != 0 && hostTimestamp() >= timeout.timestamp)
) {
restoreChannel(msg_.portId, msg_.channelId, UpgradeHandshakeError.Timeout);
return false;
Expand Down
7 changes: 7 additions & 0 deletions contracts/core/24-host/IBCHost.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@ contract IBCHost is IIBCHostErrors, IBCStore {
// In ibc-solidity, the prefix is not required, but for compatibility with ibc-go this must be a non-empty value.
bytes internal constant DEFAULT_COMMITMENT_PREFIX = bytes("ibc");

/**
* @dev hostTimestamp returns the current timestamp(Unix time in nanoseconds) of the host chain.
*/
function hostTimestamp() internal view virtual returns (uint64) {
return uint64(block.timestamp) * 1e9;
}

/**
* @dev _getCommitmentPrefix returns the prefix of the commitment proof.
*/
Expand Down
94 changes: 73 additions & 21 deletions tests/foundry/src/ICS04Upgrade.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
pragma solidity ^0.8.20;

import "./helpers/IBCTestHelper.t.sol";
import {Vm} from "forge-std/Test.sol";
import {Vm, console2} from "forge-std/Test.sol";
import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";
import {Upgrade, UpgradeFields, Timeout} from "../../../contracts/proto/Channel.sol";
import {LocalhostClientLib} from "../../../contracts/clients/09-localhost/LocalhostClient.sol";
import {LocalhostHelper} from "../../../contracts/clients/09-localhost/LocalhostHelper.sol";
import {IIBCChannelRecvPacket, IIBCChannelAcknowledgePacket} from "../../../contracts/core/04-channel/IIBCChannel.sol";
import {IIBCChannelUpgrade, IIBCChannelUpgradeBase} from "../../../contracts/core/04-channel/IIBCChannelUpgrade.sol";
import {IIBCChannelUpgradeBase} from "../../../contracts/core/04-channel/IIBCChannelUpgrade.sol";
import {TestIBCChannelUpgradableMockApp} from "./helpers/TestIBCChannelUpgradableMockApp.t.sol";
import {ICS04UpgradeTestHelper} from "./helpers/ICS04UpgradeTestHelper.t.sol";
import {ICS04PacketEventTestHelper} from "./helpers/ICS04PacketTestHelper.t.sol";
Expand Down Expand Up @@ -412,8 +412,8 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper

Timeout.Data[3] memory timeouts = [
Timeout.Data({height: H(block.number), timestamp: 0}),
Timeout.Data({height: H(0), timestamp: uint64(block.timestamp)}),
Timeout.Data({height: H(block.number), timestamp: uint64(block.timestamp)})
Timeout.Data({height: H(0), timestamp: getTimestamp(0)}),
Timeout.Data({height: H(block.number), timestamp: getTimestamp()})
];
HandshakeCallbacks memory callbacks = emptyCallbacks();
callbacks.openInitAndFlushing.callback = _testUpgradeTimeoutAbortAck;
Expand Down Expand Up @@ -457,8 +457,8 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper

Timeout.Data[3] memory timeouts = [
Timeout.Data({height: H(block.number), timestamp: 0}),
Timeout.Data({height: H(0), timestamp: uint64(block.timestamp)}),
Timeout.Data({height: H(block.number), timestamp: uint64(block.timestamp)})
Timeout.Data({height: H(0), timestamp: getTimestamp()}),
Timeout.Data({height: H(block.number), timestamp: getTimestamp()})
];
HandshakeCallbacks memory callbacks = emptyCallbacks();
callbacks.flushingAndFlushing.callback = _testUpgradeTimeoutAbortConfirm;
Expand Down Expand Up @@ -497,7 +497,7 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
}

function testUpgradeTimeoutUpgrade() public {
CallbacksTimeout[] memory cases = new CallbacksTimeout[](10);
CallbacksTimeout[] memory cases = new CallbacksTimeout[](16);
for (uint256 i = 0; i < cases.length; i++) {
cases[i].callbacks = emptyCallbacks();
}
Expand All @@ -511,8 +511,8 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
i++;

cases[i].callbacks.flushingAndFlushing.callback = _testUpgradeTimeoutUpgradeSuccess;
cases[i].t0 = Timeout.Data({height: H(0), timestamp: uint64(block.timestamp + 1)});
cases[i].t1 = Timeout.Data({height: H(0), timestamp: uint64(block.timestamp + 1)});
cases[i].t0 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
cases[i].t1 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
i++;

cases[i].callbacks.openInitAndFlushing.callback = _testUpgradeTimeoutUpgradeSuccess;
Expand All @@ -521,50 +521,85 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
cases[i].t1 = Timeout.Data({height: H(block.number + 1), timestamp: 0});
i++;

cases[i].callbacks.openInitAndFlushing.callback = _testUpgradeTimeoutUpgradeSuccess;
cases[i].callbacks.openInitAndFlushing.reverse = true;
cases[i].t0 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
cases[i].t1 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
i++;

cases[i].callbacks.flushingAndComplete.callback = _testUpgradeTimeoutUpgradeSuccess;
cases[i].callbacks.flushingAndComplete.reverse = true;
cases[i].t0 = Timeout.Data({height: H(block.number + 1), timestamp: 0});
cases[i].t1 = Timeout.Data({height: H(block.number + 1), timestamp: 0});
i++;

cases[i].callbacks.flushingAndComplete.callback = _testUpgradeTimeoutUpgradeSuccess;
cases[i].callbacks.flushingAndComplete.reverse = true;
cases[i].t0 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
cases[i].t1 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
i++;

// ------------------------------ Failure Cases ------------------------------ //

cases[i].callbacks.flushingAndFlushing.callback = _testUpgradeTimeoutUpgradeFailNotReached;
cases[i].callbacks.flushingAndFlushing.callback = _testUpgradeTimeoutUpgradeFailTimeoutHeightNotReached;
cases[i].t0 = Timeout.Data({height: H(block.number + 1), timestamp: 0});
cases[i].t1 = Timeout.Data({height: H(block.number + 1), timestamp: 0});
i++;

cases[i].callbacks.flushingAndFlushing.callback = _testUpgradeTimeoutUpgradeFailNotReached;
cases[i].t0 = Timeout.Data({height: H(0), timestamp: uint64(block.timestamp + 1)});
cases[i].t1 = Timeout.Data({height: H(0), timestamp: uint64(block.timestamp + 1)});
cases[i].callbacks.flushingAndFlushing.callback = _testUpgradeTimeoutUpgradeFailTimeoutTimestampNotReached;
cases[i].t0 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
cases[i].t1 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
i++;

cases[i].callbacks.flushingAndComplete.callback = _testUpgradeTimeoutUpgradeFailReached;
cases[i].t0 = Timeout.Data({height: H(block.number + 1), timestamp: 0});
cases[i].t1 = Timeout.Data({height: H(block.number + 1), timestamp: 0});
i++;

cases[i].callbacks.flushingAndComplete.callback = _testUpgradeTimeoutUpgradeFailReached;
cases[i].t0 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
cases[i].t1 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
i++;

cases[i].callbacks.openSucAndComplete.callback = _testUpgradeTimeoutUpgradeFailReachedAlreadyUpgraded;
cases[i].callbacks.openSucAndComplete.reverse = true;
cases[i].t0 = Timeout.Data({height: H(block.number + 1), timestamp: 0});
cases[i].t1 = Timeout.Data({height: H(block.number + 1), timestamp: 0});
i++;

cases[i].callbacks.openSucAndComplete.callback = _testUpgradeTimeoutUpgradeFailReachedAlreadyUpgraded;
cases[i].callbacks.openSucAndComplete.reverse = true;
cases[i].t0 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
cases[i].t1 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
i++;

cases[i].callbacks.completeAndComplete.callback = _testUpgradeTimeoutUpgradeFailReachedAlreadyCompleted;
cases[i].t0 = Timeout.Data({height: H(block.number + 1), timestamp: 0});
cases[i].t1 = Timeout.Data({height: H(block.number + 1), timestamp: 0});
i++;

cases[i].callbacks.completeAndComplete.callback = _testUpgradeTimeoutUpgradeFailReachedAlreadyCompleted;
cases[i].t0 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
cases[i].t1 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
i++;

cases[i].callbacks.completeAndComplete.callback = _testUpgradeTimeoutUpgradeFailReachedAlreadyCompleted;
cases[i].callbacks.completeAndComplete.reverse = true;
cases[i].t0 = Timeout.Data({height: H(block.number + 1), timestamp: 0});
cases[i].t1 = Timeout.Data({height: H(block.number + 1), timestamp: 0});
i++;

cases[i].callbacks.completeAndComplete.callback = _testUpgradeTimeoutUpgradeFailReachedAlreadyCompleted;
cases[i].callbacks.completeAndComplete.reverse = true;
cases[i].t0 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
cases[i].t1 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
i++;

require(i == cases.length, "invalid number of cases");

for (uint256 i = 0; i < cases.length; i++) {
(uint256 height, uint256 timestamp) = (block.number, block.timestamp);
console2.log("case:", i);
(uint256 height, uint256 timestampSec) = (block.number, block.timestamp);
(ChannelInfo memory channel0, ChannelInfo memory channel1) =
createMockAppLocalhostChannel(Channel.Order.ORDER_UNORDERED);
handshakeUpgradeWithCallbacks(
Expand All @@ -589,7 +624,7 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
);
// restore the block height and timestamp
vm.roll(height);
vm.warp(timestamp);
vm.warp(timestampSec);
}
}

Expand Down Expand Up @@ -1822,7 +1857,7 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
vm.roll(uint256(timeout.height.revision_height));
}
if (timeout.timestamp != 0) {
vm.warp(uint256(timeout.timestamp));
vm.warp(uint256(timeout.timestamp / 1e9));
}
(Channel.Data memory counterpartyChannel,) = handler.getChannel(channel1.portId, channel1.channelId);
handler.timeoutChannelUpgrade(
Expand All @@ -1848,7 +1883,7 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
return false;
}

function _testUpgradeTimeoutUpgradeFailNotReached(
function _testUpgradeTimeoutUpgradeFailTimeoutHeightNotReached(
IIBCHandler handler,
ChannelInfo memory channel0,
ChannelInfo memory channel1
Expand All @@ -1861,8 +1896,25 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
proofChannel: LocalhostClientLib.sentinelProof(),
proofHeight: H(block.number)
});
// IBCChannelUpgradeTimeoutHeightNotReached or IBCChannelUpgradeTimeoutTimestampNotReached
vm.expectRevert();
vm.expectRevert(IIBCChannelUpgradeErrors.IBCChannelUpgradeTimeoutHeightNotReached.selector);
handler.timeoutChannelUpgrade(msg_);
return true;
}

function _testUpgradeTimeoutUpgradeFailTimeoutTimestampNotReached(
IIBCHandler handler,
ChannelInfo memory channel0,
ChannelInfo memory channel1
) internal returns (bool) {
(Channel.Data memory counterpartyChannel,) = handler.getChannel(channel1.portId, channel1.channelId);
IIBCChannelUpgradeBase.MsgTimeoutChannelUpgrade memory msg_ = IIBCChannelUpgradeBase.MsgTimeoutChannelUpgrade({
portId: channel0.portId,
channelId: channel0.channelId,
counterpartyChannel: counterpartyChannel,
proofChannel: LocalhostClientLib.sentinelProof(),
proofHeight: H(block.number)
});
vm.expectRevert(IIBCChannelUpgradeErrors.IBCChannelUpgradeTimeoutTimestampNotReached.selector);
handler.timeoutChannelUpgrade(msg_);
return true;
}
Expand All @@ -1877,7 +1929,7 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
vm.roll(uint256(timeout.height.revision_height));
}
if (timeout.timestamp != 0) {
vm.warp(uint256(timeout.timestamp));
vm.warp(uint256(timeout.timestamp / 1e9));
}
(Channel.Data memory counterpartyChannel,) = handler.getChannel(channel1.portId, channel1.channelId);
IIBCChannelUpgradeBase.MsgTimeoutChannelUpgrade memory msg_ = IIBCChannelUpgradeBase.MsgTimeoutChannelUpgrade({
Expand Down Expand Up @@ -1923,7 +1975,7 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
vm.roll(uint256(timeout.height.revision_height));
}
if (timeout.timestamp != 0) {
vm.warp(uint256(timeout.timestamp));
vm.warp(uint256(timeout.timestamp / 1e9));
}
(Channel.Data memory counterpartyChannel,) = handler.getChannel(channel1.portId, channel1.channelId);
IIBCChannelUpgradeBase.MsgTimeoutChannelUpgrade memory msg_ = IIBCChannelUpgradeBase.MsgTimeoutChannelUpgrade({
Expand Down
9 changes: 9 additions & 0 deletions tests/foundry/src/helpers/IBCTestHelper.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,15 @@ abstract contract IBCTestHelper is Test {
return H(0, revisionHeight);
}

function getTimestamp() internal returns (uint64) {
return getTimestamp(0);
}

function getTimestamp(int256 offsetSecs) internal returns (uint64) {
assertTrue(int256(block.timestamp) + offsetSecs >= 0, "getTimestamp: negative timestamp");
return uint64(uint256((int256(block.timestamp) + offsetSecs) * 1e9));
}

function roll(uint256 bn) internal {
require(prevBlockNumber == 0, "roll: already rolled");
prevBlockNumber = block.number;
Expand Down

0 comments on commit 64ca8f4

Please sign in to comment.