Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update close channel api to be future compatible #88

Merged
merged 2 commits into from
Apr 22, 2024
Merged

update close channel api to be future compatible #88

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
24eefb9
update close channel api to be future compatible
RnkSngh Apr 17, 2024
bf6114e
add upgrade logic ot uch
RnkSngh Apr 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 8 additions & 23 deletions contracts/core/Dispatcher.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,31 +247,16 @@ contract Dispatcher is OwnableUpgradeable, UUPSUpgradeable, IDispatcher {
}

/**
* @dev Emits a `CloseIbcChannel` event with the given `channelId` and the address of the message sender
* @notice Close the specified IBC channel by channel ID
* Must be called by the channel owner, ie. _portChannelMap[msg.sender][channelId] must exist
* @notice Initializes a close channel handshake process. It is directly called by the dapp which wants to close
* the channel
*/
function closeIbcChannel(bytes32 channelId) external {
Channel memory channel = _portChannelMap[msg.sender][channelId];
if (channel.counterpartyChannelId == bytes32(0)) {
revert IBCErrors.channelNotOwnedBySender();
}
function channelCloseInit(bytes32 channelId) external {}
RnkSngh marked this conversation as resolved.
Show resolved Hide resolved

(bool success, bytes memory data) = _callIfContract(
msg.sender,
abi.encodeWithSelector(
IbcChannelReceiver.onCloseIbcChannel.selector,
channelId,
channel.counterpartyPortId,
channel.counterpartyChannelId
)
);
if (success) {
emit CloseIbcChannel(msg.sender, channelId);
} else {
emit CloseIbcChannelError(address(msg.sender), data);
}
}
/**
* @notice Confirms a close channel handshake process. It is called by a relayer on behalf of the dapp whhich
* initializes the channel closefter after the IBC/VIBC hub chain has processed ChanCloseConfirm event.
*/
function channelCloseConfirm(address portAddress, bytes32 channelId, Ics23Proof calldata proof) external {}

/**
* This func is called by a 'relayer' after the IBC/VIBC hub chain has processed ChanCloseConfirm event.
Expand Down
27 changes: 6 additions & 21 deletions contracts/core/UniversalChannelHandler.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ import {
import {IbcReceiver} from "../interfaces/IbcReceiver.sol";
import {IbcReceiverBaseUpgradeable} from "../interfaces/IbcReceiverUpgradeable.sol";
import {ChannelOrder, ChannelEnd, IbcPacket, AckPacket, UniversalPacket, IbcUtils} from "../libs/Ibc.sol";
import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol";

contract UniversalChannelHandler is IbcReceiverBaseUpgradeable, IbcUniversalChannelMW {
contract UniversalChannelHandler is IbcReceiverBaseUpgradeable, UUPSUpgradeable, IbcUniversalChannelMW {
uint256[49] private __gap;

bytes32[] public connectedChannels;
Expand All @@ -34,28 +35,10 @@ contract UniversalChannelHandler is IbcReceiverBaseUpgradeable, IbcUniversalChan
function initialize(IbcDispatcher _dispatcher) public initializer {
__IbcReceiverBase_init(_dispatcher);
}
/**
* @dev Close a universal channel.
* Cannot send or receive packets after the channel is closed.
* @param channelId The channel id of the channel to be closed.
*/

function closeChannel(bytes32 channelId) external onlyOwner {
dispatcher.closeIbcChannel(channelId);
}
function onChanCloseInit(bytes32 channelId, string calldata, bytes32) external onlyIbcDispatcher {}

function onCloseIbcChannel(bytes32 channelId, string calldata, bytes32) external onlyIbcDispatcher {
// logic to determin if the channel should be closed
bool channelFound = false;
for (uint256 i = 0; i < connectedChannels.length; i++) {
if (connectedChannels[i] == channelId) {
delete connectedChannels[i];
channelFound = true;
break;
}
}
if (!channelFound) revert ChannelNotFound();
}
function onChanCloseConfirm(bytes32 channelId, string calldata, bytes32) external onlyIbcDispatcher {}
dshiell marked this conversation as resolved.
Show resolved Hide resolved

function openChannel(
string calldata version,
Expand Down Expand Up @@ -195,6 +178,8 @@ contract UniversalChannelHandler is IbcReceiverBaseUpgradeable, IbcUniversalChan
dispatcher = _dispatcher;
}

function _authorizeUpgrade(address newImplementation) internal override onlyOwner {}

function _connectChannel(bytes32 channelId, string calldata version)
internal
returns (string memory checkedVersion)
Expand Down
31 changes: 4 additions & 27 deletions contracts/examples/Mars.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,27 +49,6 @@ contract Mars is IbcReceiverBase, IbcReceiver {
timeoutPackets.push(packet);
}

