Skip to content

Commit 64ca8f4

Browse files
bluelebluele
authored
Merge pull request #285 from hyperledger-labs/fix-04-upgrade-timeout-unit
Fix to use nanosecond unit as 04-upgrade timeout Signed-off-by: Jun Kimura <jun.kimura@datachain.jp>
2 parents d60289f + b254c91 commit 64ca8f4

File tree

7 files changed

+106
-38
lines changed

7 files changed

+106
-38
lines changed

‎.gas-snapshot

+12-12
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,13 @@ IBCTest:testBenchmarkRecvPacket() (gas: 158870)
88
IBCTest:testBenchmarkSendPacket() (gas: 128432)
99
IBCTest:testBenchmarkUpdateMockClient() (gas: 160229)
1010
IBCTest:testToUint128((uint64,uint64)) (runs: 256, μ: 947, ~: 947)
11-
TestICS02:testCreateClient() (gas: 36496843)
12-
TestICS02:testInvalidCreateClient() (gas: 36394062)
13-
TestICS02:testInvalidUpdateClient() (gas: 36392970)
14-
TestICS02:testRegisterClient() (gas: 36048636)
15-
TestICS02:testRegisterClientDuplicatedClientType() (gas: 36033945)
16-
TestICS02:testRegisterClientInvalidClientType() (gas: 36063406)
17-
TestICS02:testUpdateClient() (gas: 36561170)
11+
TestICS02:testCreateClient() (gas: 36540928)
12+
TestICS02:testInvalidCreateClient() (gas: 36438147)
13+
TestICS02:testInvalidUpdateClient() (gas: 36437055)
14+
TestICS02:testRegisterClient() (gas: 36092721)
15+
TestICS02:testRegisterClientDuplicatedClientType() (gas: 36078030)
16+
TestICS02:testRegisterClientInvalidClientType() (gas: 36107491)
17+
TestICS02:testUpdateClient() (gas: 36605255)
1818
TestICS03Handshake:testConnOpenAck() (gas: 1858230)
1919
TestICS03Handshake:testConnOpenConfirm() (gas: 2054143)
2020
TestICS03Handshake:testConnOpenInit() (gas: 1429838)
@@ -41,9 +41,9 @@ TestICS04Handshake:testInvalidChanOpenInit() (gas: 1760558)
4141
TestICS04Handshake:testInvalidChanOpenTry() (gas: 1775528)
4242
TestICS04Packet:testAcknowledgementPacket() (gas: 3351116)
4343
TestICS04Packet:testInvalidSendPacket() (gas: 3551583)
44-
TestICS04Packet:testRecvPacket() (gas: 10996130)
44+
TestICS04Packet:testRecvPacket() (gas: 10996942)
4545
TestICS04Packet:testRecvPacketTimeoutHeight() (gas: 3259727)
46-
TestICS04Packet:testRecvPacketTimeoutTimestamp() (gas: 3278877)
46+
TestICS04Packet:testRecvPacketTimeoutTimestamp() (gas: 3279103)
4747
TestICS04Packet:testSendPacket() (gas: 6412442)
4848
TestICS04Packet:testTimeoutOnClose() (gas: 3553289)
4949
TestICS04Upgrade:testUpgradeAuthorityCancel() (gas: 46742335)
@@ -58,9 +58,9 @@ TestICS04Upgrade:testUpgradeOutOfSync() (gas: 3903220)
5858
TestICS04Upgrade:testUpgradeRelaySuccessAtCounterpartyFlushComplete() (gas: 5237770)
5959
TestICS04Upgrade:testUpgradeRelaySuccessAtFlushing() (gas: 5610957)
6060
TestICS04Upgrade:testUpgradeSendPacketFailAtFlushingOrFlushComplete() (gas: 4071025)
61-
TestICS04Upgrade:testUpgradeTimeoutAbortAck() (gas: 17681581)
62-
TestICS04Upgrade:testUpgradeTimeoutAbortConfirm() (gas: 21317741)
63-
TestICS04Upgrade:testUpgradeTimeoutUpgrade() (gas: 44264077)
61+
TestICS04Upgrade:testUpgradeTimeoutAbortAck() (gas: 17694261)
62+
TestICS04Upgrade:testUpgradeTimeoutAbortConfirm() (gas: 21333518)
63+
TestICS04Upgrade:testUpgradeTimeoutUpgrade() (gas: 71049040)
6464
TestICS04Upgrade:testUpgradeToOrdered() (gas: 56509529)
6565
TestICS04Upgrade:testUpgradeToUnordered() (gas: 45113829)
6666
TestICS04UpgradeApp:testUpgradeAuthorizationChanneNotFound() (gas: 61690)

