feat: enforce additive-only storage layout changes for PDPVerifier#263
feat: enforce additive-only storage layout changes for PDPVerifier#263Chaitu-Tatipamula wants to merge 6 commits intoFilOzone:mainfrom
Conversation
Chaitu-Tatipamula
force-pushed
the
main
branch
4 times, most recently
from
cd8e935 to
4f8bf8c
Compare
| .PHONY: check-layout | ||
| check-layout: | ||
| @echo "Checking storage layout for destructive changes..." | ||
| @bash tools/check_storage_layout.sh No newline at end of file |
There was a problem hiding this comment.
check-layout only validates the checked-in PDPVerifierLayout.sol; it never regenerates it from src/PDPVerifier.sol. That means CI can pass with a stale layout file. Please make this check rebuild the layout first and fail if the regenerated output differs from the committed file, then run the additive-only validation.
tools/check_storage_layout.sh
Outdated
| if ! git show "$BASE_REF:$FULL_LAYOUT_FILE" > "$TEMP_BASE_LAYOUT" 2>/dev/null; then | ||
| echo "Warning: Could not retrieve base layout, assuming new file" | ||
| if validate_layout_format "$LAYOUT_FILE"; then | ||
| echo "Storage layout format validated" | ||
| exit 0 | ||
| else | ||
| exit 1 |
There was a problem hiding this comment.
This fallback is too permissive for CI: when the base ref or base layout cannot be loaded, the script warns and exits successfully after format-only validation. That downgrades the safety check to a no-op. Please make missing base refs a hard failure in CI, and only skip comparison when the layout file truly does not exist yet on the base branch.
| - name: Check storage layout | ||
| run: | | ||
| export PATH="/home/runner/.config/.foundry/bin:$PATH"; | ||
| make check-layout; |
There was a problem hiding this comment.
This new step depends on the PR base ref, but the workflow doesn’t fetch it, so the check can silently degrade in shallow clones. Please either set fetch-depth: 0 on checkout or explicitly fetch ${{ github.base_ref }} before make check-layout.
github-project-automation
bot
moved this from 🔎 Awaiting review
to ⌨️ In Progress
in FOC
- Update Makefile 'check-layout' to regenerate layouts and enforce git diff check - Restructure check_storage_layout.sh CI fallback to strictly fail - Transition script to use precise JSON metadata layout validation - Ensure 'fetch-depth: 0' in Github Actions for full historical diffing - Auto-generate PDPVerifierLayout.json snapshot
2 participants
Summary
Implements storage slot safety for the PDPVerifier upgradable contract
by enforcing that storage layout changes are strictly additive,
preventing storage collision bugs during contract upgrades.
Resolves #258
What's Checked
Changes
PDPVerifierLayout.solfrom contract source usingforge inspectchanges only add new slots at the end; flags destructive changes like
removed slots, moved slots, or inserted slots
(must be committed and kept in sync)
gen,check-layout,clean-gentargets formanaging storage layout
verify storage layout on all pushes/PRs
existing CI
Usage