From 696ad3de6e0b0cc07f47466d688d3c79f8a9440a Mon Sep 17 00:00:00 2001 From: David Dada Date: Fri, 10 Jul 2026 09:22:10 +0100 Subject: [PATCH] feat: adopt ERC-8153 exportSelectors for on-chain facet self-description 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..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. --- CONTRIBUTING.md | 7 +- foundry.toml | 2 +- script/DeployDiamond.s.sol | 15 +-- src/facets/DiamondCutFacet.sol | 8 +- src/facets/DiamondLoupeFacet.sol | 13 +- src/facets/ERC165Facet.sol | 8 +- src/facets/OwnableFacet.sol | 20 ++- src/interfaces/IFacet.sol | 24 ++++ test/DiamondTester.t.sol | 12 +- test/ExportSelectorsTester.t.sol | 182 +++++++++++++++++++++++++++ test/InitializableTester.t.sol | 14 ++- test/helpers/GetSelectors.sol | 31 ----- test/helpers/Selectors.sol | 34 +++++ test/mocks/MockFacetA.sol | 9 +- test/mocks/MockFacetB.sol | 9 +- test/states/DeployedDiamondState.sol | 7 +- 16 files changed, 330 insertions(+), 65 deletions(-) create mode 100644 src/interfaces/IFacet.sol create mode 100644 test/ExportSelectorsTester.t.sol delete mode 100644 test/helpers/GetSelectors.sol create mode 100644 test/helpers/Selectors.sol diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1069db3..11d6010 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -18,10 +18,8 @@ forge build ## Running Tests -Tests use FFI for selector extraction, so the `--ffi` flag is required: - ```sh -forge test --ffi -vvv +forge test -vvv ``` ## Making Changes @@ -34,7 +32,7 @@ forge test --ffi -vvv - `src/initializers/` — Initializer contracts 3. Add or update tests in `test/` to cover your changes. 4. Run `forge fmt` to format your code. -5. Ensure all tests pass with `forge test --ffi`. +5. Ensure all tests pass with `forge test`. 6. Open a pull request against `main`. ## Guidelines @@ -42,6 +40,7 @@ forge test --ffi -vvv - Keep gas efficiency in mind — this library is optimized for minimal overhead. - Follow existing code style and naming conventions. - One logical change per PR — avoid bundling unrelated changes. +- Selectors are self-reported on-chain via ERC-8153 `exportSelectors()`. When you add an external/public function to a facet, you MUST also add its selector to that facet's `exportSelectors()`, extend the expected set in `test/ExportSelectorsTester.t.sol`, and exercise it through the diamond with a routed-call test. A function omitted from `exportSelectors()` will not be cut in or routed — and no test will fail unless you add one. - Include test cases for both success and failure paths. ## Reporting Issues diff --git a/foundry.toml b/foundry.toml index 2ca62a0..eae3d42 100644 --- a/foundry.toml +++ b/foundry.toml @@ -9,7 +9,7 @@ remappings = [ ] optimizer_runs = 1_000_000 via_ir = false -ffi = true +ffi = false [profile.ci] optimizer_runs = 1_000_000 diff --git a/script/DeployDiamond.s.sol b/script/DeployDiamond.s.sol index dfb568f..144f799 100644 --- a/script/DeployDiamond.s.sol +++ b/script/DeployDiamond.s.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; -import {GetSelectors} from "@diamond-test/helpers/GetSelectors.sol"; +import {Selectors} from "@diamond-test/helpers/Selectors.sol"; import {MockDiamond} from "@diamond-test/mocks/MockDiamond.sol"; import {DiamondCutFacet} from "@diamond/facets/DiamondCutFacet.sol"; import {DiamondLoupeFacet} from "@diamond/facets/DiamondLoupeFacet.sol"; @@ -11,6 +11,7 @@ import {DiamondInit} from "@diamond/initializers/DiamondInit.sol"; import {ERC165Init} from "@diamond/initializers/ERC165Init.sol"; import {MultiInit} from "@diamond/initializers/MultiInit.sol"; import {OwnableInit} from "@diamond/initializers/OwnableInit.sol"; +import {IFacet} from "@diamond/interfaces/IFacet.sol"; import {ContextLib} from "@diamond/libraries/ContextLib.sol"; import {FacetCut, FacetCutAction} from "@diamond/libraries/DiamondLib.sol"; import {Script} from "forge-std/Script.sol"; @@ -20,7 +21,7 @@ import {Script} from "forge-std/Script.sol"; /// @author David Dada /// /// @dev Uses Foundry's `Script` and a helper contract to deploy and wire up DiamondCutFacet, DiamondLoupeFacet, and OwnableFacet -contract DeployDiamond is Script, GetSelectors { +contract DeployDiamond is Script { /// @notice Executes the deployment of the Diamond contract with the initial facets and ERC165 interface setup /// @dev Broadcasts transactions using Foundry's scripting environment (`vm.startBroadcast()` and `vm.stopBroadcast()`). /// Deploys three core facets, sets up DiamondArgs, encodes an initializer call, and constructs the Diamond. @@ -39,32 +40,32 @@ contract DeployDiamond is Script, GetSelectors { // Create an array of FacetCut entries for standard facets FacetCut[] memory cut = new FacetCut[](4); - // Add DiamondCutFacet to the cut list + // Add DiamondCutFacet to the cut list (selectors read on-chain via ERC-8153 exportSelectors) cut[0] = FacetCut({ facetAddress: address(diamondCutFacet), action: FacetCutAction.Add, - functionSelectors: _getSelectors("DiamondCutFacet") + functionSelectors: Selectors.decode(IFacet(address(diamondCutFacet)).exportSelectors()) }); // Add DiamondLoupeFacet to the cut list cut[1] = FacetCut({ facetAddress: address(diamondLoupeFacet), action: FacetCutAction.Add, - functionSelectors: _getSelectors("DiamondLoupeFacet") + functionSelectors: Selectors.decode(IFacet(address(diamondLoupeFacet)).exportSelectors()) }); // Add ERC165Facet to the cut list cut[2] = FacetCut({ facetAddress: address(erc165Facet), action: FacetCutAction.Add, - functionSelectors: _getSelectors("ERC165Facet") + functionSelectors: Selectors.decode(IFacet(address(erc165Facet)).exportSelectors()) }); // Add OwnableFacet to the cut list cut[3] = FacetCut({ facetAddress: address(ownableFacet), action: FacetCutAction.Add, - functionSelectors: _getSelectors("OwnableFacet") + functionSelectors: Selectors.decode(IFacet(address(ownableFacet)).exportSelectors()) }); // Deploy the Diamond contract with the facets and initialization args diff --git a/src/facets/DiamondCutFacet.sol b/src/facets/DiamondCutFacet.sol index 3f8ce46..8a2298b 100644 --- a/src/facets/DiamondCutFacet.sol +++ b/src/facets/DiamondCutFacet.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.20; import {IDiamondCut} from "@diamond/interfaces/IDiamondCut.sol"; +import {IFacet} from "@diamond/interfaces/IFacet.sol"; import {DiamondLib, FacetCut} from "@diamond/libraries/DiamondLib.sol"; import {OwnableLib} from "@diamond/libraries/OwnableLib.sol"; @@ -13,7 +14,7 @@ import {OwnableLib} from "@diamond/libraries/OwnableLib.sol"; /// @dev Note: /// Remember to add the loupe functions from DiamondLoupeFacet to the diamond. /// The loupe functions are required by the EIP2535 Diamonds standard. -contract DiamondCutFacet is IDiamondCut { +contract DiamondCutFacet is IDiamondCut, IFacet { /// @notice Add/replace/remove any number of functions and optionally execute /// a function with delegatecall /// @param _diamondCut Contains the facet addresses and function selectors @@ -26,4 +27,9 @@ contract DiamondCutFacet is IDiamondCut { // Call the diamond cut function from the library DiamondLib.diamondCut(_diamondCut, _init, _calldata); } + + /// @inheritdoc IFacet + function exportSelectors() external pure returns (bytes memory selectors_) { + selectors_ = abi.encodePacked(this.diamondCut.selector); + } } diff --git a/src/facets/DiamondLoupeFacet.sol b/src/facets/DiamondLoupeFacet.sol index ed2fc1b..9963b22 100644 --- a/src/facets/DiamondLoupeFacet.sol +++ b/src/facets/DiamondLoupeFacet.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.20; import {IDiamondLoupe} from "@diamond/interfaces/IDiamondLoupe.sol"; +import {IFacet} from "@diamond/interfaces/IFacet.sol"; import {DiamondLib, DiamondStorage, Facet} from "@diamond/libraries/DiamondLib.sol"; /// @title DiamondLoupeFacet @@ -10,7 +11,7 @@ import {DiamondLib, DiamondStorage, Facet} from "@diamond/libraries/DiamondLib.s /// @author Modified by David Dada (https://github.com/dadadave80) /// /// @dev Implements the IDiamondLoupe interface as defined in EIP-2535 -contract DiamondLoupeFacet is IDiamondLoupe { +contract DiamondLoupeFacet is IDiamondLoupe, IFacet { /// @notice Gets all facet addresses and their function selectors. /// @return facets_ Facet function facets() external view returns (Facet[] memory facets_) { @@ -44,4 +45,14 @@ contract DiamondLoupeFacet is IDiamondLoupe { function facetAddress(bytes4 _functionSelector) external view override returns (address) { return DiamondLib.diamondStorage().selectorToFacetAndPosition[_functionSelector].facetAddress; } + + /// @inheritdoc IFacet + function exportSelectors() external pure returns (bytes memory selectors_) { + selectors_ = abi.encodePacked( + this.facets.selector, + this.facetFunctionSelectors.selector, + this.facetAddresses.selector, + this.facetAddress.selector + ); + } } diff --git a/src/facets/ERC165Facet.sol b/src/facets/ERC165Facet.sol index f5ad69b..e135558 100644 --- a/src/facets/ERC165Facet.sol +++ b/src/facets/ERC165Facet.sol @@ -1,13 +1,14 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; +import {IFacet} from "@diamond/interfaces/IFacet.sol"; import {ERC165Lib} from "@diamond/libraries/ERC165Lib.sol"; /// @title ERC165Facet /// @notice Diamond facet that implements the ERC-165 standard interface detection /// @author David Dada (https://github.com/dadadave80) /// @dev Delegates to ERC165Lib which hardcodes support for ERC-165, ERC-173, IDiamondCut, and IDiamondLoupe -contract ERC165Facet { +contract ERC165Facet is IFacet { /// @notice Query if a contract implements an interface /// @param _interfaceId The interface identifier, as specified in ERC-165 /// @dev Interface identification is specified in ERC-165. This function @@ -17,4 +18,9 @@ contract ERC165Facet { function supportsInterface(bytes4 _interfaceId) external view returns (bool) { return ERC165Lib.supportsInterface(_interfaceId); } + + /// @inheritdoc IFacet + function exportSelectors() external pure returns (bytes memory selectors_) { + selectors_ = abi.encodePacked(this.supportsInterface.selector); + } } diff --git a/src/facets/OwnableFacet.sol b/src/facets/OwnableFacet.sol index a6b723b..141ea77 100644 --- a/src/facets/OwnableFacet.sol +++ b/src/facets/OwnableFacet.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; +import {IFacet} from "@diamond/interfaces/IFacet.sol"; import {OwnableLib} from "@diamond/libraries/OwnableLib.sol"; /// @title OwnableRolesFacet @@ -15,7 +16,7 @@ import {OwnableLib} from "@diamond/libraries/OwnableLib.sol"; /// While the ownable portion follows /// [EIP-173](https://eips.ethereum.org/EIPS/eip-173) for compatibility, /// the nomenclature for the 2-step ownership handover may be unique to this codebase. -contract OwnableFacet { +contract OwnableFacet is IFacet { /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/ /* PUBLIC UPDATE FUNCTIONS */ /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/ @@ -61,6 +62,23 @@ contract OwnableFacet { return OwnableLib.ownershipHandoverExpiresAt(_pendingOwner); } + /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/ + /* ERC-8153 SELF-DESCRIPTION */ + /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/ + + /// @inheritdoc IFacet + function exportSelectors() external pure returns (bytes memory selectors_) { + selectors_ = abi.encodePacked( + this.transferOwnership.selector, + this.renounceOwnership.selector, + this.requestOwnershipHandover.selector, + this.cancelOwnershipHandover.selector, + this.completeOwnershipHandover.selector, + this.owner.selector, + this.ownershipHandoverExpiresAt.selector + ); + } + /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/ /* MODIFIERS */ /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/ diff --git a/src/interfaces/IFacet.sol b/src/interfaces/IFacet.sol new file mode 100644 index 0000000..16dc62b --- /dev/null +++ b/src/interfaces/IFacet.sol @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +/// @title IFacet +/// @notice ERC-8153 facet self-description interface. +/// @author David Dada (https://github.com/dadadave80) +/// +/// @dev Every facet reports its own function selectors on-chain, so upgraders and +/// deployment tooling no longer depend on off-chain selector extraction. +/// See https://eips.ethereum.org/EIPS/eip-8153. +/// +/// @custom:security Selectors are SELF-REPORTED by the facet. Only build `FacetCut`s from the +/// `exportSelectors()` of facets whose bytecode you trust (e.g. facets you deployed +/// yourself). A malicious facet can claim selectors it does not implement — including +/// `diamondCut` or ownership selectors — to hijack routing. `Add` cuts fail closed on +/// selector collision, but `Replace` cuts and cuts on a fresh diamond do not. +interface IFacet { + /// @notice Returns this facet's externally-callable function selectors, ABI-packed. + /// @dev The returned length is always a multiple of 4. `exportSelectors` MUST exclude + /// itself: it is called directly on the facet address, never routed through the + /// diamond, and every facet declares it (routing it would collide across facets). + /// @return selectors_ The facet's selectors packed as consecutive 4-byte values. + function exportSelectors() external pure returns (bytes memory selectors_); +} diff --git a/test/DiamondTester.t.sol b/test/DiamondTester.t.sol index 3e47478..3fd336a 100644 --- a/test/DiamondTester.t.sol +++ b/test/DiamondTester.t.sol @@ -1,10 +1,12 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; +import {Selectors} from "@diamond-test/helpers/Selectors.sol"; import {Utils} from "@diamond-test/helpers/Utils.sol"; import {DeployedDiamondState} from "@diamond-test/states/DeployedDiamondState.sol"; import {IDiamondCut} from "@diamond/interfaces/IDiamondCut.sol"; import {IDiamondLoupe} from "@diamond/interfaces/IDiamondLoupe.sol"; +import {IFacet} from "@diamond/interfaces/IFacet.sol"; import {Facet} from "@diamond/libraries/DiamondLib.sol"; /// @title DiamondTester @@ -36,12 +38,12 @@ contract DiamondTester is DeployedDiamondState { /* LOUPE — Inspecting the Cut */ /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/ - /// @notice Every expected selector is registered under its facet - function testLoupe_SelectorsAreComplete() public { + /// @notice Every selector a facet self-reports (ERC-8153) is registered under that facet + function testLoupe_SelectorsAreComplete() public view { for (uint256 i; i < facetAddresses.length; ++i) { - bytes4[] memory fromGenSelectors = _getSelectors(facetNames[i]); - for (uint256 j; j < fromGenSelectors.length; ++j) { - assertEq(facetAddresses[i], diamondLoupe.facetAddress(fromGenSelectors[j])); + bytes4[] memory exported = Selectors.decode(IFacet(facetAddresses[i]).exportSelectors()); + for (uint256 j; j < exported.length; ++j) { + assertEq(facetAddresses[i], diamondLoupe.facetAddress(exported[j])); } } } diff --git a/test/ExportSelectorsTester.t.sol b/test/ExportSelectorsTester.t.sol new file mode 100644 index 0000000..531d5f3 --- /dev/null +++ b/test/ExportSelectorsTester.t.sol @@ -0,0 +1,182 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {Selectors, Selectors__LengthNotMultipleOf4} from "@diamond-test/helpers/Selectors.sol"; +import {Utils} from "@diamond-test/helpers/Utils.sol"; +import {MockFacetA} from "@diamond-test/mocks/MockFacetA.sol"; +import {MockFacetB} from "@diamond-test/mocks/MockFacetB.sol"; +import {DiamondCutFacet} from "@diamond/facets/DiamondCutFacet.sol"; +import {DiamondLoupeFacet} from "@diamond/facets/DiamondLoupeFacet.sol"; +import {ERC165Facet} from "@diamond/facets/ERC165Facet.sol"; +import {OwnableFacet} from "@diamond/facets/OwnableFacet.sol"; +import {IDiamondCut} from "@diamond/interfaces/IDiamondCut.sol"; +import {IDiamondLoupe} from "@diamond/interfaces/IDiamondLoupe.sol"; +import {IFacet} from "@diamond/interfaces/IFacet.sol"; +import {Test} from "forge-std/Test.sol"; + +/// @title ExportSelectorsTester +/// @notice Verifies each facet's ERC-8153 `exportSelectors()` output and the `Selectors` decoder. +/// @dev Expected selector sets are listed from an INDEPENDENT source (interface constants, +/// contract-qualified externals, instance-form refs) — never re-derived from +/// `exportSelectors()` — so these assertions cannot pass by self-agreement. +contract ExportSelectorsTester is Test { + DiamondCutFacet cut; + DiamondLoupeFacet loupe; + ERC165Facet erc165; + OwnableFacet ownable; + MockFacetA mockA; + MockFacetB mockB; + + function setUp() public { + cut = new DiamondCutFacet(); + loupe = new DiamondLoupeFacet(); + erc165 = new ERC165Facet(); + ownable = new OwnableFacet(); + mockA = new MockFacetA(); + mockB = new MockFacetB(); + } + + /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/ + /* EXACT-SET ASSERTIONS PER FACET */ + /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/ + + /// @notice DiamondCutFacet exports exactly its `diamondCut` selector. + function testExport_DiamondCut() public view { + bytes4[] memory expected = new bytes4[](1); + expected[0] = IDiamondCut.diamondCut.selector; + _assertExports(IFacet(address(cut)), expected); + } + + /// @notice DiamondLoupeFacet exports exactly its four loupe selectors. + function testExport_Loupe() public view { + bytes4[] memory expected = new bytes4[](4); + expected[0] = IDiamondLoupe.facets.selector; + expected[1] = IDiamondLoupe.facetFunctionSelectors.selector; + expected[2] = IDiamondLoupe.facetAddresses.selector; + expected[3] = IDiamondLoupe.facetAddress.selector; + _assertExports(IFacet(address(loupe)), expected); + } + + /// @notice ERC165Facet exports exactly its `supportsInterface` selector. + function testExport_ERC165() public view { + bytes4[] memory expected = new bytes4[](1); + expected[0] = ERC165Facet.supportsInterface.selector; + _assertExports(IFacet(address(erc165)), expected); + } + + /// @notice OwnableFacet exports exactly its seven ownership selectors. + function testExport_Ownable() public view { + bytes4[] memory expected = new bytes4[](7); + expected[0] = ownable.transferOwnership.selector; + expected[1] = ownable.renounceOwnership.selector; + expected[2] = ownable.requestOwnershipHandover.selector; + expected[3] = ownable.cancelOwnershipHandover.selector; + expected[4] = ownable.completeOwnershipHandover.selector; + expected[5] = ownable.owner.selector; + expected[6] = ownable.ownershipHandoverExpiresAt.selector; + _assertExports(IFacet(address(ownable)), expected); + } + + /// @notice MockFacetA exports exactly its three selectors. + function testExport_MockA() public view { + bytes4[] memory expected = new bytes4[](3); + expected[0] = MockFacetA.funcA1.selector; + expected[1] = MockFacetA.funcA2.selector; + expected[2] = MockFacetA.funcA3.selector; + _assertExports(IFacet(address(mockA)), expected); + } + + /// @notice MockFacetB exports exactly its two selectors. + function testExport_MockB() public view { + bytes4[] memory expected = new bytes4[](2); + expected[0] = MockFacetB.funcA1.selector; + expected[1] = MockFacetB.funcB1.selector; + _assertExports(IFacet(address(mockB)), expected); + } + + /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/ + /* INVARIANTS ACROSS ALL FACETS */ + /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/ + + /// @notice ERC-8153: every facet's packed payload is non-empty and a multiple of 4 bytes. + function testExport_LengthIsNonZeroMultipleOf4() public view { + IFacet[6] memory facets = _allFacets(); + for (uint256 i; i < facets.length; ++i) { + bytes memory packed = facets[i].exportSelectors(); + assertGt(packed.length, 0, "empty selector payload"); + assertEq(packed.length % 4, 0, "payload not a multiple of 4"); + } + } + + /// @notice ERC-8153: `exportSelectors` MUST NOT include its own selector. + function testExport_ExcludesItself() public view { + IFacet[6] memory facets = _allFacets(); + for (uint256 i; i < facets.length; ++i) { + bytes4[] memory sels = Selectors.decode(facets[i].exportSelectors()); + for (uint256 j; j < sels.length; ++j) { + assertTrue(sels[j] != IFacet.exportSelectors.selector, "must exclude exportSelectors"); + } + } + } + + /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/ + /* SELECTORS DECODER */ + /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/ + + /// @notice `decode` inverts tight 4-byte packing for any selector set. + function testFuzz_DecodeRoundTrip(bytes4[] memory _sels) public pure { + bytes memory packed; + for (uint256 i; i < _sels.length; ++i) { + packed = abi.encodePacked(packed, _sels[i]); + } + bytes4[] memory got = Selectors.decode(packed); + assertEq(got.length, _sels.length); + for (uint256 i; i < _sels.length; ++i) { + assertEq(got[i], _sels[i]); + } + } + + /// @notice `decode` of an empty payload yields an empty array. + function testDecode_Empty() public pure { + assertEq(Selectors.decode("").length, 0); + } + + /// @notice `decode` reverts when the payload length is not a multiple of 4. + function testDecode_RevertsOnNonMultipleOf4() public { + // Routed through an external call so `expectRevert` sees the revert at a lower depth + // (the library function is `internal` and would otherwise be inlined at this depth). + vm.expectRevert(abi.encodeWithSelector(Selectors__LengthNotMultipleOf4.selector, uint256(3))); + this.decodeExternal(hex"abcdef"); + } + + /*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/ + /* HELPERS */ + /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/ + + /// @dev External wrapper so `decode` reverts can be asserted with `vm.expectRevert`. + function decodeExternal(bytes calldata _packed) external pure returns (bytes4[] memory selectors_) { + selectors_ = Selectors.decode(_packed); + } + + function _allFacets() internal view returns (IFacet[6] memory facets_) { + facets_ = [ + IFacet(address(cut)), + IFacet(address(loupe)), + IFacet(address(erc165)), + IFacet(address(ownable)), + IFacet(address(mockA)), + IFacet(address(mockB)) + ]; + } + + function _assertExports(IFacet _facet, bytes4[] memory _expected) internal view { + bytes4[] memory got = Selectors.decode(_facet.exportSelectors()); + assertEq(got.length, _expected.length, "selector count mismatch"); + for (uint256 i; i < _expected.length; ++i) { + assertTrue(Utils.containsElement(got, _expected[i]), "expected selector missing"); + } + for (uint256 i; i < got.length; ++i) { + assertTrue(Utils.containsElement(_expected, got[i]), "unexpected selector present"); + } + } +} diff --git a/test/InitializableTester.t.sol b/test/InitializableTester.t.sol index 73efe38..a01f8f9 100644 --- a/test/InitializableTester.t.sol +++ b/test/InitializableTester.t.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; -import {GetSelectors} from "@diamond-test/helpers/GetSelectors.sol"; +import {Selectors} from "@diamond-test/helpers/Selectors.sol"; import {ReinitializableDiamond} from "@diamond-test/mocks/ReinitializableDiamond.sol"; import {DiamondCutFacet} from "@diamond/facets/DiamondCutFacet.sol"; import {DiamondLoupeFacet} from "@diamond/facets/DiamondLoupeFacet.sol"; @@ -11,13 +11,15 @@ import {DiamondInit} from "@diamond/initializers/DiamondInit.sol"; import {ERC165Init} from "@diamond/initializers/ERC165Init.sol"; import {MultiInit} from "@diamond/initializers/MultiInit.sol"; import {OwnableInit} from "@diamond/initializers/OwnableInit.sol"; +import {IFacet} from "@diamond/interfaces/IFacet.sol"; import {ContextLib} from "@diamond/libraries/ContextLib.sol"; import {FacetCut, FacetCutAction} from "@diamond/libraries/DiamondLib.sol"; import {Initialized, InvalidInitialization} from "@diamond/libraries/InitializableLib.sol"; +import {Test} from "forge-std/Test.sol"; /// @title InitializableTester /// @notice Tests for the initializable Diamond pattern -contract InitializableTester is GetSelectors { +contract InitializableTester is Test { ReinitializableDiamond diamond; DiamondCutFacet diamondCutFacet; DiamondLoupeFacet diamondLoupeFacet; @@ -48,28 +50,28 @@ contract InitializableTester is GetSelectors { FacetCut({ facetAddress: address(diamondCutFacet), action: FacetCutAction.Add, - functionSelectors: _getSelectors("DiamondCutFacet") + functionSelectors: Selectors.decode(IFacet(address(diamondCutFacet)).exportSelectors()) }) ); cuts.push( FacetCut({ facetAddress: address(diamondLoupeFacet), action: FacetCutAction.Add, - functionSelectors: _getSelectors("DiamondLoupeFacet") + functionSelectors: Selectors.decode(IFacet(address(diamondLoupeFacet)).exportSelectors()) }) ); cuts.push( FacetCut({ facetAddress: address(erc165Facet), action: FacetCutAction.Add, - functionSelectors: _getSelectors("ERC165Facet") + functionSelectors: Selectors.decode(IFacet(address(erc165Facet)).exportSelectors()) }) ); cuts.push( FacetCut({ facetAddress: address(ownableFacet), action: FacetCutAction.Add, - functionSelectors: _getSelectors("OwnableFacet") + functionSelectors: Selectors.decode(IFacet(address(ownableFacet)).exportSelectors()) }) ); diff --git a/test/helpers/GetSelectors.sol b/test/helpers/GetSelectors.sol deleted file mode 100644 index 66b4b63..0000000 --- a/test/helpers/GetSelectors.sol +++ /dev/null @@ -1,31 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.20; - -import {Test} from "forge-std/Test.sol"; - -/// @notice Helper contract for extracting function selectors from facet contracts. -/// @author David Dada (https://github.com/dadadave80) -abstract contract GetSelectors is Test { - /// @notice Extracts function selectors from a given facet using `forge inspect` via FFI. - /// @dev Uses FFI to call `forge inspect methodIdentifiers` which returns only - /// the small methodIdentifiers JSON, avoiding the full artifact dump in traces. - /// @param _facet The name of the facet contract to inspect. - /// @return selectors_ An array of function selectors extracted from the facet. - function _getSelectors(string memory _facet) internal returns (bytes4[] memory selectors_) { - string[] memory cmd = new string[](5); - cmd[0] = "forge"; - cmd[1] = "inspect"; - cmd[2] = _facet; - cmd[3] = "methodIdentifiers"; - cmd[4] = "--json"; - - string memory json = string(vm.ffi(cmd)); - string[] memory signatures = vm.parseJsonKeys(json, ""); - uint256 len = signatures.length; - selectors_ = new bytes4[](len); - - for (uint256 i; i < len; ++i) { - selectors_[i] = bytes4(keccak256(bytes(signatures[i]))); - } - } -} diff --git a/test/helpers/Selectors.sol b/test/helpers/Selectors.sol new file mode 100644 index 0000000..2e18320 --- /dev/null +++ b/test/helpers/Selectors.sol @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +/// @notice Thrown when decoding a selector payload whose length is not a multiple of 4. +error Selectors__LengthNotMultipleOf4(uint256 length); + +/// @notice Decodes ERC-8153 `exportSelectors()` output (packed 4-byte selectors) into a `bytes4[]`. +/// @author David Dada (https://github.com/dadadave80) +/// +/// @dev Replaces the previous off-chain FFI selector extraction (`GetSelectors.sol`). Selectors are +/// now read on-chain from each facet via `IFacet.exportSelectors()` — no `--ffi` flag required. +/// Usage: `Selectors.decode(IFacet(facet).exportSelectors())`. +/// +/// @custom:security The `IFacet(facet).exportSelectors()` payload is SELF-REPORTED by the facet. +/// Only decode-and-cut selectors from facets you trust (e.g. facets you deployed). A malicious +/// facet can report selectors it does not implement to hijack routing — see `IFacet`. +library Selectors { + /// @notice Decodes ABI-packed 4-byte selectors into a `bytes4[]`. + /// @dev Reverts if `_packed.length` is not a multiple of 4 (a malformed ERC-8153 payload). + /// @param _packed Consecutive 4-byte selectors. + /// @return selectors_ The unpacked selectors. + function decode(bytes memory _packed) internal pure returns (bytes4[] memory selectors_) { + if (_packed.length % 4 != 0) revert Selectors__LengthNotMultipleOf4(_packed.length); + uint256 n = _packed.length / 4; + selectors_ = new bytes4[](n); + for (uint256 i; i < n; ++i) { + uint256 o = i * 4; + selectors_[i] = bytes4( + (uint32(uint8(_packed[o])) << 24) | (uint32(uint8(_packed[o + 1])) << 16) + | (uint32(uint8(_packed[o + 2])) << 8) | uint32(uint8(_packed[o + 3])) + ); + } + } +} diff --git a/test/mocks/MockFacetA.sol b/test/mocks/MockFacetA.sol index a3cf966..e42bac6 100644 --- a/test/mocks/MockFacetA.sol +++ b/test/mocks/MockFacetA.sol @@ -1,7 +1,9 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; -contract MockFacetA { +import {IFacet} from "@diamond/interfaces/IFacet.sol"; + +contract MockFacetA is IFacet { function funcA1() external pure returns (uint256) { return 1; } @@ -13,4 +15,9 @@ contract MockFacetA { function funcA3() external pure returns (uint256) { return 3; } + + /// @inheritdoc IFacet + function exportSelectors() external pure returns (bytes memory selectors_) { + selectors_ = abi.encodePacked(this.funcA1.selector, this.funcA2.selector, this.funcA3.selector); + } } diff --git a/test/mocks/MockFacetB.sol b/test/mocks/MockFacetB.sol index 978f48f..a7875e4 100644 --- a/test/mocks/MockFacetB.sol +++ b/test/mocks/MockFacetB.sol @@ -1,7 +1,9 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; -contract MockFacetB { +import {IFacet} from "@diamond/interfaces/IFacet.sol"; + +contract MockFacetB is IFacet { /// @dev Same signature as MockFacetA.funcA1 — same selector, different return value. function funcA1() external pure returns (uint256) { return 100; @@ -10,4 +12,9 @@ contract MockFacetB { function funcB1() external pure returns (uint256) { return 200; } + + /// @inheritdoc IFacet + function exportSelectors() external pure returns (bytes memory selectors_) { + selectors_ = abi.encodePacked(this.funcA1.selector, this.funcB1.selector); + } } diff --git a/test/states/DeployedDiamondState.sol b/test/states/DeployedDiamondState.sol index 0f8f924..e788daf 100644 --- a/test/states/DeployedDiamondState.sol +++ b/test/states/DeployedDiamondState.sol @@ -2,15 +2,15 @@ pragma solidity ^0.8.20; import {DeployDiamond} from "@diamond-script/DeployDiamond.s.sol"; -import {GetSelectors} from "@diamond-test/helpers/GetSelectors.sol"; import {DiamondLoupeFacet} from "@diamond/facets/DiamondLoupeFacet.sol"; import {ERC165Facet} from "@diamond/facets/ERC165Facet.sol"; import {OwnableFacet} from "@diamond/facets/OwnableFacet.sol"; import {IDiamondCut} from "@diamond/interfaces/IDiamondCut.sol"; +import {Test} from "forge-std/Test.sol"; /// @notice Provides shared state for tests involving a freshly deployed Diamond contract. /// @dev Sets up references to deployed facets, interfaces, and the diamond itself for testing. -abstract contract DeployedDiamondState is GetSelectors { +abstract contract DeployedDiamondState is Test { DeployDiamond deployDiamond; /// @notice Instance of the deployed Diamond contract. address public diamond; @@ -30,9 +30,6 @@ abstract contract DeployedDiamondState is GetSelectors { /// @notice Stores the facet addresses returned from the diamond loupe. address[] public facetAddresses; - /// @notice List of facet contract names used in deployment. - string[4] public facetNames = ["DiamondCutFacet", "DiamondLoupeFacet", "ERC165Facet", "OwnableFacet"]; - address public diamondOwner = address(this); /// @notice Deploys the Diamond contract and initializes interface references and facet addresses.