‎contracts/clients/09-localhost/LocalhostClient.sol

+1-1
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ contract LocalhostClient is ILightClient, ILightClientErrors {
9494
} else if (height.revision_height > block.number) {
9595
revert InvalidHeightRevisionHeight();
9696
}
97-
return uint64(block.timestamp);
97+
return uint64(block.timestamp) * 1e9;
9898
}
9999

100100
/**

‎contracts/core/04-channel/IBCChannelPacketSendRecv.sol

+2-2
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,8 @@ contract IBCChannelPacketSendRecv is
133133
{
134134
revert IBCChannelTimeoutPacketHeight(block.number, msg_.packet.timeoutHeight.revision_height);
135135
}
136-
if (msg_.packet.timeoutTimestamp != 0 && block.timestamp * 1e9 >= msg_.packet.timeoutTimestamp) {
137-
revert IBCChannelTimeoutPacketTimestamp(block.timestamp * 1e9, msg_.packet.timeoutTimestamp);
136+
if (msg_.packet.timeoutTimestamp != 0 && hostTimestamp() >= msg_.packet.timeoutTimestamp) {
137+
revert IBCChannelTimeoutPacketTimestamp(hostTimestamp(), msg_.packet.timeoutTimestamp);
138138
}
139139

140140
verifyPacketCommitment(

‎contracts/core/04-channel/IBCChannelUpgrade.sol

+2-2
Original file line numberDiff line numberDiff line change
@@ -549,7 +549,7 @@ contract IBCChannelUpgradeInitTryAck is IBCChannelUpgradeBase, IIBCChannelUpgrad
549549
Timeout.Data calldata timeout = msg_.counterpartyUpgrade.timeout;
550550
if (
551551
(timeout.height.revision_height != 0 && block.number >= timeout.height.revision_height)
552-
|| (timeout.timestamp != 0 && block.timestamp >= timeout.timestamp)
552+
|| (timeout.timestamp != 0 && hostTimestamp() >= timeout.timestamp)
553553
) {
554554
restoreChannel(msg_.portId, msg_.channelId, UpgradeHandshakeError.Timeout);
555555
return false;
@@ -620,7 +620,7 @@ contract IBCChannelUpgradeConfirmTimeoutCancel is IBCChannelUpgradeBase, IIBCCha
620620
Timeout.Data calldata timeout = msg_.counterpartyUpgrade.timeout;
621621
if (
622622
(timeout.height.revision_height != 0 && block.number >= timeout.height.revision_height)
623-
|| (timeout.timestamp != 0 && block.timestamp >= timeout.timestamp)
623+
|| (timeout.timestamp != 0 && hostTimestamp() >= timeout.timestamp)
624624
) {
625625
restoreChannel(msg_.portId, msg_.channelId, UpgradeHandshakeError.Timeout);
626626
return false;

‎contracts/core/24-host/IBCHost.sol

+7
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,13 @@ contract IBCHost is IIBCHostErrors, IBCStore {
1010
// In ibc-solidity, the prefix is not required, but for compatibility with ibc-go this must be a non-empty value.
1111
bytes internal constant DEFAULT_COMMITMENT_PREFIX = bytes("ibc");
1212

13+
/**
14+
* @dev hostTimestamp returns the current timestamp(Unix time in nanoseconds) of the host chain.
15+
*/
16+
function hostTimestamp() internal view virtual returns (uint64) {
17+
return uint64(block.timestamp) * 1e9;
18+
}
19+
1320
/**
1421
* @dev _getCommitmentPrefix returns the prefix of the commitment proof.
1522
*/

‎tests/foundry/src/ICS04Upgrade.t.sol

+73-21
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@
22
pragma solidity ^0.8.20;
33

44
import "./helpers/IBCTestHelper.t.sol";
5-
import {Vm} from "forge-std/Test.sol";
5+
import {Vm, console2} from "forge-std/Test.sol";
66
import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";
77
import {Upgrade, UpgradeFields, Timeout} from "../../../contracts/proto/Channel.sol";
88
import {LocalhostClientLib} from "../../../contracts/clients/09-localhost/LocalhostClient.sol";
99
import {LocalhostHelper} from "../../../contracts/clients/09-localhost/LocalhostHelper.sol";
1010
import {IIBCChannelRecvPacket, IIBCChannelAcknowledgePacket} from "../../../contracts/core/04-channel/IIBCChannel.sol";
11-
import {IIBCChannelUpgrade, IIBCChannelUpgradeBase} from "../../../contracts/core/04-channel/IIBCChannelUpgrade.sol";
11+
import {IIBCChannelUpgradeBase} from "../../../contracts/core/04-channel/IIBCChannelUpgrade.sol";
1212
import {TestIBCChannelUpgradableMockApp} from "./helpers/TestIBCChannelUpgradableMockApp.t.sol";
1313
import {ICS04UpgradeTestHelper} from "./helpers/ICS04UpgradeTestHelper.t.sol";
1414
import {ICS04PacketEventTestHelper} from "./helpers/ICS04PacketTestHelper.t.sol";
@@ -412,8 +412,8 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
412412

413413
Timeout.Data[3] memory timeouts = [
414414
Timeout.Data({height: H(block.number), timestamp: 0}),
415-
Timeout.Data({height: H(0), timestamp: uint64(block.timestamp)}),
416-
Timeout.Data({height: H(block.number), timestamp: uint64(block.timestamp)})
415+
Timeout.Data({height: H(0), timestamp: getTimestamp(0)}),
416+
Timeout.Data({height: H(block.number), timestamp: getTimestamp()})
417417
];
418418
HandshakeCallbacks memory callbacks = emptyCallbacks();
419419
callbacks.openInitAndFlushing.callback = _testUpgradeTimeoutAbortAck;
@@ -457,8 +457,8 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
457457

458458
Timeout.Data[3] memory timeouts = [
459459
Timeout.Data({height: H(block.number), timestamp: 0}),
460-
Timeout.Data({height: H(0), timestamp: uint64(block.timestamp)}),
461-
Timeout.Data({height: H(block.number), timestamp: uint64(block.timestamp)})
460+
Timeout.Data({height: H(0), timestamp: getTimestamp()}),
461+
Timeout.Data({height: H(block.number), timestamp: getTimestamp()})
462462
];
463463
HandshakeCallbacks memory callbacks = emptyCallbacks();
464464
callbacks.flushingAndFlushing.callback = _testUpgradeTimeoutAbortConfirm;
@@ -497,7 +497,7 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
497497
}
498498

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

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

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

524+
cases[i].callbacks.openInitAndFlushing.callback = _testUpgradeTimeoutUpgradeSuccess;
525+
cases[i].callbacks.openInitAndFlushing.reverse = true;
526+
cases[i].t0 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
527+
cases[i].t1 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
528+
i++;
529+
524530
cases[i].callbacks.flushingAndComplete.callback = _testUpgradeTimeoutUpgradeSuccess;
525531
cases[i].callbacks.flushingAndComplete.reverse = true;
526532
cases[i].t0 = Timeout.Data({height: H(block.number + 1), timestamp: 0});
527533
cases[i].t1 = Timeout.Data({height: H(block.number + 1), timestamp: 0});
528534
i++;
529535

536+
cases[i].callbacks.flushingAndComplete.callback = _testUpgradeTimeoutUpgradeSuccess;
537+
cases[i].callbacks.flushingAndComplete.reverse = true;
538+
cases[i].t0 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
539+
cases[i].t1 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
540+
i++;
541+
530542
// ------------------------------ Failure Cases ------------------------------ //
531543

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

537-
cases[i].callbacks.flushingAndFlushing.callback = _testUpgradeTimeoutUpgradeFailNotReached;
538-
cases[i].t0 = Timeout.Data({height: H(0), timestamp: uint64(block.timestamp + 1)});
539-
cases[i].t1 = Timeout.Data({height: H(0), timestamp: uint64(block.timestamp + 1)});
549+
cases[i].callbacks.flushingAndFlushing.callback = _testUpgradeTimeoutUpgradeFailTimeoutTimestampNotReached;
550+
cases[i].t0 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
551+
cases[i].t1 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
540552
i++;
541553

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

559+
cases[i].callbacks.flushingAndComplete.callback = _testUpgradeTimeoutUpgradeFailReached;
560+
cases[i].t0 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
561+
cases[i].t1 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
562+
i++;
563+
547564
cases[i].callbacks.openSucAndComplete.callback = _testUpgradeTimeoutUpgradeFailReachedAlreadyUpgraded;
548565
cases[i].callbacks.openSucAndComplete.reverse = true;
549566
cases[i].t0 = Timeout.Data({height: H(block.number + 1), timestamp: 0});
550567
cases[i].t1 = Timeout.Data({height: H(block.number + 1), timestamp: 0});
551568
i++;
552569

570+
cases[i].callbacks.openSucAndComplete.callback = _testUpgradeTimeoutUpgradeFailReachedAlreadyUpgraded;
571+
cases[i].callbacks.openSucAndComplete.reverse = true;
572+
cases[i].t0 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
573+
cases[i].t1 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
574+
i++;
575+
553576
cases[i].callbacks.completeAndComplete.callback = _testUpgradeTimeoutUpgradeFailReachedAlreadyCompleted;
554577
cases[i].t0 = Timeout.Data({height: H(block.number + 1), timestamp: 0});
555578
cases[i].t1 = Timeout.Data({height: H(block.number + 1), timestamp: 0});
556579
i++;
557580

581+
cases[i].callbacks.completeAndComplete.callback = _testUpgradeTimeoutUpgradeFailReachedAlreadyCompleted;
582+
cases[i].t0 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
583+
cases[i].t1 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
584+
i++;
585+
558586
cases[i].callbacks.completeAndComplete.callback = _testUpgradeTimeoutUpgradeFailReachedAlreadyCompleted;
559587
cases[i].callbacks.completeAndComplete.reverse = true;
560588
cases[i].t0 = Timeout.Data({height: H(block.number + 1), timestamp: 0});
561589
cases[i].t1 = Timeout.Data({height: H(block.number + 1), timestamp: 0});
562590
i++;
563591

592+
cases[i].callbacks.completeAndComplete.callback = _testUpgradeTimeoutUpgradeFailReachedAlreadyCompleted;
593+
cases[i].callbacks.completeAndComplete.reverse = true;
594+
cases[i].t0 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
595+
cases[i].t1 = Timeout.Data({height: H(0), timestamp: getTimestamp(1)});
596+
i++;
597+
564598
require(i == cases.length, "invalid number of cases");
565599

566600
for (uint256 i = 0; i < cases.length; i++) {
567-
(uint256 height, uint256 timestamp) = (block.number, block.timestamp);
601+
console2.log("case:", i);
602+
(uint256 height, uint256 timestampSec) = (block.number, block.timestamp);
568603
(ChannelInfo memory channel0, ChannelInfo memory channel1) =
569604
createMockAppLocalhostChannel(Channel.Order.ORDER_UNORDERED);
570605
handshakeUpgradeWithCallbacks(
@@ -589,7 +624,7 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
589624
);
590625
// restore the block height and timestamp
591626
vm.roll(height);
592-
vm.warp(timestamp);
627+
vm.warp(timestampSec);
593628
}
594629
}
595630

@@ -1822,7 +1857,7 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
18221857
vm.roll(uint256(timeout.height.revision_height));
18231858
}
18241859
if (timeout.timestamp != 0) {
1825-
vm.warp(uint256(timeout.timestamp));
1860+
vm.warp(uint256(timeout.timestamp / 1e9));
18261861
}
18271862
(Channel.Data memory counterpartyChannel,) = handler.getChannel(channel1.portId, channel1.channelId);
18281863
handler.timeoutChannelUpgrade(
@@ -1848,7 +1883,7 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
18481883
return false;
18491884
}
18501885

