From a2c82e56b3bf0b44fa9b24187ecc35834e274443 Mon Sep 17 00:00:00 2001 From: Wieedze Date: Mon, 18 May 2026 12:27:16 +0200 Subject: [PATCH 1/2] feat(versioned-proxy)!: Role 1 multi-admin (drop 2-step transfer) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BREAKING: replaces the single-slot proxyAdmin + 2-step ownership transfer with a whitelist of proxyAdmins (mirrors Role 2 fee-admins). Storage layout, constructor signature, and admin API all change. Existing deployed proxies are incompatible — fresh deploys required. Rationale: the user wanted Role 1 to feel symmetric with Role 2 (a list of admins, instant grant/revoke). The 2-step transfer existed to prevent fat-fingered transfers of the upgrade authority. That safety is now delegated to putting a Safe multisig in the proxyAdmins list — the Safe's internal quorum provides the ceremony. Contract changes (src/IntuitionVersionedFeeProxy.sol): - Storage: drop `address proxyAdmin` + `address pendingProxyAdmin`, add `mapping(address=>bool) proxyAdmins` + `uint256 proxyAdminCount` - Constructor: `address admin` → `address[] initialProxyAdmins` (reverts on empty list or any zero entry, dedupes silently) - Functions: drop `transferProxyAdmin` + `acceptProxyAdmin`, add `setProxyAdmin(address, bool)` with: * idempotent reject (revert `ProxyAdminAlreadySet` if status already matches — keeps the on-chain event log truthful) * last-admin guard (revert `LastProxyAdmin` if revoke would empty the whitelist — anti-lock-out) - Modifier `onlyProxyAdmin`: mapping lookup instead of address == - Views: drop `proxyAdmin()` + `pendingProxyAdmin()`, add `isProxyAdmin(address)` + `proxyAdminCount()`. The full admin list is reconstructed from events (same pattern as Role 2) Interface (src/interfaces/IIntuitionVersionedFeeProxy.sol): - Events: drop `ProxyAdminTransferStarted/Transferred`, add `ProxyAdminGranted(address)` + `ProxyAdminRevoked(address)` - Function/view changes mirror the contract Errors (src/libraries/Errors.sol): - Drop `VersionedFeeProxy_NotPendingProxyAdmin` - Add `VersionedFeeProxy_ProxyAdminAlreadySet` (idempotent reject) - Add `VersionedFeeProxy_LastProxyAdmin` (anti-lock-out) Factory (src/IntuitionFeeProxyFactory.sol): - `createProxy` signature unchanged - Now passes the full `initialAdmins[]` to the proxy constructor. Both roles seed from the same list — diverge via setters later if your team / Safe layout needs it. Tests: - All inline `VersionedFactory.deploy(scalar, ...)` updated to `deploy([addrs], ...)` across 4 test files - IntuitionVersionedFeeProxy.test.ts: 2-step transfer block replaced by 7-test setProxyAdmin block covering grant, revoke, idempotent guards (×2), last-admin guard, grant+revoke roundtrip, non-admin reject. supportsInterface selector list updated. - IntuitionFeeProxyFactory.test.ts: asserts both admins are now in Role 1 (proxyAdminCount == initialAdmins.length) 226 tests passing. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/IntuitionFeeProxyFactory.sol | 9 +- .../src/IntuitionVersionedFeeProxy.sol | 115 ++++++++------ .../IIntuitionVersionedFeeProxy.sol | 37 +++-- packages/contracts/src/libraries/Errors.sol | 12 +- .../test/IntuitionFeeProxyFactory.test.ts | 6 +- .../test/IntuitionFeeProxyV2.test.ts | 14 +- .../test/IntuitionFeeProxyV2Sponsored.test.ts | 2 +- .../test/IntuitionFeeProxyV2_1.test.ts | 4 +- .../test/IntuitionVersionedFeeProxy.test.ts | 147 ++++++++++-------- 9 files changed, 206 insertions(+), 140 deletions(-) diff --git a/packages/contracts/src/IntuitionFeeProxyFactory.sol b/packages/contracts/src/IntuitionFeeProxyFactory.sol index a94b380..f4e2a40 100644 --- a/packages/contracts/src/IntuitionFeeProxyFactory.sol +++ b/packages/contracts/src/IntuitionFeeProxyFactory.sol @@ -212,10 +212,13 @@ contract IntuitionFeeProxyFactory is (ethMultiVault, depositFixedFee, depositPercentageFee, initialAdmins) ); - address proxyAdmin_ = initialAdmins.length > 0 ? initialAdmins[0] : address(0); - + // Pass the full initialAdmins array to the proxy's Role 1 whitelist. + // Both Role 1 (proxyAdmins, upgrade authority) and Role 2 + // (whitelistedAdmins, fees) start from the same set — admins can + // diverge the two later via setProxyAdmin / setWhitelistedAdmin + // if their team / safe layout needs it. proxy = address( - new IntuitionVersionedFeeProxy(proxyAdmin_, version, impl, initData, name) + new IntuitionVersionedFeeProxy(initialAdmins, version, impl, initData, name) ); proxiesByDeployer[msg.sender].push(proxy); diff --git a/packages/contracts/src/IntuitionVersionedFeeProxy.sol b/packages/contracts/src/IntuitionVersionedFeeProxy.sol index fcfdeee..f61aa45 100644 --- a/packages/contracts/src/IntuitionVersionedFeeProxy.sol +++ b/packages/contracts/src/IntuitionVersionedFeeProxy.sol @@ -16,9 +16,11 @@ import {Errors} from "./libraries/Errors.sol"; /// with the logic implementation's regular storage. /// - No `receive()`: direct ETH transfers revert. All legitimate fee flows /// carry calldata, so a bare transfer would only be a mis-send. -/// - Admin gating is a single `proxyAdmin` address; users should point it at a -/// multisig. Transferable via `transferProxyAdmin`. -/// - ⚠️ **`name` is admin-controlled metadata, NOT a trust anchor.** The +/// - Admin gating is a **whitelist of proxyAdmins** (any of them can act). +/// Grant / revoke via `setProxyAdmin`. The last remaining admin cannot +/// self-revoke (a runtime guard prevents accidental lock-out). Recommended +/// setup: at least one Gnosis Safe in the list. +/// - ⚠️ **`name` is admin-controlled metadata, NOT a trust anchor.** Any /// proxy-admin can rename the proxy at any time — including to mimic a /// known brand. Consumers MUST derive identity / "official" status from /// the proxy address itself (e.g. the Factory's `isProxyFromFactory` @@ -52,14 +54,18 @@ contract IntuitionVersionedFeeProxy is IIntuitionVersionedFeeProxy { bytes32[] versionList; mapping(bytes32 => address) implementations; mapping(bytes32 => bool) versionExists; - address proxyAdmin; + // Multi-admin whitelist for Role 1 (upgrade authority). Any admin in + // the mapping can register versions, set default, rename, and grant / + // revoke other proxyAdmins. Mirrors the Role 2 (fee-admin) shape so + // both rotation flows feel symmetric. The pre-rotation single-slot + // 2-step model (proxyAdmin + pendingProxyAdmin) was dropped — the + // safety it provided is delivered by the recommendation to put a + // Safe multisig in the list, which gives N-of-M signing semantics. + mapping(address => bool) proxyAdmins; + // Count tracked alongside the mapping so the last-admin guard is O(1). + // Replaces the 2-step pending-admin pattern. + uint256 proxyAdminCount; bytes32 name; - // 2-step admin transfer: `pendingProxyAdmin` holds the candidate set by - // the current admin via `transferProxyAdmin`. Only that address can - // promote itself via `acceptProxyAdmin`. Prevents fat-fingered - // transfers to lost / wrong addresses. Appended — ERC-7201 namespaced - // slot mask (~0xff) reserves 256 slots, plenty of room. - address pendingProxyAdmin; } function _layout() private pure returns (Layout storage s) { @@ -72,7 +78,7 @@ contract IntuitionVersionedFeeProxy is IIntuitionVersionedFeeProxy { // ============ Modifiers ============ modifier onlyProxyAdmin() { - if (msg.sender != _layout().proxyAdmin) { + if (!_layout().proxyAdmins[msg.sender]) { revert Errors.VersionedFeeProxy_NotProxyAdmin(); } _; @@ -80,19 +86,22 @@ contract IntuitionVersionedFeeProxy is IIntuitionVersionedFeeProxy { // ============ Constructor ============ - /// @param admin Proxy-admin authorized for version management (recommend: Safe) + /// @param initialProxyAdmins Initial whitelist (at least one non-zero, no + /// duplicates). For production: include at least one Safe multisig. /// @param initialVersion Identifier for the initial registered version (e.g. bytes32("v2.0.0")) /// @param initialImpl Address of the initial logic implementation /// @param initData Calldata forwarded via delegatecall to initialize the logic - /// @param initialName Optional human-readable name (bytes32 — empty for none, editable by proxyAdmin via setName) + /// @param initialName Optional human-readable name (bytes32 — empty for none, editable by a proxyAdmin via setName) constructor( - address admin, + address[] memory initialProxyAdmins, bytes32 initialVersion, address initialImpl, bytes memory initData, bytes32 initialName ) { - if (admin == address(0)) revert Errors.IntuitionFeeProxy_ZeroAddress(); + if (initialProxyAdmins.length == 0) { + revert Errors.IntuitionFeeProxy_NoAdminsProvided(); + } if (initialVersion == bytes32(0)) revert Errors.VersionedFeeProxy_InvalidVersion(); if (initialImpl == address(0) || initialImpl.code.length == 0) { revert Errors.VersionedFeeProxy_InvalidImplementation(); @@ -107,7 +116,23 @@ contract IntuitionVersionedFeeProxy is IIntuitionVersionedFeeProxy { } Layout storage s = _layout(); - s.proxyAdmin = admin; + + // Seed the proxyAdmin whitelist. Reject zero addresses and dedupe + // (silently — duplicates in the list would otherwise inflate the + // count and break the last-admin guard). + uint256 added; + uint256 len = initialProxyAdmins.length; + for (uint256 i = 0; i < len; i++) { + address a = initialProxyAdmins[i]; + if (a == address(0)) revert Errors.IntuitionFeeProxy_ZeroAddress(); + if (!s.proxyAdmins[a]) { + s.proxyAdmins[a] = true; + emit ProxyAdminGranted(a); + unchecked { ++added; } + } + } + s.proxyAdminCount = added; + s.implementations[initialVersion] = initialImpl; s.versionExists[initialVersion] = true; s.versionList.push(initialVersion); @@ -116,7 +141,6 @@ contract IntuitionVersionedFeeProxy is IIntuitionVersionedFeeProxy { _mirrorEip1967(initialImpl); - emit ProxyAdminTransferred(address(0), admin); emit VersionRegistered(initialVersion, initialImpl); emit DefaultVersionChanged(bytes32(0), initialVersion); if (initialName != bytes32(0)) { @@ -213,11 +237,11 @@ contract IntuitionVersionedFeeProxy is IIntuitionVersionedFeeProxy { /// `receiver == msg.sender || approvals[receiver][msg.sender] & DEPOSIT`, /// and `msg.sender` from the MultiVault's POV is this proxy. A new /// impl could legally route deposits to any address that has approved - /// this proxy on the MultiVault. The trust model relies on - /// `proxyAdmin` being a Safe multisig (M-of-N, M ≥ 3 recommended). - /// Users who want to be insulated from default-version switches MUST - /// pin a specific version via `executeAtVersion(version, …)` rather - /// than relying on the fallback. + /// this proxy on the MultiVault. The trust model relies on the + /// `proxyAdmins` whitelist including a Safe multisig (M-of-N, M ≥ 3 + /// recommended). Users who want to be insulated from default-version + /// switches MUST pin a specific version via `executeAtVersion(version, …)` + /// rather than relying on the fallback. function setDefaultVersion(bytes32 version) external onlyProxyAdmin { Layout storage s = _layout(); if (!s.versionExists[version]) revert Errors.VersionedFeeProxy_VersionNotFound(); @@ -229,22 +253,25 @@ contract IntuitionVersionedFeeProxy is IIntuitionVersionedFeeProxy { } /// @inheritdoc IIntuitionVersionedFeeProxy - function transferProxyAdmin(address newAdmin) external onlyProxyAdmin { - if (newAdmin == address(0)) revert Errors.IntuitionFeeProxy_ZeroAddress(); - Layout storage s = _layout(); - s.pendingProxyAdmin = newAdmin; - emit ProxyAdminTransferStarted(s.proxyAdmin, newAdmin); - } - - /// @inheritdoc IIntuitionVersionedFeeProxy - function acceptProxyAdmin() external { + function setProxyAdmin(address admin, bool status) external onlyProxyAdmin { + if (admin == address(0)) revert Errors.IntuitionFeeProxy_ZeroAddress(); Layout storage s = _layout(); - address pending = s.pendingProxyAdmin; - if (msg.sender != pending) revert Errors.VersionedFeeProxy_NotPendingProxyAdmin(); - address old = s.proxyAdmin; - s.proxyAdmin = pending; - delete s.pendingProxyAdmin; - emit ProxyAdminTransferred(old, pending); + bool current = s.proxyAdmins[admin]; + if (current == status) revert Errors.VersionedFeeProxy_ProxyAdminAlreadySet(); + // Last-admin guard: refuse to revoke the only remaining proxyAdmin, + // otherwise the role would be permanently lost (no one can grant it + // back). Mirrors the Role 2 (fee-admin) `adminCount > 0` invariant. + if (!status && s.proxyAdminCount == 1) { + revert Errors.VersionedFeeProxy_LastProxyAdmin(); + } + s.proxyAdmins[admin] = status; + if (status) { + unchecked { ++s.proxyAdminCount; } + emit ProxyAdminGranted(admin); + } else { + unchecked { --s.proxyAdminCount; } + emit ProxyAdminRevoked(admin); + } } /// @inheritdoc IIntuitionVersionedFeeProxy @@ -279,13 +306,13 @@ contract IntuitionVersionedFeeProxy is IIntuitionVersionedFeeProxy { } /// @inheritdoc IIntuitionVersionedFeeProxy - function proxyAdmin() external view returns (address) { - return _layout().proxyAdmin; + function isProxyAdmin(address candidate) external view returns (bool) { + return _layout().proxyAdmins[candidate]; } - /// @notice The candidate admin awaiting acceptance, or address(0) if none. - function pendingProxyAdmin() external view returns (address) { - return _layout().pendingProxyAdmin; + /// @inheritdoc IIntuitionVersionedFeeProxy + function proxyAdminCount() external view returns (uint256) { + return _layout().proxyAdminCount; } // ============ ERC-165 ============ @@ -355,10 +382,6 @@ contract IntuitionVersionedFeeProxy is IIntuitionVersionedFeeProxy { } } - // NOTE: no `receive()` — ETH transfers without calldata revert. Fee flows - // all come with calldata (createAtoms / deposit / …), so bare transfers - // would only be mis-sends. - // ============ Internal ============ /// @dev Writes `impl` into the EIP-1967 implementation slot and emits diff --git a/packages/contracts/src/interfaces/IIntuitionVersionedFeeProxy.sol b/packages/contracts/src/interfaces/IIntuitionVersionedFeeProxy.sol index b3b1427..048f7f3 100644 --- a/packages/contracts/src/interfaces/IIntuitionVersionedFeeProxy.sol +++ b/packages/contracts/src/interfaces/IIntuitionVersionedFeeProxy.sol @@ -13,10 +13,11 @@ interface IIntuitionVersionedFeeProxy { event VersionRegistered(bytes32 indexed version, address indexed implementation); event VersionRemoved(bytes32 indexed version); event DefaultVersionChanged(bytes32 indexed oldVersion, bytes32 indexed newVersion); - event ProxyAdminTransferred(address indexed oldAdmin, address indexed newAdmin); - /// @notice Emitted when a new proxy-admin transfer is initiated (pending the new admin's acceptance). - event ProxyAdminTransferStarted(address indexed currentAdmin, address indexed pendingAdmin); + /// @notice Emitted when an address gains the proxyAdmin role. + event ProxyAdminGranted(address indexed admin); + /// @notice Emitted when an address loses the proxyAdmin role. + event ProxyAdminRevoked(address indexed admin); /// @notice Emitted when the proxy's human-readable name is set or changed. event NameChanged(bytes32 indexed oldName, bytes32 indexed newName); @@ -27,17 +28,20 @@ interface IIntuitionVersionedFeeProxy { function removeVersion(bytes32 version) external; function setDefaultVersion(bytes32 version) external; - /// @notice Initiate a proxy-admin transfer. The new admin takes over only - /// after they call `acceptProxyAdmin`. Passing `address(0)` reverts. - /// Calling again before acceptance overwrites the pending admin. - function transferProxyAdmin(address newAdmin) external; - - /// @notice Finalize a pending proxy-admin transfer. Only callable by the - /// address set as `pendingProxyAdmin` via `transferProxyAdmin`. - function acceptProxyAdmin() external; + /// @notice Grant or revoke the proxyAdmin role for an address. + /// `status = true` adds; `status = false` removes. Idempotent + /// calls (status already matches) revert with + /// `ProxyAdminAlreadySet`. The last remaining admin cannot + /// self-revoke (revert `LastProxyAdmin`). + /// @dev Any current proxyAdmin can call this. For production, the + /// recommended setup is to keep at least one Gnosis Safe + /// multisig in the whitelist — the Safe's internal quorum + /// provides the safety net the previous 2-step transfer flow + /// used to enforce. + function setProxyAdmin(address admin, bool status) external; /// @notice Set or rename the proxy's human-readable label. Pass bytes32(0) to clear. - /// @dev ⚠️ **The name is NOT a trust anchor.** The proxy-admin can + /// @dev ⚠️ **The name is NOT a trust anchor.** Any proxy-admin can /// rename the proxy at any time — including to mimic a known /// brand. Frontends MUST NOT use `name` to derive an "official" /// / "verified" badge. Use the Factory's `isProxyFromFactory` @@ -50,8 +54,13 @@ interface IIntuitionVersionedFeeProxy { function getImplementation(bytes32 version) external view returns (address); function getDefaultVersion() external view returns (bytes32); function getVersions() external view returns (bytes32[] memory); - function proxyAdmin() external view returns (address); - function pendingProxyAdmin() external view returns (address); + + /// @notice Returns true if `candidate` is currently a proxyAdmin. + function isProxyAdmin(address candidate) external view returns (bool); + + /// @notice Returns the number of addresses currently holding the proxyAdmin role. + function proxyAdminCount() external view returns (uint256); + /// @notice Returns the proxy's current human-readable label. /// @dev ⚠️ See the warning on `setName` — a name is admin-controlled /// metadata, never a source of trust. diff --git a/packages/contracts/src/libraries/Errors.sol b/packages/contracts/src/libraries/Errors.sol index 8765e18..5fffc97 100644 --- a/packages/contracts/src/libraries/Errors.sol +++ b/packages/contracts/src/libraries/Errors.sol @@ -99,8 +99,16 @@ library Errors { /// @notice Delegatecall into the versioned implementation failed without returndata error VersionedFeeProxy_DelegateCallFailed(); - /// @notice `acceptProxyAdmin` called by an address that is not the pending admin - error VersionedFeeProxy_NotPendingProxyAdmin(); + /// @notice `setProxyAdmin` called with status matching the current state + /// (granting an already-admin or revoking a non-admin). Idempotent + /// calls revert so the on-chain log of grant/revoke events stays + /// truthful (every emit corresponds to a real state change). + error VersionedFeeProxy_ProxyAdminAlreadySet(); + + /// @notice `setProxyAdmin(admin, false)` would leave the proxy with zero + /// admins, permanently locking the role. The last remaining + /// proxyAdmin cannot self-revoke. + error VersionedFeeProxy_LastProxyAdmin(); /// @notice The new impl's `STORAGE_COMPAT_ID` differs from the proxy's /// reference (default version's impl). Registering an incompatible diff --git a/packages/contracts/test/IntuitionFeeProxyFactory.test.ts b/packages/contracts/test/IntuitionFeeProxyFactory.test.ts index d299599..841b63d 100644 --- a/packages/contracts/test/IntuitionFeeProxyFactory.test.ts +++ b/packages/contracts/test/IntuitionFeeProxyFactory.test.ts @@ -162,7 +162,11 @@ describe("IntuitionFeeProxyFactory (UUPS)", function () { "IntuitionVersionedFeeProxy", proxyAddr, )) as unknown as IntuitionVersionedFeeProxy; - expect(await versioned.proxyAdmin()).to.equal(admin1.address); // = initialAdmins[0] + // Factory now passes the full initialAdmins array to Role 1 — every + // initial admin is also a proxyAdmin. proxyAdminCount mirrors Role 2. + expect(await versioned.isProxyAdmin(admin1.address)).to.equal(true); + expect(await versioned.isProxyAdmin(admin2.address)).to.equal(true); + expect(await versioned.proxyAdminCount()).to.equal(2n); expect(await versioned.getDefaultVersion()).to.equal(V2); expect(await versioned.getVersions()).to.deep.equal([V2]); expect(await versioned.getImplementation(V2)).to.equal(await implV2.getAddress()); diff --git a/packages/contracts/test/IntuitionFeeProxyV2.test.ts b/packages/contracts/test/IntuitionFeeProxyV2.test.ts index 11e3d50..2118ced 100644 --- a/packages/contracts/test/IntuitionFeeProxyV2.test.ts +++ b/packages/contracts/test/IntuitionFeeProxyV2.test.ts @@ -43,7 +43,7 @@ describe("IntuitionFeeProxyV2", function () { // delegating the initializer call on deployment. const VersionedFactory = await ethers.getContractFactory("IntuitionVersionedFeeProxy"); const proxyDeployment = await VersionedFactory.deploy( - admin1.address, // proxy-admin = first admin in V2 admins list + [admin1.address], // proxyAdmins whitelist (post Role 1 multi-admin) INITIAL_VERSION, await impl.getAddress(), initData, @@ -125,7 +125,7 @@ describe("IntuitionFeeProxyV2", function () { ]); const VersionedFactory = await ethers.getContractFactory("IntuitionVersionedFeeProxy"); await expect( - VersionedFactory.deploy(admin1.address, INITIAL_VERSION, await impl.getAddress(), initData, ethers.ZeroHash) + VersionedFactory.deploy([admin1.address], INITIAL_VERSION, await impl.getAddress(), initData, ethers.ZeroHash) ).to.be.revertedWithCustomError(impl, "IntuitionFeeProxy_InvalidMultiVaultAddress"); }); @@ -141,7 +141,7 @@ describe("IntuitionFeeProxyV2", function () { ]); const VersionedFactory = await ethers.getContractFactory("IntuitionVersionedFeeProxy"); await expect( - VersionedFactory.deploy(admin1.address, INITIAL_VERSION, await impl.getAddress(), initData, ethers.ZeroHash) + VersionedFactory.deploy([admin1.address], INITIAL_VERSION, await impl.getAddress(), initData, ethers.ZeroHash) ).to.be.revertedWithCustomError(impl, "IntuitionFeeProxy_FeePercentageTooHigh"); }); @@ -157,7 +157,7 @@ describe("IntuitionFeeProxyV2", function () { ]); const VersionedFactory = await ethers.getContractFactory("IntuitionVersionedFeeProxy"); await expect( - VersionedFactory.deploy(admin1.address, INITIAL_VERSION, await impl.getAddress(), initData, ethers.ZeroHash) + VersionedFactory.deploy([admin1.address], INITIAL_VERSION, await impl.getAddress(), initData, ethers.ZeroHash) ).to.be.revertedWithCustomError(impl, "IntuitionFeeProxy_FixedFeeTooHigh"); }); @@ -173,7 +173,7 @@ describe("IntuitionFeeProxyV2", function () { ]); const VersionedFactory = await ethers.getContractFactory("IntuitionVersionedFeeProxy"); await expect( - VersionedFactory.deploy(admin1.address, INITIAL_VERSION, await impl.getAddress(), initData, ethers.ZeroHash) + VersionedFactory.deploy([admin1.address], INITIAL_VERSION, await impl.getAddress(), initData, ethers.ZeroHash) ).to.be.revertedWithCustomError(impl, "IntuitionFeeProxy_NoAdminsProvided"); }); @@ -189,7 +189,7 @@ describe("IntuitionFeeProxyV2", function () { ]); const VersionedFactory = await ethers.getContractFactory("IntuitionVersionedFeeProxy"); await expect( - VersionedFactory.deploy(admin1.address, INITIAL_VERSION, await impl.getAddress(), initData, ethers.ZeroHash) + VersionedFactory.deploy([admin1.address], INITIAL_VERSION, await impl.getAddress(), initData, ethers.ZeroHash) ).to.be.revertedWithCustomError(impl, "IntuitionFeeProxy_NoAdminsProvided"); }); @@ -206,7 +206,7 @@ describe("IntuitionFeeProxyV2", function () { ]); const VersionedFactory = await ethers.getContractFactory("IntuitionVersionedFeeProxy"); const p = await VersionedFactory.deploy( - alice.address, + [alice.address], INITIAL_VERSION, await impl.getAddress(), initData, diff --git a/packages/contracts/test/IntuitionFeeProxyV2Sponsored.test.ts b/packages/contracts/test/IntuitionFeeProxyV2Sponsored.test.ts index d8cf46e..94db77b 100644 --- a/packages/contracts/test/IntuitionFeeProxyV2Sponsored.test.ts +++ b/packages/contracts/test/IntuitionFeeProxyV2Sponsored.test.ts @@ -39,7 +39,7 @@ describe("IntuitionFeeProxyV2Sponsored (B1 full-sponsorship)", function () { const VerFactory = await ethers.getContractFactory("IntuitionVersionedFeeProxy"); const vp = await VerFactory.deploy( - admin1.address, + [admin1.address], INITIAL_VERSION, await impl.getAddress(), initData, diff --git a/packages/contracts/test/IntuitionFeeProxyV2_1.test.ts b/packages/contracts/test/IntuitionFeeProxyV2_1.test.ts index 1d8f03e..fd52a71 100644 --- a/packages/contracts/test/IntuitionFeeProxyV2_1.test.ts +++ b/packages/contracts/test/IntuitionFeeProxyV2_1.test.ts @@ -52,7 +52,7 @@ describe("IntuitionFeeProxyV2_1 — canonical versioning demo", function () { const Versioned = await ethers.getContractFactory("IntuitionVersionedFeeProxy"); const proxyDeployment = (await Versioned.deploy( - admin.address, + [admin.address], V2_LABEL, await v2Impl.getAddress(), initData, @@ -270,7 +270,7 @@ describe("IntuitionFeeProxyV2_1Sponsored — sponsored versioning demo", functio const Versioned = await ethers.getContractFactory("IntuitionVersionedFeeProxy"); const proxyDeployment = await Versioned.deploy( - admin.address, + [admin.address], V2_SPONSORED_LABEL, await v2sImpl.getAddress(), initData, diff --git a/packages/contracts/test/IntuitionVersionedFeeProxy.test.ts b/packages/contracts/test/IntuitionVersionedFeeProxy.test.ts index a9db3a8..59bf477 100644 --- a/packages/contracts/test/IntuitionVersionedFeeProxy.test.ts +++ b/packages/contracts/test/IntuitionVersionedFeeProxy.test.ts @@ -35,7 +35,7 @@ describe("IntuitionVersionedFeeProxy (ERC-7936)", function () { const VersionedFactory = await ethers.getContractFactory("IntuitionVersionedFeeProxy"); const versioned = (await VersionedFactory.deploy( - proxyAdmin.address, + [proxyAdmin.address, admin2.address, admin3.address], V2, await implV2.getAddress(), initData, @@ -75,10 +75,13 @@ describe("IntuitionVersionedFeeProxy (ERC-7936)", function () { describe("Construction", function () { it("registers initial version, sets default and proxy-admin, runs initializer", async function () { - const { versioned, proxyAdmin, implV2, proxyAsV2, mv } = + const { versioned, proxyAdmin, admin2, admin3, implV2, proxyAsV2, mv } = await loadFixture(deployFixture); - expect(await versioned.proxyAdmin()).to.equal(proxyAdmin.address); + expect(await versioned.isProxyAdmin(proxyAdmin.address)).to.equal(true); + expect(await versioned.isProxyAdmin(admin2.address)).to.equal(true); + expect(await versioned.isProxyAdmin(admin3.address)).to.equal(true); + expect(await versioned.proxyAdminCount()).to.equal(3n); expect(await versioned.getDefaultVersion()).to.equal(V2); expect(await versioned.getVersions()).to.deep.equal([V2]); expect(await versioned.getImplementation(V2)).to.equal(await implV2.getAddress()); @@ -89,11 +92,19 @@ describe("IntuitionVersionedFeeProxy (ERC-7936)", function () { expect(await proxyAsV2.adminCount()).to.equal(3n); }); - it("reverts on zero admin", async function () { + it("reverts on empty initial proxyAdmins array", async function () { + const { implV2 } = await loadFixture(deployFixture); + const VersionedFactory = await ethers.getContractFactory("IntuitionVersionedFeeProxy"); + await expect( + VersionedFactory.deploy([], V2, await implV2.getAddress(), "0x01", ethers.ZeroHash), + ).to.be.revertedWithCustomError(VersionedFactory, "IntuitionFeeProxy_NoAdminsProvided"); + }); + + it("reverts on zero address in initial proxyAdmins", async function () { const { implV2 } = await loadFixture(deployFixture); const VersionedFactory = await ethers.getContractFactory("IntuitionVersionedFeeProxy"); await expect( - VersionedFactory.deploy(ethers.ZeroAddress, V2, await implV2.getAddress(), "0x", ethers.ZeroHash), + VersionedFactory.deploy([ethers.ZeroAddress], V2, await implV2.getAddress(), "0x01", ethers.ZeroHash), ).to.be.revertedWithCustomError(VersionedFactory, "IntuitionFeeProxy_ZeroAddress"); }); @@ -102,10 +113,10 @@ describe("IntuitionVersionedFeeProxy (ERC-7936)", function () { const VersionedFactory = await ethers.getContractFactory("IntuitionVersionedFeeProxy"); await expect( VersionedFactory.deploy( - proxyAdmin.address, + [proxyAdmin.address], ethers.ZeroHash, await implV2.getAddress(), - "0x", + "0x01", ethers.ZeroHash, ), ).to.be.revertedWithCustomError(VersionedFactory, "VersionedFeeProxy_InvalidVersion"); @@ -115,7 +126,7 @@ describe("IntuitionVersionedFeeProxy (ERC-7936)", function () { const [, , , , , , eoa] = await ethers.getSigners(); const VersionedFactory = await ethers.getContractFactory("IntuitionVersionedFeeProxy"); await expect( - VersionedFactory.deploy(eoa.address, V2, eoa.address, "0x", ethers.ZeroHash), + VersionedFactory.deploy([eoa.address], V2, eoa.address, "0x01", ethers.ZeroHash), ).to.be.revertedWithCustomError(VersionedFactory, "VersionedFeeProxy_InvalidImplementation"); }); @@ -131,7 +142,7 @@ describe("IntuitionVersionedFeeProxy (ERC-7936)", function () { ]); const VersionedFactory = await ethers.getContractFactory("IntuitionVersionedFeeProxy"); await expect( - VersionedFactory.deploy(proxyAdmin.address, V2, await impl.getAddress(), badInit, ethers.ZeroHash), + VersionedFactory.deploy([proxyAdmin.address], V2, await impl.getAddress(), badInit, ethers.ZeroHash), ).to.be.revertedWithCustomError(impl, "IntuitionFeeProxy_FeePercentageTooHigh"); }); @@ -143,7 +154,7 @@ describe("IntuitionVersionedFeeProxy (ERC-7936)", function () { const VersionedFactory = await ethers.getContractFactory("IntuitionVersionedFeeProxy"); await expect( VersionedFactory.deploy( - proxyAdmin.address, + [proxyAdmin.address], V2, await implV2.getAddress(), "0x", @@ -456,14 +467,13 @@ describe("IntuitionVersionedFeeProxy (ERC-7936)", function () { "registerVersion(bytes32,address)", "removeVersion(bytes32)", "setDefaultVersion(bytes32)", - "transferProxyAdmin(address)", - "acceptProxyAdmin()", + "setProxyAdmin(address,bool)", "setName(bytes32)", "getImplementation(bytes32)", "getDefaultVersion()", "getVersions()", - "proxyAdmin()", - "pendingProxyAdmin()", + "isProxyAdmin(address)", + "proxyAdminCount()", "getName()", "executeAtVersion(bytes32,bytes)", ]; @@ -482,86 +492,95 @@ describe("IntuitionVersionedFeeProxy (ERC-7936)", function () { }); }); - // ============ transferProxyAdmin ============ + // ============ setProxyAdmin ============ - describe("transferProxyAdmin", function () { - it("2-step transfer: pending is set, accept finalises, old admin loses powers", async function () { + describe("setProxyAdmin", function () { + it("grant: adds an address to the whitelist + increments count + emits event", async function () { const { versioned, proxyAdmin, newAdmin } = await loadFixture(deployFixture); const v3Addr = await deployV3Impl(); - // Step 1 — current admin initiates transfer (pending only) - await expect(versioned.connect(proxyAdmin).transferProxyAdmin(newAdmin.address)) - .to.emit(versioned, "ProxyAdminTransferStarted") - .withArgs(proxyAdmin.address, newAdmin.address); - expect(await versioned.proxyAdmin()).to.equal(proxyAdmin.address); - expect(await versioned.pendingProxyAdmin()).to.equal(newAdmin.address); + expect(await versioned.isProxyAdmin(newAdmin.address)).to.equal(false); + const countBefore = await versioned.proxyAdminCount(); - // Before acceptance: old admin still has powers - await expect(versioned.connect(proxyAdmin).registerVersion(V3, v3Addr)) + await expect(versioned.connect(proxyAdmin).setProxyAdmin(newAdmin.address, true)) + .to.emit(versioned, "ProxyAdminGranted") + .withArgs(newAdmin.address); + + expect(await versioned.isProxyAdmin(newAdmin.address)).to.equal(true); + expect(await versioned.proxyAdminCount()).to.equal(countBefore + 1n); + + // New admin can now act + await expect(versioned.connect(newAdmin).registerVersion(V3, v3Addr)) .to.emit(versioned, "VersionRegistered") .withArgs(V3, v3Addr); - // And new admin cannot yet act - await expect( - versioned.connect(newAdmin).registerVersion(ethers.encodeBytes32String("v99"), v3Addr), - ).to.be.revertedWithCustomError(versioned, "VersionedFeeProxy_NotProxyAdmin"); + }); + + it("revoke: removes an address + decrements count + emits event", async function () { + const { versioned, proxyAdmin, admin2 } = await loadFixture(deployFixture); + const countBefore = await versioned.proxyAdminCount(); + + await expect(versioned.connect(proxyAdmin).setProxyAdmin(admin2.address, false)) + .to.emit(versioned, "ProxyAdminRevoked") + .withArgs(admin2.address); - // Step 2 — pending admin accepts - await expect(versioned.connect(newAdmin).acceptProxyAdmin()) - .to.emit(versioned, "ProxyAdminTransferred") - .withArgs(proxyAdmin.address, newAdmin.address); - expect(await versioned.proxyAdmin()).to.equal(newAdmin.address); - expect(await versioned.pendingProxyAdmin()).to.equal(ethers.ZeroAddress); + expect(await versioned.isProxyAdmin(admin2.address)).to.equal(false); + expect(await versioned.proxyAdminCount()).to.equal(countBefore - 1n); - // Old admin now reverts + // Revoked admin can no longer act + const v3Addr = await deployV3Impl(); await expect( - versioned.connect(proxyAdmin).registerVersion(ethers.encodeBytes32String("v100"), v3Addr), + versioned.connect(admin2).registerVersion(V3, v3Addr), ).to.be.revertedWithCustomError(versioned, "VersionedFeeProxy_NotProxyAdmin"); }); - it("transferProxyAdmin reverts on zero address", async function () { + it("setProxyAdmin reverts on zero address", async function () { const { versioned, proxyAdmin } = await loadFixture(deployFixture); await expect( - versioned.connect(proxyAdmin).transferProxyAdmin(ethers.ZeroAddress), + versioned.connect(proxyAdmin).setProxyAdmin(ethers.ZeroAddress, true), ).to.be.revertedWithCustomError(versioned, "IntuitionFeeProxy_ZeroAddress"); }); - it("non-admin cannot initiate transfer", async function () { + it("non-admin cannot grant or revoke", async function () { const { versioned, nonAdmin, newAdmin } = await loadFixture(deployFixture); await expect( - versioned.connect(nonAdmin).transferProxyAdmin(newAdmin.address), + versioned.connect(nonAdmin).setProxyAdmin(newAdmin.address, true), ).to.be.revertedWithCustomError(versioned, "VersionedFeeProxy_NotProxyAdmin"); }); - it("acceptProxyAdmin reverts when called by non-pending address", async function () { - const { versioned, proxyAdmin, newAdmin, nonAdmin } = await loadFixture(deployFixture); - - // No pending admin yet → anyone should revert + it("idempotent grant (already admin) reverts with ProxyAdminAlreadySet", async function () { + const { versioned, proxyAdmin, admin2 } = await loadFixture(deployFixture); await expect( - versioned.connect(newAdmin).acceptProxyAdmin(), - ).to.be.revertedWithCustomError(versioned, "VersionedFeeProxy_NotPendingProxyAdmin"); + versioned.connect(proxyAdmin).setProxyAdmin(admin2.address, true), + ).to.be.revertedWithCustomError(versioned, "VersionedFeeProxy_ProxyAdminAlreadySet"); + }); - // After transferProxyAdmin, only `newAdmin` can accept - await versioned.connect(proxyAdmin).transferProxyAdmin(newAdmin.address); - await expect( - versioned.connect(nonAdmin).acceptProxyAdmin(), - ).to.be.revertedWithCustomError(versioned, "VersionedFeeProxy_NotPendingProxyAdmin"); + it("idempotent revoke (not an admin) reverts with ProxyAdminAlreadySet", async function () { + const { versioned, proxyAdmin, nonAdmin } = await loadFixture(deployFixture); await expect( - versioned.connect(proxyAdmin).acceptProxyAdmin(), - ).to.be.revertedWithCustomError(versioned, "VersionedFeeProxy_NotPendingProxyAdmin"); + versioned.connect(proxyAdmin).setProxyAdmin(nonAdmin.address, false), + ).to.be.revertedWithCustomError(versioned, "VersionedFeeProxy_ProxyAdminAlreadySet"); }); - it("transferProxyAdmin overwrites a pending candidate", async function () { - const { versioned, proxyAdmin, newAdmin, nonAdmin } = await loadFixture(deployFixture); - await versioned.connect(proxyAdmin).transferProxyAdmin(newAdmin.address); - expect(await versioned.pendingProxyAdmin()).to.equal(newAdmin.address); + it("last-admin guard: cannot revoke the only remaining proxyAdmin", async function () { + const { versioned, proxyAdmin, admin2, admin3 } = await loadFixture(deployFixture); - await versioned.connect(proxyAdmin).transferProxyAdmin(nonAdmin.address); - expect(await versioned.pendingProxyAdmin()).to.equal(nonAdmin.address); + // Revoke admin2 + admin3 — proxyAdmin is now the only one left + await versioned.connect(proxyAdmin).setProxyAdmin(admin2.address, false); + await versioned.connect(proxyAdmin).setProxyAdmin(admin3.address, false); + expect(await versioned.proxyAdminCount()).to.equal(1n); - // Old pending can no longer accept + // The last admin self-revoking would lock the role permanently await expect( - versioned.connect(newAdmin).acceptProxyAdmin(), - ).to.be.revertedWithCustomError(versioned, "VersionedFeeProxy_NotPendingProxyAdmin"); + versioned.connect(proxyAdmin).setProxyAdmin(proxyAdmin.address, false), + ).to.be.revertedWithCustomError(versioned, "VersionedFeeProxy_LastProxyAdmin"); + }); + + it("grant + revoke roundtrip on the same address", async function () { + const { versioned, proxyAdmin, newAdmin } = await loadFixture(deployFixture); + await versioned.connect(proxyAdmin).setProxyAdmin(newAdmin.address, true); + expect(await versioned.isProxyAdmin(newAdmin.address)).to.equal(true); + await versioned.connect(proxyAdmin).setProxyAdmin(newAdmin.address, false); + expect(await versioned.isProxyAdmin(newAdmin.address)).to.equal(false); }); }); @@ -584,7 +603,7 @@ describe("IntuitionVersionedFeeProxy (ERC-7936)", function () { const VersionedFactory = await ethers.getContractFactory("IntuitionVersionedFeeProxy"); const NAME = ethers.encodeBytes32String("My DAO Fees"); const deployed = await VersionedFactory.deploy( - proxyAdmin.address, + [proxyAdmin.address], V2, await implV2.getAddress(), initData, From dd1c4353a01d0ad310dc9200039117e4e829abb0 Mon Sep 17 00:00:00 2001 From: Wieedze Date: Mon, 18 May 2026 13:27:41 +0200 Subject: [PATCH 2/2] feat(role1-multi-admin): propagate whitelist API across SDK, safe-tx, webapp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to a2c82e5 (contract-level Role 1 → multi-admin whitelist). Drags every downstream consumer onto the new API so the branch compiles and the UX matches the contract semantics. SDK - readers.readProxyVersions: drop `proxyAdmin` + `pendingProxyAdmin` outputs, return `proxyAdminCount: bigint` instead. Both the Multicall3 path and the per-field fallback are updated. safe-tx - New ops/versioned-proxy.ts exposing `setProxyAdmin(proxy, admin, status)` as an AdminOp builder (mirrors v2-admin.setWhitelistedAdmin). - Registered as `set-proxy-admin` in the CLI op-registry under a new `versioned-proxy` category. OP_REGISTRY count test bumped 10 → 11. - Unit test covers grant + revoke selector + args. webapp - useVersionedProxy: replace `useTransferProxyAdmin` + `useAcceptProxyAdmin` with `useSetProxyAdmin` + `useIsProxyAdmin`; add `useProxyAdmins` that reduces ProxyAdminGranted / ProxyAdminRevoked events into the current whitelist (no on-chain getter for the full set). - useProxyRoles: now reads `isProxyAdmin(account)` via wagmi instead of comparing against a single `proxyAdmin` address. - UpgradeAuthorityPanel rewritten as a multi-admin panel (list + per-row revoke + grant form + Safe-propose path), with a collapsible Advanced section housing the "grant both roles" convenience + the Role 1 vs Role 2 explainer. - AdminsTab + ProxyDetail: drop the obsolete `proxyAdmin` + `pendingProxyAdmin` prop plumbing. - useProxyAdminRotation hook deleted (2-step state machine no longer needed); stale AdminRotateStage type removed. - Docs.tsx: Architecture actor + Primitives + Admin Rotation sections rewritten for the whitelist model. - SafeProposeFeedback component extracted so every Safe-propose surface renders feedback identically. e2e - e2e-validate.ts step ⑫b: 2-step transfer flow replaced with whitelist grant → revoke → non-admin guard → last-admin guard. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/contracts/scripts/e2e-validate.ts | 64 +- packages/safe-tx/src/cli/op-registry.ts | 18 +- packages/safe-tx/src/ops/index.ts | 1 + packages/safe-tx/src/ops/versioned-proxy.ts | 38 + .../safe-tx/test/unit/cli/op-registry.test.ts | 6 +- .../test/unit/ops/versioned-proxy.test.ts | 37 + packages/sdk/src/readers.ts | 29 +- packages/webapp/src/components/AdminsTab.tsx | 6 - .../src/components/SafeProposeFeedback.tsx | 43 ++ .../src/components/UpgradeAuthorityPanel.tsx | 655 ++++++++++-------- .../webapp/src/hooks/useProxyAdminRotation.ts | 165 ----- packages/webapp/src/hooks/useProxyRoles.ts | 28 +- .../webapp/src/hooks/useVersionedProxy.ts | 162 ++++- packages/webapp/src/pages/Docs.tsx | 62 +- packages/webapp/src/pages/ProxyDetail.tsx | 6 +- packages/webapp/src/types.ts | 15 - 16 files changed, 739 insertions(+), 596 deletions(-) create mode 100644 packages/safe-tx/src/ops/versioned-proxy.ts create mode 100644 packages/safe-tx/test/unit/ops/versioned-proxy.test.ts create mode 100644 packages/webapp/src/components/SafeProposeFeedback.tsx delete mode 100644 packages/webapp/src/hooks/useProxyAdminRotation.ts diff --git a/packages/contracts/scripts/e2e-validate.ts b/packages/contracts/scripts/e2e-validate.ts index eeea0e9..8d3e1e6 100644 --- a/packages/contracts/scripts/e2e-validate.ts +++ b/packages/contracts/scripts/e2e-validate.ts @@ -437,48 +437,60 @@ async function main() { console.log(" ✓ adminship rotated + last-admin guard enforced"); // ════════════════════════════════════════════════════════════════════ - // ⑫b 2-step proxyAdmin transfer (versioned proxy level). - // deployer is still proxyAdmin here — removing from whitelistedAdmins - // in ⑫ doesn't affect the versioned-proxy admin slot. Transfer it to - // userA (already the sole whitelistedAdmin after ⑫) so one address - // controls both surfaces going forward. + // ⑫b proxyAdmin whitelist rotation (versioned proxy level). + // deployer is still the sole proxyAdmin here — removing it from + // whitelistedAdmins in ⑫ doesn't affect the versioned-proxy admin + // list. Grant userA, then revoke deployer so one address controls + // both surfaces going forward. // ════════════════════════════════════════════════════════════════════ - console.log("\n⑫b 2-step proxyAdmin transfer: deployer → userA …"); + console.log("\n⑫b proxyAdmin whitelist rotation: deployer → userA …"); assertEq( - (await versioned.proxyAdmin()).toLowerCase(), - deployer.address.toLowerCase(), - "proxyAdmin still = deployer (whitelist revoke didn't touch versioned admin)", + await versioned.isProxyAdmin(deployer.address), + true, + "deployer still proxyAdmin (whitelist revoke didn't touch versioned admin)", ); - await (await versioned.connect(deployer).transferProxyAdmin(userA.address)).wait(); + await (await versioned.connect(deployer).setProxyAdmin(userA.address, true)).wait(); assertEq( - (await versioned.pendingProxyAdmin()).toLowerCase(), - userA.address.toLowerCase(), - "pendingProxyAdmin = userA", + await versioned.isProxyAdmin(userA.address), + true, + "userA granted proxyAdmin", + ); + assertEq( + await versioned.proxyAdminCount(), + 2n, + "proxyAdminCount = 2 after grant", ); - // Guard: only the pending address can accept + // Guard: non-admin cannot mutate the list await expectRevertWithName( - () => versioned.connect(nonAdmin).acceptProxyAdmin(), - "VersionedFeeProxy_NotPendingProxyAdmin", - "non-pending accept blocked", + () => versioned.connect(nonAdmin).setProxyAdmin(nonAdmin.address, true), + "VersionedFeeProxy_NotProxyAdmin", + "non-admin setProxyAdmin blocked", ); - await (await versioned.connect(userA).acceptProxyAdmin()).wait(); + // Revoke deployer — userA becomes the sole admin + await (await versioned.connect(deployer).setProxyAdmin(deployer.address, false)).wait(); assertEq( - (await versioned.proxyAdmin()).toLowerCase(), - userA.address.toLowerCase(), - "proxyAdmin now = userA", + await versioned.isProxyAdmin(deployer.address), + false, + "deployer revoked", ); assertEq( - await versioned.pendingProxyAdmin(), - ethers.ZeroAddress, - "pendingProxyAdmin cleared", + await versioned.proxyAdminCount(), + 1n, + "proxyAdminCount = 1 after revoke", ); // Old proxyAdmin locked out await expectRevertWithName( - () => versioned.connect(deployer).transferProxyAdmin(deployer.address), + () => versioned.connect(deployer).setProxyAdmin(deployer.address, true), "VersionedFeeProxy_NotProxyAdmin", "old proxyAdmin locked out", ); - console.log(" ✓ proxyAdmin rotated via 2-step, old admin locked out"); + // Last-admin guard — userA cannot self-revoke + await expectRevertWithName( + () => versioned.connect(userA).setProxyAdmin(userA.address, false), + "VersionedFeeProxy_LastProxyAdmin", + "last proxyAdmin cannot self-revoke", + ); + console.log(" ✓ proxyAdmin rotated via whitelist, last-admin guard enforced"); // ════════════════════════════════════════════════════════════════════ // ⑬ Final withdrawAll by userA (now sole admin) diff --git a/packages/safe-tx/src/cli/op-registry.ts b/packages/safe-tx/src/cli/op-registry.ts index 39c4ec1..a990762 100644 --- a/packages/safe-tx/src/cli/op-registry.ts +++ b/packages/safe-tx/src/cli/op-registry.ts @@ -2,6 +2,7 @@ import { getAddress, stringToHex, type Address, type Hex } from 'viem' import * as factory from '../ops/factory.js' import * as uups from '../ops/uups-upgrade.js' import * as v2 from '../ops/v2-admin.js' +import * as versionedProxy from '../ops/versioned-proxy.js' import type { AdminOp } from '../types.js' /** @@ -21,7 +22,7 @@ export type OpFlag = { export type OpRegistration = { name: string - category: 'v2-admin' | 'factory' | 'uups' + category: 'v2-admin' | 'factory' | 'uups' | 'versioned-proxy' description: string flags: OpFlag[] build: (args: Record) => AdminOp @@ -133,6 +134,21 @@ export const OP_REGISTRY: OpRegistration[] = [ build: ({ factory: f }) => factory.acceptOwnership(f as Address), }, + // ----- versioned-proxy (Role 1) ----- + { + name: 'set-proxy-admin', + category: 'versioned-proxy', + description: + 'Add or remove an address from the versioned proxy Role 1 (proxyAdmins) whitelist', + flags: [ + { name: 'proxy', type: 'address', description: 'Versioned proxy address', required: true }, + { name: 'admin', type: 'address', description: 'Admin address to toggle', required: true }, + { name: 'status', type: 'bool', description: 'true = grant, false = revoke', required: true }, + ], + build: ({ proxy, admin, status }) => + versionedProxy.setProxyAdmin(proxy as Address, admin as Address, status as boolean), + }, + // ----- uups ----- { name: 'upgrade-to-and-call', diff --git a/packages/safe-tx/src/ops/index.ts b/packages/safe-tx/src/ops/index.ts index aaa888b..3d7ece7 100644 --- a/packages/safe-tx/src/ops/index.ts +++ b/packages/safe-tx/src/ops/index.ts @@ -1,3 +1,4 @@ export * as v2Admin from './v2-admin.js' export * as factory from './factory.js' export * as uups from './uups-upgrade.js' +export * as versionedProxy from './versioned-proxy.js' diff --git a/packages/safe-tx/src/ops/versioned-proxy.ts b/packages/safe-tx/src/ops/versioned-proxy.ts new file mode 100644 index 0000000..411be0c --- /dev/null +++ b/packages/safe-tx/src/ops/versioned-proxy.ts @@ -0,0 +1,38 @@ +import { encodeFunctionData, type Address } from 'viem' +import type { AdminOp } from '../types.js' + +/** + * Admin operations exposed by IntuitionVersionedFeeProxy. Role 1 + * (proxyAdmin) whitelist mutation, mirrors the Role 2 + * `setWhitelistedAdmin` pattern — instant grant/revoke, no 2-step + * ceremony. The contract enforces last-admin guard + idempotent reject. + */ +const VERSIONED_PROXY_ABI = [ + { + type: 'function', + name: 'setProxyAdmin', + inputs: [ + { name: 'admin', type: 'address' }, + { name: 'status', type: 'bool' }, + ], + outputs: [], + stateMutability: 'nonpayable', + }, +] as const + +export function setProxyAdmin( + proxy: Address, + admin: Address, + status: boolean, +): AdminOp { + return { + to: proxy, + value: 0n, + data: encodeFunctionData({ + abi: VERSIONED_PROXY_ABI, + functionName: 'setProxyAdmin', + args: [admin, status], + }), + description: `setProxyAdmin(${admin}, ${status}) on ${proxy}`, + } +} diff --git a/packages/safe-tx/test/unit/cli/op-registry.test.ts b/packages/safe-tx/test/unit/cli/op-registry.test.ts index d7cab1f..33c57d7 100644 --- a/packages/safe-tx/test/unit/cli/op-registry.test.ts +++ b/packages/safe-tx/test/unit/cli/op-registry.test.ts @@ -8,13 +8,15 @@ import { } from '../../../src/cli/op-registry.js' describe('OP_REGISTRY', () => { - it('contains 10 ops total (5 v2-admin + 4 factory + 1 uups)', () => { - expect(OP_REGISTRY).toHaveLength(10) + it('contains 11 ops total (5 v2-admin + 4 factory + 1 versioned-proxy + 1 uups)', () => { + expect(OP_REGISTRY).toHaveLength(11) const v2 = OP_REGISTRY.filter((o) => o.category === 'v2-admin') const fac = OP_REGISTRY.filter((o) => o.category === 'factory') + const vp = OP_REGISTRY.filter((o) => o.category === 'versioned-proxy') const up = OP_REGISTRY.filter((o) => o.category === 'uups') expect(v2).toHaveLength(5) expect(fac).toHaveLength(4) + expect(vp).toHaveLength(1) expect(up).toHaveLength(1) }) diff --git a/packages/safe-tx/test/unit/ops/versioned-proxy.test.ts b/packages/safe-tx/test/unit/ops/versioned-proxy.test.ts new file mode 100644 index 0000000..29648ac --- /dev/null +++ b/packages/safe-tx/test/unit/ops/versioned-proxy.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from 'vitest' +import { decodeFunctionData, getAddress, toFunctionSelector } from 'viem' +import * as versionedProxy from '../../../src/ops/versioned-proxy.js' + +const PROXY = getAddress('0xf10D442D0fB934D4037DC30769a6EfCf2f54F7B6') +const ADMIN = getAddress('0xc634457aD68b037E2D5aA1C10c3930d7e4E2d551') + +const SELECTOR_SET_PROXY_ADMIN = toFunctionSelector('setProxyAdmin(address,bool)') + +const ABI = [ + { + type: 'function', + name: 'setProxyAdmin', + inputs: [{ type: 'address' }, { type: 'bool' }], + outputs: [], + stateMutability: 'nonpayable', + }, +] as const + +describe('versioned-proxy AdminOp builders', () => { + it('setProxyAdmin grant', () => { + const op = versionedProxy.setProxyAdmin(PROXY, ADMIN, true) + expect(op.to).toBe(PROXY) + expect(op.value).toBe(0n) + expect(op.data.slice(0, 10)).toBe(SELECTOR_SET_PROXY_ADMIN) + expect(op.description).toContain('setProxyAdmin') + const decoded = decodeFunctionData({ abi: ABI, data: op.data }) + expect(decoded.functionName).toBe('setProxyAdmin') + expect(decoded.args).toEqual([ADMIN, true]) + }) + + it('setProxyAdmin revoke', () => { + const op = versionedProxy.setProxyAdmin(PROXY, ADMIN, false) + const decoded = decodeFunctionData({ abi: ABI, data: op.data }) + expect(decoded.args).toEqual([ADMIN, false]) + }) +}) diff --git a/packages/sdk/src/readers.ts b/packages/sdk/src/readers.ts index 38e58fc..71362d1 100644 --- a/packages/sdk/src/readers.ts +++ b/packages/sdk/src/readers.ts @@ -190,12 +190,16 @@ export async function readSponsoredMetrics( } } -/** Registered version labels + current default + current + pending proxy admin. +/** Registered version labels + current default + size of the proxyAdmin whitelist. * * Hybrid dispatch: Multicall3 with `allowFailure: true` on chains that - * support it (per-field revert tolerant — older impls missing - * `pendingProxyAdmin` don't break the read), parallel per-field reads + * support it (per-field revert tolerant), parallel per-field reads * otherwise. Both return identical shape. + * + * Role 1 became a whitelist post 2-step retirement — callers that need + * the full admin list should reduce the `ProxyAdminGranted` / + * `ProxyAdminRevoked` event log (no on-chain getter is exposed for the + * full set). */ export async function readProxyVersions( client: PublicClient, @@ -203,41 +207,36 @@ export async function readProxyVersions( ): Promise<{ versions: readonly `0x${string}`[] defaultVersion: `0x${string}` | undefined - proxyAdmin: Address | undefined - pendingProxyAdmin: Address | undefined + proxyAdminCount: bigint }> { const abi = IntuitionVersionedFeeProxyABI as any const contracts = [ { abi, address: proxy, functionName: 'getVersions' }, { abi, address: proxy, functionName: 'getDefaultVersion' }, - { abi, address: proxy, functionName: 'proxyAdmin' }, - { abi, address: proxy, functionName: 'pendingProxyAdmin' }, + { abi, address: proxy, functionName: 'proxyAdminCount' }, ] as const try { - const [versions, defaultVersion, proxyAdmin, pendingProxyAdmin] = + const [versions, defaultVersion, proxyAdminCount] = await client.multicall({ allowFailure: true, contracts: contracts as any }) return { versions: versions.status === 'success' ? (versions.result as readonly `0x${string}`[]) : [], defaultVersion: defaultVersion.status === 'success' ? (defaultVersion.result as `0x${string}`) : undefined, - proxyAdmin: proxyAdmin.status === 'success' ? (proxyAdmin.result as Address) : undefined, - pendingProxyAdmin: pendingProxyAdmin.status === 'success' ? (pendingProxyAdmin.result as Address) : undefined, + proxyAdminCount: proxyAdminCount.status === 'success' ? (proxyAdminCount.result as bigint) : 0n, } } catch { // Fallback — no Multicall3 on this chain. Per-field try-catch so a // single-field revert doesn't poison the whole read. const safe = (p: Promise): Promise => p.catch(() => undefined) - const [versions, defaultVersion, proxyAdmin, pendingProxyAdmin] = await Promise.all([ + const [versions, defaultVersion, proxyAdminCount] = await Promise.all([ safe(client.readContract(contracts[0] as any) as Promise), safe(client.readContract(contracts[1] as any) as Promise<`0x${string}`>), - safe(client.readContract(contracts[2] as any) as Promise
), - safe(client.readContract(contracts[3] as any) as Promise
), + safe(client.readContract(contracts[2] as any) as Promise), ]) return { versions: versions ?? [], defaultVersion, - proxyAdmin, - pendingProxyAdmin, + proxyAdminCount: proxyAdminCount ?? 0n, } } } diff --git a/packages/webapp/src/components/AdminsTab.tsx b/packages/webapp/src/components/AdminsTab.tsx index d5d4d1c..5098489 100644 --- a/packages/webapp/src/components/AdminsTab.tsx +++ b/packages/webapp/src/components/AdminsTab.tsx @@ -7,8 +7,6 @@ import { UpgradeAuthorityPanel } from './UpgradeAuthorityPanel' interface Props { proxy: Address - proxyAdmin: Address | undefined - pendingProxyAdmin: Address | undefined account: Address | undefined isFeeAdmin: boolean isVersionsFetching: boolean @@ -17,8 +15,6 @@ interface Props { export function AdminsTab({ proxy, - proxyAdmin, - pendingProxyAdmin, account, isFeeAdmin, isVersionsFetching, @@ -35,8 +31,6 @@ export function AdminsTab({

+ {proposed && ( +
+
+ Proposed. safeTxHash:{' '} + {proposed.safeTxHash} +
+
+ Owners can co-sign and execute in{' '} + + Den + + . +
+
+ )} + {error && ( +

Safe propose: {error}

+ )} + + ) +} diff --git a/packages/webapp/src/components/UpgradeAuthorityPanel.tsx b/packages/webapp/src/components/UpgradeAuthorityPanel.tsx index dbdfcdb..0cf2910 100644 --- a/packages/webapp/src/components/UpgradeAuthorityPanel.tsx +++ b/packages/webapp/src/components/UpgradeAuthorityPanel.tsx @@ -2,124 +2,149 @@ import { useEffect, useState } from 'react' import { isAddress, type Address } from 'viem' import { useWaitForTransactionReceipt } from 'wagmi' +import { ops } from '@intuition-fee-proxy/safe-tx' +import { useSetWhitelistedAdmin } from '../hooks/useProxy' +import { usePostTxRefreshing } from '../hooks/usePostTxRefreshing' +import { useSafePropose } from '../hooks/useSafePropose' +import { useSafeStatuses } from '../hooks/useSafeStatus' import { - useAcceptProxyAdmin, - useTransferProxyAdmin, + useProxyAdmins, + useSetProxyAdmin, } from '../hooks/useVersionedProxy' import AddressDisplay from './Address' -import { useProxyAdminRotation } from '../hooks/useProxyAdminRotation' -import { usePostTxRefreshing } from '../hooks/usePostTxRefreshing' import { ProxyAdminSafeBanner } from './ProxyAdminSafeBanner' import { SafeBadge } from './SafeBadge' +import { SafeProposeFeedback } from './SafeProposeFeedback' import { Spinner } from './Spinner' -const ZERO = '0x0000000000000000000000000000000000000000' - interface Props { proxy: Address - proxyAdmin: Address | undefined - pendingProxyAdmin: Address | undefined account: Address | undefined isConnectedFeeAdmin: boolean onTransferred: () => void isRefreshing: boolean } +/** + * Role 1 panel — multi-admin whitelist (post 2-step retirement). Mirrors + * `AdminsPanel` (Role 2) in structure: list + per-row revoke + grant + * form. Direct write for current proxyAdmins, "Propose via Safe" path + * when at least one Safe sits in the whitelist. + * + * Default visible: the list and grant form. + * Advanced (collapsible, closed by default): the "grant both roles in + * one click" convenience + the Role 1 vs Role 2 explainer. + */ export function UpgradeAuthorityPanel({ proxy, - proxyAdmin, - pendingProxyAdmin, account, isConnectedFeeAdmin, onTransferred, isRefreshing, }: Props) { - const isYou = Boolean( - account && - proxyAdmin && - account.toLowerCase() === proxyAdmin.toLowerCase(), - ) - const hasPending = Boolean( - pendingProxyAdmin && pendingProxyAdmin.toLowerCase() !== ZERO, + const { admins: proxyAdmins, isLoading, refetch } = useProxyAdmins(proxy) + const adminStatuses = useSafeStatuses(proxyAdmins) + const safeAdmin = proxyAdmins.find( + (a) => adminStatuses[a.toLowerCase()]?.kind === 'safe', ) - const isPendingYou = Boolean( + const safePropose = useSafePropose({ safeAddress: safeAdmin }) + + const isYou = Boolean( account && - pendingProxyAdmin && - account.toLowerCase() === pendingProxyAdmin.toLowerCase(), + proxyAdmins.some( + (a) => account.toLowerCase() === a.toLowerCase(), + ), ) - const hasBothRoles = isYou && isConnectedFeeAdmin - // Standalone "Grant Role 1" — separate write instance from the rotation. - const [newAdminInput, setNewAdminInput] = useState('') - const { - transferAdmin, - hash: transferHash, - isPending: transferPending, - error: transferError, - reset: resetTransfer, - } = useTransferProxyAdmin(proxy) - const transferReceipt = useWaitForTransactionReceipt({ hash: transferHash }) - - // Step 2 — pending admin accepts + // Direct setProxyAdmin write (the user's own EOA acts as a proxyAdmin) const { - acceptAdmin, - hash: acceptHash, - isPending: acceptPending, - error: acceptError, - reset: resetAccept, - } = useAcceptProxyAdmin(proxy) - const acceptReceipt = useWaitForTransactionReceipt({ hash: acceptHash }) - - // Bridges the window between "tx mined" and "useProxyVersions reflects - // the new on-chain state" — drives the card-title spinner. - const postTx = usePostTxRefreshing(isRefreshing) + setProxyAdmin, + hash, + isPending, + error: writeError, + reset, + } = useSetProxyAdmin(proxy) + const receipt = useWaitForTransactionReceipt({ hash }) - // Rotation state machine (grant + transfer + external-accept detection). - const rotation = useProxyAdminRotation({ - proxy, - proxyAdmin, - account, - onWriteSuccess: () => { - onTransferred() - postTx.start() - }, - }) - - // Transient success flash after acceptProxyAdmin lands — tells the - // new admin visually that they now hold the role. Auto-dismisses. - const [acceptedFlash, setAcceptedFlash] = useState(false) + const [addDraft, setAddDraft] = useState('') + const [pendingTarget, setPendingTarget] = useState
(null) + const postTx = usePostTxRefreshing(isRefreshing) - // Standalone transfer: clear input, refetch parent, bridge spinner. useEffect(() => { - if (transferReceipt.isSuccess) { - setNewAdminInput('') - resetTransfer() + if (receipt.isSuccess) { + refetch() + reset() + setAddDraft('') onTransferred() postTx.start() } - }, [transferReceipt.isSuccess]) + }, [hash, receipt.isSuccess]) - // Accept: refetch parent, flash success, bridge spinner. useEffect(() => { - if (acceptReceipt.isSuccess) { - resetAccept() - onTransferred() - postTx.start() - setAcceptedFlash(true) - const t = setTimeout(() => setAcceptedFlash(false), 6000) - return () => clearTimeout(t) + if ( + !isPending && + !receipt.isLoading && + !receipt.isSuccess && + pendingTarget + ) { + setPendingTarget(null) } - }, [acceptReceipt.isSuccess]) + }, [isPending, receipt.isLoading, receipt.isSuccess, writeError]) - const inputValid = isAddress(newAdminInput.trim()) + const addValid = + isAddress(addDraft) && + !proxyAdmins.some((a) => a.toLowerCase() === addDraft.toLowerCase()) - const titleBusy = - postTx.active || - transferPending || - transferReceipt.isLoading || - acceptPending || - acceptReceipt.isLoading || - rotation.busy + const busy = isPending || receipt.isLoading || safePropose.isProposing + + async function onGrant() { + if (!addValid) return + setPendingTarget('ADD') + try { + await setProxyAdmin(addDraft as Address, true) + } catch (e) { + console.error(e) + setPendingTarget(null) + } + } + + async function onRevoke(addr: Address) { + setPendingTarget(addr) + try { + await setProxyAdmin(addr, false) + } catch (e) { + console.error(e) + setPendingTarget(null) + } + } + + async function onProposeGrant() { + if (!addValid || !safeAdmin) return + safePropose.reset() + try { + await safePropose.propose( + ops.versionedProxy.setProxyAdmin(proxy, addDraft as Address, true), + ) + } catch (e) { + console.error(e) + } + } + + async function onProposeRevoke(addr: Address) { + if (!safeAdmin) return + safePropose.reset() + try { + await safePropose.propose( + ops.versionedProxy.setProxyAdmin(proxy, addr, false), + ) + } catch (e) { + console.error(e) + } + } + + // The form is interactive if (a) the user is a direct proxyAdmin or + // (b) a Safe is in the whitelist (Safe owners can propose). + const canInteract = isYou || Boolean(safeAdmin) return (
@@ -129,180 +154,277 @@ export function UpgradeAuthorityPanel({ Role 1

- Upgrade authority (proxyAdmin) - {titleBusy && } + Upgrade authority (proxyAdmins) + {(postTx.active || (isLoading && proxyAdmins.length > 0)) && ( + + )}

- Single slot · 2-step grant + {proxyAdmins.length} address{proxyAdmins.length === 1 ? '' : 'es'} · + instant grant/revoke

- Only one address can hold this role at a time. - For production, put a Gnosis Safe multisig here — - the Safe itself handles N signers / threshold / signer rotation - internally, so you get “multi-human proxyAdmin” - without the contract knowing anything about it. + Controls implementation registration and default version. Mirrors + Role 2's pattern (whitelist + last-admin guard). Use a Safe for + production — its internal quorum is the safety net we used to + get from the 2-step transfer.

- + - {acceptedFlash && ( -
- - ✓ - - - You are now proxyAdmin — the transfer has been - finalised on-chain. - + {isLoading && proxyAdmins.length === 0 && ( +
+
+
)} -
- - current - - {proxyAdmin ? ( - <> - - 0 && ( +
    + {proxyAdmins.map((addr) => { + const isSelf = + account && addr.toLowerCase() === account.toLowerCase() + const isLast = proxyAdmins.length === 1 + const canDirectRevoke = isYou && !isLast + const canProposeRevoke = Boolean(safeAdmin) && !isLast + return ( +
  • +
    + + + {isSelf && ( + + you + + )} +
    +
    + {canDirectRevoke && ( + + )} + {canProposeRevoke && ( + + )} + {isLast && ( + + Last admin — cannot revoke + + )} +
    +
    • + ) + })} +
+ )} + + {canInteract && ( +
+
+
+ Grant Role 1 to a new address +
+
+ Adds an address to the proxyAdmins whitelist. Use a Safe + for production deployments. +
+
+
+ setAddDraft(e.target.value)} + placeholder="0x…" + className="input font-mono text-xs flex-1 min-w-[260px]" /> {isYou && ( - - you - + )} - - ) : ( - — - )} -
- - {hasPending && ( -
-
- - pending - - - {isPendingYou && ( - - you - + {safeAdmin && ( + )}
-

- A transfer has been initiated. The pending address must sign{' '} - acceptProxyAdmin() from - their wallet to finalise. Until then the current admin keeps all - powers and can overwrite the pending candidate. -

- {isPendingYou && ( - + {addDraft && !isAddress(addDraft) && ( +

Invalid address.

)} - {acceptError && ( -

- {acceptError.message.split('\n')[0]} -

+ {addDraft && isAddress(addDraft) && !addValid && ( +

Already a proxyAdmin.

)} -
- )} - - {isYou && ( -
- -
- setNewAdminInput(e.target.value)} - className="input flex-1 min-w-[18rem] font-mono text-xs" - /> - -
- {transferError && ( -

- {transferError.message.split('\n')[0]} + {writeError && ( +

+ {writeError.message.split('\n')[0]}

)} -

- Grants Role 1 only — the target must then call{' '} - acceptProxyAdmin(). - Fee admin rights (Role 2) are untouched. Use the combined grant - below if you hold both roles. -

)} - {hasBothRoles && } + -

- Role 1 controls which logic the proxy delegates to — - register new implementations, change default version, rename. It{' '} - cannot touch fees, withdrawals, or the sponsor - pool; those are Role 2 below. -

+ { + refetch() + onTransferred() + postTx.start() + }} + />
) } /** - * Subcomponent for the "grant both roles" convenience flow. Pure renderer - * over the rotation state machine — no local state of its own. + * Collapsible "Advanced" section — closed by default. + * + * Houses two things: + * 1. The grant-both-roles convenience (only enabled when the caller is + * a direct admin on BOTH roles — granting both via Safe is just two + * separate ProposeViaSafe flows, no point bundling). + * 2. The Role 1 vs Role 2 explainer paragraph (operational reference, + * not needed in the main flow once the user is familiar). */ -function RotateBothRolesForm({ - rotation, +function AdvancedSection({ + proxy, + canGrantBoth, + onWriteDone, }: { - rotation: ReturnType + proxy: Address + canGrantBoth: boolean + onWriteDone: () => void }) { - const buttonLabel = (() => { - switch (rotation.stage) { - case 'grant': - return rotation.grantPending ? 'Sign Role 2…' : 'Granting Role 2…' - case 'transfer': - return rotation.transferPending ? 'Sign Role 1…' : 'Granting Role 1…' - case 'done': - return 'Waiting for accept…' - default: - return 'Grant both roles' + const [open, setOpen] = useState(false) + + return ( +
+ + + {open && ( +
+ {canGrantBoth && } +

+ Role 1 vs Role 2.{' '} + Role 1 (proxyAdmins) controls which logic the proxy + delegates to — register new implementations, change default + version, rename. It cannot touch fees, + withdrawals, or the sponsor pool; those are Role 2 below. + Both roles use the same whitelist model with last-admin + guard. Keep them disjoint if your dev team and ops team + are distinct. +

+
+ )} +
+ ) +} + +/** + * Convenience: grant Role 1 + Role 2 to a new address in two back-to-back + * transactions. Only available when the caller currently holds both roles + * (direct admin on each). For Safe-mediated grants, use the per-role + * Propose via Safe buttons — each goes through its own quorum anyway. + */ +function GrantBothRolesForm({ + proxy, + onDone, +}: { + proxy: Address + onDone: () => void +}) { + const [input, setInput] = useState('') + const [stage, setStage] = useState<'idle' | 'role1' | 'role2'>('idle') + + const role1 = useSetProxyAdmin(proxy) + const role1Receipt = useWaitForTransactionReceipt({ hash: role1.hash }) + const role2 = useSetWhitelistedAdmin(proxy) + const role2Receipt = useWaitForTransactionReceipt({ hash: role2.hash }) + + const inputValid = isAddress(input.trim()) + + // Once Role 1 grant mines, fire Role 2 grant. + useEffect(() => { + if (stage === 'role1' && role1Receipt.isSuccess) { + setStage('role2') + role2.setAdmin(input.trim() as Address, true).catch(() => setStage('idle')) + } + }, [stage, role1Receipt.isSuccess]) + + // Once Role 2 grant mines, refresh parent. + useEffect(() => { + if (stage === 'role2' && role2Receipt.isSuccess) { + setStage('idle') + setInput('') + onDone() + } + }, [stage, role2Receipt.isSuccess]) + + const busy = stage !== 'idle' + + function start() { + if (!inputValid) return + setStage('role1') + role1.setProxyAdmin(input.trim() as Address, true).catch(() => setStage('idle')) + } + + const label = (() => { + if (stage === 'role1') { + return role1.isPending ? 'Sign Role 1…' : 'Granting Role 1…' + } + if (stage === 'role2') { + return role2.isPending ? 'Sign Role 2…' : 'Granting Role 2…' } + return 'Grant both roles' })() return ( @@ -317,79 +439,34 @@ function RotateBothRolesForm({

You currently hold both roles. This runs{' '} + setProxyAdmin(new, true){' '} + then{' '} setWhitelistedAdmin(new, true){' '} - then transferProxyAdmin(new){' '} - back-to-back (2 signatures). After the new admin calls{' '} - acceptProxyAdmin(), they - can revoke you as fee admin from their wallet. + back-to-back (2 signatures). Both grants are instant — no + 2-step ceremony.

- {rotation.stage !== 'complete' && ( -
- rotation.setInput(e.target.value)} - disabled={rotation.stage !== 'idle' && rotation.stage !== 'done'} - className="input flex-1 min-w-[18rem] font-mono text-xs" - /> - -
- )} - {rotation.stage !== 'idle' && rotation.stage !== 'complete' && ( -
    -
  1. - {rotation.grantConfirmed ? '✓ ' : ''}Grant fee admin to new - address (immediate) -
    2. -
  2. - {rotation.stage === 'done' ? '✓ ' : ''}Initiate proxyAdmin - transfer (pending until accepted) -
    3. -
  3. - New admin calls{' '} - acceptProxyAdmin(){' '} - (auto-detected) -
    4. -
  4. - (optional) Revoke your old fee admin rights from the new - admin's wallet -
    5. -
- )} - {rotation.stage === 'complete' && ( -
-

- ✓ Rotation complete — the new address is now proxyAdmin + fee - admin. You can optionally revoke yourself as fee admin from the - new admin's wallet. -

- -
- )} - {rotation.error && ( +
+ setInput(e.target.value)} + disabled={busy} + className="input flex-1 min-w-[18rem] font-mono text-xs" + /> + +
+ {(role1.error || role2.error) && (

- {rotation.error.split('\n')[0]} + {(role1.error ?? role2.error)!.message.split('\n')[0]}

)} diff --git a/packages/webapp/src/hooks/useProxyAdminRotation.ts b/packages/webapp/src/hooks/useProxyAdminRotation.ts deleted file mode 100644 index d119073..0000000 --- a/packages/webapp/src/hooks/useProxyAdminRotation.ts +++ /dev/null @@ -1,165 +0,0 @@ -import { useEffect, useRef, useState } from 'react' -import { isAddress, type Address } from 'viem' -import { useWaitForTransactionReceipt } from 'wagmi' - -import { useSetWhitelistedAdmin } from './useProxy' -import { useTransferProxyAdmin } from './useVersionedProxy' -import type { AdminRotateStage } from '../types' - -export interface ProxyAdminRotation { - /** Input address for the target admin. */ - input: string - setInput: (v: string) => void - /** Where we are in the 5-state machine. */ - stage: AdminRotateStage - /** User-facing error from any step of the chain. */ - error: string | null - /** True while any signature/mining inside the rotation is in flight. */ - busy: boolean - /** Receipts — exposed so the caller can show step-by-step progress. */ - grantConfirmed: boolean - transferConfirmed: boolean - /** Granular flags for per-step button text ("Sign Role 2…" etc). */ - grantPending: boolean - transferPending: boolean - /** True when the trimmed input is a valid hex address. */ - isValid: boolean - /** Start the 2-tx chain. Precondition: isValid. */ - start: () => Promise - /** Clear all state — run manually once the user acknowledges completion. */ - reset: () => void -} - -/** - * Encapsulates the "grant both roles" 5-state machine + the 2 writes it - * drives (setWhitelistedAdmin + transferProxyAdmin) and the external - * acceptance detection. Keeps UpgradeAuthorityPanel free of state-machine - * wiring — the panel just renders based on `stage`. - * - * Callbacks: - * - `onWriteSuccess` — fired when EITHER write mines. Callers refetch - * `useProxyVersions` to pick up the new pending/current admin. - */ -export function useProxyAdminRotation({ - proxy, - proxyAdmin, - account, - onWriteSuccess, -}: { - proxy: Address - proxyAdmin: Address | undefined - account: Address | undefined - onWriteSuccess: () => void -}): ProxyAdminRotation { - const { - setAdmin: grantFeeAdmin, - hash: grantHash, - isPending: grantPending, - reset: resetGrant, - } = useSetWhitelistedAdmin(proxy) - const grantReceipt = useWaitForTransactionReceipt({ hash: grantHash }) - - const { - transferAdmin, - hash: transferHash, - isPending: transferPending, - reset: resetTransfer, - } = useTransferProxyAdmin(proxy) - const transferReceipt = useWaitForTransactionReceipt({ hash: transferHash }) - - const [input, setInput] = useState('') - const [stage, setStage] = useState('idle') - const [error, setError] = useState(null) - - function resetAll() { - setStage('idle') - setInput('') - setError(null) - resetGrant() - resetTransfer() - } - - // Fire proxyAdmin transfer once the fee-admin grant confirms. - useEffect(() => { - if (stage === 'grant' && grantReceipt.isSuccess) { - resetGrant() - setStage('transfer') - transferAdmin(input.trim() as Address).catch((e: any) => { - setError(e?.message ?? 'transferProxyAdmin failed') - setStage('idle') - }) - } - }, [stage, grantReceipt.isSuccess, resetGrant, input, transferAdmin]) - - // Transfer mined → move to "done" (awaiting external accept). - useEffect(() => { - if (transferReceipt.isSuccess) { - resetTransfer() - onWriteSuccess() - if (stage === 'transfer') setStage('done') - } - }, [transferReceipt.isSuccess, resetTransfer, onWriteSuccess, stage]) - - // Refresh parent reads when grant mines too (not strictly needed for - // versions, but keeps behaviour symmetric with the panel's onTransferred). - useEffect(() => { - if (grantReceipt.isSuccess) onWriteSuccess() - }, [grantReceipt.isSuccess, onWriteSuccess]) - - // Detect external acceptance: on-chain proxyAdmin flipped to our target. - useEffect(() => { - if (stage !== 'done' || !proxyAdmin || !input) return - if (proxyAdmin.toLowerCase() === input.trim().toLowerCase()) { - setStage('complete') - } - }, [stage, proxyAdmin, input]) - - // Fresh wallet = fresh rotation form. Previous wallet's in-flight state - // would be confusing for the new principal. - const prevAccount = useRef(account) - useEffect(() => { - if (prevAccount.current !== account) { - resetAll() - prevAccount.current = account - } - // `resetAll` is intentionally omitted — it's stable for our purposes - // and listing it would require a useCallback dance for no gain. - - }, [account]) - - const isValid = isAddress(input.trim()) - - async function start() { - setError(null) - setStage('grant') - try { - await grantFeeAdmin(input.trim() as Address, true) - } catch (e: any) { - setError(e?.message ?? 'setWhitelistedAdmin failed') - setStage('idle') - } - } - - const busy = - grantPending || - grantReceipt.isLoading || - transferPending || - transferReceipt.isLoading || - stage === 'grant' || - stage === 'transfer' - - return { - input, - setInput, - stage, - error, - busy, - grantConfirmed: grantReceipt.isSuccess, - transferConfirmed: transferReceipt.isSuccess, - grantPending, - transferPending, - isValid, - start, - reset: resetAll, - } -} diff --git a/packages/webapp/src/hooks/useProxyRoles.ts b/packages/webapp/src/hooks/useProxyRoles.ts index 9b6180c..fd4666f 100644 --- a/packages/webapp/src/hooks/useProxyRoles.ts +++ b/packages/webapp/src/hooks/useProxyRoles.ts @@ -1,9 +1,12 @@ +import { useReadContract } from 'wagmi' import type { Address } from 'viem' +import { IntuitionVersionedFeeProxyABI } from '@intuition-fee-proxy/sdk' + export interface ProxyRoles { /** True when the connected wallet is a whitelisted fee admin (Role 2). */ isFeeAdmin: boolean - /** True when the connected wallet is the on-chain proxyAdmin (Role 1). */ + /** True when the connected wallet is in the proxyAdmins whitelist (Role 1). */ isProxyAdmin: boolean /** True when the connected wallet holds both roles simultaneously. */ hasBothRoles: boolean @@ -12,21 +15,30 @@ export interface ProxyRoles { } /** - * Derives the four role booleans from `account` + `proxyAdmin` + a precomputed - * `isFeeAdmin`. Pure — no side effects, no wagmi calls. + * Derives the four role booleans from `account` + on-chain + * `isProxyAdmin(account)` lookup + a precomputed `isFeeAdmin`. + * + * Role 1 became a whitelist (post 2-step retirement), so a single + * `proxyAdmin` address comparison is no longer enough — we hit the + * contract's `isProxyAdmin(addr)` view. */ export function useProxyRoles({ + proxy, account, - proxyAdmin, isFeeAdmin, }: { + proxy: Address | undefined account: Address | undefined - proxyAdmin: Address | undefined isFeeAdmin: boolean }): ProxyRoles { - const isProxyAdmin = Boolean( - account && proxyAdmin && account.toLowerCase() === proxyAdmin.toLowerCase(), - ) + const result = useReadContract({ + abi: IntuitionVersionedFeeProxyABI as any, + address: proxy, + functionName: 'isProxyAdmin', + args: account ? [account] : undefined, + query: { enabled: Boolean(proxy && account) }, + }) + const isProxyAdmin = Boolean(result.data) const hasBothRoles = isProxyAdmin && isFeeAdmin const isViewer = !isFeeAdmin && !isProxyAdmin diff --git a/packages/webapp/src/hooks/useVersionedProxy.ts b/packages/webapp/src/hooks/useVersionedProxy.ts index 4a8fed1..9937268 100644 --- a/packages/webapp/src/hooks/useVersionedProxy.ts +++ b/packages/webapp/src/hooks/useVersionedProxy.ts @@ -1,5 +1,18 @@ -import { useReadContract, useReadContracts, useWriteContract } from 'wagmi' -import { hexToString, stringToHex, type Address, type Hex } from 'viem' +import { useEffect, useState } from 'react' +import { + useBlockNumber, + usePublicClient, + useReadContract, + useReadContracts, + useWriteContract, +} from 'wagmi' +import { + getAddress, + hexToString, + stringToHex, + type Address, + type Hex, +} from 'viem' import { IntuitionVersionedFeeProxyABI } from '@intuition-fee-proxy/sdk' @@ -10,15 +23,13 @@ export function useProxyVersions(proxy: Address | undefined) { contracts: [ { abi, address: proxy, functionName: 'getVersions' }, { abi, address: proxy, functionName: 'getDefaultVersion' }, - { abi, address: proxy, functionName: 'proxyAdmin' }, - { abi, address: proxy, functionName: 'pendingProxyAdmin' }, + { abi, address: proxy, functionName: 'proxyAdminCount' }, ], allowFailure: false, query: { enabled: Boolean(proxy), - // Auto-poll so `proxyAdmin` / `pendingProxyAdmin` reflect - // acceptance that happens from another wallet or tab without - // forcing the user to refresh. + // Auto-poll so the admin count reflects grants/revokes happening + // from another wallet or tab without forcing the user to refresh. refetchInterval: 10_000, }, }) @@ -27,15 +38,14 @@ export function useProxyVersions(proxy: Address | undefined) { ...result, versions: (result.data?.[0] as Hex[] | undefined) ?? [], defaultVersion: result.data?.[1] as Hex | undefined, - proxyAdmin: result.data?.[2] as Address | undefined, - pendingProxyAdmin: result.data?.[3] as Address | undefined, + proxyAdminCount: (result.data?.[2] as bigint | undefined) ?? 0n, } } /** * Cheap 1-read hook for pages that only need the currently-active version - * label (Explore card, etc.). Avoids the 3-read overhead of - * `useProxyVersions` when the versions list / proxyAdmin aren't needed. + * label (Explore card, etc.). Avoids the overhead of `useProxyVersions` + * when the versions list / admin count aren't needed. * * Decodes the bytes32 to a human-readable label ("v2.0.0"). Empty string * if the proxy has no default set yet (shouldn't happen — Factory always @@ -108,44 +118,130 @@ export function useSetDefaultVersion(proxy: Address | undefined) { } /** - * Step 1 of the 2-step proxy-admin transfer. Only callable by the current - * `proxyAdmin`. Sets `pendingProxyAdmin = newAdmin`; the target must then - * call `acceptProxyAdmin()` from their own wallet to finalise. Passing a - * wrong address is recoverable — just call again with the correct one. + * Grant or revoke the Role 1 (proxyAdmin) whitelist for an address. + * Mirrors the Role 2 `setWhitelistedAdmin` pattern — instant, no 2-step + * ceremony. The contract enforces: + * - idempotent reject (revert if status already matches) + * - last-admin guard (revert if revoke would empty the whitelist) */ -export function useTransferProxyAdmin(proxy: Address | undefined) { +export function useSetProxyAdmin(proxy: Address | undefined) { const { writeContractAsync, data, isPending, error, reset } = useWriteContract() - function transferAdmin(newAdmin: Address) { + function setProxyAdmin(admin: Address, status: boolean) { if (!proxy) throw new Error('Proxy address missing') return writeContractAsync({ abi, address: proxy, - functionName: 'transferProxyAdmin', - args: [newAdmin], + functionName: 'setProxyAdmin', + args: [admin, status], }) } - return { transferAdmin, hash: data, isPending, error, reset } + return { setProxyAdmin, hash: data, isPending, error, reset } } /** - * Step 2 of the 2-step proxy-admin transfer. Must be called by the address - * currently set as `pendingProxyAdmin`. Promotes caller to `proxyAdmin`. + * Reconstruct the current proxyAdmin whitelist from the on-chain + * `ProxyAdminGranted` / `ProxyAdminRevoked` event log. Mirrors the + * Role 2 `useAdmins` pattern — the contract doesn't expose a getter + * for the full list, so we reduce events. */ -export function useAcceptProxyAdmin(proxy: Address | undefined) { - const { writeContractAsync, data, isPending, error, reset } = useWriteContract() +export function useProxyAdmins(proxy: Address | undefined) { + const publicClient = usePublicClient() + const { data: currentBlock } = useBlockNumber({ watch: true }) + const [admins, setAdmins] = useState([]) + const [isLoading, setIsLoading] = useState(false) + const [error, setError] = useState(null) + const [refreshKey, setRefreshKey] = useState(0) - function acceptAdmin() { - if (!proxy) throw new Error('Proxy address missing') - return writeContractAsync({ - abi, - address: proxy, - functionName: 'acceptProxyAdmin', - }) - } + useEffect(() => { + if (!publicClient || !proxy || !currentBlock) return + let cancelled = false + setIsLoading(true) + setError(null) + Promise.all([ + publicClient.getLogs({ + address: proxy, + event: { + type: 'event', + name: 'ProxyAdminGranted', + inputs: [{ type: 'address', name: 'admin', indexed: true }], + }, + fromBlock: 0n, + toBlock: currentBlock, + }), + publicClient.getLogs({ + address: proxy, + event: { + type: 'event', + name: 'ProxyAdminRevoked', + inputs: [{ type: 'address', name: 'admin', indexed: true }], + }, + fromBlock: 0n, + toBlock: currentBlock, + }), + ]) + .then(([grants, revokes]) => { + if (cancelled) return + type LogShape = { args: { admin: Address }; blockNumber: bigint; logIndex: number } + // Order all events by (block, logIndex) and replay them. + const events: Array<{ admin: Address; granted: boolean; bn: bigint; li: number }> = + [ + ...grants.map((l) => { + const x = l as unknown as LogShape + return { + admin: x.args.admin, + granted: true, + bn: x.blockNumber, + li: x.logIndex, + } + }), + ...revokes.map((l) => { + const x = l as unknown as LogShape + return { + admin: x.args.admin, + granted: false, + bn: x.blockNumber, + li: x.logIndex, + } + }), + ].sort((a, b) => (a.bn === b.bn ? a.li - b.li : a.bn < b.bn ? -1 : 1)) + + const set = new Set() + for (const e of events) { + const a = getAddress(e.admin) + if (e.granted) set.add(a) + else set.delete(a) + } + setAdmins(Array.from(set) as Address[]) + setIsLoading(false) + }) + .catch((e) => { + if (cancelled) return + setError(e as Error) + setIsLoading(false) + }) + return () => { + cancelled = true + } + }, [publicClient, proxy, currentBlock ? Number(currentBlock) : 0, refreshKey]) - return { acceptAdmin, hash: data, isPending, error, reset } + return { admins, isLoading, error, refetch: () => setRefreshKey((k) => k + 1) } +} + +/** Read whether a given address is currently a proxyAdmin. */ +export function useIsProxyAdmin( + proxy: Address | undefined, + candidate: Address | undefined, +) { + const result = useReadContract({ + abi, + address: proxy, + functionName: 'isProxyAdmin', + args: candidate ? [candidate] : undefined, + query: { enabled: Boolean(proxy && candidate) }, + }) + return { ...result, isProxyAdmin: Boolean(result.data) } } /** Read the proxy's human-readable name (bytes32, decoded to string). */ diff --git a/packages/webapp/src/pages/Docs.tsx b/packages/webapp/src/pages/Docs.tsx index d53a8e3..3d84096 100644 --- a/packages/webapp/src/pages/Docs.tsx +++ b/packages/webapp/src/pages/Docs.tsx @@ -504,8 +504,8 @@ function Architecture() { } /> -

Proxy admin (single address / Safe)

+

Proxy admins (whitelisted)

- Both admin roles are rotatable, but the mechanics differ on - purpose — the slower path covers the higher-impact role. + Both admin roles are whitelists with the same shape — N addresses, + instant grant/revoke, last-admin guard. The 2-step ceremony Role + 1 used to have was retired in favour of the multi-admin model + (any current admin can grant a replacement, then the old admin + revokes itself).

- Current admin calls transferProxyAdmin(newAdmin), - which only sets pendingProxyAdmin — no powers - move. The target must then sign{' '} - acceptProxyAdmin() from their own wallet to - finalise. Until then the outgoing admin keeps full powers - and can overwrite the pending candidate. + Whitelist of upgrade-authority addresses.{' '} + setProxyAdmin(addr, true/false) grants or + revokes in a single tx. Any current proxyAdmin can mutate + the list. The contract reverts if status already matches + and refuses to revoke the last remaining admin. } /> @@ -986,10 +988,10 @@ function AdminRotation() { term="Role 2 — fee admins (instant, N addresses)" desc={ <> - Whitelist-style. setWhitelistedAdmin(addr, true/false){' '} - adds or removes in a single tx. Any fee admin can grant or - revoke any other — except the last one cannot self-revoke. - Multiple fee admins can coexist; all share the same powers. + Same shape as Role 1, different surface.{' '} + setWhitelistedAdmin(addr, true/false) mutates + the fee-admin whitelist. Any fee admin can grant or revoke + any other — except the last one cannot self-revoke. } /> @@ -997,15 +999,13 @@ function AdminRotation() { term="Convenience — Grant both roles" desc={ <> - When a single wallet holds both roles and wants to hand the - whole proxy off, the webapp exposes a combined flow that - fires setWhitelistedAdmin(new, true) then{' '} - transferProxyAdmin(new) back-to-back (2 sigs, - 1 click). The target still has to{' '} - acceptProxyAdmin() from their side; acceptance - is auto-detected via the on-chain state poll. The outgoing - admin optionally revokes themselves as fee admin from the - new owner's wallet afterwards. + When a single wallet holds both roles and wants to hand + the whole proxy off, the webapp exposes a combined flow + that fires setProxyAdmin(new, true) then{' '} + setWhitelistedAdmin(new, true) back-to-back + (2 sigs, 1 click). Both grants are instant. The outgoing + admin optionally revokes itself from each whitelist + afterwards. } /> diff --git a/packages/webapp/src/pages/ProxyDetail.tsx b/packages/webapp/src/pages/ProxyDetail.tsx index 6fcd16b..4f67531 100644 --- a/packages/webapp/src/pages/ProxyDetail.tsx +++ b/packages/webapp/src/pages/ProxyDetail.tsx @@ -59,8 +59,6 @@ function ProxyDetail({ proxy }: { proxy: Address }) { const { versions, defaultVersion, - proxyAdmin, - pendingProxyAdmin, refetch: refetchVersions, isFetching: isVersionsFetching, } = useProxyVersions(proxy) @@ -78,8 +76,8 @@ function ProxyDetail({ proxy }: { proxy: Address }) { const family: ProxyFamily = channel === 'sponsored' ? 'sponsored' : 'standard' const { isProxyAdmin, isViewer } = useProxyRoles({ + proxy, account, - proxyAdmin, isFeeAdmin, }) @@ -173,8 +171,6 @@ function ProxyDetail({ proxy }: { proxy: Address }) { {tab === 'admins' && (