function onCloseIbcChannel(bytes32 channelId, string calldata, bytes32) external virtual onlyIbcDispatcher {
// logic to determin if the channel should be closed
bool channelFound = false;
for (uint256 i = 0; i < connectedChannels.length; i++) {
dshiell marked this conversation as resolved.
Show resolved Hide resolved
if (connectedChannels[i] == channelId) {
delete connectedChannels[i];
channelFound = true;
break;
}
}
if (!channelFound) revert ChannelNotFound();
}

/**
* This func triggers channel closure from the dApp.
* Func args can be arbitary, as long as dispatcher.closeIbcChannel is invoked propperly.
*/
function triggerChannelClose(bytes32 channelId) external onlyOwner {
dispatcher.closeIbcChannel(channelId);
}

/**
* @dev Sends a packet with a greeting message over a specified channel.
* @param message The greeting message to be sent.
Expand Down Expand Up @@ -134,6 +113,10 @@ contract Mars is IbcReceiverBase, IbcReceiver {
}
revert UnsupportedVersion();
}

function onChanCloseInit(bytes32 channelId, string calldata, bytes32) external onlyIbcDispatcher {}

function onChanCloseConfirm(bytes32 channelId, string calldata, bytes32) external onlyIbcDispatcher {}
}

/*
Expand Down Expand Up @@ -170,12 +153,6 @@ contract RevertingStringMars is Mars {
require(false, "connect ibc channel is reverting");
}

// solhint-disable-next-line
function onCloseIbcChannel(bytes32, string calldata, bytes32) external view override onlyIbcDispatcher {
// solhint-disable-next-line
require(false, "close ibc channel is reverting");
}

// solhint-disable-next-line
function onAcknowledgementPacket(IbcPacket calldata, AckPacket calldata) external view override onlyIbcDispatcher {
// solhint-disable-next-line
Expand Down
3 changes: 2 additions & 1 deletion contracts/interfaces/IDispatcher.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ interface IDispatcher is IbcDispatcher, IbcEventsEmitter {
Ics23Proof calldata proof
) external;

function closeIbcChannel(bytes32 channelId) external;
function channelCloseConfirm(address portAddress, bytes32 channelId, Ics23Proof calldata proof) external;
function channelCloseInit(bytes32 channelId) external;

function sendPacket(bytes32 channelId, bytes calldata packet, uint64 timeoutTimestamp) external;

Expand Down
3 changes: 2 additions & 1 deletion contracts/interfaces/IbcDispatcher.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ interface IbcDispatcher is IbcPacketSender {
string calldata counterpartyPortId
) external;

function closeIbcChannel(bytes32 channelId) external;
function channelCloseConfirm(address portAddress, bytes32 channelId, Ics23Proof calldata proof) external;
function channelCloseInit(bytes32 channelId) external;

function portPrefix() external view returns (string memory portPrefix);
}
Expand Down
24 changes: 19 additions & 5 deletions contracts/interfaces/IbcReceiver.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,25 @@ interface IbcChannelReceiver {
external;

function onChanOpenConfirm(bytes32 channelId) external;
function onCloseIbcChannel(
bytes32 channelId,
string calldata counterpartyPortIdentifier,
bytes32 counterpartyChannelId
) external;
/**
* @notice Handles the channel close init event
* @param channelId The unique identifier of the channel
* @dev Make sure to validate channelId and counterpartyVersion
* @param counterpartyPortId The unique identifier of the counterparty's port
* @param counterpartyChannelId The unique identifier of the counterparty's channel
*/
function onChanCloseInit(bytes32 channelId, string calldata counterpartyPortId, bytes32 counterpartyChannelId)
external;

/**
* @notice Handles the channel close confirmation event
* @param channelId The unique identifier of the channel
* @dev Make sure to validate channelId and counterpartyVersion
* @param counterpartyPortId The unique identifier of the counterparty's port
* @param counterpartyChannelId The unique identifier of the counterparty's channel
*/
function onChanCloseConfirm(bytes32 channelId, string calldata counterpartyPortId, bytes32 counterpartyChannelId)
external;
}

