diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 0000000..b35e747 --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,32 @@ +name: Linter + +on: + push: + branches: ["main"] + pull_request: + branches: ["main"] + +jobs: + build: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v7 + + - name: Install Foundry + uses: foundry-rs/foundry-toolchain@v1 + with: + version: v1.7.1 + cache: true + + - name: Install Dependencies + run: | + forge install + + - name: fmt + run: | + forge fmt --check + + - name: Lint + run: | + forge lint --deny notes --quiet diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..d9e6c16 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,28 @@ +name: Test + +on: + push: + branches: ["main"] + pull_request: + branches: ["main"] + +jobs: + build: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v7 + + - name: Install Foundry + uses: foundry-rs/foundry-toolchain@v1 + with: + version: v1.7.1 + cache: true + + - name: Install Dependencies + run: | + forge install + + - name: Run tests + run: | + forge test diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..e95eb1a --- /dev/null +++ b/.gitignore @@ -0,0 +1,6 @@ +# forge +out/ +cache/ + +# VIM +*.swp diff --git a/.gitmodules b/.gitmodules new file mode 100644 index 0000000..19d3c08 --- /dev/null +++ b/.gitmodules @@ -0,0 +1,6 @@ +[submodule "lib/forge-std"] + path = lib/forge-std + url = https://github.com/foundry-rs/forge-std +[submodule "lib/safe-smart-account"] + path = lib/safe-smart-account + url = https://github.com/safe-fndn/safe-smart-account diff --git a/foundry.lock b/foundry.lock new file mode 100644 index 0000000..518ec4e --- /dev/null +++ b/foundry.lock @@ -0,0 +1,14 @@ +{ + "lib/forge-std": { + "tag": { + "name": "v1.16.2", + "rev": "bf647bd6046f2f7da30d0c2bf435e5c76a780c1b" + } + }, + "lib/safe-smart-account": { + "tag": { + "name": "v1.5.0", + "rev": "dc437e8fba8b4805d76bcbd1c668c9fd3d1e83be" + } + } +} \ No newline at end of file diff --git a/foundry.toml b/foundry.toml new file mode 100644 index 0000000..f8344f3 --- /dev/null +++ b/foundry.toml @@ -0,0 +1,27 @@ +[profile.default] +src = 'src' +test = 'test' +script = 'script' +out = 'out' +libs = ['lib'] +cache_path = 'cache' +solc = "0.8.36" +via_ir = false +optimizer = true +optimizer_runs = 2000 +bytecode_hash = "none" + +# For dependencies +remappings = [ + 'forge-std/=lib/forge-std/src/', + '@safe/=lib/safe-smart-account/contracts/', + #'@fvm-solidity/=lib/pdp/lib/fvm-solidity/src/', +] + +[lint] +severity = ["high", "med", "low", "info", "gas", "code-size"] +exclude_lints = [ + "incorrect-shift", + "multi-contract-file", + "unwrapped-modifier-logic", +] diff --git a/lib/forge-std b/lib/forge-std new file mode 160000 index 0000000..bf647bd --- /dev/null +++ b/lib/forge-std @@ -0,0 +1 @@ +Subproject commit bf647bd6046f2f7da30d0c2bf435e5c76a780c1b diff --git a/lib/safe-smart-account b/lib/safe-smart-account new file mode 160000 index 0000000..77901a5 --- /dev/null +++ b/lib/safe-smart-account @@ -0,0 +1 @@ +Subproject commit 77901a5a1ad835b74ad3b72f73a8412cfe491c57 diff --git a/src/lib/Epoch.sol b/src/lib/Epoch.sol new file mode 100644 index 0000000..e747525 --- /dev/null +++ b/src/lib/Epoch.sol @@ -0,0 +1,53 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT +pragma solidity ^0.8.36; + +type Epoch is uint96; + +using { + add as +, + sub as -, + equals as ==, + greaterThan as >, + lessThan as <, + greaterThanOrEqualTo as >=, + lessThanOrEqualTo as <= +} for Epoch global; + +/// @return epoch The current block number +function currentEpoch() view returns (Epoch epoch) { + assembly ("memory-safe") { + epoch := number() + } +} + +function add(Epoch epoch, Epoch other) pure returns (Epoch sum) { + assembly ("memory-safe") { + sum := add(epoch, other) + } +} + +function sub(Epoch epoch, Epoch other) pure returns (Epoch difference) { + assembly ("memory-safe") { + difference := sub(epoch, other) + } +} + +function equals(Epoch epoch, Epoch other) pure returns (bool) { + return Epoch.unwrap(epoch) == Epoch.unwrap(other); +} + +function greaterThan(Epoch epoch, Epoch other) pure returns (bool) { + return Epoch.unwrap(epoch) > Epoch.unwrap(other); +} + +function lessThan(Epoch epoch, Epoch other) pure returns (bool) { + return Epoch.unwrap(epoch) < Epoch.unwrap(other); +} + +function greaterThanOrEqualTo(Epoch epoch, Epoch other) pure returns (bool) { + return Epoch.unwrap(epoch) >= Epoch.unwrap(other); +} + +function lessThanOrEqualTo(Epoch epoch, Epoch other) pure returns (bool) { + return Epoch.unwrap(epoch) <= Epoch.unwrap(other); +} diff --git a/src/lib/IsASafe.sol b/src/lib/IsASafe.sol new file mode 100644 index 0000000..4cd76ad --- /dev/null +++ b/src/lib/IsASafe.sol @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT +pragma solidity ^0.8.36; + +import {IProxy} from "@safe/proxies/SafeProxy.sol"; + +library IsASafe { + error NotSafeProxy(address account); + error UnusualSafeMasterCopy(address account, address masterCopy); + + function isProbablyASafe(address account) internal view { + uint256 codesize = account.code.length; + // observed Safe proxy codesize range is 110 (v1.0.0) to 171 (v1.3.0) + require(codesize > 80 && codesize < 240, NotSafeProxy(account)); + address implementation = IProxy(account).masterCopy(); + // observed Safe masterCopy size is 20869 (v1.5.0) to 24421 (v1.4.1) + require(implementation.code.length > 8000, UnusualSafeMasterCopy(account, implementation)); + } +} diff --git a/src/lib/OwnerSet.sol b/src/lib/OwnerSet.sol new file mode 100644 index 0000000..a6259f0 --- /dev/null +++ b/src/lib/OwnerSet.sol @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT +pragma solidity ^0.8.36; + +// OwnerSet is a space-efficient bitmask +// Each owner has a unique representative bit assigned during addOwner + +type OwnerSet is uint160; + +using {equals as ==, notEquals as !=, or as |, xor as ^, and as &} for OwnerSet global; + +OwnerSet constant EMPTY_SET = OwnerSet.wrap(uint160(0)); +OwnerSet constant FULL_SET = OwnerSet.wrap(type(uint160).max); + +function equals(OwnerSet a, OwnerSet b) pure returns (bool) { + return OwnerSet.unwrap(a) == OwnerSet.unwrap(b); +} + +function notEquals(OwnerSet a, OwnerSet b) pure returns (bool) { + return OwnerSet.unwrap(a) != OwnerSet.unwrap(b); +} + +function or(OwnerSet a, OwnerSet b) pure returns (OwnerSet) { + return OwnerSet.wrap(OwnerSet.unwrap(a) | OwnerSet.unwrap(b)); +} + +function xor(OwnerSet a, OwnerSet b) pure returns (OwnerSet) { + return OwnerSet.wrap(OwnerSet.unwrap(a) ^ OwnerSet.unwrap(b)); +} + +function and(OwnerSet a, OwnerSet b) pure returns (OwnerSet) { + return OwnerSet.wrap(OwnerSet.unwrap(a) & OwnerSet.unwrap(b)); +} diff --git a/src/lib/Owners.sol b/src/lib/Owners.sol new file mode 100644 index 0000000..bf33023 --- /dev/null +++ b/src/lib/Owners.sol @@ -0,0 +1,105 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT +pragma solidity ^0.8.36; + +import {EMPTY_SET, FULL_SET, OwnerSet} from "./OwnerSet.sol"; + +library OwnersLibrary { + struct OwnerInfo { + uint8 bitId; // [0, 160] + } + + /// @custom:storage-location erc7201:Solstice.Owners + struct Owners { + mapping(address => OwnerInfo) ownerInfo; + uint8 nextBitCursor; // [0, 160) + OwnerSet allOwners; + } + + // keccak256(abi.encode(uint256(keccak256("Solstice.Owners")) - 1)) & ~bytes32(uint256(0xff)); + bytes32 private constant OWNERS_SLOT = 0x7d2e7f914625694dd929b468ac404d7943373f4d24421c78ac93b57cc8efb500; + + function getOwnersSlot() internal pure returns (Owners storage owners) { + assembly ("memory-safe") { + owners.slot := OWNERS_SLOT + } + } + + event OwnerAdded(address indexed owner); + event OwnerRemoved(address indexed owner); + + function isOwner(address someone) internal view returns (bool) { + return getOwnersSlot().ownerInfo[someone].bitId != 0; + } + + /// @dev Returns EMPTY_SET if `owner` is not a current owner. + function asOwnerSet(address owner) internal view returns (OwnerSet mask) { + uint8 ownerBit = getOwnersSlot().ownerInfo[owner].bitId; + assembly ("memory-safe") { + mask := shl(sub(ownerBit, 1), 1) + } + } + + function getAllOwners() internal view returns (OwnerSet) { + return getOwnersSlot().allOwners; + } + + // Proposed owner is already an owner + error AlreadyOwner(address owner); + // Unsupported ownership count (> 160) + error MaximumOwnersReached(); + + /// @param owner The address to grant ownership to + function addOwner(address owner) internal { + require(!isOwner(owner), AlreadyOwner(owner)); + + Owners storage owners = getOwnersSlot(); + uint8 ownerBit = owners.nextBitCursor; + OwnerSet allOwners = owners.allOwners; + + require(allOwners != FULL_SET, MaximumOwnersReached()); + + OwnerSet ownerSet = EMPTY_SET; + + // assign next free bit + while (true) { + assembly ("memory-safe") { + ownerSet := shl(ownerBit, 1) + ownerBit := add(1, ownerBit) + } + if (ownerSet & allOwners == EMPTY_SET) { + break; + } else { + ownerBit %= 160; + } + } + + owners.ownerInfo[owner].bitId = ownerBit; + owners.allOwners = allOwners | ownerSet; + owners.nextBitCursor = ownerBit % 160; + + emit OwnerAdded(owner); + } + + // Address to remove is not a current owner + error NotOwner(address owner); + error CannotRemoveLastOwner(); + + /// @param owner The address to revoke ownership from + /// @dev A removed owner's bit may be recycled to a future owner by addOwner. + /// @dev Veto stale PendingTasks when removing an owner to avoid approvals carrying over. + function removeOwner(address owner) internal { + require(isOwner(owner), NotOwner(owner)); + + Owners storage owners = getOwnersSlot(); + OwnerSet mask = asOwnerSet(owner); + + OwnerSet nextOwners = owners.allOwners ^ mask; + require(nextOwners != EMPTY_SET, CannotRemoveLastOwner()); + + owners.allOwners = nextOwners; + + delete owners.ownerInfo[owner]; + + emit OwnerRemoved(owner); + } +} diff --git a/src/lib/PendingTask.sol b/src/lib/PendingTask.sol new file mode 100644 index 0000000..3e84ac2 --- /dev/null +++ b/src/lib/PendingTask.sol @@ -0,0 +1,30 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT +pragma solidity ^0.8.36; + +import {Epoch} from "./Epoch.sol"; +import {OwnerSet} from "./OwnerSet.sol"; + +struct PendingTask { + Epoch modified; + OwnerSet approvals; +} + +struct PendingTaskInfo { + PendingTask task; +} + +library PendingTaskLibrary { + /// @custom:storage-location erc7201:Solstice.PendingTasks + struct PendingTasks { + mapping(bytes32 taskId => PendingTaskInfo) tasks; + } + + // keccak256(abi.encode(uint256(keccak256("Solstice.PendingTasks")) - 1)) & ~bytes32(uint256(0xff)); + bytes32 private constant PENDING_TASKS_SLOT = 0x635f64a8ec66823e68578973f5bc466fd4e0eadd655f760cfc91e860524aa300; + + function getTasksSlot() internal pure returns (mapping(bytes32 taskId => PendingTaskInfo) storage tasks) { + assembly ("memory-safe") { + tasks.slot := PENDING_TASKS_SLOT + } + } +} diff --git a/src/lib/UnanimousGovernance.sol b/src/lib/UnanimousGovernance.sol new file mode 100644 index 0000000..2d230cf --- /dev/null +++ b/src/lib/UnanimousGovernance.sol @@ -0,0 +1,78 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT +pragma solidity ^0.8.36; + +import {Epoch, currentEpoch} from "./Epoch.sol"; +import {EMPTY_SET, OwnerSet} from "./OwnerSet.sol"; +import {PendingTask, PendingTaskInfo, PendingTaskLibrary} from "./PendingTask.sol"; +import {OwnersLibrary} from "./Owners.sol"; + +contract UnanimousGovernance { + using OwnersLibrary for address; + + Epoch constant NO_HOLD = Epoch.wrap(0); + Epoch constant UNSUBMITTED = Epoch.wrap(0); + + event Submitted(bytes32 indexed taskId); + event Approved(bytes32 indexed taskId, address indexed owner); + event Rejected(bytes32 indexed taskId, address indexed owner); + + error HoldUntil(Epoch until); + error NotOwner(address account); + error AlreadyApproved(); + + /// @notice Executes the wrapped function once every current owner has approved `taskId`. + /// @dev If `hold` is zero, execution happens on the approval that reaches unanimity. + /// @dev Otherwise, once unanimous, execution becomes permissionless after `hold` epochs elapse. + /// @param taskId The identifier of the task being approved, usually keccak256(msg.data) + /// @param hold The number of epochs to wait after unanimity before execution becomes permissionless + modifier unanimous(bytes32 taskId, Epoch hold) { + // load + PendingTaskInfo storage taskInfo = PendingTaskLibrary.getTasksSlot()[taskId]; + PendingTask memory loaded = taskInfo.task; + OwnerSet allOwners = OwnersLibrary.getAllOwners(); + + // modify + if (loaded.approvals & allOwners == allOwners) { + // already approved: permissionless completion + Epoch until = loaded.modified + hold; + require(currentEpoch() >= until, HoldUntil(until)); + // execute + delete taskInfo.task; + _; + } else { + // approve + require(msg.sender.isOwner(), NotOwner(msg.sender)); + OwnerSet ownerBit = msg.sender.asOwnerSet(); + if (loaded.modified == UNSUBMITTED) { + emit Submitted(taskId); + } else { + require(loaded.approvals & ownerBit == EMPTY_SET, AlreadyApproved()); + } + loaded.modified = currentEpoch(); + loaded.approvals = loaded.approvals | ownerBit; + + // store result + emit Approved(taskId, msg.sender); + if (hold == NO_HOLD && loaded.approvals & allOwners == allOwners) { + delete taskInfo.task; + // execute now + _; + } else { + // wait + taskInfo.task = loaded; + } + } + } + + /// @param taskId The identifier of the pending task to reject + function _veto(bytes32 taskId) internal { + // load + PendingTaskInfo storage taskInfo = PendingTaskLibrary.getTasksSlot()[taskId]; + + // modify + require(msg.sender.isOwner(), NotOwner(msg.sender)); + delete taskInfo.task; + + emit Rejected(taskId, msg.sender); + } +} diff --git a/test/OwnerSet.t.sol b/test/OwnerSet.t.sol new file mode 100644 index 0000000..8c65579 --- /dev/null +++ b/test/OwnerSet.t.sol @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT +pragma solidity ^0.8.36; + +import {Test} from "forge-std/Test.sol"; +import {EMPTY_SET, FULL_SET, OwnerSet} from "../src/lib/OwnerSet.sol"; + +contract OwnerSetTest is Test { + function test_emptySet_isZero() public pure { + assertEq(OwnerSet.unwrap(EMPTY_SET), 0); + } + + function test_fullSet_isMaxUint160() public pure { + assertEq(OwnerSet.unwrap(FULL_SET), type(uint160).max); + } + + function testFuzz_equals_reflexive(uint160 raw) public pure { + OwnerSet set = OwnerSet.wrap(raw); + assertTrue(set == set); + } + + function testFuzz_notEquals_isNegationOfEquals(uint160 a, uint160 b) public pure { + OwnerSet setA = OwnerSet.wrap(a); + OwnerSet setB = OwnerSet.wrap(b); + assertEq(setA == setB, !(setA != setB)); + } + + function testFuzz_notEquals_differingBits(uint160 a, uint160 b) public pure { + vm.assume(a != b); + assertTrue(OwnerSet.wrap(a) != OwnerSet.wrap(b)); + } + + function testFuzz_or_matchesRawBitwiseOr(uint160 a, uint160 b) public pure { + OwnerSet result = OwnerSet.wrap(a) | OwnerSet.wrap(b); + assertEq(OwnerSet.unwrap(result), a | b); + } + + function testFuzz_xor_matchesRawBitwiseXor(uint160 a, uint160 b) public pure { + OwnerSet result = OwnerSet.wrap(a) ^ OwnerSet.wrap(b); + assertEq(OwnerSet.unwrap(result), a ^ b); + } + + function testFuzz_and_matchesRawBitwiseAnd(uint160 a, uint160 b) public pure { + OwnerSet result = OwnerSet.wrap(a) & OwnerSet.wrap(b); + assertEq(OwnerSet.unwrap(result), a & b); + } + + function testFuzz_or_emptySetIsIdentity(uint160 a) public pure { + assertTrue((OwnerSet.wrap(a) | EMPTY_SET) == OwnerSet.wrap(a)); + } + + function testFuzz_and_fullSetIsIdentity(uint160 a) public pure { + assertTrue((OwnerSet.wrap(a) & FULL_SET) == OwnerSet.wrap(a)); + } + + function testFuzz_and_emptySetIsAnnihilator(uint160 a) public pure { + assertTrue((OwnerSet.wrap(a) & EMPTY_SET) == EMPTY_SET); + } + + function testFuzz_xor_selfInverse(uint160 a) public pure { + assertTrue((OwnerSet.wrap(a) ^ OwnerSet.wrap(a)) == EMPTY_SET); + } + + function testFuzz_xor_isReversibleWithSameOperand(uint160 a, uint160 b) public pure { + OwnerSet setA = OwnerSet.wrap(a); + OwnerSet setB = OwnerSet.wrap(b); + assertTrue((setA ^ setB) ^ setB == setA); + } +} diff --git a/test/Owners.t.sol b/test/Owners.t.sol new file mode 100644 index 0000000..93c6bcd --- /dev/null +++ b/test/Owners.t.sol @@ -0,0 +1,219 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT +pragma solidity ^0.8.36; + +import {Test} from "forge-std/Test.sol"; +import {EMPTY_SET, FULL_SET, OwnerSet} from "../src/lib/OwnerSet.sol"; +import {OwnersLibrary} from "../src/lib/Owners.sol"; + +// wraps the internal library so vm.expectRevert has a real call frame to target +contract OwnersHarness { + function isOwner(address someone) external view returns (bool) { + return OwnersLibrary.isOwner(someone); + } + + function getAllOwners() external view returns (OwnerSet) { + return OwnersLibrary.getAllOwners(); + } + + function asOwnerSet(address owner) external view returns (OwnerSet) { + return OwnersLibrary.asOwnerSet(owner); + } + + function addOwner(address owner) external { + OwnersLibrary.addOwner(owner); + } + + function removeOwner(address owner) external { + OwnersLibrary.removeOwner(owner); + } + + // test-only bootstrap: jumps the bit-scan cursor and bitmap directly, so tests can exercise + // addOwner's wraparound without registering ~160 real owners first + function seedBitState(uint8 nextBitCursor, OwnerSet allOwners) external { + OwnersLibrary.Owners storage owners = OwnersLibrary.getOwnersSlot(); + owners.nextBitCursor = nextBitCursor; + owners.allOwners = allOwners; + } +} + +contract OwnersTest is Test { + OwnersHarness harness; + + function setUp() public { + harness = new OwnersHarness(); + } + + function test_isOwner_falseInitially(address someone) public view { + assertFalse(harness.isOwner(someone)); + } + + function test_allOwners_emptyInitially() public view { + assertTrue(harness.getAllOwners() == EMPTY_SET); + } + + function testFuzz_addOwner(address owner) public { + vm.assume(owner != address(0)); + harness.addOwner(owner); + assertTrue(harness.isOwner(owner)); + + assertTrue(harness.getAllOwners() == harness.asOwnerSet(owner)); + + vm.expectRevert(abi.encodeWithSelector(OwnersLibrary.AlreadyOwner.selector, owner)); + harness.addOwner(owner); + } + + function testFuzz_addOwner_multipleOwners(address a, address b) public { + vm.assume(a != address(0) && b != address(0) && a != b); + harness.addOwner(a); + harness.addOwner(b); + + assertTrue(harness.isOwner(a)); + assertTrue(harness.isOwner(b)); + + // distinct owners get distinct bits + assertTrue(harness.asOwnerSet(a) != harness.asOwnerSet(b)); + assertTrue(harness.getAllOwners() == (harness.asOwnerSet(a) | harness.asOwnerSet(b))); + + vm.expectRevert(abi.encodeWithSelector(OwnersLibrary.AlreadyOwner.selector, a)); + harness.addOwner(a); + vm.expectRevert(abi.encodeWithSelector(OwnersLibrary.AlreadyOwner.selector, b)); + harness.addOwner(b); + } + + function testFuzz_removeOwner(address owner) public { + address companion = makeAddr("removeOwnerCompanion"); + vm.assume(owner != address(0) && owner != companion); + + harness.addOwner(companion); + harness.addOwner(owner); + harness.removeOwner(owner); + assertFalse(harness.isOwner(owner)); + assertTrue(harness.getAllOwners() == harness.asOwnerSet(companion)); + } + + function testFuzz_removeOwner_revertsWhenRemovingLastOwner(address owner) public { + vm.assume(owner != address(0)); + harness.addOwner(owner); + + vm.expectRevert(OwnersLibrary.CannotRemoveLastOwner.selector); + harness.removeOwner(owner); + + assertTrue(harness.isOwner(owner)); + } + + function testFuzz_removeOwner_leavesOtherOwnersIntact(address a, address b, address c) public { + vm.assume(a != address(0) && b != address(0) && c != address(0)); + vm.assume(a != b && a != c && b != c); + + harness.addOwner(a); + harness.addOwner(b); + harness.addOwner(c); + + OwnerSet bMask = harness.asOwnerSet(b); + harness.removeOwner(b); + + assertFalse(harness.isOwner(b)); + assertTrue(harness.isOwner(a)); + assertTrue(harness.isOwner(c)); + assertTrue(harness.getAllOwners() & bMask == EMPTY_SET); + assertTrue(harness.getAllOwners() == (harness.asOwnerSet(a) | harness.asOwnerSet(c))); + } + + function testFuzz_removeOwner_revertsForNonOwner(address a) public { + vm.assume(a != address(0)); + vm.expectRevert(abi.encodeWithSelector(OwnersLibrary.NotOwner.selector, a)); + harness.removeOwner(a); + } + + function testFuzz_removeOwner_revertsAfterAlreadyRemoved(address a) public { + address companion = makeAddr("alreadyRemovedCompanion"); + vm.assume(a != address(0) && a != companion); + + harness.addOwner(companion); + harness.addOwner(a); + harness.removeOwner(a); + + vm.expectRevert(abi.encodeWithSelector(OwnersLibrary.NotOwner.selector, a)); + harness.removeOwner(a); + } + + function testFuzz_addOwner_afterRemove_canReAdd(address owner) public { + address companion = makeAddr("reAddCompanion"); + vm.assume(owner != address(0) && owner != companion); + + harness.addOwner(companion); + harness.addOwner(owner); + harness.removeOwner(owner); + harness.addOwner(owner); + + assertTrue(harness.isOwner(owner)); + } + + function test_addOwner_revertsWhenFull() public { + harness.seedBitState(0, FULL_SET); + address owner = makeAddr("overflowOwner"); + + vm.expectRevert(OwnersLibrary.MaximumOwnersReached.selector); + harness.addOwner(owner); + } + + // loop-around coverage: + // addOwner scans forward from `nextBitCursor` for a free bit, wrapping the + // scan back to bit 0 (via `ownerBit %= 160`) once it runs past bit 159, the + // top of the uint160 bitmap. These tests seed that boundary condition + // directly instead of registering ~160 real owners to reach it. + + function test_addOwner_cursorAtTopBit_resetsCursorToZeroAfterClaimingIt() public { + // the top bit (159) is free: addOwner claims it directly, no scanning needed, + // but the stored cursor must still wrap to 0 (160 % 160 == 0) for next time + harness.seedBitState(159, EMPTY_SET); + + address owner = makeAddr("topBitOwner"); + harness.addOwner(owner); + assertTrue(harness.asOwnerSet(owner) == OwnerSet.wrap(uint160(1) << 159)); + + address nextOwner = makeAddr("afterTopBitOwner"); + harness.addOwner(nextOwner); + assertTrue(harness.asOwnerSet(nextOwner) == OwnerSet.wrap(1)); + } + + function test_addOwner_wrapsPastTopBitToFindFreeLowBit() public { + // bits 155-159 are occupied; cursor starts at the top (159), so the very + // first candidate collides and the scan must wrap around to bit 0 + OwnerSet occupiedTop = OwnerSet.wrap(uint160(0x1F) << 155); + harness.seedBitState(159, occupiedTop); + + address owner = makeAddr("wraparoundOwner"); + harness.addOwner(owner); + + assertTrue(harness.asOwnerSet(owner) == OwnerSet.wrap(1)); + assertTrue(harness.getAllOwners() == (occupiedTop | OwnerSet.wrap(1))); + } + + function test_addOwner_wrapsAndSkipsOccupiedLowBitsBeforeFindingFree() public { + // bit 159 (top) and bits 0,1,2 are occupied; bit 3 is the first free slot + // once the scan wraps around and walks past the occupied low bits + OwnerSet occupied = OwnerSet.wrap((uint160(1) << 159) | uint160(0x7)); + harness.seedBitState(159, occupied); + + address owner = makeAddr("wraparoundOwner2"); + harness.addOwner(owner); + + assertTrue(harness.asOwnerSet(owner) == OwnerSet.wrap(uint160(1) << 3)); + assertTrue(harness.getAllOwners() == (occupied | OwnerSet.wrap(uint160(1) << 3))); + } + + function test_addOwner_wrapsAllTheWayAroundToFindOnlyFreeBit() public { + // every bit is occupied except bit 5; the cursor starts at the very top + // (159), which is itself occupied, so the scan must wrap from 159 back to + // 0 and then walk up through the occupied low bits before landing on 5 + OwnerSet almostFull = FULL_SET ^ OwnerSet.wrap(uint160(1) << 5); + harness.seedBitState(159, almostFull); + + address owner = makeAddr("lastFreeBitOwner"); + harness.addOwner(owner); + + assertTrue(harness.asOwnerSet(owner) == OwnerSet.wrap(uint160(1) << 5)); + assertTrue(harness.getAllOwners() == FULL_SET); + } +} diff --git a/test/UnanimousGovernance.t.sol b/test/UnanimousGovernance.t.sol new file mode 100644 index 0000000..db33c3b --- /dev/null +++ b/test/UnanimousGovernance.t.sol @@ -0,0 +1,320 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT +pragma solidity ^0.8.36; + +import {Test} from "forge-std/Test.sol"; +import {Epoch, currentEpoch} from "../src/lib/Epoch.sol"; +import {EMPTY_SET, OwnerSet} from "../src/lib/OwnerSet.sol"; +import {PendingTask, PendingTaskLibrary} from "../src/lib/PendingTask.sol"; +import {OwnersLibrary} from "../src/lib/Owners.sol"; +import {UnanimousGovernance} from "../src/lib/UnanimousGovernance.sol"; + +// wraps the internal modifier with two administrator actions so vm.expectRevert/vm.expectEmit +// have a real call frame to target. Mirrors planned usage: each action's hold is a constant +// baked into its `unanimous` invocation, and the taskId is just keccak256(msg.data), i.e. the +// method selector plus its parameters. +contract UnanimousGovernanceHarness is UnanimousGovernance { + Epoch public constant ADD_OWNER_HOLD = Epoch.wrap(0); + Epoch public constant REMOVE_OWNER_HOLD = Epoch.wrap(10); + + // test-only bootstrap: seeds the owner set without going through the unanimous modifier + function seedOwner(address owner) external { + OwnersLibrary.addOwner(owner); + } + + function isOwner(address someone) external view returns (bool) { + return OwnersLibrary.isOwner(someone); + } + + function asOwnerSet(address owner) external view returns (OwnerSet) { + return OwnersLibrary.asOwnerSet(owner); + } + + // exposes raw pending-task state so tests can assert on it directly, + // rather than only inferring it from events or final owner-set membership + function getPendingTask(bytes32 taskId) external view returns (Epoch modified, OwnerSet approvals) { + PendingTask memory task = PendingTaskLibrary.getTasksSlot()[taskId].task; + return (task.modified, task.approvals); + } + + function addOwnerTaskId(address owner) public pure returns (bytes32) { + return keccak256(abi.encodeWithSelector(this.addOwner.selector, owner)); + } + + function removeOwnerTaskId(address owner) public pure returns (bytes32) { + return keccak256(abi.encodeWithSelector(this.removeOwner.selector, owner)); + } + + function addOwner(address owner) external unanimous(keccak256(msg.data), ADD_OWNER_HOLD) { + OwnersLibrary.addOwner(owner); + } + + function removeOwner(address owner) external unanimous(keccak256(msg.data), REMOVE_OWNER_HOLD) { + OwnersLibrary.removeOwner(owner); + } + + function vetoAddOwner(address owner) external { + _veto(addOwnerTaskId(owner)); + } + + function vetoRemoveOwner(address owner) external { + _veto(removeOwnerTaskId(owner)); + } +} + +contract UnanimousGovernanceTest is Test { + UnanimousGovernanceHarness harness; + + address alice = makeAddr("alice"); + address bob = makeAddr("bob"); + address carol = makeAddr("carol"); + address stranger = makeAddr("stranger"); + address newOwner = makeAddr("newOwner"); + + function setUp() public { + harness = new UnanimousGovernanceHarness(); + } + + function test_addOwner_singleOwner_executesImmediately() public { + harness.seedOwner(alice); + bytes32 taskId = harness.addOwnerTaskId(newOwner); + + vm.expectEmit(true, false, false, false, address(harness)); + emit UnanimousGovernance.Submitted(taskId); + vm.expectEmit(true, true, false, false, address(harness)); + emit UnanimousGovernance.Approved(taskId, alice); + vm.expectEmit(true, false, false, false, address(harness)); + emit OwnersLibrary.OwnerAdded(newOwner); + + vm.prank(alice); + harness.addOwner(newOwner); + + assertTrue(harness.isOwner(newOwner)); + } + + function test_addOwner_twoOwners_requiresBothApprovals() public { + harness.seedOwner(alice); + harness.seedOwner(bob); + bytes32 taskId = harness.addOwnerTaskId(newOwner); + + vm.expectEmit(true, false, false, false, address(harness)); + emit UnanimousGovernance.Submitted(taskId); + vm.expectEmit(true, true, false, false, address(harness)); + emit UnanimousGovernance.Approved(taskId, alice); + vm.prank(alice); + harness.addOwner(newOwner); + + // only one of two owners has approved: not yet executed + assertFalse(harness.isOwner(newOwner)); + (Epoch modified, OwnerSet approvals) = harness.getPendingTask(taskId); + assertTrue(modified == currentEpoch()); + assertTrue(approvals == harness.asOwnerSet(alice)); + + vm.expectEmit(true, true, false, false, address(harness)); + emit UnanimousGovernance.Approved(taskId, bob); + vm.expectEmit(true, false, false, false, address(harness)); + emit OwnersLibrary.OwnerAdded(newOwner); + vm.prank(bob); + harness.addOwner(newOwner); + + assertTrue(harness.isOwner(newOwner)); + // executed: the task record is deleted + (modified, approvals) = harness.getPendingTask(taskId); + assertTrue(modified == Epoch.wrap(0)); + assertTrue(approvals == EMPTY_SET); + } + + function test_addOwner_threeOwners_partialApprovalDoesNotExecute() public { + harness.seedOwner(alice); + harness.seedOwner(bob); + harness.seedOwner(carol); + bytes32 taskId = harness.addOwnerTaskId(newOwner); + + vm.prank(alice); + harness.addOwner(newOwner); + vm.prank(bob); + harness.addOwner(newOwner); + assertFalse(harness.isOwner(newOwner)); + (Epoch modified, OwnerSet approvals) = harness.getPendingTask(taskId); + assertTrue(modified == currentEpoch()); + assertTrue(approvals == (harness.asOwnerSet(alice) | harness.asOwnerSet(bob))); + + vm.prank(carol); + harness.addOwner(newOwner); + assertTrue(harness.isOwner(newOwner)); + (modified, approvals) = harness.getPendingTask(taskId); + assertTrue(modified == Epoch.wrap(0)); + assertTrue(approvals == EMPTY_SET); + } + + function test_addOwner_nonOwner_cannotApprove() public { + harness.seedOwner(alice); + + vm.expectRevert(abi.encodeWithSelector(UnanimousGovernance.NotOwner.selector, stranger)); + vm.prank(stranger); + harness.addOwner(newOwner); + } + + function test_removeOwner_withHold_delaysExecutionUntilPermissionlessFinalize() public { + harness.seedOwner(alice); + harness.seedOwner(bob); + bytes32 taskId = harness.removeOwnerTaskId(bob); + + // unanimous requires every current owner to approve, including the + // one being removed, so bob must approve his own removal + vm.prank(alice); + harness.removeOwner(bob); + + Epoch approvalEpoch = currentEpoch(); + vm.prank(bob); + harness.removeOwner(bob); + + // both owners approved, but the hold delays execution + assertTrue(harness.isOwner(bob)); + (Epoch modified, OwnerSet approvals) = harness.getPendingTask(taskId); + assertTrue(modified == approvalEpoch); + assertTrue(approvals == (harness.asOwnerSet(alice) | harness.asOwnerSet(bob))); + + // too early: reverts even for an owner + Epoch until = approvalEpoch + harness.REMOVE_OWNER_HOLD(); + vm.expectRevert(abi.encodeWithSelector(UnanimousGovernance.HoldUntil.selector, until)); + vm.prank(alice); + harness.removeOwner(bob); + + // the revert left the pending task untouched + (modified, approvals) = harness.getPendingTask(taskId); + assertTrue(modified == approvalEpoch); + assertTrue(approvals == (harness.asOwnerSet(alice) | harness.asOwnerSet(bob))); + + vm.roll(Epoch.unwrap(approvalEpoch + harness.REMOVE_OWNER_HOLD())); + + // permissionless: a non-owner can finalize once the hold has elapsed + vm.expectEmit(true, false, false, false, address(harness)); + emit OwnersLibrary.OwnerRemoved(bob); + vm.prank(stranger); + harness.removeOwner(bob); + + assertFalse(harness.isOwner(bob)); + (modified, approvals) = harness.getPendingTask(taskId); + assertTrue(modified == Epoch.wrap(0)); + assertTrue(approvals == EMPTY_SET); + } + + function test_veto_duringHoldingPeriod_cancelsBeforeFinalize() public { + harness.seedOwner(alice); + harness.seedOwner(bob); + bytes32 taskId = harness.removeOwnerTaskId(bob); + + vm.prank(alice); + harness.removeOwner(bob); + Epoch approvalEpoch = currentEpoch(); + vm.prank(bob); + harness.removeOwner(bob); + + // fully approved, but still within the hold: veto is still possible + assertTrue(harness.isOwner(bob)); + (Epoch modified, OwnerSet approvals) = harness.getPendingTask(taskId); + assertTrue(modified == approvalEpoch); + assertTrue(approvals == (harness.asOwnerSet(alice) | harness.asOwnerSet(bob))); + + vm.expectEmit(true, true, false, false, address(harness)); + emit UnanimousGovernance.Rejected(taskId, alice); + vm.prank(alice); + harness.vetoRemoveOwner(bob); + + assertTrue(harness.isOwner(bob)); + (modified, approvals) = harness.getPendingTask(taskId); + assertTrue(modified == Epoch.wrap(0)); + assertTrue(approvals == EMPTY_SET); + + // even once the original hold window would have elapsed, the task + // was cleared, so finalizing now just restarts approval from scratch + vm.roll(Epoch.unwrap(currentEpoch() + harness.REMOVE_OWNER_HOLD())); + + vm.expectEmit(true, false, false, false, address(harness)); + emit UnanimousGovernance.Submitted(taskId); + vm.prank(alice); + harness.removeOwner(bob); + + assertTrue(harness.isOwner(bob)); + (modified, approvals) = harness.getPendingTask(taskId); + assertTrue(modified == currentEpoch()); + assertTrue(approvals == harness.asOwnerSet(alice)); + } + + function test_veto_resetsPendingTask() public { + harness.seedOwner(alice); + harness.seedOwner(bob); + bytes32 taskId = harness.addOwnerTaskId(newOwner); + + vm.prank(alice); + harness.addOwner(newOwner); + (Epoch modified, OwnerSet approvals) = harness.getPendingTask(taskId); + assertTrue(modified == currentEpoch()); + assertTrue(approvals == harness.asOwnerSet(alice)); + + vm.expectEmit(true, true, false, false, address(harness)); + emit UnanimousGovernance.Rejected(taskId, bob); + vm.prank(bob); + harness.vetoAddOwner(newOwner); + + (modified, approvals) = harness.getPendingTask(taskId); + assertTrue(modified == Epoch.wrap(0)); + assertTrue(approvals == EMPTY_SET); + + // the task was cleared: a fresh Submitted event fires on the next approval + vm.expectEmit(true, false, false, false, address(harness)); + emit UnanimousGovernance.Submitted(taskId); + vm.prank(alice); + harness.addOwner(newOwner); + + assertFalse(harness.isOwner(newOwner)); + (modified, approvals) = harness.getPendingTask(taskId); + assertTrue(modified == currentEpoch()); + assertTrue(approvals == harness.asOwnerSet(alice)); + } + + function test_veto_onlyOwnerCanVeto() public { + harness.seedOwner(alice); + harness.seedOwner(bob); + + vm.prank(alice); + harness.addOwner(newOwner); + + vm.expectRevert(abi.encodeWithSelector(UnanimousGovernance.NotOwner.selector, stranger)); + vm.prank(stranger); + harness.vetoAddOwner(newOwner); + } + + function test_doubleApproval_byOwner_reverts() public { + harness.seedOwner(alice); + harness.seedOwner(bob); + harness.seedOwner(carol); + + // alice approves, then tries to approve again before the others have + // weighed in: the bitmask approval accounting now rejects this outright, + // instead of silently xor-cancelling her first approval + vm.prank(alice); + harness.addOwner(newOwner); + + vm.expectRevert(UnanimousGovernance.AlreadyApproved.selector); + vm.prank(alice); + harness.addOwner(newOwner); + + bytes32 taskId = harness.addOwnerTaskId(newOwner); + (Epoch modified, OwnerSet approvals) = harness.getPendingTask(taskId); + assertTrue(modified == currentEpoch()); + assertTrue(approvals == harness.asOwnerSet(alice)); + + // the revert didn't consume or corrupt the pending approval: bob and + // carol can still approve and execute normally + vm.prank(bob); + harness.addOwner(newOwner); + vm.prank(carol); + harness.addOwner(newOwner); + + assertTrue(harness.isOwner(newOwner)); + (modified, approvals) = harness.getPendingTask(taskId); + assertTrue(modified == Epoch.wrap(0)); + assertTrue(approvals == EMPTY_SET); + } +}