test(midas): audit harness, PoCs, invariants, and verified findings report - #197
Open
0xmikko wants to merge 1 commit into
Open
test(midas): audit harness, PoCs, invariants, and verified findings report#1970xmikko wants to merge 1 commit into
0xmikko wants to merge 1 commit into
Conversation
…eport Adds a standalone Midas audit suite under contracts/test/audit/midas/ plus a verified findings report and mathematical proofs under specs/adapters/midas/. All Midas-side claims are verified against midas-apps/contracts at revision 90a5f246. New test files (33 tests, all passing on top of dc1d8b5): - MidasAuditTestBase.sol: shared mocks (configurable data feed, issuance and redemption vault, credit manager/facade/access control, AddressProvider) - MidasDecimalMath.fuzz.t.sol (7 fuzz): precision, monotonicity, overflow for _convertToE18 and _calculateTokenOutAmount - MidasFindings.poc.t.sol (7 PoCs): R01, R02, R06, R09, R10, R018, R019 - MidasWithdraw.invariant.t.sol (8): exact transfer, atomic revert, pending-set consistency for withdraw/withdrawFromRedeemer - MidasLiquidator.unit.t.sol (8): first tests for MidasLiquidator (was 0 coverage), including collateral forwarding and flag lifecycle - MidasSixDecimalFlow.t.sol (2): end-to-end 6-decimal round-trip - MidasGatewayResiduals.poc.t.sol (3): residual-input PoCs, GREEN after the sweeping fix in 7eaa0db (verified unreachable with real Midas since _tokenTransferFromUser always pulls the exact calculated amount) Verified findings (confirmed against Midas source): - MID-R01 high: phantom uses current mToken rate while Midas can settle at the saved rate via safeBulkApproveRequestAtSavedRate (line 327-341), which bypasses _requireVariationTolerance because priceDif=0 - MID-R02 high: rejectRequest (line 378-386) locks mToken in vault with no recovery path; phantom retains full value - MID-R10 medium: Canceled (status=2) treated as pending by Gearbox pendingTokenOutAmount; enum has only Pending/Processed/Canceled - MID-R06 medium: transferRedeemer does not check the 10-pending cap on the destination - MID-R09 medium: isTransferAllowed is a global boolean, not scoped to the liquidated account - MID-R15 medium: liquidator _forwardCollateral(true) sweeps pre-existing residual balance to the next caller - MID-R018/R019 medium: _sweepTokens sweeps pre-existing gateway balances (donations, dust) to the next caller; introduced by 7eaa0db Refuted / unreachable: - MID-R03 refuted: request.amountMToken stores net-after-fee (line 706), phantom uses the net amount, fees cannot inflate valuation - MID-R015/016/017 unreachable with real Midas: _tokenTransferFromUser (ManageableVault line 415-429) always pulls the exact amount; sweeping fix is defensive against future partial-fill vault upgrades Reports: - MidasAuditReport.md: full audit report with source line references - MidasMathematicalProofs.md: 12 formal proofs covering decimal math, withdraw accounting, liquidator lifecycle, sweep behavior, and fee non-inflation Test results: 90 passed, 1 failed (upstream test_U_MID_G_08 regression after 7eaa0db — the upstream MidasRedemptionVaultMock does not pull mToken from the caller while the real vault does; not an audit-test failure).
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.
Summary
Standalone Midas audit suite + verified findings report. All Midas-side claims verified against
midas-apps/contractsat revision90a5f246(cloned to~/Coding/midas).What's new
33 new audit tests (all passing on top of
dc1d8b5) undercontracts/test/audit/midas/:MidasAuditTestBase.solMidasDecimalMath.fuzz.t.sol_convertToE18precision,_calculateTokenOutAmountmonotonicity, overflow, zero-inputMidasFindings.poc.t.solMidasWithdraw.invariant.t.solwithdraw/withdrawFromRedeemeraccounting: exact transfer, atomic revert, pending-set consistencyMidasLiquidator.unit.t.solMidasLiquidator(was 0 coverage): collateral forwarding, flag lifecycle, validation revertsMidasSixDecimalFlow.t.solMidasGatewayResiduals.poc.t.sol2 reports under
specs/adapters/midas/:MidasAuditReport.md— full audit report with Midas source line referencesMidasMathematicalProofs.md— 12 formal proofsVerified findings (confirmed against Midas source)
safeBulkApproveRequestAtSavedRate(RedemptionVault.sol:327-341) bypasses variation tolerance; phantom uses current raterejectRequest(RedemptionVault.sol:378-386) locks mToken with no recovery path; phantom retains valueCanceled(status=2) treated as pending by Gearbox; enum has only Pending/Processed/Canceled (IManageableVault.sol:19-23)transferRedeemerdoes not check 10-pending cap on destinationisTransferAllowedis global, not scoped to liquidated account_forwardCollateral(true)sweeps pre-existing residual to next caller_sweepTokenssweeps pre-existing gateway balances to next caller (introduced by7eaa0db)Refuted / unreachable
MID-R03refuted:request.amountMTokenstores net-after-fee (RedemptionVault.sol:706); phantom uses net amount; fees cannot inflate valuationMID-R015/016/017unreachable with real Midas:_tokenTransferFromUser(ManageableVault.sol:415-429) always pulls exact amount; sweeping fix in7eaa0dbis defensive against future partial-fill vault upgradesTest results
The 1 failure is an upstream test-mock regression (
test_U_MID_G_08), not an audit-test failure and not a production bug — the upstreamMidasRedemptionVaultMock.redeemRequestdoes not pull mToken from the caller while the real Midas vault does, so the new_sweepMToken()returns it to the account. The audit harness models the pull correctly.Notes
MidasSecurityResearch.md(pre-existing research base) is not included in this PR — it lives in the main worktree and was the starting point for this verification pass.~/Coding/midasat revision90a5f246(RedemptionVault.sol,DepositVault.sol,ManageableVault.sol,IManageableVault.sol).