Savings donation-attack guard, audits, and multi-chain upgrade deployment - #32
Merged
Merged
Conversation
…ation, silently disabling burn ratio protection
…SettersGovernor::adjustStablecoins
…r than 0 and 1 potentially leading to DOS
…ct-out mint and burn for certain collateral decimals
…ost-check causes Surplus::processSurplus(collateralAddress,0) DoS
…z test OpenZeppelin v5 uses custom errors (SafeCastOverflowedUintDowncast) instead of string reverts. Use generic vm.expectRevert() and return early to avoid asserting on reverted call results.
…for surplusBufferRatio > 100%
Prevents CI from resolving a newer OZ version where __UUPSUpgradeable_init was removed, breaking the build.
… unset ratio Removed the check for surplusBufferRatio being zero in Surplus.sol and added a new error SurplusBufferRatioNotSet in Errors.sol. Updated LibSurplus to use the new error and adjusted tests to reflect these changes, ensuring proper handling of surplus calculations.
…r normalizer (amends issue 18)
…re protocol remains above buffer ratio
…nd streamline loops for improved clarity and efficiency
Audit/Cyfrin february 2026
… address for better tracking
…ning all stables Ceil rounding in getCollateralRatio could return stablecoinsIssued > totalSupply, allowing users to bypass the CannotBurnAllStableIssued check in the Redeemer. Floor ensures amountBurnt >= stablecoinsIssued always triggers the safety check. Ceil is kept locally as divisor for collatRatio computation (conservative).
…leIssued - Fix Burn test to use >= comparison matching contract Floor rounding logic - Add quoteIn check to skip no-op swaps (amountOut == 0 from oracle rounding) - Add BurningAllStableIssued tests with manager and whitelist for Burn - Add BurningAllStableIssued tests with whitelist and manager+whitelist for Redeem - Add RedeemWithForfeit BurningAllStableIssued test without manager - Add RedeemWithForfeit BurningAllStableIssued test with whitelist - Update MultiRedemptionCurve test to use getTotalIssued() (Floor) for revert checks
…commendation Replace stablecoinsIssued check with normalizedStables guard in both _quoteRedemptionCurve and _updateNormalizer to prevent the fee bypass attack via the redemption path (renormalization truncation to zero).
…savings-donation Cyfrin Savings Fix — audit remediation (L-1, L-2, I-3, I-4)
…overnor for atomic upgrade
…cannot mint paused yield
…toredAssets after non-atomic upgrade
…orized deposit/redeem
…-savings-donation Bailsec Parallel Savings Fix — audit remediation (Issues 01-05, 07)
…nd-pause-refactor feat: savings donation-attack guard + parallelizer pause refactor
Bumps the rocketh family to 0.19, hardhat-deploy to 2.0.9 and hardhat to 3.9.1, which the new hardhat-deploy requires. The 0.19 API replaces `setup` with `setupDeployScripts` and moves `UserConfig` to `rocketh/types`. hardhat-deploy 2 also writes one artifact module per contract under generated/artifacts/ with named exports, so the default import no longer resolves and is replaced by a namespace import on the index.
Compiling with viaIR pushed Swapper to 27239 bytes, above the 24576 byte contract size limit, which blocked deployment. Turning it off brings the facet down to 19109 bytes and aligns hardhat with the foundry profiles, which already set via_ir to false.
processSurplus and release were held by the governor, which forced the multisig to run what are routine keeper jobs. They now sit behind a dedicated KEEPER_ROLE so they can be automated without handing out governor rights. Adds KEEPER_ROLE to the constants, splits the two selectors out of the governor batch in SetParallelizerRoles, and updates the test access manager config and the fixtures that wire selector roles.
surplusBufferRatio was the only surplus storage field without a getter, so its value could only be read from raw storage even though it gates surplus extraction. Adds getSurplusBufferRatio alongside the other surplus getters. Also covers the setter itself, which had no dedicated test: the InvalidParam guard below BASE_9, the emitted event, the value at the BASE_9 boundary and the access control check.
…ementation Adds two scripts that only deploy code and never execute privileged calls, since the wiring is done by the multisig. PrepareParallelizerUpgrade deploys the changed facets, diffs their selectors against the on chain loupe and prints the resulting diamondCut. DeploySavingsImplementation deploys the implementation and prints the upgradeToAndCall payload.
hardhat-deploy 2 writes generated/artifacts/ and generated/abis/, and no longer regenerates generated/artifacts.ts nor generated/types/. Both were left behind by the previous format, unreferenced and frozen at their last compile. artifacts.ts was the more harmful of the two: rocketh resolved its default export ahead of the new directory, so deployments would have silently used stale bytecode. Verified by recompiling from a clean tree, where neither path is recreated.
Records the facets and the savings implementation deployed on Sonic, including the Surplus facet, which was missing from production since the original deployment. The .chainId marker is replaced by .chain, the format rocketh 0.19 writes.
Keeps the abis emitted by the compile in the repository, as the record of what the deployment was built from.
…gs fix Certora engagement covering the Parallelizer facets, the Savings vault and FlashParallelToken, verified at mitigation commit 7c24bc4. All 490 rule runs verify, including the three donation properties that fail on the pre-fix code and confirm the storedAssets remediation.
Records the facets and the savings implementation deployed on Base, including the Surplus facet, which was missing from production since the original deployment. The .chainId marker is replaced by .chain, the format rocketh 0.19 writes.
Records the facets and the savings implementation deployed on Avalanche, including the Surplus facet, which was missing from production since the original deployment.
Records the facets and the savings implementation deployed on Ethereum mainnet, including the Surplus facet, which was missing from production since the original deployment. The .chainId marker is replaced by .chain, the format rocketh 0.19 writes.
Records the facets and the savings implementation deployed on HyperEVM, including the Surplus facet, which was missing from production since the original deployment. The .chainId marker is replaced by .chain, the format rocketh 0.19 writes.
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.
Overview
Hardens the Savings vault against the dormant-accrual donation attack, refactors pausing into explicit calls, migrates the deploy toolchain to rocketh 0.19, and records the upgrade deployment across every production chain. The Savings changes carry both the Cyfrin and Bailsec remediations plus a Certora formal verification pass.
Savings donation-attack guard
storedAssetsnow tracks the real backing, andtotalAssets()projectsstoredAssetsinstead of the rawbalanceOf, so a direct token transfer can no longer feed the rate-multiplied accrual.recoverSurplus(to)lets governance sweep untracked surplus (scoped toasset()), which stays out oftotalAssets().initializeStoredAssets()(reinitializer(2),restricted) seeds the tracked balance on upgrade, bounded by alastUpdatefreshness window.Pause refactor
togglePauseis split into explicitpause()/unpause()on both the Savings vault and the Parallelizer, withAlreadyPaused/NotPausedguards.pause()settles yield up to the pause moment,unpause()drops the paused interval instead of accruing it.Audit remediation
lastUpdatere-anchoring, pausedtotalAssets(), paused-aware accrual in the rate setters, ERC4626max*checks reintroduced across the standard and EIP-3009 entry points, and anonlyInitializedguard against a non-atomic upgrade.New surface
getSurplusBufferRatio()exposes the surplus buffer ratio, which was the only surplus storage field without a getter.processSurplusandreleasemove to a dedicated keeper role, so routine surplus jobs no longer require governor rights.Toolchain
setuptosetupDeployScriptsand adapting to the new per-contract artifact layout.viaIRso the Swapper facet stays under the EIP-170 contract size limit.Deployments
0xd3a452b3, implementation0xd256379d) that replaces the previous one.Notes
upgradeToAndCall) is executed separately by governance and is not part of this branch.