diff --git a/governance/xc_admin/packages/xc_admin_common/src/__tests__/GovernancePayload.test.ts b/governance/xc_admin/packages/xc_admin_common/src/__tests__/GovernancePayload.test.ts index 38eaa9c971..245b9b487d 100644 --- a/governance/xc_admin/packages/xc_admin_common/src/__tests__/GovernancePayload.test.ts +++ b/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 { .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"); } diff --git a/governance/xc_admin/packages/xc_admin_common/src/governance_payload/PythGovernanceAction.ts b/governance/xc_admin/packages/xc_admin_common/src/governance_payload/PythGovernanceAction.ts index 85d1dc4de2..f461791204 100644 --- a/governance/xc_admin/packages/xc_admin_common/src/governance_payload/PythGovernanceAction.ts +++ b/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 && diff --git a/governance/xc_admin/packages/xc_admin_common/src/governance_payload/WithdrawFee.ts b/governance/xc_admin/packages/xc_admin_common/src/governance_payload/WithdrawFee.ts new file mode 100644 index 0000000000..9523285062 --- /dev/null +++ b/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, + }); + } +} diff --git a/governance/xc_admin/packages/xc_admin_common/src/governance_payload/index.ts b/governance/xc_admin/packages/xc_admin_common/src/governance_payload/index.ts index cb423946a4..2919ec93e1 100644 --- a/governance/xc_admin/packages/xc_admin_common/src/governance_payload/index.ts +++ b/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"; diff --git a/target_chains/ethereum/contracts/contracts/pyth/Pyth.sol b/target_chains/ethereum/contracts/contracts/pyth/Pyth.sol index 01296bcf6c..516b19237c 100644 --- a/target_chains/ethereum/contracts/contracts/pyth/Pyth.sol +++ b/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( diff --git a/target_chains/ethereum/contracts/contracts/pyth/PythGovernance.sol b/target_chains/ethereum/contracts/contracts/pyth/PythGovernance.sol index 5bb1bd3265..bc3b0ce19c 100644 --- a/target_chains/ethereum/contracts/contracts/pyth/PythGovernance.sol +++ b/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); + } } diff --git a/target_chains/ethereum/contracts/contracts/pyth/PythGovernanceInstructions.sol b/target_chains/ethereum/contracts/contracts/pyth/PythGovernanceInstructions.sol index b4a5f3db4f..da1d2178bf 100644 --- a/target_chains/ethereum/contracts/contracts/pyth/PythGovernanceInstructions.sol +++ b/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(); + } } diff --git a/target_chains/ethereum/contracts/forge-test/PythGovernance.t.sol b/target_chains/ethereum/contracts/forge-test/PythGovernance.t.sol index fa83593232..148f507088 100644 --- a/target_chains/ethereum/contracts/forge-test/PythGovernance.t.sol +++ b/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); } diff --git a/target_chains/ethereum/contracts/package.json b/target_chains/ethereum/contracts/package.json index 814972bb7e..0a354e174d 100644 --- a/target_chains/ethereum/contracts/package.json +++ b/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": {