OpenZeppelin / OpenZeppelin/openzeppelin-adapters

Consider routing access-control writes through shared runCapabilityWrite (typed-error mapping)

Open
#39 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
2
Forks
1
Avg merge
1h 47m
Merged PRs (30d)
14

Description

Context

Note: The code referenced below is not yet committed or pushed to remote. It currently lives on the local feature branch 002-ri-evm-capabilities (RI EVM capabilities POC) and has only been committed up to the IRS capability (692f1a6); the ERC-3643 work that introduces runCapabilityWrite is still uncommitted at the time of filing. Paths/symbols below will not resolve on main until that branch lands. This issue exists to capture the follow-up so it isn't lost.

While implementing the RI capabilities (IRS, ERC-3643), we extracted a shared write skeleton:

  • packages/adapter-evm-core/src/shared/executor.tsrunCapabilityWrite(params, mapError)

It centralizes the common write control flow: submit assembled calldata via the injected executor → catch → log → map the failure to a typed error. Both EvmIRSService.execute and EvmErc3643Service.execute now use it, each supplying its own capability-specific mapError policy (IRS → IdentityOperationFailed; ERC-3643 → RecipientNotVerified / HolderFrozen / InsufficientBalance / ComplianceModuleRejected / fallback RICapabilityOperationFailed).

Problem / Opportunity

The third write-capable service, EvmAccessControlService, does not use this skeleton. Its write entry point delegates straight to the executor with no catch and no error mapping:

// packages/adapter-evm-core/src/access-control/service.ts
protected async executeAction(txData, executionConfig, onStatusChange?, runtimeApiKey?): Promise<OperationResult> {
  return this.executeTransaction(txData, executionConfig, onStatusChange, runtimeApiKey);
}

It was intentionally left as-is during the RI work because adopting runCapabilityWrite there is a behavioral change, not a like-for-like refactor: it would start catching every write error, logging it, and wrapping it into a typed error. That requires its own decisions and is out of scope for the POC branch.

Proposed follow-up

Evaluate giving access-control writes consistent typed-error mapping via runCapabilityWrite:

  • Define an access-control mapError policy (likely mapping known reverts → existing AccessControlError subclasses such as OperationFailed, with a sensible fallback).
  • Route executeAction through runCapabilityWrite with that mapper.
  • Update/extend access-control write tests that currently assert on raw/unwrapped errors.
  • Confirm no consumer (e.g. role-manager, ui-builder) depends on the current raw-error shape from access-control writes.

Acceptance criteria

  • Access-control write failures surface as typed AccessControlErrors consistent with the IRS/ERC-3643 pattern, or a deliberate decision is recorded that access-control should keep propagating raw errors (with rationale).
  • All existing access-control tests pass (updated as needed).
  • No behavioral regression for downstream consumers.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading packages/adapter-evm-core/src/shared/executor.ts and packages/adapter-evm-core/src/access-control/service.ts, then inspect the existing access-control write tests and downstream consumers such as role-manager and ui-builder. Determine whether access-control should adopt typed error mapping or deliberately retain raw errors, and verify the chosen behavior with passing tests and no consumer regressions.

Written by the indexing model from the issue text.

Assessment

Tech stack
blockchain, typescript
Domain
backend-api-design, blockchain
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.