/**
Expand Down
25 changes: 25 additions & 0 deletions test/universal.channel.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,31 @@ contract UniversalChannelPacketTest is Base, IbcMwEventsEmitter {
vm.stopPrank();
}

function test_uch_uch_upgrade__ok() public {
IUniversalChannelHandler uch = eth1.ucHandlerProxy();
UniversalChannelHandler newUCHImplementation = new UniversalChannelHandler();
vm.startPrank(address(eth1)); // Prank eth1 since that address is the owner
assertFalse(address(getProxyImplementation(address(uch), vm)) == address(newUCHImplementation));
UUPSUpgradeable(address(uch)).upgradeTo(address(address(newUCHImplementation)));

assertEq(
address(getProxyImplementation(address(uch), vm)),
address(newUCHImplementation),
"new uch implementation not set correctly in uch proxy"
);
vm.stopPrank();
}

function test_nonOwner_cannot_upgrade_uch() public {
IUniversalChannelHandler uch = eth1.ucHandlerProxy();
UniversalChannelHandler newUCHImplementation = new UniversalChannelHandler();
address notOwner = vm.addr(1);
vm.startPrank(notOwner);
vm.expectRevert("Ownable: caller is not the owner");
UUPSUpgradeable(address(uch)).upgradeTo(address(address(newUCHImplementation)));
vm.stopPrank();
}

/**
* Test packet flow from chain A to chain B via UniversalChannel MW and optionally other MW that sits on top of
* UniversalChannel MW.
Expand Down
92 changes: 32 additions & 60 deletions test/upgradeableProxy/upgrades/DispatcherV2.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,10 @@ contract DispatcherV2 is OwnableUpgradeable, UUPSUpgradeable, IDispatcher {
revert IBCErrors.invalidCounterPartyPortId();
}

// Have to encode here to avoid stack-too-deep error
bytes memory chanOpenInitArgs = abi.encode(ordering, connectionHops, counterpartyPortId, version);
(bool success, bytes memory data) =
_callIfContract(msg.sender, abi.encodeWithSelector(IbcChannelReceiver.onChanOpenInit.selector, version));
_callIfContract(msg.sender, bytes.concat(IbcChannelReceiver.onChanOpenInit.selector, chanOpenInitArgs));

if (success) {
emit ChannelOpenInit(
Expand Down Expand Up @@ -149,7 +151,16 @@ contract DispatcherV2 is OwnableUpgradeable, UUPSUpgradeable, IDispatcher {

address receiver = _getAddressFromPort(local.portId);
(bool success, bytes memory data) = _callIfContract(
receiver, abi.encodeWithSelector(IbcChannelReceiver.onChanOpenTry.selector, counterparty.version)
receiver,
abi.encodeWithSelector(
IbcChannelReceiver.onChanOpenTry.selector,
ordering,
connectionHops,
local.channelId,
counterparty.portId,
counterparty.channelId,
counterparty.version
)
);

if (success) {
Expand Down Expand Up @@ -188,7 +199,9 @@ contract DispatcherV2 is OwnableUpgradeable, UUPSUpgradeable, IDispatcher {
address receiver = _getAddressFromPort(local.portId);
(bool success, bytes memory data) = _callIfContract(
receiver,
abi.encodeWithSelector(IbcChannelReceiver.onChanOpenAck.selector, local.channelId, counterparty.version)
abi.encodeWithSelector(
IbcChannelReceiver.onChanOpenAck.selector, local.channelId, counterparty.channelId, counterparty.version
)
);

if (success) {
Expand Down Expand Up @@ -233,31 +246,16 @@ contract DispatcherV2 is OwnableUpgradeable, UUPSUpgradeable, IDispatcher {
}

/**
* @dev Emits a `CloseIbcChannel` event with the given `channelId` and the address of the message sender
* @notice Close the specified IBC channel by channel ID
* Must be called by the channel owner, ie. _portChannelMap[msg.sender][channelId] must exist
* @notice Initializes a close channel handshake process. It is directly called by the dapp which wants to close
* the channel
*/
function closeIbcChannel(bytes32 channelId) external {
Channel memory channel = _portChannelMap[msg.sender][channelId];
if (channel.counterpartyChannelId == bytes32(0)) {
revert IBCErrors.channelNotOwnedBySender();
}
function channelCloseInit(bytes32 channelId) external {}

(bool success, bytes memory data) = _callIfContract(
msg.sender,
abi.encodeWithSelector(
IbcChannelReceiver.onCloseIbcChannel.selector,
channelId,
channel.counterpartyPortId,
channel.counterpartyChannelId
)
);
if (success) {
emit CloseIbcChannel(msg.sender, channelId);
} else {
emit CloseIbcChannelError(address(msg.sender), data);
}
}
/**
* @notice Confirms a close channel handshake process. It is called by a relayer on behalf of the dapp whhich
* initializes the channel closefter after the IBC/VIBC hub chain has processed ChanCloseConfirm event.
*/
function channelCloseConfirm(address portAddress, bytes32 channelId, Ics23Proof calldata proof) external {}
RnkSngh marked this conversation as resolved.
Show resolved Hide resolved

