hyperledger-labs / yui-ibc-solidity

IBC in Solidity
Apache License 2.0
134 stars 62 forks source link

Question about proof of acknowledgement #233

Closed michwqy closed 1 year ago

michwqy commented 1 year ago

As said in this,

The storage location for each commitment is calculated as follows: assume that the slot of the commitments state variable is s and a ICS-23 commitment path is p, the storage location is keccak256(keccak256(p) . s).

Path of commitment and slot are used to calculate the storage location, which is the key in state trie of Ethereum. But according to the code,

function writeAcknowledgement(
        string calldata destinationPortId,
        string calldata destinationChannel,
        uint64 sequence,
        bytes calldata acknowledgement
    ) external {
        ...
        bytes32 ackCommitmentKey =
            IBCCommitment.packetAcknowledgementCommitmentKey(destinationPortId, destinationChannel, sequence);
        bytes32 ackCommitment = commitments[ackCommitmentKey];
        require(ackCommitment == bytes32(0), "acknowledgement for packet already exists");
        commitments[ackCommitmentKey] = keccak256(abi.encodePacked(sha256(acknowledgement)));
}       

The default value of the storage location is zero.

I want to know if the relayer can claim that the value on the storage path is zero before the acknowledgement is written and obtain corresponding proof (like the key of proof is the storage location and the value is zero)? If it is possible, something wrong will happen in function onAcknowledgementPacket in app/20-transfer. Because zero is not equal toICS20Lib.KECCAK256_SUCCESSFUL_ACKNOWLEDGEMENT_JSON.

    function onAcknowledgementPacket(Packet.Data calldata packet, bytes calldata acknowledgement, address)
            external
            virtual
            override
            onlyIBC
        {
            if (keccak256(acknowledgement) != ICS20Lib.KECCAK256_SUCCESSFUL_ACKNOWLEDGEMENT_JSON) {
                _refundTokens(ICS20Lib.unmarshalJSON(packet.data), packet.source_port, packet.source_channel);
            }
        }
bluele commented 1 year ago

@michwqy Thanks for your question.

According to ICS-04, the commitment for acknowledgment is hash(acknowledgement), which is a non-zero value. The implementation is here: https://github.com/hyperledger-labs/yui-ibc-solidity/blob/v0.3.17/contracts/core/04-channel/IBCPacket.sol#L221 Therefore, even if the relayer submits a non-existence proof of commitment, the attempt fails because the corresponding acknowledgment cannot be given.

michwqy commented 1 year ago

I'm not sure if I understand correctly. Assume the commit path like path = acks/p1/ch1/s1 , thus commitments[path] = 0 before the acknowledgement is written. Let the storage location is S, the relayer can get the proof where key is S, and value is 0. But the relayer can't construct the acknowledgement where sha256(acknowledgement) = 0 (maybe keccak256((sha256(acknowledgement))) = 0). Thus relayer can't submit a acknowledgement that can pass the proof. Is my understanding right?

bluele commented 1 year ago

@michwqy Thanks for the clarification. Yes, you are right.

michwqy commented 1 year ago

Thanks. I will close this issue.