[executor] call contract with value (#1206)

* call contract with value

* address feedback

Author: Dev Kalra

target_chains/ethereum/contracts/contracts/executor/Executor.sol

@@ -26,6 +26,8 @@ contract Executor {
         // This argument is included to support multiple Executors on the same blockchain.
         address executorAddress;
         address callAddress;
+        // callAddress will be called with given value
+        uint value;
         bytes callData;
     }
 
@@ -87,7 +89,9 @@ contract Executor {
         if (len == 0) revert ExecutorErrors.InvalidContractTarget();
 
         bool success;
-        (success, response) = address(callAddress).call(gi.callData);
+        (success, response) = address(callAddress).call{value: gi.value}(
+            gi.callData
+        );
 
         // Check if the call was successful or not.
         if (!success) {
@@ -163,6 +167,9 @@ contract Executor {
         gi.callAddress = encodedInstruction.toAddress(index);
         index += 20;
 
+        gi.value = encodedInstruction.toUint256(index);
+        index += 32;
+
         // As solidity performs math operations in a checked mode
         // if the length of the encoded instruction be smaller than index
         // it will revert. So we don't need any extra check.

target_chains/ethereum/contracts/forge-test/Executor.t.sol

@@ -41,7 +41,8 @@ contract ExecutorTest is Test, WormholeTestUtils {
     function testExecute(
         address callAddress,
         bytes memory callData,
-        uint64 sequence
+        uint64 sequence,
+        uint value
     ) internal returns (bytes memory vaa) {
         bytes memory payload = abi.encodePacked(
             uint32(0x5054474d),
@@ -50,6 +51,7 @@ contract ExecutorTest is Test, WormholeTestUtils {
             CHAIN_ID,
             address(executor),
             callAddress,
+            value,
             callData
         );
 
@@ -66,7 +68,8 @@ contract ExecutorTest is Test, WormholeTestUtils {
     }
 
     function getTestUpgradeVaa(
-        address newImplementation
+        address newImplementation,
+        uint value
     ) internal returns (bytes memory vaa) {
         bytes memory payload = abi.encodePacked(
             uint32(0x5054474d),
@@ -75,6 +78,7 @@ contract ExecutorTest is Test, WormholeTestUtils {
             CHAIN_ID,
             address(executor),
             address(executor),
+            value,
             abi.encodeWithSelector(
                 ExecutorUpgradable.upgradeTo.selector,
                 newImplementation
@@ -101,19 +105,19 @@ contract ExecutorTest is Test, WormholeTestUtils {
     }
 
     function testUpgradeCallSucceedsForContractWithCorrectMagic() public {
-        bytes memory vaa = getTestUpgradeVaa(address(executor2));
+        bytes memory vaa = getTestUpgradeVaa(address(executor2), 0);
         executor.execute(vaa);
     }
 
     function testUpgradeCallFailsForNotUUPSContract() public {
-        bytes memory vaa = getTestUpgradeVaa(address(callable));
+        bytes memory vaa = getTestUpgradeVaa(address(callable), 0);
 
         vm.expectRevert("ERC1967Upgrade: new implementation is not UUPS");
         executor.execute(vaa);
     }
 
     function testUpgradeCallFailsForInvalidMagic() public {
-        bytes memory vaa = getTestUpgradeVaa(address(executorInvalidMagic));
+        bytes memory vaa = getTestUpgradeVaa(address(executorInvalidMagic), 0);
 
         vm.expectRevert(ExecutorErrors.InvalidMagicValue.selector);
         executor.execute(vaa);
@@ -127,14 +131,70 @@ contract ExecutorTest is Test, WormholeTestUtils {
         testExecute(
             address(callable),
             abi.encodeWithSelector(ICallable.foo.selector),
-            1
+            1,
+            0
         );
         assertEq(callable.fooCount(), c + 1);
         assertEq(callable.lastCaller(), address(executor));
         // Sanity check to make sure the check above is meaningful.
         assert(address(executor) != address(this));
     }
 
+    function testCallWithValueSucceeds() public {
+        callable.reset();
+
+        uint32 c = callable.fooCount();
+        assertEq(callable.lastCaller(), address(bytes20(0)));
+
+        uint value = 1;
+        vm.deal(address(executor), value);
+
+        testExecute(
+            address(callable),
+            abi.encodeWithSelector(ICallable.fooPayable.selector),
+            1,
+            value
+        );
+        assertEq(callable.fooCount(), c + 1);
+        assertEq(callable.lastCaller(), address(executor));
+        assertEq(address(executor).balance, 0);
+        assertEq(address(callable).balance, value);
+        // Sanity check to make sure the check above is meaningful.
+        assert(address(executor) != address(this));
+    }
+
+    function testCallWithValueInsufficientBalance() public {
+        callable.reset();
+
+        assertEq(callable.lastCaller(), address(bytes20(0)));
+
+        uint value = 5;
+        vm.deal(address(executor), 1);
+
+        bytes memory payload = abi.encodePacked(
+            uint32(0x5054474d),
+            PythGovernanceInstructions.GovernanceModule.EvmExecutor,
+            Executor.ExecutorAction.Execute,
+            CHAIN_ID,
+            address(executor),
+            address(callable),
+            value,
+            abi.encodeWithSelector(ICallable.fooPayable.selector)
+        );
+
+        bytes memory vaa = generateVaa(
+            uint32(block.timestamp),
+            OWNER_CHAIN_ID,
+            OWNER_EMITTER,
+            1,
+            payload,
+            NUM_SIGNERS
+        );
+
+        vm.expectRevert();
+        executor.execute(vaa);
+    }
+
     function testCallWithArgsSucceeds() public {
         callable.reset();
 
@@ -143,10 +203,34 @@ contract ExecutorTest is Test, WormholeTestUtils {
         testExecute(
             address(callable),
             abi.encodeWithSelector(ICallable.fooWithArgs.selector, 17),
-            1
+            1,
+            0
+        );
+        assertEq(callable.fooCount(), c + 17);
+        assertEq(callable.lastCaller(), address(executor));
+        // Sanity check to make sure the check above is meaningful.
+        assert(address(executor) != address(this));
+    }
+
+    function testCallWithArgsAndValueSucceeds() public {
+        callable.reset();
+
+        uint32 c = callable.fooCount();
+        assertEq(callable.lastCaller(), address(bytes20(0)));
+
+        uint value = 1;
+        vm.deal(address(executor), value);
+
+        testExecute(
+            address(callable),
+            abi.encodeWithSelector(ICallable.fooPayableWithArgs.selector, 17),
+            1,
+            value
         );
         assertEq(callable.fooCount(), c + 17);
         assertEq(callable.lastCaller(), address(executor));
+        assertEq(address(executor).balance, 0);
+        assertEq(address(callable).balance, value);
         // Sanity check to make sure the check above is meaningful.
         assert(address(executor) != address(this));
     }
@@ -156,7 +240,8 @@ contract ExecutorTest is Test, WormholeTestUtils {
         testExecute(
             address(callable),
             abi.encodeWithSelector(ICallable.foo.selector),
-            1
+            1,
+            0
         );
         assertEq(callable.fooCount(), c + 1);
     }
@@ -178,6 +263,7 @@ contract ExecutorTest is Test, WormholeTestUtils {
                 CHAIN_ID,
                 address(executor),
                 address(callable),
+                uint(0),
                 abi.encodeWithSelector(ICallable.foo.selector)
             );
 
@@ -205,6 +291,7 @@ contract ExecutorTest is Test, WormholeTestUtils {
             CHAIN_ID,
             address(executor),
             address(callable),
+            uint(0),
             abi.encodeWithSelector(ICallable.foo.selector)
         );
 
@@ -229,6 +316,7 @@ contract ExecutorTest is Test, WormholeTestUtils {
             CHAIN_ID,
             address(executor),
             address(callable),
+            uint(0),
             abi.encodeWithSelector(ICallable.foo.selector)
         );
 
@@ -249,7 +337,8 @@ contract ExecutorTest is Test, WormholeTestUtils {
         testExecute(
             address(callable),
             abi.encodeWithSelector(ICallable.foo.selector),
-            3
+            3,
+            0
         );
 
         bytes memory payload = abi.encodePacked(
@@ -259,6 +348,7 @@ contract ExecutorTest is Test, WormholeTestUtils {
             CHAIN_ID,
             address(executor),
             address(callable),
+            uint(0),
             abi.encodeWithSelector(ICallable.foo.selector)
         );
 
@@ -278,7 +368,8 @@ contract ExecutorTest is Test, WormholeTestUtils {
         testExecute(
             address(callable),
             abi.encodeWithSelector(ICallable.foo.selector),
-            4
+            4,
+            0
         );
         assertEq(callable.fooCount(), 1);
     }
@@ -291,6 +382,7 @@ contract ExecutorTest is Test, WormholeTestUtils {
             CHAIN_ID,
             address(executor),
             address(callable),
+            uint(0),
             abi.encodeWithSelector(ICallable.foo.selector)
         );
 
@@ -320,6 +412,7 @@ contract ExecutorTest is Test, WormholeTestUtils {
             uint16(3),
             address(executor),
             address(callable),
+            uint(0),
             abi.encodeWithSelector(ICallable.foo.selector)
         );
 
@@ -344,6 +437,7 @@ contract ExecutorTest is Test, WormholeTestUtils {
             CHAIN_ID,
             address(0x1),
             address(callable),
+            uint(0),
             abi.encodeWithSelector(ICallable.foo.selector)
         );
 
@@ -368,6 +462,7 @@ contract ExecutorTest is Test, WormholeTestUtils {
             CHAIN_ID,
             address(executor),
             address(callable),
+            uint(0),
             abi.encodeWithSelector(ICallable.foo.selector)
         );
 
@@ -392,6 +487,7 @@ contract ExecutorTest is Test, WormholeTestUtils {
             CHAIN_ID,
             address(executor),
             address(callable),
+            uint(0),
             abi.encodeWithSelector(ICallable.reverts.selector)
         );
 
@@ -416,6 +512,7 @@ contract ExecutorTest is Test, WormholeTestUtils {
             CHAIN_ID,
             address(executor),
             address(100),
+            uint(0),
             abi.encodeWithSelector(ICallable.foo.selector)
         );
 
@@ -436,8 +533,12 @@ contract ExecutorTest is Test, WormholeTestUtils {
 interface ICallable {
     function foo() external;
 
+    function fooPayable() external payable;
+
     function fooWithArgs(uint32 inc) external;
 
+    function fooPayableWithArgs(uint32 inc) external payable;
+
     function reverts() external;
 
     function reset() external;
@@ -459,11 +560,21 @@ contract TestCallable is ICallable {
         lastCaller = msg.sender;
     }
 
+    function fooPayable() external payable override {
+        fooCount += 1;
+        lastCaller = msg.sender;
+    }
+
     function fooWithArgs(uint32 inc) external override {
         fooCount += inc;
         lastCaller = msg.sender;
     }
 
+    function fooPayableWithArgs(uint32 inc) external payable override {
+        fooCount += inc;
+        lastCaller = msg.sender;
+    }
+
     function reverts() external override {
         revert("call should revert");
     }