Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Apr 17, 2025

Conversation

cctdaniel
Copy link
Contributor

@cctdaniel cctdaniel commented Apr 10, 2025

Summary

Added support for the WithdrawFee governance action that allows withdrawing collected fees from Pyth contracts to a specified address. The changes include:

  • New WithdrawFee action in governance payload system
  • Added serialization/deserialization support in xc_admin_common
  • Added Solidity implementation for fee withdrawal
  • Added test coverage for the new action

Rationale

This feature is necessary to:

  1. Enable fee collection from Pyth contracts
  2. Provide a secure, governance-controlled mechanism to withdraw accumulated fees
  3. Allow flexible fee distribution to specified addresses

How has this been tested?

  • Current tests cover my changes

    • Added WithdrawFee case to existing governance payload serialization tests
    • Added WithdrawFee to property-based testing via governanceActionArb
    • Existing governance message verification tests cover the new action
  • Added new tests

    • Added serialization/deserialization tests for WithdrawFee action
    • Added test coverage for hex string to Buffer conversion in governance payload tests
  • Manually tested the code

    • Verified correct encoding/decoding of withdrawal amounts and target addresses
    • Confirmed proper integration with existing governance message flow
    • Validated error handling for insufficient fees

The changes are well-tested through:

  1. Unit tests for TypeScript serialization
  2. Integration with existing governance payload system
  3. Property-based testing for round-trip serialization
  4. Solidity contract tests for fee withdrawal functionality

@cctdaniel cctdaniel requested a review from a team as a code owner April 10, 2025 06:42
Copy link

vercel bot commented Apr 10, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

6 Skipped Deployments
Name Status Preview Comments Updated (UTC)
api-reference ⬜️ Ignored (Inspect) Visit Preview Apr 17, 2025 2:12am
component-library ⬜️ Ignored (Inspect) Visit Preview Apr 17, 2025 2:12am
entropy-debugger ⬜️ Ignored (Inspect) Visit Preview Apr 17, 2025 2:12am
insights ⬜️ Ignored (Inspect) Visit Preview Apr 17, 2025 2:12am
proposals ⬜️ Ignored (Inspect) Visit Preview Apr 17, 2025 2:12am
staking ⬜️ Ignored (Inspect) Visit Preview Apr 17, 2025 2:12am

Copy link
Collaborator

@ali-bahjati ali-bahjati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very nice. I made a comment to change the schema to be consistent; will approve after that.

@cctdaniel cctdaniel merged commit 5ed1e16 into main Apr 17, 2025
11 checks passed
@cctdaniel cctdaniel deleted the withdraw-fees branch April 17, 2025 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants