Merge branch 'main' into alexander/on-recv

.gas-snapshot

@@ -1,8 +1,8 @@
-AckReentryTest:test_reentry_on_ack_message() (gas: 531516)
+AckReentryTest:test_reentry_on_ack_message() (gas: 531465)
 CallReentryTest:test_reentry_on_call_message() (gas: 560516)
 EscrowInformationTest:test_check_escrow_events() (gas: 100189)
 EscrowInformationTest:test_check_escrow_state() (gas: 97583)
-EscrowInformationTest:test_gas_refund(uint256) (runs: 256, μ: 148866, ~: 153744)
+EscrowInformationTest:test_gas_refund(uint256) (runs: 256, μ: 149291, ~: 153744)
 EscrowWrongGasPaymentTest:test_fail_not_enough_gas_sent() (gas: 94126)
 EscrowWrongGasPaymentTest:test_place_incentive() (gas: 90028)
 GasSpendControlTest:test_fail_relayer_has_to_provide_enough_gas() (gas: 605769)
@@ -39,4 +39,4 @@ SendMessagePaymentTest:test_send_message_with_additional_cost() (gas: 102845)
 TargetDeltaZeroTest:test_target_delta_zero(uint16) (runs: 256, μ: 257234, ~: 257234)
 TestProcessMessageDisabled:test_process_message_disabled(bytes,bytes,address) (runs: 256, μ: 10140, ~: 10117)
 TimeOverflowTest:test_larger_than_uint_time_is_fine() (gas: 253470)
-TimeOverflowTest:test_overflow_in_unchecked_is_fine() (gas: 255682)
+TimeOverflowTest:test_overflow_in_unchecked_is_fine() (gas: 255682)

.github/workflows/test.yml

@@ -1,6 +1,6 @@
 name: test
 
-on: workflow_dispatch
+on: [push, fork]
 
 env:
   FOUNDRY_PROFILE: ci

README.md

@@ -1,6 +1,6 @@
 Contracts within this repo have not been audited and should not be used in production.
 
-# Generalized Incentive Escrow
+# Generalised Incentive Escrow
 
 This repository contains an implementation of a generalized Incentive Scheme. 
 
@@ -122,7 +122,7 @@ The implementation is not perfect. Below the most notable implementation strange
 
 If a message reverts, ran out of gas, or otherwise failed to return an ack the implementation should do its best to not revert but instead send the original message back prepended with 0xff as the acknowledgment.
 