/**
* This func is called by a 'relayer' after the IBC/VIBC hub chain has processed ChanCloseConfirm event.
Expand All @@ -281,7 +279,8 @@ contract DispatcherV2 is OwnableUpgradeable, UUPSUpgradeable, IDispatcher {
//
// // confirm with dApp by calling its callback
// IbcChannelReceiver reciever = IbcChannelReceiver(portAddress);
// reciever.onCloseIbcChannel(channelId, channel.counterpartyPortId, channel.counterpartyChannelId);
// reciever.onCloseIbcChannel(channelId, channel.counterpartyPortId,
// channel.counterpartyChannelId);
// delete _portChannelMap[portAddress][channelId];
// emit CloseIbcChannel(portAddress, channelId);
// }
Expand Down Expand Up @@ -453,23 +452,6 @@ contract DispatcherV2 is OwnableUpgradeable, UUPSUpgradeable, IDispatcher {
emit WriteAckPacket(receiver, packet.dest.channelId, packet.sequence, ack);
}

// TODO: add async writeAckPacket
// // this can be invoked sync or async by the IBC-dApp
// function writeAckPacket(IbcPacket calldata packet, AckPacket calldata ackPacket) external {
// // verify `receiver` is the original packet sender
// require(
// portIdAddressMatch(address(msg.sender), packet.src.portId),
// 'Receiver is not the original packet sender'
// );
// }

// TODO: remove below writeTimeoutPacket() function
// 1. core SC is responsible to generate timeout packet
// 2. user contract are not free to generate timeout with different criteria
// 3. [optional]: we may wish relayer to trigger timeout process, but in this case, belowunction won't do
// the job, as it doesn't have proofs.
// There is no strong reason to do this, as relayer can always do the regular `recvPacket` flow, which will
// do proper timeout generation.
/**
* Generate a timeout packet for the given packet
*/
Expand All @@ -481,7 +463,6 @@ contract DispatcherV2 is OwnableUpgradeable, UUPSUpgradeable, IDispatcher {
address receiver = _getAddressFromPort(packet.src.portId);
// verify packet does not have a receipt
bool hasReceipt = _recvPacketReceipt[receiver][packet.dest.channelId][packet.sequence];

if (hasReceipt) {
revert IBCErrors.packetReceiptAlreadyExists();
}
Expand Down Expand Up @@ -517,20 +498,6 @@ contract DispatcherV2 is OwnableUpgradeable, UUPSUpgradeable, IDispatcher {
return _lightClient.getState(height);
}

// verify an EVM address matches an IBC portId.
// IBC_PortID = portPrefix + address (hex string without 0x prefix, case-insensitive)
// function portIdAddressMatch(address addr, string calldata portId) public view returns (bool isMatch) {
// if (keccak256(abi.encodePacked(portPrefix)) != keccak256(abi.encodePacked(portId[0:portPrefixLen]))) {
// return false;
// }
// string memory portSuffix = portId[portPrefixLen:];
// isMatch = Ibc._hexStrToAddress(portSuffix) == addr;
// }

function _getAddressFromPort(string calldata port) internal view returns (address) {
return Ibc._hexStrToAddress(port[portPrefixLen:]);
}

// Prerequisite: must verify sender is authorized to send packet on the channel
function _sendPacket(address sender, bytes32 channelId, bytes memory packet, uint64 timeoutTimestamp) internal {
// current packet sequence
Expand Down Expand Up @@ -572,7 +539,8 @@ contract DispatcherV2 is OwnableUpgradeable, UUPSUpgradeable, IDispatcher {
_nextSequenceSend[address(portAddress)][local.channelId] = 1;
_nextSequenceRecv[address(portAddress)][local.channelId] = 1;
_nextSequenceAck[address(portAddress)][local.channelId] = 1;
_channelIdToConnection[local.channelId] = connectionHops[0]; // Set channel to connection mapping for finding
_channelIdToConnection[local.channelId] = connectionHops[0]; // Set channel to connection mapping for
// finding
}

// Returns the result of the call if no revert, otherwise returns the error if thrown.
Expand Down Expand Up @@ -600,4 +568,8 @@ contract DispatcherV2 is OwnableUpgradeable, UUPSUpgradeable, IDispatcher {
|| (packet.timeoutHeight.revision_height != 0 && block.number >= packet.timeoutHeight.revision_height)
);
}

function _getAddressFromPort(string calldata port) internal view returns (address addr) {
addr = Ibc._hexStrToAddress(port[portPrefixLen:]);
}
}
Loading