Skip to content

feat(adapter-evm-core): dual-management ONCHAINID deploy for operator saga - #64

Merged
pasevin merged 2 commits into
mainfrom
fix/irs-onboard-dual-management
Jul 27, 2026
Merged

feat(adapter-evm-core): dual-management ONCHAINID deploy for operator saga#64
pasevin merged 2 commits into
mainfrom
fix/irs-onboard-dual-management

Conversation

@pasevin

@pasevin pasevin commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes a structural defect in ERC-3643 holder onboarding: the saga runs entirely as the operator, but deployOnchainId called IdFactory createIdentity, which grants MANAGEMENT only to the holder. attachClaim therefore reverts at gas estimation because the operator has no key on the identity.

The bug (on-chain proof)

Sepolia identity 0xAac4B7E3FC0C6f4a00E03B053103FebAF8d02339 (HD-11 holder 0xd82124230713bf68b587b6d3d136d89cd4c2b5ce):

Party MANAGEMENT(1) CLAIM(3)
Operator 0xDD601cb1dDb4471e88C51A5f64A9d54294179142 false false
Holder 0xd821…b5ce true false

The saga was structurally impossible, not flaky.

Why this was invisible until now

PR #62 (@openzeppelin/adapter-evm@2.3.2) fixed a different defect: deployOnchainId resolved identity via a post-deploy getIdentity eth_call that could miss even after confirmation, aborting the saga before attachClaim. Every onboarding attempt died at deploy resolution. Fixing that revealed this second defect — the two are sequential, not unrelated.

Trust-model decision (deliberately ratified)

createIdentityWithManagementKeys was 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:

  1. Deploy via createIdentityWithManagementKeys — operator receives MANAGEMENT.
  2. 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: deployOnchainIdgrantHolderManagementKeyattachClaimregisterIdentity.

grantHolderManagementKey must run before attachClaim. 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 on grantHolderManagementKey in service.ts; do not reorder for convenience.

Breaking-ish surface (minor bump)

  • createIRS now requires operatorManagementKey — the EOA that will execute attachClaim (never inferred from the tx signer; IdFactory onlyOwner may be a relayer contract). Missing/malformed values throw InvalidOperatorManagementKeyError at construction.
  • Every future identity’s key layout changes (operator MANAGEMENT at deploy; holder MANAGEMENT after grant).

Changeset: minor — consumer-visible identity key layout and new required construction input; not a patch.

New API

createIRS(config, {
  signAndBroadcast,
  addresses: { identityRegistry, identityFactory, trustedIssuersRegistry },
  operatorManagementKey: OPERATOR_EOA, // required
});

await irs.deployOnchainId({ holder }, execConfig);
await irs.grantHolderManagementKey({ onchainId, holder }, execConfig);
await irs.attachClaim({ onchainId, claim }, execConfig);
await irs.registerIdentity({ holder, onchainId, country }, execConfig);

Type EvmIRSCapability exports grantHolderManagementKey and getFactoryIdentity until @openzeppelin/ui-types catches up.

Post-deploy, the adapter verifies operatorManagementKey holds 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.ts were run against unfixed origin/main source (implementation stashed, test file only): 6/6 failed. Strongest pin:

AssertionError: expected 'createIdentity' to be 'createIdentityWithManagementKeys'

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)

-core saga must add the grant step and teach AC-3 resume logic the new partial state: factory-linked + holder lacks MANAGEMENT → resume at grantHolderManagementKey (skip deploy).

Test plan

  • Red-first: 6/6 new tests fail against unfixed source
  • Green: 6/6 pass with fix
  • pnpm typecheck, pnpm lint, pnpm test, pnpm build (adapter-evm-core + adapter-evm)

Made with Cursor

… 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>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 operatorManagementKey in createIRS and deploy identities via createIdentityWithManagementKeys with operator MANAGEMENT.
  • Add grantHolderManagementKey write to grant the holder MANAGEMENT (intended to run before attachClaim).
  • 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

  • grantHolderManagementKey performs a post-tx keyHasPurpose read without catching RPC failures. A transient read error will throw a raw viem exception (not IdentityOperationFailed) and also hides the fact that the tx may have succeeded, which makes recovery logic harder. Catch read errors and rethrow IdentityOperationFailed with onchainId and 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.

Comment thread packages/adapter-evm-core/src/irs/service.ts Outdated
…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>
@pasevin
pasevin merged commit a86996a into main Jul 27, 2026
9 checks passed
@pasevin
pasevin deleted the fix/irs-onboard-dual-management branch July 27, 2026 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants