feat: world chain account implementation by 0xOsiris · Pull Request #656 · worldcoin/world-chain

0xOsiris · 2026-05-19T23:37:09Z

Note

High Risk
High risk because it introduces new core account-router contracts that rely on DELEGATECALL for signature/policy validation and manage verifier installation/storage invariants, which are security-critical paths.

Overview
Implements the WIP-1001 account router as a new WorldChainAccount contract deployed behind a global beacon (WorldChainAccountUpgradeableBeacon) and reached via WorldChainAccountBeaconProxy.

The router now enforces manager-only installation of an immutable admin verifier and a rotating session key ring (ERC-7201 namespaced storage), and dispatches EIP-1271 signature checks plus session-policy evaluation to installed verifiers via controlled DELEGATECALL (rejecting unknown verifiers before any verifier code runs).

Adds Dispatch fallback handling with an explicit UnknownSelector error, defines IWorldChainAccount + IWorldChainAccountRouterErrors, and updates wip-1001.md references from IWorldChainAccountRouter to IWorldChainAccount.

^{Reviewed by Cursor Bugbot for commit 8da0f22. Configure here.}

0xOsiris · 2026-05-19T23:49:35Z

@cursoragent review

cursor · 2026-05-19T23:49:40Z

You need to increase your spend limit or enable usage-based billing to run background agents. Go to Cursor

0xOsiris · 2026-05-19T23:49:57Z

@cursor review

0xOsiris · 2026-05-19T23:50:27Z

/otto review with GPT 5.5 Max Thinking

agentotto · 2026-05-19T23:50:53Z

Let me fetch the PR details first.
Spawning a GPT-5.5 subagent with workspace to do a thorough review.
I've spawned a GPT-5.5 subagent (with high reasoning) to do the review. It will:

Clone the repo and checkout the contracts/world-chain-account-router branch
Thoroughly analyze all changed files with a security focus (DELEGATECALL safety, ERC-7201 storage, EIP-1271, beacon proxy upgrades, access control, session key rotation, etc.)
Post inline comments at specific file/line locations
Post a brief top-level summary with the overall verdict

I'll let you know when it's done! 🔍

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 8da0f22. Configure here.}

agentotto

Security review inline findings.

agentotto · 2026-05-19T23:55:59Z

+
+    /// @inheritdoc IWorldChainAccount
+    function isValidSignatureForAdmin(bytes32 hash, bytes calldata signature)
+        external


Critical: these validation dispatch functions perform DELEGATECALL but are callable by any EOA/contract. The restricted-frame invariant only holds when the manager uses STATICCALL; a direct CALL to this function executes the installed verifier in the account's storage with normal state-mutating EVM semantics, so verifier code can SSTORE account validation state, emit logs, make external calls/reenter, etc. This violates the interface's no-non-manager mutation boundary and makes verifier bugs exploitable by anyone. Please gate isValidSignatureForAdmin, isValidSignatureForVerifier, and evaluateSessionPolicyForVerifier with onlyManager (or equivalent) before any delegatecall; the manager can still STATICCALL them.

+1 I think this comment makes sense. Those fns should be only callable by the world chain account manager, so we could add a modifier that restricts access to it

This is not the case

Why? From the spec it seems these fns are only callable by world chain account manager. If we don't want this, I would specify in the wip better

agentotto · 2026-05-19T23:55:59Z

