chore: normalize SPDX license headers to LGPL-3.0-only across src, script, test#2014
chore: normalize SPDX license headers to LGPL-3.0-only across src, script, test#20140xDEnYO wants to merge 1 commit into
Conversation
…ript, test Per [CONV:LICENSE] every own .sol file must start with the exact LGPL header immediately followed by pragma. 196 files violated this (UNLICENSED/Unlicense/ Unlicensed/MIT variants, doubled headers, missing SPDX lines, directives above the header) and kept propagating because new files are created by copying siblings. - normalize all 196 violations; external copies keep their upstream license: LiFiDEXAggregator.sol (Sushi RouteProcessor4, UNLICENSED) and ITokenMessenger.sol (Circle CCTP, Apache-2.0) - add script/utils/check-spdx-headers.sh as single source of truth (incl. external-copy allowlist) - post-edit hook warns agents on wrong first lines (src/, script/, test/) - new spdxLicenseChecker.yml CI workflow enforces repo-wide on every PR Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedToo many files! This PR contains 199 files, which is 49 over the limit of 150. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (199)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
the
requires-types
Test Coverage ReportLine Coverage: 89.98% (3386 / 3763 lines) |
🤖 GitHub Action: Security Alerts Review 🔍🟢 Dismissed Security Alerts with Comments 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: 🟢 View Alert - File: ✅ No unresolved security alerts! 🎉 |
ReviewVerified the mechanics against the branch directly. Correct and well done:
Main finding — enforcement only closes half the rule:
The gap is real. Scanning line 2 of every
These are all pre-existing (untouched here — they already had LGPL on line 1), so this is not a regression. But it means the copy-chain problem this PR exists to kill is only half-closed: the PR establishes that Cheap fix within the script's spirit: also assert line 2 is Minor / non-blocking:
Verdict: Solid, mechanically sound, safe to merge for its stated scope (line-1 normalization + enforcement). One substantive recommendation: extend the checker to also enforce "line 2 = pragma" — otherwise the rule's second half stays unenforced and ~33 existing violations remain invisible to CI. |
3 participants
Which Linear task belongs to this PR?
EXSC-588
Why did I implement it this way?
Problem.
[CONV:LICENSE](.agents/rules/100-solidity-basics.md) mandates// SPDX-License-Identifier: LGPL-3.0-onlyas line 1 of our own.solfiles, but 198 files undersrc/,script/, andtest/deviated (UNLICENSED,Unlicense,Unlicensed,MIT, doubled// //headers, missing SPDX lines, solhint directives above the header). Because new files are written by copying siblings — and intest/roughly half the siblings were wrong — the prose rule kept losing (PR #1996 reintroduced two variants this way). This PR fixes the data and adds mechanical enforcement so the copy chain stops.What was done.
pragma:UNLICENSED, 49Unlicense, 3Unlicensed, 3MIT, 5 doubled// // …LGPL-3.0-only, 1 doubled// // …MIT)pragma// solhint-disable…as line 1 (two of them withUnlicenseon line 2) → SPDX/pragma now first, directive moved belowpragma(solhint file-level disables apply from their line onward and all long lines sit further down; verified withbunx solhint— 0 errors, 0 new warnings)src/Helpers/ReentrancyGuard.sol+src/Helpers/Validatable.sol(part of the 190) additionally had/// @custom:version 1.0.0between SPDX andpragma(forbidden by the rule) → moved into the contract-level NatSpec blocksrc/violations for external copies — per the rule, external copies retain their original license. Two are genuine external copies and were left untouched:src/Periphery/LiFiDEXAggregator.sol— copy of SushiSwapRouteProcessor4.sol(pinned commit in its NatSpec); upstream first line verified as// SPDX-License-Identifier: UNLICENSED, so the current header IS the retained upstream licensesrc/Interfaces/ITokenMessenger.sol— Circle CCTP interface,Apache-2.0with Circle copyright blocksrc/offenders all carry@author LI.FINatSpec or are LiFi-authored facets/helpers → normalized to LGPLscript/utils/check-spdx-headers.shholds the expected header and the external-copy allowlist. Both consumers call it, so the allowlist can't drift:.agents/hooks/post-edit-validate.sh— agents editing a.solundersrc/,script/, ortest/with a wrong first line now get a[CONV:LICENSE]warning echoed back immediately (non-blocking, like the existing solhint feedback).github/workflows/spdxLicenseChecker.yml— repo-wide check on every PR, so human edits (andHUSKY=0commits) are caught too. Chose CI over the pre-commit hook: pre-commit is bypassable and already heavy; the check is a <1s grep in CI.Verification.
bash script/utils/check-spdx-headers.shrepo-wide: cleanforge build: exit 0bun test(forge test --evm-version cancun): 1677/1678 passed; the single failure (EcoFacetTest.testBase_CanBridgeTokens_fuzzed) was a drpc Base RPC timeout, suite re-run green 43/43 — infrastructure flake, unrelated to header changes.solfiles is one of {LGPL header, relocated@custom:version 1.0.0, relocated solhint directive, blank line}; every removed line is one of the wrong header variants or a relocated directive/pr-ready(local CodeRabbit): the full 199-file diff exceeds the CLI's 150-file hard limit, so the review was run scoped per directory over the three files carrying non-mechanical logic:.agents/hooks/post-edit-validate.sh→ No findings ✔.github/workflows/spdxLicenseChecker.yml→ No findings ✔script/utils/check-spdx-headers.sh→ not completable locally — repeated attempts (5+ rounds over ~2h) hit the lifinance org's shared rolling CodeRabbit rate limit, never a code finding. It is a single self-contained bash validator, dry-run tested here (flags a deliberately broken header with exit 1, stays silent on the allowlisted external copy, passes repo-wide) andbash -nclean; its only caller,.agents/hooks/post-edit-validate.sh, reviewed clean above. Cloud CodeRabbit reviews it in CI as the safety net..soledits are outside the CodeRabbit review surface; they are covered by the scripted diff audit above (every added/removed line matched an expected header/directive pattern) and by the new CI check. Cloud CodeRabbit will also run in CI as the standard safety net.Checklist before requesting a review
/pr-ready(local CodeRabbit) on this branch and resolved (or explicitly documented) all findings — see.agents/commands/pr-ready.mdChecklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)