Skip to content

Conversation

@cctdaniel
Copy link
Contributor

@cctdaniel cctdaniel commented Apr 15, 2025

Summary

Added a withdrawFee function to EntropyGovernance.sol that allows admin or owner to withdraw accumulated Pyth fees to a specified address. This includes:

  • New withdrawFee function with authorization checks
  • New FeeWithdrawn event
  • Comprehensive test coverage in EntropyAuthorized.t.sol

Rationale

  • Enables fee collection from the Entropy contract, similar to Pyth's implementation
  • Provides a secure way to withdraw accumulated fees with proper authorization checks
  • Maintains accounting of fees through accruedPythFeesInWei state variable
  • Follows similar patterns to existing governance functions

How has this been tested?

  • Current tests cover my changes
  • Added new tests
  • Manually tested the code

Added 5 new test cases in EntropyAuthorized.t.sol:

  1. testWithdrawFeeByAdmin - Verifies admin can withdraw fees
  2. testWithdrawFeeByOwner - Verifies owner can withdraw fees
  3. testWithdrawFeeByUnauthorized - Verifies unauthorized users cannot withdraw
  4. testWithdrawFeeInsufficientBalance - Verifies withdrawal amount cannot exceed balance
  5. testWithdrawFeeToZeroAddress - Verifies cannot withdraw to zero address

Tests verify:

  • Authorization checks work correctly
  • Fee accounting is accurate
  • Events are emitted properly
  • Edge cases are handled (zero address, insufficient balance)
  • Integration with existing provider registration and fee collection

Each test follows the pattern:

  1. Register a provider
  2. Set Pyth fee
  3. Make requests to accrue fees
  4. Verify fee accrual
  5. Test withdrawal scenarios

@vercel
Copy link

vercel bot commented Apr 15, 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 1:11am
component-library ⬜️ Ignored (Inspect) Visit Preview Apr 17, 2025 1:11am
entropy-debugger ⬜️ Ignored (Inspect) Visit Preview Apr 17, 2025 1:11am
insights ⬜️ Ignored (Inspect) Visit Preview Apr 17, 2025 1:11am
proposals ⬜️ Ignored (Inspect) Visit Preview Apr 17, 2025 1:11am
staking ⬜️ Ignored (Inspect) Visit Preview Apr 17, 2025 1:11am


function testWithdrawFeeByAdmin() public {
// Register provider1 first
bytes32[] memory hashChain = generateHashChain(provider1, 0, 100);
Copy link
Contributor

Choose a reason for hiding this comment

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

there's a lot of code duplication in these tests. I suggest adding a helper method that sets up the contract with fees so the test simply has to do the withdraw call.

@cctdaniel cctdaniel merged commit d359f67 into main Apr 17, 2025
10 checks passed
@cctdaniel cctdaniel deleted the entropy-withdraw-fee branch April 17, 2025 01:16
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.

4 participants