+    ///         and running each verifier's installation hook. The hash rotation atomically retires
+    ///         every entry from the prior key ring without an explicit per-element wipe.
+    function _installKeyRing(WorldChainAccountVerifier[] calldata sessionVerifiers) internal {
+        IWorldChainAccount.KeyRingStorage storage $ = _keyRingStorage();


High: _installKeyRing does not enforce the WIP key-ring invariants at the router boundary (length in 1..MAX_SESSION_VERIFIERS, non-zero deployed verifier addresses, and no duplicate verifier addresses) before committing keyRingHash/running hooks. Duplicates are especially problematic because keyRingHash commits to multiple descriptors while sessionKeyRing is keyed only by address; the hook runs multiple times and final verifier storage may correspond to only the last descriptor. Relying only on manager validation leaves a manager/predeploy upgrade bug able to install a ring the router cannot faithfully represent. Please validate the full set first, then set keyRingHash and run hooks.

+1 this comment makes sense

as mentioned on slack, we should revisit this!

agentotto · 2026-05-19T23:55:59Z

+/// @custom:security-contact security@toolsforhumanity.com
+contract WorldChainAccountUpgradeableBeacon is UpgradeableBeacon {
+    constructor(address implementation_, address initialOwner) UpgradeableBeacon(implementation_, initialOwner) {}
+}


High: a global UpgradeableBeacon lets the owner change account logic, the immutable MANAGER, and storage assumptions for every account without changing each account's runtime code hash. WIP domains/address derivation and txpool cache invalidation bind WORLD_CHAIN_ACCOUNT_ROUTER_CODE_HASH/keyRingHash, not the beacon implementation hash/version, so an upgrade can silently change validation semantics for already-created accounts and pending signatures. If upgradeability remains, the spec/code should bind or gate implementation versions (e.g. fork/timelock governance plus storage-layout checks and golden tests); otherwise the router implementation should be immutable.

agentotto · 2026-05-19T23:55:59Z

+contract WorldChainAccountBeaconProxy is BeaconProxy {
+    /// @notice The single global `WorldChainAccountUpgradeableBeacon` predeploy.
+    /// TODO: FIXME(Osiris): Update Spec
+    address internal constant WORLD_CHAIN_ACCOUNT_BEACON = 0x000000000000000000000000000000000000B0cC;


Spec mismatch: this hard-coded beacon predeploy is not currently an activation parameter in WIP-1001, while the proxy runtime/code hash depends on the immutable beacon address. If clients/managers derive WORLD_CHAIN_ACCOUNT_ROUTER_CODE_HASH with a different beacon address, accounts can satisfy the expected proxy code shape while pointing at the wrong or nonexistent beacon and all calls brick. Please finalize the beacon address in the spec/activation params and add code-hash golden tests.

+1, this needs to be added to the wip

/// TODO: FIXME(Osiris): Update Spec

agentotto · 2026-05-19T23:56:11Z

Overall verdict: changes requested before merge. The implementation is compact and broadly follows the WIP-1001 shape, but this is a consensus/account-auth boundary and the inline blockers should be resolved with adversarial tests before landing.

I verified the documented ERC-7201 slot derivation and compiled the changed contracts with solc-js 0.8.28 in this workspace. I could not run the Foundry suite here because forge is not installed in the workspace image, so please rely on CI/full local Foundry runs for final validation.

agentotto · 2026-05-19T23:56:38Z

The GPT-5.5 subagent has completed the review. Here's a summary of what was done:

✅ Review complete on PR #656 — "feat: world chain account implementation"

The subagent:

Cloned the repo, checked out contracts/world-chain-account-router, and read all 8 changed files
Verified the ERC-7201 slot derivation matches the documented formula
Posted 6 inline security review comments on specific file+line locations
Posted a top-level summary comment

Links:

🔍 Inline review: feat: world chain account implementation #656 (review)
💬 Summary comment: feat: world chain account implementation #656 (comment)

Note: Foundry tests could not be run because forge isn't installed in the workspace image — manual or CI test verification is recommended before merge given the high-risk nature of this PR.

alessandromazza98

There are some comments that make sense to be addressed.

Requesting changes to point those out, once they're addressed, it's good to be merged

alessandromazza98 · 2026-05-20T08:01:57Z

+contract WorldChainAccountBeaconProxy is BeaconProxy {
+    /// @notice The single global `WorldChainAccountUpgradeableBeacon` predeploy.
+    /// TODO: FIXME(Osiris): Update Spec
+    address internal constant WORLD_CHAIN_ACCOUNT_BEACON = 0x000000000000000000000000000000000000B0cC;


+1, this needs to be added to the wip

alessandromazza98 · 2026-05-20T09:14:40Z

+    ///         and running each verifier's installation hook. The hash rotation atomically retires
+    ///         every entry from the prior key ring without an explicit per-element wipe.
+    function _installKeyRing(WorldChainAccountVerifier[] calldata sessionVerifiers) internal {
+        IWorldChainAccount.KeyRingStorage storage $ = _keyRingStorage();


+1 this comment makes sense

alessandromazza98 · 2026-05-20T09:18:22Z

+        IWorldChainSessionVerifier.ExecutionTraceContext calldata context
+    ) external override sessionInstalled(verifier) returns (bool allowed) {
+        bytes memory ret =
+            verifier.functionDelegateCall(abi.encodeCall(IWorldChainSessionVerifier.evaluateSessionPolicy, (context)));


alessandromazza98 · 2026-05-20T09:20:06Z

+
+    /// @inheritdoc IWorldChainAccount
+    function isValidSignatureForAdmin(bytes32 hash, bytes calldata signature)
+        external


+1 I think this comment makes sense. Those fns should be only callable by the world chain account manager, so we could add a modifier that restricts access to it

alessandromazza98

I'm actually approving because I don't want to block you, but please take a look at those comments and address them before merging

Kemperino · 2026-05-20T14:31:01Z

+    struct KeyRingStorage {
+        address adminVerifier;
+        bytes32 keyRingHash;
+        mapping(address verifier => bytes32 keyRingHash) sessionKeyRing;


not unique in current design.

i still think this struct generally is very elegant implementation tho 👍

Kemperino · 2026-05-20T14:35:20Z

+    ///         caller other than `WORLD_CHAIN_ACCOUNT_MANAGER`. Replaces any previously installed
+    ///         key ring on `setKeyRing`.
+    /// @param sessionVerifiers The ordered session verifier set to install.
+    function installKeyRing(WorldChainAccountVerifier[] calldata sessionVerifiers) external;


super nit, but it probably should be Keyring not KeyRing

Kemperino · 2026-05-20T14:41:06Z

+    ///      verifier writes any validation-affecting state into its own deterministic account
+    ///      storage namespace. Uses OpenZeppelin's `Address.functionDelegateCall`, which reverts
+    ///      with the callee's returndata on failure.
+    function _runInstallHook(address verifier, bytes calldata installation) private {


would it make sense to add a function to the session verifier namespace which defines the storage slot (erc7201) of that verifier (e.g based on a name) and double checks no verifiers clash? otherwise they might override each others storage and you look yourself out of your account potentially.

Yes, this is a great idea

Kemperino · 2026-05-20T14:54:28Z

+    ///         and running each verifier's installation hook. The hash rotation atomically retires
+    ///         every entry from the prior key ring without an explicit per-element wipe.
+    function _installKeyRing(WorldChainAccountVerifier[] calldata sessionVerifiers) internal {
+        IWorldChainAccount.KeyRingStorage storage $ = _keyRingStorage();


as mentioned on slack, we should revisit this!

Kemperino · 2026-05-20T14:55:27Z

+import {IWorldChainAccountRouterErrors} from "../interfaces/IWorldChainAccountRouterErrors.sol";
+
+/// @title Dispatch
+/// @author 0xOsiris, World Contributors


it better not have a vulnerability then xD

0xOsiris added 12 commits May 15, 2026 12:07

feat: add smart contract scaffolding

8c678c4

chore: outline TODOs

a156f64

wip: add fallback router scaffolding

3f6e8db

wip: cleanup

e4b7e7c

wip

02a0a4b

wip

8932334

wip: world chain account beacon/proxy

0a6246f

chore: remove session verifier/admin verifier

d4e13e4

chore: remove world chain account manager

358d495

fix: dispatch

acbad0e

chore: cleanup comments

3763f8e

Merge branch 'main' into contracts/world-chain-account-router

8da0f22

0xOsiris marked this pull request as ready for review May 19, 2026 23:49

0xOsiris requested review from 0xForerunner, Dzejkop, Kemperino, alessandromazza98, cichaczem, karankurbur, kilianglas, murph and piohei as code owners May 19, 2026 23:49

cursor Bot reviewed May 19, 2026

View reviewed changes

agentotto Bot reviewed May 19, 2026

View reviewed changes

alessandromazza98 requested changes May 20, 2026

View reviewed changes

alessandromazza98 approved these changes May 20, 2026

View reviewed changes

Kemperino reviewed May 20, 2026

View reviewed changes

fix: key ring bound checks

a60d205

Kemperino mentioned this pull request May 20, 2026

feat: WIP-1001: Introduce Session Verifier Instances #660

Merged

0xOsiris added 5 commits May 20, 2026 11:30

fix: add zero address check

f4e4679

test: add tests

48d740c

test: add tests

01bf729

chore: update specification

72e25b8

chore: cleanup nameing

6d1add1

Conversation

0xOsiris commented May 19, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xOsiris commented May 19, 2026

Uh oh!

cursor Bot commented May 19, 2026

Uh oh!

0xOsiris commented May 19, 2026

Uh oh!

0xOsiris commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

agentotto Bot commented May 19, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

agentotto Bot left a comment

Choose a reason for hiding this comment

Uh oh!

agentotto Bot May 19, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agentotto Bot May 19, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agentotto Bot May 19, 2026

Choose a reason for hiding this comment

Uh oh!

agentotto Bot May 19, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

agentotto Bot commented May 19, 2026

Uh oh!

agentotto Bot commented May 19, 2026

Uh oh!

alessandromazza98 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alessandromazza98 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

0xOsiris commented May 19, 2026 •

edited by cursor Bot

Loading

0xOsiris commented May 19, 2026 •

edited

Loading