Merge pull request #2573 from pyth-network/withdraw-fees

feat(target_chains/ethereum): add WithdrawFee action and implement related functionality in governance payload

Author: cctdaniel

governance/xc_admin/packages/xc_admin_common/src/__tests__/GovernancePayload.test.ts

@@ -33,6 +33,7 @@ import {
   SetDataSources,
 } from "../governance_payload/SetDataSources";
 import { SetTransactionFee } from "../governance_payload/SetTransactionFee";
+import { WithdrawFee } from "../governance_payload/WithdrawFee";
 
 test("GovernancePayload ser/de", (done) => {
   jest.setTimeout(60000);
@@ -431,6 +432,21 @@ function governanceActionArb(): Arbitrary<PythGovernanceAction> {
         .map(({ v, e }) => {
           return new SetTransactionFee(header.targetChainId, v, e);
         });
+    } else if (header.action === "WithdrawFee") {
+      return fc
+        .record({
+          targetAddress: hexBytesArb({ minLength: 20, maxLength: 20 }),
+          value: fc.bigUintN(64),
+          expo: fc.bigUintN(64),
+        })
+        .map(({ targetAddress, value, expo }) => {
+          return new WithdrawFee(
+            header.targetChainId,
+            Buffer.from(targetAddress, "hex"),
+            value,
+            expo,
+          );
+        });
     } else {
       throw new Error("Unsupported action type");
     }

governance/xc_admin/packages/xc_admin_common/src/governance_payload/PythGovernanceAction.ts

@@ -17,6 +17,7 @@ export const TargetAction = {
   SetWormholeAddress: 6,
   SetFeeInToken: 7,
   SetTransactionFee: 8,
+  WithdrawFee: 9,
 } as const;
 
 export const EvmExecutorAction = {
@@ -49,6 +50,8 @@ export function toActionName(
         return "SetFeeInToken";
       case 8:
         return "SetTransactionFee";
+      case 9:
+        return "WithdrawFee";
     }
   } else if (
     deserialized.moduleId == MODULE_EVM_EXECUTOR &&

governance/xc_admin/packages/xc_admin_common/src/governance_payload/WithdrawFee.ts

@@ -0,0 +1,51 @@
+import {
+  PythGovernanceActionImpl,
+  PythGovernanceHeader,
+} from "./PythGovernanceAction";
+import * as BufferLayout from "@solana/buffer-layout";
+import * as BufferLayoutExt from "./BufferLayoutExt";
+import { ChainName } from "../chains";
+
+/** Withdraw fees from the target chain to the specified address */
+export class WithdrawFee extends PythGovernanceActionImpl {
+  static layout: BufferLayout.Structure<
+    Readonly<{ targetAddress: string; value: bigint; expo: bigint }>
+  > = BufferLayout.struct([
+    BufferLayoutExt.hexBytes(20, "targetAddress"), // Ethereum address as hex string
+    BufferLayoutExt.u64be("value"), // uint64 for value
+    BufferLayoutExt.u64be("expo"), // uint64 for exponent
+  ]);
+
+  constructor(
+    targetChainId: ChainName,
+    readonly targetAddress: Buffer,
+    readonly value: bigint,
+    readonly expo: bigint,
+  ) {
+    super(targetChainId, "WithdrawFee");
+  }
+
+  static decode(data: Buffer): WithdrawFee | undefined {
+    const decoded = PythGovernanceActionImpl.decodeWithPayload(
+      data,
+      "WithdrawFee",
+      WithdrawFee.layout,
+    );
+    if (!decoded) return undefined;
+
+    return new WithdrawFee(
+      decoded[0].targetChainId,
+      Buffer.from(decoded[1].targetAddress, "hex"),
+      decoded[1].value,
+      decoded[1].expo,
+    );
+  }
+
+  encode(): Buffer {
+    return super.encodeWithPayload(WithdrawFee.layout, {
+      targetAddress: this.targetAddress.toString("hex"),
+      value: this.value,
+      expo: this.expo,
+    });
+  }
+}

governance/xc_admin/packages/xc_admin_common/src/governance_payload/index.ts

@@ -21,6 +21,7 @@ import {
 } from "./SetWormholeAddress";
 import { EvmExecute } from "./ExecuteAction";
 import { SetTransactionFee } from "./SetTransactionFee";
+import { WithdrawFee } from "./WithdrawFee";
 
 /** Decode a governance payload */
 export function decodeGovernancePayload(
@@ -76,6 +77,8 @@ export function decodeGovernancePayload(
       return EvmExecute.decode(data);
     case "SetTransactionFee":
       return SetTransactionFee.decode(data);
+    case "WithdrawFee":
+      return WithdrawFee.decode(data);
     default:
       return undefined;
   }
@@ -92,3 +95,4 @@ export * from "./SetFee";
 export * from "./SetTransactionFee";
 export * from "./SetWormholeAddress";
 export * from "./ExecuteAction";
+export * from "./WithdrawFee";

target_chains/ethereum/contracts/contracts/pyth/Pyth.sol

@@ -555,7 +555,7 @@ abstract contract Pyth is
     }
 
     function version() public pure returns (string memory) {
-        return "1.4.4-alpha.4";
+        return "1.4.4-alpha.5";
     }
 
     function calculateTwap(

target_chains/ethereum/contracts/contracts/pyth/PythGovernance.sol

@@ -39,6 +39,7 @@ abstract contract PythGovernance is
         address newWormholeAddress
     );
     event TransactionFeeSet(uint oldFee, uint newFee);
+    event FeeWithdrawn(address targetAddress, uint fee);
 
     function verifyGovernanceVM(
         bytes memory encodedVM
@@ -100,6 +101,8 @@ abstract contract PythGovernance is
             );
         } else if (gi.action == GovernanceAction.SetTransactionFee) {
             setTransactionFee(parseSetTransactionFeePayload(gi.payload));
+        } else if (gi.action == GovernanceAction.WithdrawFee) {
+            withdrawFee(parseWithdrawFeePayload(gi.payload));
         } else {
             revert PythErrors.InvalidGovernanceMessage();
         }
@@ -255,4 +258,14 @@ abstract contract PythGovernance is
 
         emit TransactionFeeSet(oldFee, transactionFeeInWei());
     }
+
+    function withdrawFee(WithdrawFeePayload memory payload) internal {
+        if (payload.fee > address(this).balance)
+            revert PythErrors.InsufficientFee();
+
+        (bool success, ) = payload.targetAddress.call{value: payload.fee}("");
+        require(success, "Failed to withdraw fees");
+
+        emit FeeWithdrawn(payload.targetAddress, payload.fee);
+    }
 }

