In-line with main - #116
Merged
Merged
Conversation
Updated signature validation, added deployment scripts
## Signature validation - Bind the ERC-1271 challenge to version, `validUntil`, chain id and the wallet address. One signature no longer works on every chain, or on a second wallet holding the same owner key. - Reject a zero `validUntil` on a user operation. The EntryPoint rewrote zero to "never expires" while the ERC-1271 path read the same six bytes as already expired. - Return `SIG_VALIDATION_FAILED` instead of `0xffffffff` for a short signature. The old value was packed `validationData`, read as an aggregator address, and it reverted the whole bundle. - Guard the `abi.decode` of the signature body. Forty malformed bytes used to revert before reaching any of the validity checks. - Check that `typeIndex` and `challengeIndex` sit inside `clientDataJSON`, and that `challengeIndex` points at `"challenge":"` rather than any other quoted field. - Require `authenticatorData` to be longer than 32 bytes before reading the flags byte. - Remove the payable `fallback` from `Account4337`. A call naming a function the wallet does not have used to succeed silently. ## Device wallet keys and registration - Require a P256 owner key to be a point on the curve, on all three deploy paths and on `transferOwnership`. The old guard tested `bytes32.length != 0`, which rejects nothing. - Make `DeviceWallet.transferOwnership` update the registry. Rotating used to leave the retired key registered and the new one claimable by a second wallet. - Refuse a device identifier or owner key that is already registered. Both were overwritten silently before. - Add the CREATE2 salt to `postCreateAccount` so the address, identifier and owner key must derive each other. It previously accepted any address, an EOA included, as a valid device wallet. - Make `Account4337.initialize` internal, so `init` from a constructor is the only path to an owner key. ## ETH access and money movement - Delete `payETHForDataBundles`. It was an uncapped ETH exit to the vault with no price check of its own. - Refuse ETH access at bind time. `deployESIMWallet` and `addESIMWallet` took the flag from the caller, so the admin could undo a user's signed revocation by attaching a fresh wallet. Passing `true` now reverts. - Leave `toggleAccessToETH` as the only way access is ever granted, still `onlySelf`. - Send deployment funding to the wallet balance instead of the EntryPoint deposit. `createAccount` had a path that stranded ETH in the factory. - Charge the batch budget for ETH actually sent, not for what was requested. - Record a data bundle purchase before paying the vault. ## Admin, vault and pause - Move admin rotation to `Registry`. The address sat in two contracts and only one had a setter, so a rotation left the wallets still authorising the retired key. - Make `requestAdminUpdate` owner-only. The admin used to nominate its own replacement, so a stolen key could not be removed by anyone. - Strip the incumbent the moment a nomination lands. The role goes dormant instead of being held by two addresses. - Add `disableAdmin` and `enableAdmin`, owner-only, for suspending a key without naming a replacement. - Move the vault address to `Registry` and make it rotatable, owner-only. The registry copy was the one receiving money and it had no setter at all. - Add a protocol pause: `pause()` on the admin key, `unpause()` on the owner. - Read `upgradeManager()` through to `owner()` on both registries. It was a storage copy that kept naming the deploy-time address. - Refuse `renounceOwnership` on all five Ownable contracts. Renouncing a factory would freeze every wallet on its current beacon logic. ## eSIM wallet ownership - Restrict `removeESIMWallet` to the device wallet or the wallet being removed. One sibling could unbind another, strip its ETH access and pull its balance. - Clear the association and `canPullETH` before the ETH callback, and mark `removeESIMWallet` and `requestTransferOwnership` `nonReentrant`. - Register only wallets the factory deployed. `bindESIMWallet` decided by asking the target contract, which any contract can answer however its author likes. - Require the caller of `bindESIMWallet` and `toggleESIMWalletStandbyStatus` to be the wallet's current owner. The registry's stale association was accepted as an alternative caller. - Delete `updateDeviceWalletAssociatedWithESIMWallet`. It set the association without touching the standby marker, which is not a step in the transfer sequence. - Stop clearing `isESIMWalletValid` on removal. A released wallet read as an address the protocol had never seen, and its remaining purchase history could not be copied. - Re-bind an eSIM wallet when its owner cancels a pending transfer. Self-cancelling used to leave it orphaned. - Allow a pending transfer to be pointed at a different device wallet in one call. The second request used to revert. - Clear `dataBundlePriceCap` on `acceptOwnershipTransfer`, so an incoming owner is not bound by the outgoing one's ceiling. - Make a device wallet deploy eSIM wallets only for itself, and give a taken CREATE2 salt a named revert. - Emit `OwnershipTransferred` once per transfer instead of twice. ## Batching, so a device can always be deployed - Split history out of lazy deployment. The old call grew with both the eSIM count and each history length, and stayed inside a block only by silently dropping all but the last five entries per eSIM. - Add `setHistoryForLazyWallet(eSIMIdentifier, maxEntries)`, up to 50 an call, with a cursor so a dropped transaction is retried by repeating the identical call. - Batch lazy deployment itself: `deployLazyWalletAndSetESIMIdentifier` takes `maxWallets` and returns a remainder, and `deployMoreESIMWalletsForLazyDevice` continues from a cursor. Past roughly 45 eSIMs, a device could not be deployed at all. - Probe past an occupied CREATE2 salt on the lazy path. Landing on one used to freeze the device permanently, since the salt has no setter. - Report a full purchase history to the wallet now that the five-entry trim is gone. ## Identifier ownership between the two routes - Refuse a device identifier that has lazy records against it, on `deployDeviceWalletForUsers` and `postCreateAccount`. - Refuse an eSIM identifier reserved for a different device on `setESIMUniqueIdentifierForAnESIMWallet`. - Refuse an eSIM identifier a wallet already holds on `batchPopulateHistory`. - Record which eSIM wallet holds each identifier, readable through `eSIMWalletForIdentifier` and `isESIMIdentifierClaimed`. - Cap identifiers at 64 bytes on the paths that accept a new one. - Refuse an identifier switch once either device has a deployed wallet. - Rename `isLazyWalletDeployed` to `Registry.isDeviceIdentifierAlreadyUsed`. It always read the registry, so the old name was wrong for a device deployed the ordinary way. ## Price ceiling - Require a non-zero default price cap at `Registry.initialize`, and reject `setDefaultDataBundlePriceCap(0)`. Zero was the fail-open state. - Keep the per-wallet ceiling on the owning device wallet, which needs a signed `execute`. The admin names the price on every purchase, so neither ceiling is the admin's to set. ## ProtocolAdmin - Add the timelock: OZ `TimelockController` with an immutable `minDelayFloor` that `getMinDelay` clamps to, so `updateDelay(0)` cannot neuter it. - Give the guardian three named instant powers and nothing else: `unpauseInstantly`, `revokeCancellersInstantly`, `disableAdminInstantly`. There is no general fast path and no instant upgrade. - Refuse a guardian that also holds `CANCELLER_ROLE` or `PROPOSER_ROLE`, at construction and at runtime. One that could cancel would evict every other canceller and then cancel its own eviction forever. - Never let the guardian touch `PROPOSER_ROLE`. Zero proposers is unrecoverable, and a bricked admin plus a pause is a pause nobody can release. - Grant `EXECUTOR_ROLE` alongside `GUARDIAN_ROLE` on the runtime path, which the constructor already did. - Add `acceptOwnershipBatch`, permissionless, so the timelock can take ownership during its own installation without serving a delay first. - Add `disableAndNominate`, a scheduled operation that suspends an admin and names its replacement in one payload. ## Tests and verification - 646 tests across 61 suites, up from 211. Seven are fork tests against the deployed EntryPoint. - Certora specs for `Registry`, `ESIMWallet`, `DeviceWallet`, `DeviceWalletFactory`, `ProtocolAdmin` and one cross-contract spec over the first three. Six clean jobs, zero assert failures. - Invariant campaigns for the protocol, the admin, purchase history and ETH access, each with its own handler. - Storage layout pinned for every contract behind a proxy, so a moved slot fails CI. - Two gas baselines: `.gas-snapshot` per test body, and `snapshots/*.json` per operation with no setup in the figure. - WebAuthn assertions can be signed in tests through an FFI signer, which the challenge derivation work needed. ## Tooling - Replace the three hardhat deploy scripts with five forge scripts under `scripts/deploy/` and `scripts/upgrade/`. All three would have produced a broken deployment: no timelock, no price cap, and the v0.7 EntryPoint. - Key `deployments/address.json` by chain name and chain id, and record codehash, implementation, beacon, deploy parameters, role lists, status and build provenance. - Move to solc 0.8.36 with `evm_version = "osaka"`. Forge and hardhat still emit byte-identical bytecode. - Bump `openzeppelin-contracts-upgradeable` to v5.4.0, `account-abstraction` to v0.8.0, plus forge-std, solady and the upgrades plugin. Storage layouts identical throughout. - Replace all 92 `require` strings with custom errors, declared in one file. - Full NatSpec on every contract, with each one split into named sections.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.