Skip to content

Commit

Permalink
clean up errors + add close channel checks in test cases
Browse files Browse the repository at this point in the history
  • Loading branch information
@RnkSngh
RnkSngh committed Mar 29, 2024
1 parent 643ecfd commit edcf78e
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 12 deletions.
17 changes: 15 additions & 2 deletions test/Dispatcher.closeChannel.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ contract DispatcherCloseChannelTest is PacketSenderTestBase {

function test_closeChannelInit_mustOwner() public {
Mars earth = new Mars(dispatcherProxy);
vm.expectRevert(abi.encodeWithSignature(IBCErrors.channelNotOwnedBySender()));
vm.expectRevert(abi.encodeWithSelector(IBCErrors.channelNotOwnedBySender.selector));
earth.triggerChannelClose(channelId);
assertNotEq0(abi.encode(dispatcherProxy.getChannel(address(mars), channelId)), abi.encode(defaultChannel));
}
Expand All @@ -49,7 +49,7 @@ contract DispatcherCloseChannelTest is PacketSenderTestBase {
}

function test_closeChannelConfirm_mustOwner() public {
vm.expectRevert(abi.encodeWithSignature(IBCErrors.channelNotOwnedByPortAddress()));
vm.expectRevert(abi.encodeWithSelector(IBCErrors.channelNotOwnedByPortAddress.selector));
dispatcherProxy.channelCloseConfirm(address(mars), "channel-999", validProof);
assertNotEq0(abi.encode(dispatcherProxy.getChannel(address(mars), channelId)), abi.encode(defaultChannel));
}
Expand Down Expand Up @@ -78,6 +78,8 @@ contract DispatcherCloseChannelTest is PacketSenderTestBase {
}

contract DappRevertTestsCloseChannel is DappRevertTests {
Channel defaultChannel; // Uninitialized struct to compare that structs are deleted

RevertingStringCloseChannelMars revertingStringCloseMars;
string portId = "eth1.7E5F4552091A69125d5DfCb7b8C2659029395Bdf";
LocalEnd _local;
Expand All @@ -104,6 +106,12 @@ contract DappRevertTestsCloseChannel is DappRevertTests {
abi.encodeWithSignature("Error(string)", "close ibc channel is reverting")
);
revertingStringCloseMars.triggerChannelClose(ch0.channelId);

// Channels should still be deleted on dapp revert
assertEq(
abi.encode(defaultChannel),
abi.encode(dispatcherProxy.getChannel(address(revertingStringCloseMars), ch0.channelId))
);
}

function test_ibc_channel_close_confirm_dapp_revert() public {
Expand All @@ -114,5 +122,10 @@ contract DappRevertTestsCloseChannel is DappRevertTests {
);

dispatcherProxy.channelCloseConfirm(address(revertingStringCloseMars), ch0.channelId, validProof);
// Channels should still be deleted on dapp revert
assertEq(
abi.encode(defaultChannel),
abi.encode(dispatcherProxy.getChannel(address(revertingStringCloseMars), ch0.channelId))
);
}
}
20 changes: 10 additions & 10 deletions test/Dispatcher.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ contract DispatcherSendPacketTestSuite is ChannelOpenTestBaseSetup {
// sendPacket fails if calling dApp doesn't own the channel
function test_mustOwner() public {
Mars earth = new Mars(dispatcherProxy);
vm.expectRevert(abi.encodeWithSignature(IBCErrors.channelNotOwnedBySender()));
vm.expectRevert(abi.encodeWithSelector(IBCErrors.channelNotOwnedBySender.selector));
earth.greet(payload, channelId, timeoutTimestamp);
}
}
Expand Down Expand Up @@ -343,7 +343,7 @@ contract DispatcherRecvPacketTestSuite is ChannelOpenTestBaseSetup {
dispatcherProxy.recvPacket(
IbcReceiver(mars), IbcPacket(src, dest, 1, payload, ZERO_HEIGHT, maxTimeout), validProof
);
vm.expectRevert(abi.encodeWithSignature(IBCErrors.unexpectedPacketSequence()));
vm.expectRevert(abi.encodeWithSelector(IBCErrors.unexpectedPacketSequence.selector));
dispatcherProxy.recvPacket(
IbcReceiver(mars), IbcPacket(src, dest, 3, payload, ZERO_HEIGHT, maxTimeout), validProof
);
Expand Down Expand Up @@ -371,14 +371,14 @@ contract DispatcherAckPacketTestSuite is PacketSenderTestBase {

// cannot ack packets if packet commitment is missing
function test_missingPacket() public {
vm.expectRevert(abi.encodeWithSignature(IBCErrors.packetCommitmentNotFound()));
vm.expectRevert(abi.encodeWithSelector(IBCErrors.packetCommitmentNotFound.selector));
dispatcherProxy.acknowledgement(IbcReceiver(mars), genPacket(1), genAckPacket("1"), validProof);

sendPacket();
dispatcherProxy.acknowledgement(IbcReceiver(mars), sentPacket, ackPacket, validProof);

// packet commitment is removed after ack
vm.expectRevert(abi.encodeWithSignature(IBCErrors.packetCommitmentNotFound()));
vm.expectRevert(abi.encodeWithSelector(IBCErrors.packetCommitmentNotFound.selector));
dispatcherProxy.acknowledgement(IbcReceiver(mars), sentPacket, ackPacket, validProof);
}

Expand All @@ -391,7 +391,7 @@ contract DispatcherAckPacketTestSuite is PacketSenderTestBase {
dispatcherProxy.acknowledgement(IbcReceiver(mars), genPacket(1), genAckPacket("1"), validProof);

// only 2nd ack is allowed; so the 3rd ack fails
vm.expectRevert(abi.encodeWithSignature(IBCErrors.unexpectedPacketSequence()));
vm.expectRevert(abi.encodeWithSelector(IBCErrors.unexpectedPacketSequence.selector));

dispatcherProxy.acknowledgement(IbcReceiver(mars), genPacket(3), genAckPacket("3"), validProof);
}
Expand All @@ -407,7 +407,7 @@ contract DispatcherAckPacketTestSuite is PacketSenderTestBase {
IbcPacket memory packetEarth = sentPacket;
packetEarth.src = earthEnd;

vm.expectRevert(abi.encodeWithSignature(IBCErrors.receiverNotOriginPacketSender()));
vm.expectRevert(abi.encodeWithSelector(IBCErrors.receiverNotOriginPacketSender.selector));
dispatcherProxy.acknowledgement(IbcReceiver(mars), packetEarth, ackPacket, validProof);
}

Expand All @@ -419,7 +419,7 @@ contract DispatcherAckPacketTestSuite is PacketSenderTestBase {
IbcPacket memory packet = sentPacket;
packet.src = invalidSrc;

vm.expectRevert(abi.encodeWithSignature(IBCErrors.packetCommitmentNotFound()));
vm.expectRevert(abi.encodeWithSelector(IBCErrors.packetCommitmentNotFound.selector));
dispatcherProxy.acknowledgement(IbcReceiver(mars), packet, ackPacket, validProof);
}
}
Expand Down Expand Up @@ -452,14 +452,14 @@ contract DispatcherTimeoutPacketTestSuite is PacketSenderTestBase {

// cannot timeout packets if packet commitment is missing
function test_missingPacket() public {
vm.expectRevert(abi.encodeWithSignature(IBCErrors.packetCommitmentNotFound()));
vm.expectRevert(abi.encodeWithSelector(IBCErrors.packetCommitmentNotFound.selector));
dispatcherProxy.timeout(IbcReceiver(mars), genPacket(1), validProof);

sendPacket();
dispatcherProxy.timeout(IbcReceiver(mars), sentPacket, validProof);

// packet commitment is removed after timeout
vm.expectRevert(abi.encodeWithSignature(IBCErrors.packetCommitmentNotFound()));
vm.expectRevert(abi.encodeWithSelector(IBCErrors.packetCommitmentNotFound.selector));
dispatcherProxy.timeout(IbcReceiver(mars), sentPacket, validProof);
}

Expand Down Expand Up @@ -487,7 +487,7 @@ contract DispatcherTimeoutPacketTestSuite is PacketSenderTestBase {
IbcPacket memory packet = sentPacket;
packet.src = invalidSrc;

vm.expectRevert(abi.encodeWithSignature(IBCErrors.packetCommitmentNotFound()));
vm.expectRevert(abi.encodeWithSelector(IBCErrors.packetCommitmentNotFound.selector));
dispatcherProxy.timeout(IbcReceiver(mars), packet, validProof);
}

Expand Down

0 comments on commit edcf78e

Please sign in to comment.