Add 4626 unsupported fee note#6428
Add 4626 unsupported fee note#6428james-toussaint wants to merge 3 commits intoOpenZeppelin:masterfrom
Conversation
|
WalkthroughThis pull request updates the documentation in the ERC4626 vault implementation to clarify assumptions about asset transfers. The changes document that deposit, mint, redeem, and burn operations assume the full requested underlying amount is received during transfer. Additionally, the documentation specifies that fee-on-transfer tokens and other non-standard tokens that deliver less than the transferred amount are not supported by this implementation. No function signatures or code logic were modified. Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@contracts/token/ERC20/extensions/ERC4626.sol`:
- Around line 21-23: Update the comment in the ERC4626 contract that currently
states the transfer assumption applies to "{deposit}, {mint}, {redeem} and
{burn}": include "{withdraw}" in the list of user-facing workflows and remove or
clarify "burn" as an internal share operation (e.g., say "burn (internal)") so
the wording aligns with the public ERC-4626 API; ensure references use the exact
function names deposit, mint, redeem, withdraw and mention burn only as an
internal mechanic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 85310720-d072-466a-967a-b87b78a68524
📒 Files selected for processing (1)
contracts/token/ERC20/extensions/ERC4626.sol
Fixes M-10.
PR Checklist
npx changeset add)