-For EVM this is currently limited by [Solidity #13869](https://github.com/ethereum/solidity/issues/13869). Calls to contracts which doesn't implement the proper endpoint will fail.
+For EVM this is currently limited by [Solidity #13869](https://github.com/ethereum/solidity/issues/13869), [Solidity #14467](https://github.com/ethereum/solidity/issues/14467). Calls to contracts which doesn't implement the proper endpoint will fail.
 - Relayers should emulate the call before calling the function to avoid wasting gas.
 - If contracts expect the call to execute (or rely on the ack), contracts need to make sure they are calling a contract that implements proper interfaces for receiving calls.
 
@@ -149,10 +149,10 @@ The destination-to-source relayer will result in the message reverting. They sho
 Because of the centralization associated with adding new chains / deployments, applications has to opt-in to these new chains. To understand the issue better, examine the following flow:
 
 1. An escrow with honest logic with no flaws exist on chain Alpha. 
-2. An application on chain Alpha can be drained by sending the fradulent key `0xabcdef` to the source chain. Ordinarly this never happens. This application trusts Alpha.
+2. An application on chain Alpha can be drained by sending the fraudulent key `0xabcdef` to the source chain. Ordinarily this never happens. This application trusts Alpha.
 3. The administrator adds another deployment on chain Beta with same address as Alpha but with another bytecode deployed. Specifically, when the administrator calls this contract it sends `0xabcdef` to the application.
 4. The application adds chain Beta to the allow list since the address matches the Beta address (thinking the byte code deployed must be the same).
-5. The fradulent deployment on Beta sends `0xabcedf` to the application on chain Alpha
+5. The fraudulent deployment on Beta sends `0xabcedf` to the application on chain Alpha
 6. On Alpha the message is verified.
 
 As a result, each application needs to tell the escrow where the other escrow sits and which escrow is allowed to send it messages. These mappings are 1:1, each chain identifier is only allowed a single escrow deployment.

src/IncentivizedMessageEscrow.sol

@@ -353,9 +353,17 @@ abstract contract IncentivizedMessageEscrow is IIncentivizedMessageEscrow, Bytes
         // Ensure that if the call reverts it doesn't boil up.
         // We don't need any return values and don't care if the call reverts.
         // This call implies we need reentry protection, since we need to call it before we delete the incentive map.
-        fromApplication.call{gas: maxGasAck}(
-            abi.encodeWithSignature("ackMessage(bytes32,bytes32,bytes)", destinationIdentifier, messageIdentifier, message[CTX1_MESSAGE_START: ])
-        );
+        bytes memory payload = abi.encodeWithSignature("ackMessage(bytes32,bytes32,bytes)", destinationIdentifier, messageIdentifier, message[CTX1_MESSAGE_START: ]);
+        assembly ("memory-safe") {
+            // Because Solidity always create RETURNDATACOPY for external calls, even low-level calls where no variables are assigned,
+            // the contract can be attacked by a so called return bomb. This incur additional cost to the relayer they aren't paid for.
+            // To protect the relayer, the call is made in inline assembly.
+            let success := call(maxGasAck, fromApplication, 0, add(payload, 0x20), mload(payload), 0, 0)
+            // This is what the call would look like non-assembly.
+            // fromApplication.call{gas: maxGasAck}(
+            //     abi.encodeWithSignature("ackMessage(bytes32,bytes32,bytes)", destinationIdentifier, messageIdentifier, message[CTX1_MESSAGE_START: ])
+            // );
+        }
 
         // Get the gas used by the destination call.
         uint256 gasSpentOnDestination = uint48(bytes6(message[CTX1_GAS_SPENT_START:CTX1_GAS_SPENT_END]));

src/apps/mock/IncentivizedMockEscrow.sol

@@ -12,11 +12,7 @@ contract IncentivizedMockEscrow is IncentivizedMessageEscrow, Ownable2Step {
     uint256 public costOfMessages;
     uint256 public accumulator = 1;
 
-    event Message(
-        bytes32 destinationIdentifier,
-        bytes recipitent,
-        bytes message
-    );
+    event Message(bytes32 destinationIdentifier, bytes recipitent, bytes message);
 
     constructor(bytes32 uniqueChainIndex, address signer, uint256 costOfMessages_) {
         UNIQUE_SOURCE_IDENTIFIER = uniqueChainIndex;

src/apps/wormhole/IncentivizedWormholeEscrow.sol

@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.13;
+
+import { IncentivizedMessageEscrow } from "../../IncentivizedMessageEscrow.sol";
+
+import { SmallStructs } from "./external/callworm/SmallStructs.sol";
+import { WormholeVerifier } from "./external/callworm/WormholeVerifier.sol";
+import { IWormhole } from "./interfaces/IWormhole.sol";
+
+// This is a mock contract which should only be used for testing.
+contract IncentivizedWormholeEscrow is IncentivizedMessageEscrow, WormholeVerifier {
+    error BadChainIdentifier();
+
+    event WormholeMessage(
+        bytes32 destinationIdentifier,
+        bytes recipitent
+    );
+
+    IWormhole public immutable WORMHOLE;
+
+    constructor(address wormhole_) WormholeVerifier(wormhole_) {
+        WORMHOLE = IWormhole(wormhole_);
+    }
+
+    function estimateAdditionalCost() external view returns(address asset, uint256 amount) {
+        asset =  address(0);
+        amount = WORMHOLE.messageFee();
+    }
+
+    function _getMessageIdentifier(
+        bytes32 destinationIdentifier,
+        bytes calldata message
+    ) internal override view returns(bytes32) {
+        return keccak256(
+            abi.encodePacked(
+                bytes32(block.number),
+                chainId(), 
+                destinationIdentifier,
+                message
+            )
+        );
+    }
+
+    function _verifyMessage(bytes calldata _metadata, bytes calldata _message) internal view override returns(bytes32 sourceIdentifier, bytes memory implementationIdentifier, bytes calldata message_) {
+
+        (
+            SmallStructs.SmallVM memory vm,
+            bytes calldata payload,
+            bool valid,
+            string memory reason
+        ) = parseAndVerifyVM(_message);
+
+        require(valid, reason);
+
+        // Load the identifier for the calling contract.
+        implementationIdentifier = abi.encodePacked(vm.emitterAddress);
+
+        // Local "supposedly" this chain identifier.
+        bytes32 thisChainIdentifier = bytes32(payload[0:32]);
+
+        // Check that the message is intended for this chain.
+        if (thisChainIdentifier != bytes32(uint256(chainId()))) revert BadChainIdentifier();
+
+        // Local the identifier for the source chain.
+        sourceIdentifier = bytes32(bytes2(vm.emitterChainId));
+
+        // Get the application message.
+        message_ = payload[32:];
+    }
+
+    function _sendMessage(bytes32 destinationChainIdentifier, bytes memory destinationImplementation, bytes memory message) internal override returns(uint128 costOfSendMessageInNativeToken) {
+        // Get the cost of sending wormhole messages.
+        costOfSendMessageInNativeToken = uint128(WORMHOLE.messageFee());
+
+        // Emit context for relayers so they know where to send the message
+        emit WormholeMessage(destinationChainIdentifier, destinationImplementation);
+
+        // Handoff the message to wormhole.
+        WORMHOLE.publishMessage{value: costOfSendMessageInNativeToken}(
+            0,
+            abi.encodePacked(
+                destinationChainIdentifier,
+                message
+            ),
+            0   // Finality = complete.
+        );
+    }
+}

src/apps/wormhole/external/callworm/GettersGetter.sol

@@ -0,0 +1,62 @@
+// contracts/Getters.sol
+// SPDX-License-Identifier: Apache 2
+
+pragma solidity ^0.8.0;
+
+import "../wormhole/Getters.sol";
+
+contract GettersGetter {
+    Getters immutable public WORMHOLE_STATE;
+
+    constructor(address wormholeState) {
+        WORMHOLE_STATE = Getters(wormholeState);
+    }
+
+    function getGuardianSet(uint32 index) public view returns (Structs.GuardianSet memory) {
+        return WORMHOLE_STATE.getGuardianSet(index);
+    }
+
+    function getCurrentGuardianSetIndex() public view returns (uint32) {
+        return WORMHOLE_STATE.getCurrentGuardianSetIndex();
+    }
+
+    function getGuardianSetExpiry() public view returns (uint32) {
+        return WORMHOLE_STATE.getGuardianSetExpiry();
+    }
+
+    function governanceActionIsConsumed(bytes32 hash) public view returns (bool) {
+        return WORMHOLE_STATE.governanceActionIsConsumed(hash);
+    }
+
+    function isInitialized(address impl) public view returns (bool) {
+        return WORMHOLE_STATE.isInitialized(impl);
+    }
+
+    function chainId() public view returns (uint16) {
+        return WORMHOLE_STATE.chainId();
+    }
+
+    function evmChainId() public view returns (uint256) {
+        return WORMHOLE_STATE.evmChainId();
+    }
+
+    function isFork() public view returns (bool) {
+        return WORMHOLE_STATE.isFork();
+    }
+
+    function governanceChainId() public view returns (uint16){
+        return WORMHOLE_STATE.governanceChainId();
+    }
+
+    function governanceContract() public view returns (bytes32){
+        return WORMHOLE_STATE.governanceContract();
+    }
+
+    function messageFee() public view returns (uint256) {
+        return WORMHOLE_STATE.messageFee();
+    }
+
+    function nextSequence(address emitter) public view returns (uint64) {
+        return WORMHOLE_STATE.nextSequence(emitter);
+    }
+}

src/apps/wormhole/external/callworm/README.md

@@ -0,0 +1 @@
+This is an alternative implementation of the wormhole message verification with the purpose of significantly reducing gas cost but also simplify integration by always keeping the message in calldata.

src/apps/wormhole/external/callworm/SmallStructs.sol

@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: Apache 2
+
+pragma solidity ^0.8.0;
+
+interface SmallStructs {
+
+	struct SmallVM {
+		// uint8 version;
+		// uint32 timestamp;
+		// uint32 nonce;
+		uint16 emitterChainId;
+		bytes32 emitterAddress;
+		// uint64 sequence;
+		// uint8 consistencyLevel;
+
+		uint32 guardianSetIndex;
+	}
+}