refactor(excubiae): improve framework design to favor Excubia reuse; minor changes

0xjei · 0xjei · commit f8bc4904b852 · 2024-06-26T17:27:41.000+02:00
developers need to define the _check and _pass internal methods logic

support for solidity &gt;=0.8.0 with a default compiler version to 0.8.20
diff --git a/packages/excubiae/contracts/Excubia.sol b/packages/excubiae/contracts/Excubia.sol
@@ -1,16 +1,16 @@
 // SPDX-License-Identifier: MIT
-pragma solidity >=0.8.0 <0.9.0;
+pragma solidity >=0.8.0;
 
 import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
 import {IExcubia} from "./IExcubia.sol";
 
 /// @title Excubia.
 /// @notice Abstract base contract which can be extended to implement a specific excubia.
-/// @dev Inherit from this contract and implement the `_check` method to define
-/// the custom gatekeeping logic.
+/// @dev Inherit from this contract and implement the `_pass` & `_check` methods to define
+/// your custom gatekeeping logic.
 abstract contract Excubia is IExcubia, Ownable(msg.sender) {
     /// @notice The excubia-protected contract address.
-    /// @dev The gate can be any contract address that requires a prior `_check`.
+    /// @dev The gate can be any contract address that requires a prior check to enable logic.
     /// For example, the gate is a Semaphore group that requires the passerby
     /// to meet certain criteria before joining.
     address public gate;
@@ -27,18 +27,34 @@ abstract contract Excubia is IExcubia, Ownable(msg.sender) {
         if (gate != address(0)) revert GateAlreadySet();
 
         gate = _gate;
+
+        emit GateSet(_gate);
+    }
+
+    /// @inheritdoc IExcubia
+    function pass(address passerby, bytes calldata data) external onlyGate {
+        _pass(passerby, data);
     }
 
     /// @inheritdoc IExcubia
-    function pass(address passerby, bytes calldata data) public virtual onlyGate {
+    function check(address passerby, bytes calldata data) external view returns (bool) {
+        return _check(passerby, data);
+    }
+
+    /// @notice Internal function to enforce the custom gate passing logic.
+    /// @dev Calls the `_check` internal logic and emits the relative event if successful.
+    /// @param passerby The address of the entity attempting to pass the gate.
+    /// @param data Additional data required for the check (e.g., encoded token identifier).
+    function _pass(address passerby, bytes calldata data) internal virtual {
         if (!_check(passerby, data)) revert AccessDenied();
 
         emit GatePassed(passerby, gate);
     }
 
-    /// @dev Abstract internal function to be implemented with custom logic to check if the passerby can pass the gate.
+    /// @notice Internal function to define the custom gate protection logic.
+    /// @dev Custom logic to determine if the passerby can pass the gate.
     /// @param passerby The address of the entity attempting to pass the gate.
     /// @param data Additional data that may be required for the check.
-    /// @return True if the passerby passes the check, false otherwise.
-    function _check(address passerby, bytes calldata data) internal virtual returns (bool);
+    /// @return passed True if the passerby passes the check, false otherwise.
+    function _check(address passerby, bytes calldata data) internal view virtual returns (bool passed) {}
 }
diff --git a/packages/excubiae/contracts/IExcubia.sol b/packages/excubiae/contracts/IExcubia.sol
@@ -1,14 +1,18 @@
 // SPDX-License-Identifier: MIT
-pragma solidity >=0.8.0 <0.9.0;
+pragma solidity >=0.8.0;
 
 /// @title IExcubia.
 /// @notice Excubia contract interface.
 interface IExcubia {
-    /// @notice Event emitted when someone passes the `_check` method.
+    /// @notice Event emitted when someone passes the gate check.
     /// @param passerby The address of those who have successfully passed the check.
     /// @param gate The address of the excubia-protected contract address.
     event GatePassed(address indexed passerby, address indexed gate);
 
+    /// @notice Event emitted when the gate address is set.
+    /// @param gate The address of the contract set as the gate.
+    event GateSet(address indexed gate);
+
     /// @notice Error thrown when an address equal to zero is given.
     error ZeroAddress();
 
@@ -29,9 +33,15 @@ interface IExcubia {
     /// @param _gate The address of the contract to be set as the gate.
     function setGate(address _gate) external;
 
-    /// @notice Initiates the excubia's check and triggers the associated action if the check is passed.
-    /// @dev Calls `_check` to handle the logic of checking for passing the gate.
+    /// @notice Enforces the custom gate passing logic.
+    /// @dev Must call the `check` to handle the logic of checking passerby for specific gate.
     /// @param passerby The address of the entity attempting to pass the gate.
     /// @param data Additional data required for the check (e.g., encoded token identifier).
     function pass(address passerby, bytes calldata data) external;
+
+    /// @dev Defines the custom gate protection logic.
+    /// @param passerby The address of the entity attempting to pass the gate.
+    /// @param data Additional data that may be required for the check.
+    /// @return True if the passerby passes the check, false otherwise.
+    function check(address passerby, bytes calldata data) external view returns (bool);
 }
diff --git a/packages/excubiae/contracts/README.md b/packages/excubiae/contracts/README.md
@@ -57,23 +57,28 @@ yarn add @zk-kit/excubiae
 To build your own Excubia:
 
 1. Inherit from the [Excubia](./Excubia.sol) abstract contract that conforms to the [IExcubia](./IExcubia.sol) interface.
-2. Implement the `_check()` method to define your own gatekeeping logic and to prevent unwanted access (sybils, double checks).
+2. Implement the `_check()` and `_pass()` methods logic defining your own checks to prevent unwanted access (sybils, double checks).
 
 ```solidity
 // SPDX-License-Identifier: MIT
-pragma solidity >=0.8.0 <0.9.0;
+pragma solidity >=0.8.0;
 
 import { Excubia } from "excubiae/contracts/Excubia.sol";
 
 contract MyExcubia is Excubia {
     // ...
 
-    function _check(address passerby, bytes calldata data) internal override returns (bool) {
-        // Implement custom access control logic here.
+    function _pass(address passerby, bytes calldata data) internal override {
         // Implement your logic to prevent unwanted access here.
+    }
+
+    function _check(address passerby, bytes calldata data) internal view override returns (bool) {
+        // Implement custom access control logic here.
 
         return true;
     }
+
+    // ...
 }
 ```
 
diff --git a/packages/excubiae/contracts/extensions/EASExcubia.sol b/packages/excubiae/contracts/extensions/EASExcubia.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity >=0.8.0 <0.9.0;
+pragma solidity >=0.8.0;
 
 import {Excubia} from "../Excubia.sol";
 import {IEAS} from "@ethereum-attestation-service/eas-contracts/contracts/IEAS.sol";
@@ -49,17 +49,28 @@ contract EASExcubia is Excubia {
         SCHEMA = _schema;
     }
 
-    /// @notice Overrides the `_check` function to validate against specific criteria.
+    /// @notice Internal function to handle the passing logic with check.
+    /// @dev Calls the parent `_pass` function and registers the attestation to avoid double checks.
     /// @param passerby The address of the entity attempting to pass the gate.
-    /// @param data Encoded attestation ID.
-    /// @return True if the attestation meets all criteria, revert otherwise.
-    function _check(address passerby, bytes calldata data) internal override returns (bool) {
+    /// @param data Additional data required for the check (e.g., encoded attestation ID).
+    function _pass(address passerby, bytes calldata data) internal override {
+        super._pass(passerby, data);
+
         bytes32 attestationId = abi.decode(data, (bytes32));
 
         // Avoiding double check of the same attestation.
         if (registeredAttestations[attestationId]) revert AlreadyRegistered();
 
         registeredAttestations[attestationId] = true;
+    }
+
+    /// @notice Internal function to handle the gate protection (attestation check) logic.
+    /// @dev Checks if the attestation matches the schema, attester, recipient, and is not revoked.
+    /// @param passerby The address of the entity attempting to pass the gate.
+    /// @param data Additional data required for the check (e.g., encoded attestation ID).
+    /// @return True if the attestation is valid and the passerby passes the check, false otherwise.
+    function _check(address passerby, bytes calldata data) internal view override returns (bool) {
+        bytes32 attestationId = abi.decode(data, (bytes32));
 
         Attestation memory attestation = EAS.getAttestation(attestationId);
 
diff --git a/packages/excubiae/contracts/test/MockEAS.sol b/packages/excubiae/contracts/test/MockEAS.sol
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.0;
+pragma solidity >=0.8.0;
 
 /* solhint-disable max-line-length */
 import {IEAS, ISchemaRegistry, AttestationRequest, MultiAttestationRequest, DelegatedAttestationRequest, MultiDelegatedAttestationRequest, DelegatedRevocationRequest, RevocationRequest, MultiRevocationRequest, MultiDelegatedRevocationRequest} from "@ethereum-attestation-service/eas-contracts/contracts/IEAS.sol";
diff --git a/packages/excubiae/hardhat.config.ts b/packages/excubiae/hardhat.config.ts
@@ -3,7 +3,7 @@ import { HardhatUserConfig } from "hardhat/config"
 
 const hardhatConfig: HardhatUserConfig = {
     solidity: {
-        version: "0.8.23",
+        version: "0.8.20",
         settings: {
             optimizer: {
                 enabled: true
diff --git a/packages/excubiae/test/EASExcubia.test.ts b/packages/excubiae/test/EASExcubia.test.ts
@@ -55,80 +55,134 @@ describe("EASExcubia", function () {
     })
 
     describe("EASExcubia", function () {
-        it("should fail to set the gate when the caller is not the owner", async () => {
-            const [, notOwnerSigner] = await ethers.getSigners()
-
-            await expect(easExcubia.connect(notOwnerSigner).setGate(gateAddress)).to.be.revertedWithCustomError(
-                easExcubia,
-                "OwnableUnauthorizedAccount"
-            )
-        })
-
-        it("should fail to set the gate when the gate address is zero", async () => {
-            await expect(easExcubia.setGate(ZeroAddress)).to.be.revertedWithCustomError(easExcubia, "ZeroAddress")
-        })
-
-        it("Should set the gate contract address correctly", async function () {
-            await easExcubia.setGate(gateAddress).then((tx) => tx.wait())
-
-            expect(await easExcubia.gate()).to.eq(gateAddress)
-        })
-
-        it("Should fail to set the gate if already set", async function () {
-            await expect(easExcubia.setGate(gateAddress)).to.be.revertedWithCustomError(easExcubia, "GateAlreadySet")
-        })
-
-        it("should throw when the callee is not the gate", async () => {
-            await expect(
-                easExcubia.connect(signer).pass(signerAddress, invalidRecipientAttestationId)
-            ).to.be.revertedWithCustomError(easExcubia, "GateOnly")
-        })
-
-        it("should throw when the attestation is not owned by the correct recipient", async () => {
-            await expect(
-                easExcubia.connect(gate).pass(signerAddress, invalidRecipientAttestationId)
-            ).to.be.revertedWithCustomError(easExcubia, "UnexpectedRecipient")
-        })
-
-        it("should throw when the attestation has been revoked", async () => {
-            await expect(
-                easExcubia.connect(gate).pass(signerAddress, revokedAttestationId)
-            ).to.be.revertedWithCustomError(easExcubia, "RevokedAttestation")
-        })
+        describe("setGate()", function () {
+            it("should fail to set the gate when the caller is not the owner", async () => {
+                const [, notOwnerSigner] = await ethers.getSigners()
+
+                await expect(easExcubia.connect(notOwnerSigner).setGate(gateAddress)).to.be.revertedWithCustomError(
+                    easExcubia,
+                    "OwnableUnauthorizedAccount"
+                )
+            })
+
+            it("should fail to set the gate when the gate address is zero", async () => {
+                await expect(easExcubia.setGate(ZeroAddress)).to.be.revertedWithCustomError(easExcubia, "ZeroAddress")
+            })
+
+            it("Should set the gate contract address correctly", async function () {
+                const tx = await easExcubia.setGate(gateAddress)
+                const receipt = await tx.wait()
+                const event = EASExcubiaContract.interface.parseLog(
+                    receipt?.logs[0] as unknown as { topics: string[]; data: string }
+                ) as unknown as {
+                    args: {
+                        gate: string
+                    }
+                }
 
-        it("should throw when the attestation schema is not the one expected", async () => {
-            await expect(
-                easExcubia.connect(gate).pass(signerAddress, invalidSchemaAttestationId)
-            ).to.be.revertedWithCustomError(easExcubia, "UnexpectedSchema")
+                expect(receipt?.status).to.eq(1)
+                expect(event.args.gate).to.eq(gateAddress)
+                expect(await easExcubia.gate()).to.eq(gateAddress)
+            })
+
+            it("Should fail to set the gate if already set", async function () {
+                await expect(easExcubia.setGate(gateAddress)).to.be.revertedWithCustomError(
+                    easExcubia,
+                    "GateAlreadySet"
+                )
+            })
         })
 
-        it("should throw when the attestation is not signed by the attestation owner", async () => {
-            await expect(
-                easExcubia.connect(gate).pass(signerAddress, invalidAttesterAttestationId)
-            ).to.be.revertedWithCustomError(easExcubia, "UnexpectedAttester")
+        describe("check()", function () {
+            it("should throw when the attestation is not owned by the correct recipient", async () => {
+                await expect(
+                    easExcubia.check(signerAddress, invalidRecipientAttestationId)
+                ).to.be.revertedWithCustomError(easExcubia, "UnexpectedRecipient")
+            })
+
+            it("should throw when the attestation has been revoked", async () => {
+                await expect(easExcubia.check(signerAddress, revokedAttestationId)).to.be.revertedWithCustomError(
+                    easExcubia,
+                    "RevokedAttestation"
+                )
+            })
+
+            it("should throw when the attestation schema is not the one expected", async () => {
+                await expect(easExcubia.check(signerAddress, invalidSchemaAttestationId)).to.be.revertedWithCustomError(
+                    easExcubia,
+                    "UnexpectedSchema"
+                )
+            })
+
+            it("should throw when the attestation is not signed by the attestation owner", async () => {
+                await expect(
+                    easExcubia.check(signerAddress, invalidAttesterAttestationId)
+                ).to.be.revertedWithCustomError(easExcubia, "UnexpectedAttester")
+            })
+
+            it("should pass the check", async () => {
+                const passed = await easExcubia.check(signerAddress, validAttestationId)
+
+                expect(passed).to.be.true
+                // check does NOT change the state of the contract (see pass()).
+                expect(await easExcubia.registeredAttestations(validAttestationId)).to.be.false
+            })
         })
 
-        it("should pass the check", async () => {
-            const tx = await easExcubia.connect(gate).pass(signerAddress, validAttestationId)
-            const receipt = await tx.wait()
-            const event = EASExcubiaContract.interface.parseLog(
-                receipt?.logs[0] as unknown as { topics: string[]; data: string }
-            ) as unknown as {
-                args: {
-                    passerby: string
-                    gate: string
+        describe("pass()", function () {
+            it("should throw when the callee is not the gate", async () => {
+                await expect(
+                    easExcubia.connect(signer).pass(signerAddress, invalidRecipientAttestationId)
+                ).to.be.revertedWithCustomError(easExcubia, "GateOnly")
+            })
+
+            it("should throw when the attestation is not owned by the correct recipient", async () => {
+                await expect(
+                    easExcubia.connect(gate).pass(signerAddress, invalidRecipientAttestationId)
+                ).to.be.revertedWithCustomError(easExcubia, "UnexpectedRecipient")
+            })
+
+            it("should throw when the attestation has been revoked", async () => {
+                await expect(
+                    easExcubia.connect(gate).pass(signerAddress, revokedAttestationId)
+                ).to.be.revertedWithCustomError(easExcubia, "RevokedAttestation")
+            })
+
+            it("should throw when the attestation schema is not the one expected", async () => {
+                await expect(
+                    easExcubia.connect(gate).pass(signerAddress, invalidSchemaAttestationId)
+                ).to.be.revertedWithCustomError(easExcubia, "UnexpectedSchema")
+            })
+
+            it("should throw when the attestation is not signed by the attestation owner", async () => {
+                await expect(
+                    easExcubia.connect(gate).pass(signerAddress, invalidAttesterAttestationId)
+                ).to.be.revertedWithCustomError(easExcubia, "UnexpectedAttester")
+            })
+
+            it("should pass the check", async () => {
+                const tx = await easExcubia.connect(gate).pass(signerAddress, validAttestationId)
+                const receipt = await tx.wait()
+                const event = EASExcubiaContract.interface.parseLog(
+                    receipt?.logs[0] as unknown as { topics: string[]; data: string }
+                ) as unknown as {
+                    args: {
+                        passerby: string
+                        gate: string
+                    }
                 }
-            }
-
-            expect(receipt?.status).to.eq(1)
-            expect(event.args.passerby).to.eq(signerAddress)
-            expect(event.args.gate).to.eq(gateAddress)
-        })
 
-        it("should prevent to pass twice", async () => {
-            await expect(
-                easExcubia.connect(gate).pass(signerAddress, validAttestationId)
-            ).to.be.revertedWithCustomError(easExcubia, "AlreadyRegistered")
+                expect(receipt?.status).to.eq(1)
+                expect(event.args.passerby).to.eq(signerAddress)
+                expect(event.args.gate).to.eq(gateAddress)
+                expect(await easExcubia.registeredAttestations(validAttestationId)).to.be.true
+            })
+
+            it("should prevent to pass twice", async () => {
+                await expect(
+                    easExcubia.connect(gate).pass(signerAddress, validAttestationId)
+                ).to.be.revertedWithCustomError(easExcubia, "AlreadyRegistered")
+            })
         })
     })
 })