target_chains/ethereum/contracts/contracts/pyth/PythGovernanceInstructions.sol

@@ -35,7 +35,8 @@ contract PythGovernanceInstructions {
         SetValidPeriod, // 4
         RequestGovernanceDataSourceTransfer, // 5
         SetWormholeAddress, // 6
-        SetTransactionFee // 7
+        SetTransactionFee, // 7
+        WithdrawFee // 8
     }
 
     struct GovernanceInstruction {
@@ -82,6 +83,12 @@ contract PythGovernanceInstructions {
         uint newFee;
     }
 
+    struct WithdrawFeePayload {
+        address targetAddress;
+        // Fee in wei, matching the native uint256 type used for address.balance in EVM
+        uint256 fee;
+    }
+
     /// @dev Parse a GovernanceInstruction
     function parseGovernanceInstruction(
         bytes memory encodedInstruction
@@ -243,4 +250,25 @@ contract PythGovernanceInstructions {
         if (encodedPayload.length != index)
             revert PythErrors.InvalidGovernanceMessage();
     }
+
+    /// @dev Parse a WithdrawFeePayload (action 8) with minimal validation
+    function parseWithdrawFeePayload(
+        bytes memory encodedPayload
+    ) public pure returns (WithdrawFeePayload memory wf) {
+        uint index = 0;
+
+        wf.targetAddress = address(encodedPayload.toAddress(index));
+        index += 20;
+
+        uint64 val = encodedPayload.toUint64(index);
+        index += 8;
+
+        uint64 expo = encodedPayload.toUint64(index);
+        index += 8;
+
+        wf.fee = uint256(val) * uint256(10) ** uint256(expo);
+
+        if (encodedPayload.length != index)
+            revert PythErrors.InvalidGovernanceMessage();
+    }
 }

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

@@ -556,6 +556,100 @@ contract PythGovernanceTest is
         pyth.updatePriceFeeds{value: 1000}(updateData);
     }
 
