Skip to content

Cyfrin Savings Fix — audit remediation (L-1, L-2, I-3, I-4) - #29

Merged
FabienCoutant merged 6 commits into
feat/savings-donation-and-pause-refactorfrom
audit/cyfrin-june-2026-savings-donation
Jun 10, 2026
Merged

FabienCoutant merged 6 commits into
feat/savings-donation-and-pause-refactorfrom
audit/cyfrin-june-2026-savings-donation

Conversation

@FabienCoutant

Copy link
Copy Markdown
Contributor

Remediation of the Cyfrin "Parallel Savings Fix" report (v1.0, 2026-06-04), stacked on top of the audited branch (#27). One commit per finding.

Code findings

  • [L-1] initializeStoredAssets() now reverts StaleAccrual() when block.timestamp - lastUpdate > 30 min. With a fresh accrual there is no unaccrued-interest delta to compound against an inflated storedAssets. Pre-upgrade step: call setRate to refresh accrual right before the upgrade, otherwise the upgrade tx itself reverts.
  • [L-2] unpause() advances lastUpdate to the unpause timestamp instead of _accrue()-ing the paused span, so the paused interval is dropped rather than minted. Pausing now halts emission (pause() still settles yield up to the pause moment).
  • [I-3] initializeStoredAssets() gains the restricted modifier — only governance can seed storedAssets, independent of whether the upgrade runs atomically.
  • [I-4] initialize() sets lastUpdate = block.timestamp, bounding the first accrual window to "since deployment" rather than "since epoch". Defense-in-depth for future fresh deployments (inert for the existing proxy, since initialize is not re-run on upgrade).

Operational (no code change)

  • [I-1] Decommission order: call setRate(0) while the vault is still a minter (final _accrue succeeds), then revoke the minter role. Reverse order traps the rate.
  • [I-2] Never set Savings as a surplus payee: surplus is paid by direct transfer, which the donation guard excludes from totalAssets() and routes to recoverSurplus — it never reaches savers.

Tests

MAX_STORED_ASSETS_INIT_STALENESS (30 min) and StaleAccrual() added. New coverage for access control + staleness on initializeStoredAssets; pause/unpause and lastUpdate assertions updated for the new behavior. Local CI green: solhint, forge build --sizes, test:unit (222/222).

@FabienCoutant
FabienCoutant merged commit 61942aa into feat/savings-donation-and-pause-refactor Jun 10, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant