-
Notifications
You must be signed in to change notification settings - Fork 2
feat: two-safe rule #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
d8e42dd
wip
wjmelements b34b0a7
define errors for unanimous
wjmelements f46d381
fix(AddressXorSet): correct contains() bitmap reconstruction
wjmelements 3769eb3
fix(Owners): detect xor-collision owners and add test coverage
wjmelements 4ad4963
test(TwoSafeRule): add coverage for the unanimous modifier and veto
wjmelements 25affee
refactor: rename TwoSafeRuler to UnanimousGovernance
wjmelements 4ae6fac
docs(AddressXorSet): document XOR vs Bloom set tradeoff
wjmelements d90c8b6
refactor(Owners): replace AddressXorSet with bitmask-based OwnerSet
wjmelements acc647c
chore(Owners): drop dead asOwnerSet overload and add dev notes
wjmelements 540a3e6
refactor(UnanimousGovernance): simplify hold check and rename sentinel
wjmelements ca82eb8
refactor: rename bit-tracking variables for clarity
wjmelements d2b9581
docs: add natspec for currentEpoch, unanimous, _veto, and owner funct…
wjmelements 034c8f8
feat(ci): add lint and test workflows
wjmelements 8b3b458
chore(ci): bump actions/checkout to v7
wjmelements ec1799a
chore(ci): upgrade forge to 1.7.1 and fix lint CI
wjmelements 46eae2d
fix(ci): install dependencies before linting
wjmelements c4f6c6c
doc: recommend keccak256(msg.data) for taskId
wjmelements caf47cd
feat: IsASafe.isProbablyASafe
wjmelements d5ca04b
fix(owners): prevent removing the last owner
wjmelements File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| # forge | ||
| out/ | ||
| cache/ | ||
|
|
||
| # VIM | ||
| *.swp |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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" | ||
| } | ||
| } | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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", | ||
| ] |
Submodule safe-smart-account
added at
77901a
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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); | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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)); | ||
| } | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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)); | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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)); | ||
|
rvagg marked this conversation as resolved.
|
||
|
|
||
| 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); | ||
| } | ||
| } | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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; | ||
| } | ||
|
rvagg marked this conversation as resolved.
|
||
|
|
||
| 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 | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.