+    function testWithdrawFee() public {
+        // First send some ETH to the contract
+        bytes[] memory updateData = new bytes[](0);
+        pyth.updatePriceFeeds{value: 1 ether}(updateData);
+        assertEq(address(pyth).balance, 1 ether);
+
+        address recipient = makeAddr("recipient");
+
+        // Create governance VAA to withdraw fee
+        bytes memory withdrawMessage = abi.encodePacked(
+            MAGIC,
+            uint8(GovernanceModule.Target),
+            uint8(GovernanceAction.WithdrawFee),
+            TARGET_CHAIN_ID,
+            recipient,
+            uint64(5), // value = 5
+            uint64(17) // exponent = 17 (5 * 10^17 = 0.5 ether)
+        );
+
+        bytes memory vaa = encodeAndSignMessage(
+            withdrawMessage,
+            TEST_GOVERNANCE_CHAIN_ID,
+            TEST_GOVERNANCE_EMITTER,
+            1
+        );
+
+        vm.expectEmit(true, true, true, true);
+        emit FeeWithdrawn(recipient, 0.5 ether);
+
+        PythGovernance(address(pyth)).executeGovernanceInstruction(vaa);
+
+        assertEq(address(pyth).balance, 0.5 ether);
+        assertEq(recipient.balance, 0.5 ether);
+    }
+
+    function testWithdrawFeeInsufficientBalance() public {
+        // First send some ETH to the contract
+        bytes[] memory updateData = new bytes[](0);
+        pyth.updatePriceFeeds{value: 1 ether}(updateData);
+        assertEq(address(pyth).balance, 1 ether);
+
+        address recipient = makeAddr("recipient");
+
+        // Create governance VAA to withdraw fee
+        bytes memory withdrawMessage = abi.encodePacked(
+            MAGIC,
+            uint8(GovernanceModule.Target),
+            uint8(GovernanceAction.WithdrawFee),
+            TARGET_CHAIN_ID,
+            recipient,
+            uint64(2), // value = 2
+            uint64(18) // exponent = 18 (2 * 10^18 = 2 ether, more than balance)
+        );
+
+        bytes memory vaa = encodeAndSignMessage(
+            withdrawMessage,
+            TEST_GOVERNANCE_CHAIN_ID,
+            TEST_GOVERNANCE_EMITTER,
+            1
+        );
+
+        vm.expectRevert(PythErrors.InsufficientFee.selector);
+        PythGovernance(address(pyth)).executeGovernanceInstruction(vaa);
+
+        // Balances should remain unchanged
+        assertEq(address(pyth).balance, 1 ether);
+        assertEq(recipient.balance, 0);
+    }
+
+    function testWithdrawFeeInvalidGovernance() public {
+        address recipient = makeAddr("recipient");
+
+        // Create governance VAA with wrong emitter
+        bytes memory withdrawMessage = abi.encodePacked(
+            MAGIC,
+            uint8(GovernanceModule.Target),
+            uint8(GovernanceAction.WithdrawFee),
+            TARGET_CHAIN_ID,
+            recipient,
+            uint64(5), // value = 5
+            uint64(17) // exponent = 17 (5 * 10^17 = 0.5 ether)
+        );
+
+        bytes memory vaa = encodeAndSignMessage(
+            withdrawMessage,
+            TEST_GOVERNANCE_CHAIN_ID,
+            bytes32(uint256(0x1111)), // Wrong emitter
+            1
+        );
+
+        vm.expectRevert(PythErrors.InvalidGovernanceDataSource.selector);
+        PythGovernance(address(pyth)).executeGovernanceInstruction(vaa);
+    }
+
     function encodeAndSignWormholeMessage(
         bytes memory data,
         uint16 emitterChainId,
@@ -611,4 +705,5 @@ contract PythGovernanceTest is
         address newWormholeAddress
     );
     event TransactionFeeSet(uint oldFee, uint newFee);
+    event FeeWithdrawn(address recipient, uint256 fee);
 }

target_chains/ethereum/contracts/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@pythnetwork/pyth-evm-contract",
-  "version": "1.4.4-alpha.2",
+  "version": "1.4.4-alpha.5",
   "description": "",
   "private": "true",
   "devDependencies": {