1851-
function _testUpgradeTimeoutUpgradeFailNotReached(
1886+
function _testUpgradeTimeoutUpgradeFailTimeoutHeightNotReached(
18521887
IIBCHandler handler,
18531888
ChannelInfo memory channel0,
18541889
ChannelInfo memory channel1
@@ -1861,8 +1896,25 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
18611896
proofChannel: LocalhostClientLib.sentinelProof(),
18621897
proofHeight: H(block.number)
18631898
});
1864-
// IBCChannelUpgradeTimeoutHeightNotReached or IBCChannelUpgradeTimeoutTimestampNotReached
1865-
vm.expectRevert();
1899+
vm.expectRevert(IIBCChannelUpgradeErrors.IBCChannelUpgradeTimeoutHeightNotReached.selector);
1900+
handler.timeoutChannelUpgrade(msg_);
1901+
return true;
1902+
}
1903+
1904+
function _testUpgradeTimeoutUpgradeFailTimeoutTimestampNotReached(
1905+
IIBCHandler handler,
1906+
ChannelInfo memory channel0,
1907+
ChannelInfo memory channel1
1908+
) internal returns (bool) {
1909+
(Channel.Data memory counterpartyChannel,) = handler.getChannel(channel1.portId, channel1.channelId);
1910+
IIBCChannelUpgradeBase.MsgTimeoutChannelUpgrade memory msg_ = IIBCChannelUpgradeBase.MsgTimeoutChannelUpgrade({
1911+
portId: channel0.portId,
1912+
channelId: channel0.channelId,
1913+
counterpartyChannel: counterpartyChannel,
1914+
proofChannel: LocalhostClientLib.sentinelProof(),
1915+
proofHeight: H(block.number)
1916+
});
1917+
vm.expectRevert(IIBCChannelUpgradeErrors.IBCChannelUpgradeTimeoutTimestampNotReached.selector);
18661918
handler.timeoutChannelUpgrade(msg_);
18671919
return true;
18681920
}
@@ -1877,7 +1929,7 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
18771929
vm.roll(uint256(timeout.height.revision_height));
18781930
}
18791931
if (timeout.timestamp != 0) {
1880-
vm.warp(uint256(timeout.timestamp));
1932+
vm.warp(uint256(timeout.timestamp / 1e9));
18811933
}
18821934
(Channel.Data memory counterpartyChannel,) = handler.getChannel(channel1.portId, channel1.channelId);
18831935
IIBCChannelUpgradeBase.MsgTimeoutChannelUpgrade memory msg_ = IIBCChannelUpgradeBase.MsgTimeoutChannelUpgrade({
@@ -1923,7 +1975,7 @@ contract TestICS04Upgrade is ICS04UpgradeTestHelper, ICS04PacketEventTestHelper
19231975
vm.roll(uint256(timeout.height.revision_height));
19241976
}
19251977
if (timeout.timestamp != 0) {
1926-
vm.warp(uint256(timeout.timestamp));
1978+
vm.warp(uint256(timeout.timestamp / 1e9));
19271979
}
19281980
(Channel.Data memory counterpartyChannel,) = handler.getChannel(channel1.portId, channel1.channelId);
19291981
IIBCChannelUpgradeBase.MsgTimeoutChannelUpgrade memory msg_ = IIBCChannelUpgradeBase.MsgTimeoutChannelUpgrade({

‎tests/foundry/src/helpers/IBCTestHelper.t.sol

+9
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,15 @@ abstract contract IBCTestHelper is Test {
4444
return H(0, revisionHeight);
4545
}
4646

47+
function getTimestamp() internal returns (uint64) {
48+
return getTimestamp(0);
49+
}
50+
51+
function getTimestamp(int256 offsetSecs) internal returns (uint64) {
52+
assertTrue(int256(block.timestamp) + offsetSecs >= 0, "getTimestamp: negative timestamp");
53+
return uint64(uint256((int256(block.timestamp) + offsetSecs) * 1e9));
54+
}
55+
4756
function roll(uint256 bn) internal {
4857
require(prevBlockNumber == 0, "roll: already rolled");
4958
prevBlockNumber = block.number;

0 commit comments

Comments
 (0)