OpenZeppelin / OpenZeppelin/openzeppelin-adapters
Consider routing access-control writes through shared runCapabilityWrite (typed-error mapping)
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 introducesrunCapabilityWriteis still uncommitted at the time of filing. Paths/symbols below will not resolve onmainuntil 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.ts→runCapabilityWrite(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
mapErrorpolicy (likely mapping known reverts → existingAccessControlErrorsubclasses such asOperationFailed, with a sensible fallback). - Route
executeActionthroughrunCapabilityWritewith 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
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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