feat: adopt ERC-8153 exportSelectors for on-chain facet self-description#29
Merged
Conversation
Merge 'dev' changes to 'main'
Every facet now implements ERC-8153's `exportSelectors() external pure`, reporting its own selectors on-chain. Selector discovery moves from the off-chain FFI helper (`forge inspect`) to this on-chain source of truth. - Add `IFacet` interface + `exportSelectors()` to all core facets and mocks (selectors referenced as `this.<fn>.selector`, self-excluded per ERC-8153) - Add `Selectors.decode` helper; delete FFI-based `GetSelectors.sol` - Rewire deploy script and tests to read selectors via `exportSelectors()` - Set `ffi = false`; drop `--ffi` from CONTRIBUTING - Add `ExportSelectorsTester` (exact-set, self-exclusion, decoder fuzz) - Document self-reported-selector trust caveat and the selector-drift discipline rule Out of scope (kept ERC-2535): upgradeDiamond, facet events/errors, loupe.
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.
What & why
Adopts ERC-8153 "Facet-Based Diamonds" at the facet level: every facet now implements
exportSelectors() external pure returns (bytes memory), reporting its own selectors on-chain. This replaces the off-chain FFI selector extraction (forge inspectviaGetSelectors.sol) — removing the--ffirequirement (a Foundry security foot-gun), speeding up tests, and giving upgraders/tooling an on-chain source of truth.The library stays ERC-2535 —
diamondCut, theDiamondCutevent, existing errors, and the loupe are unchanged.upgradeDiamond, the new facet events/errors, and the fallback rename are deliberately out of scope.Changes
IFacet+exportSelectors()on all core facets (DiamondCut,DiamondLoupe,ERC165,Ownable) and mocks. Selectors referenced asthis.<fn>.selector(the one form that compilespurefor bothpublicandexternalfunctions), self-excluded per ERC-8153.Selectors.decodehelper; deleted FFI-basedGetSelectors.sol.exportSelectors();ffi = false;--ffidropped fromCONTRIBUTING.ExportSelectorsTester: exact-set (independent, non-circular), self-exclusion,×4invariant, decoder fuzz + revert.Verification
forge build✅ ·forge test91/91 ✅ (ffi disabled) ·forge fmt --check✅ · facets ≪ 24 KB ✅Reviewed by a multi-agent (Fable) security + quality pass — verdict: meets the production bar, no critical/high/medium findings; the surfaced low/nice-to-have items are applied.
Deferred decision
A mechanical selector-drift guard (CI
forge inspect methodIdentifierscross-check) was not added because it reintroduces FFI at the CI gate — counter to this PR's goal. Mitigated instead by the independent test assertions +CONTRIBUTINGrule. Open to adding it if maintainers prefer the stronger guard.