feat(adapter-evm-core): dual-management ONCHAINID deploy for operator saga - #64
Merged
Conversation
… saga deployOnchainId uses createIdentityWithManagementKeys so the configured operatorManagementKey receives MANAGEMENT and can complete attachClaim. grantHolderManagementKey adds holder MANAGEMENT before attach-claim so partial failures leave the holder able to rescue their identity. createIRS now requires operatorManagementKey (validated at construction). Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a structural ERC-3643 onboarding issue by changing ONCHAINID deployment to grant MANAGEMENT to the saga operator at deploy time, then explicitly granting the holder MANAGEMENT before claim attachment, ensuring the operator can execute attachClaim while still ending with a holder-manageable identity.
Changes:
- Require
operatorManagementKeyincreateIRSand deploy identities viacreateIdentityWithManagementKeyswith operator MANAGEMENT. - Add
grantHolderManagementKeywrite to grant the holder MANAGEMENT (intended to run beforeattachClaim). - Add ERC-734 key helpers/ABIs and comprehensive tests validating calldata + key layout behavior.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/adapter-evm/test/ri-capabilities-subpath-runtime.test.ts | Updates runtime capability test to pass the new required operatorManagementKey. |
| packages/adapter-evm-core/src/irs/types.ts | Extends IRS service options with required operatorManagementKey. |
| packages/adapter-evm-core/src/irs/service.ts | Deploys with operator MANAGEMENT, validates operator key purpose, and adds grantHolderManagementKey. |
| packages/adapter-evm-core/src/irs/management-key.ts | Adds construction-time validation + error type for operatorManagementKey. |
| packages/adapter-evm-core/src/irs/index.ts | Re-exports new IRS helper modules. |
| packages/adapter-evm-core/src/irs/identity-keys.ts | Adds ERC-734 key hashing and keyHasPurpose read helper. |
| packages/adapter-evm-core/src/irs/actions.ts | Switches deploy to createIdentityWithManagementKeys and adds addKey action assembly. |
| packages/adapter-evm-core/src/irs/abis.ts | Adds ABIs for createIdentityWithManagementKeys, addKey, and keyHasPurpose. |
| packages/adapter-evm-core/src/irs/tests/irs.writes.test.ts | Updates deploy write test expectations and accounts for MANAGEMENT probe. |
| packages/adapter-evm-core/src/irs/tests/irs.receipt-wait-bounds.test.ts | Updates IRS construction helper to pass operatorManagementKey. |
| packages/adapter-evm-core/src/irs/tests/irs.receipt-identity.test.ts | Updates receipt resolution tests to account for MANAGEMENT probe (no factory getIdentity call). |
| packages/adapter-evm-core/src/irs/tests/irs.onboard-management-keys.test.ts | Adds dedicated tests enforcing dual-management deploy + grant behavior and construction validation. |
| packages/adapter-evm-core/src/irs/tests/irs.factory.test.ts | Updates capability surface expectations and validates missing/invalid operatorManagementKey. |
| packages/adapter-evm-core/src/irs/tests/irs.factory-read.test.ts | Updates capability construction to include operatorManagementKey. |
| packages/adapter-evm-core/src/capabilities/irs.ts | Makes operatorManagementKey required; exports extended EvmIRSCapability including new writes/reads. |
| packages/adapter-evm-core/src/capabilities/index.ts | Re-exports EvmIRSCapability. |
| packages/adapter-evm-core/src/tests/ri-sc004-coverage.test.ts | Updates SC004 coverage expectations to include grantHolderManagementKey. |
| .changeset/irs-onboard-dual-management.md | Documents minor release for @openzeppelin/adapter-evm with new required option + new write. |
Comments suppressed due to low confidence (1)
packages/adapter-evm-core/src/irs/service.ts:308
grantHolderManagementKeyperforms a post-txkeyHasPurposeread without catching RPC failures. A transient read error will throw a raw viem exception (notIdentityOperationFailed) and also hides the fact that the tx may have succeeded, which makes recovery logic harder. Catch read errors and rethrowIdentityOperationFailedwithonchainIdand the cause.
const holderHasManagement = await identityKeyHasPurpose(
this.rpcUrl(),
onchainId,
holder,
IDENTITY_KEY_PURPOSE_MANAGEMENT
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…OperationFailed Wrap deployOnchainId and grantHolderManagementKey keyHasPurpose probes so transient RPC errors after a successful write include onchainId for saga resume. Co-authored-by: Cursor <cursoragent@cursor.com>
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
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.
Summary
Fixes a structural defect in ERC-3643 holder onboarding: the saga runs entirely as the operator, but
deployOnchainIdcalled IdFactorycreateIdentity, which grants MANAGEMENT only to the holder.attachClaimtherefore reverts at gas estimation because the operator has no key on the identity.The bug (on-chain proof)
Sepolia identity
0xAac4B7E3FC0C6f4a00E03B053103FebAF8d02339(HD-11 holder0xd82124230713bf68b587b6d3d136d89cd4c2b5ce):0xDD601cb1dDb4471e88C51A5f64A9d542941791420xd821…b5ceThe saga was structurally impossible, not flaky.
Why this was invisible until now
PR #62 (
@openzeppelin/adapter-evm@2.3.2) fixed a different defect:deployOnchainIdresolved identity via a post-deploygetIdentityeth_call that could miss even after confirmation, aborting the saga beforeattachClaim. Every onboarding attempt died at deploy resolution. Fixing that revealed this second defect — the two are sequential, not unrelated.Trust-model decision (deliberately ratified)
createIdentityWithManagementKeyswas considered, but IdFactory forbids listing the wallet among management keys (require(_managementKeys[i] != keccak256(abi.encode(_wallet)))). Operator-managed and holder-managed are mutually exclusive at deploy.We chose dual management over operator-only:
createIdentityWithManagementKeys— operator receives MANAGEMENT.grantHolderManagementKey— operator grants holder MANAGEMENT before any claim step.Operator-only would have worked for the saga but would teach “the operator permanently owns the customer identity” — unacceptable for a reference implementation.
Ordering is load-bearing
Saga order:
deployOnchainId→grantHolderManagementKey→attachClaim→registerIdentity.grantHolderManagementKeymust run beforeattachClaim. If attach or register fails partway, the holder already holds MANAGEMENT and can rescue their own identity. Running grant after attach would leave a partial failure with an identity only the operator can touch — a fresh orphan trap. This is documented ongrantHolderManagementKeyinservice.ts; do not reorder for convenience.Breaking-ish surface (minor bump)
createIRSnow requiresoperatorManagementKey— the EOA that will executeattachClaim(never inferred from the tx signer; IdFactoryonlyOwnermay be a relayer contract). Missing/malformed values throwInvalidOperatorManagementKeyErrorat construction.Changeset: minor — consumer-visible identity key layout and new required construction input; not a patch.
New API
Type
EvmIRSCapabilityexportsgrantHolderManagementKeyandgetFactoryIdentityuntil@openzeppelin/ui-typescatches up.Post-deploy, the adapter verifies
operatorManagementKeyholds MANAGEMENT (keyHasPurpose) and fails loudly if not — misconfiguration surfaces at deploy time, not at attach.Tests have teeth
New tests in
irs.onboard-management-keys.test.tswere run against unfixedorigin/mainsource (implementation stashed, test file only): 6/6 failed. Strongest pin:A calldata assertion — cannot pass against the old implementation silently.
After restore: 6/6 passed. Full suite: adapter-evm-core 1208 tests, adapter-evm 150 passed (4 skipped).
Evidence:
scratchpad/dual-management-evidence.md(Maestri canvas).Not solved here
This fixes future onboards only. Three already-orphaned Sepolia identities (
0xd821…,0x9538…,0x3d53…) are holder-managed; no adapter change reaches them. They require separate out-of-band recovery.Follow-up (out of scope for this PR)
-coresaga must add the grant step and teach AC-3 resume logic the new partial state: factory-linked + holder lacks MANAGEMENT → resume atgrantHolderManagementKey(skip deploy).Test plan
pnpm typecheck,pnpm lint,pnpm test,pnpm build(adapter-evm-core + adapter-evm)Made with Cursor