Repository navigation
fix(maintenance-pool): deposit credits the received delta, not the requested amount - #606
Open
yggdrasilworld wants to merge 2 commits into
Open
yggdrasilworld wants to merge 2 commits into
yggdrasilworld wants to merge 2 commits into
Conversation
…quested amount maintenance-pool::deposit transferred then credited to pool.balance / total_deposited / the Deposit record. With a fee-on-transfer or rebasing token, the amount that arrives is less than , so the pool's books exceeded its holdings — later withdrawals or reclaims fail with insufficient balance or eat into other pools. Use mergefi_common::measure_transfer_delta as escrow::fund and milestones::create_milestone already do: measure the contract's actual balance change across the transfer, reject actual_received <= 0, and book the received amount. Resolves MergeFi#532 Note: the fee-on-transfer regression test requires a fee-taking mock token, which isn't present in the workspace — flagging that the test gap remains; the fix mirrors the established escrow/milestones pattern.
|
Someone is attempting to deploy a commit to the chonilius' projects Team on Vercel. A member of the Team first needs to authorize it. |
The AC demands the deposit-delta fix be exercised with a fee-on-transfer mock — adds MockFeeToken (10% burns in transit) and asserts the pool credits 90 of a requested 100 deposit.
Author
|
Scope audit addendum: added the AC's demanded fee-on-transfer test — MockFeeToken (10% burns in transit), deposit of 100 credits the received 90. cargo test 43/43. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes
maintenance-pool::depositto credit the received amount (viameasure_transfer_delta), not the requested amount — the fee-on-transfer / rebasing-token bug from #532.The bug
depositcalledtoken_client.transfer(..., &amount)then creditedamounttopool.balance,total_deposited, and theDepositrecord. For a token that takes a transfer fee, the contract actually receivesamount - fee— so the pool's books exceeded its holdings, and later withdrawals/reclaims would fail or eat into other pools.The fix
Mirrors the established pattern from
escrow::fund/contributeandmilestones::create_milestone/contribute(#3):measure_transfer_deltameasures the contract's actual balance change across the transferactual_received <= 0→Error::InvalidAmountpool.balance,total_deposited, and theDepositrecord all creditactual_received, notamountHonest caveat — flagged
rustc 1.95+wasm32v1-none, which isn't installable in this environment; the fix is a faithful mirror of the provenescrow::fundusage but I could not build it. Please let CI verify.common,escrow,milestones,maintenance-poolonly). If a mock exists elsewhere or one should be added, point me at it and I'll add the regression test.Resolves #532
Payment: Base USDC
0xD40Df5c27cEa7dd78A3c345401995c319589334e