feat(portfolio-contract): creatorFacet.withdrawFees #11991
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new withdrawFees method to the portfolio contract's creator facet, allowing the creator to withdraw accumulated fees from the contract account. This functionality is particularly useful when terminating or replacing the contract.
Key changes:
- Added
withdrawFeesmethod to the creator facet with optional amount parameter - Enhanced the contract interface to include proper method signature validation
- Added comprehensive test coverage for both partial and full fee withdrawal scenarios
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/portfolio-contract/src/portfolio.contract.ts | Implements the withdrawFees method on the creator facet with optional amount parameter and proper interface validation |
| packages/portfolio-contract/test/portfolio.contract.test.ts | Adds comprehensive test coverage for the new withdrawal functionality, testing both partial and complete fee withdrawal scenarios |
| // LCA operations are prompt | ||
| const { when } = vowTools; | ||
| const acct = await when(contractAccountV); | ||
| const amount = await (optAmount || when(acct.getBalance('ubld'))); |
There was a problem hiding this comment.
The parentheses are incorrectly placed. This will always await optAmount if it exists (which could be a synchronous value), or await the balance if not. It should be const amount = optAmount || await when(acct.getBalance('ubld')); to properly handle the optional parameter.
| const amount = await (optAmount || when(acct.getBalance('ubld'))); | |
| const amount = optAmount || await when(acct.getBalance('ubld')); |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
In this case, there's an await on the prior line. Does that allow the nested await?
There was a problem hiding this comment.
The await above means our lint rule is satisfied. But my impression is that in any case, it's best not to have code paths that are sometimes a turn boundary and sometimes not.
Deploying agoric-sdk with
|
| Latest commit: |
530289b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f71ca61b.agoric-sdk.pages.dev |
| Branch Preview URL: | https://dc-withdraw-fees.agoric-sdk.pages.dev |
dckc
added
automerge:rebase
dckc
force-pushed
the
dc-withdraw-fees
branch
from
18c7332 to
530289b
Compare
Description
creator can withdraw fees; for example when terminating / replacing the contract
Security / Documentation Considerations
straightforward usage of creatorFacet, compatible with
ymaxControlScaling Considerations
n/a
Testing Considerations
straightforward unit test using mock bridge
Upgrade Considerations
upgrade compatible