Skip to content

Let trusted issuer identities add and remove their claims without target key grants #91

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Joachim-Lebrun merged 4 commits into from
Sep 16, 2026
Merged

Let trusted issuer identities add and remove their claims without target key grants #91

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e0c88da
✨ Let trusted issuer identities add and remove their claims without t…
mohamadhammoud Sep 15, 2026
e585398
♻️ Route trusted issuer claims through addClaim and remove addClaimBy…
mohamadhammoud Sep 15, 2026
f4a7e0a
🎨 forge fmt
mohamadhammoud Sep 15, 2026
1834e26
🐛 Encode expected beacon revert errors with encodeWithSelector
mohamadhammoud Sep 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions contracts/libraries/Errors.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,17 +196,6 @@ library Errors {

/* ----- ERC734Validator trusted-issuer path ----- */

/// @notice Reverts when {ERC734Validator.addClaimByTrustedIssuer} is called by a wallet
/// that the factory does not resolve to any identity. The trusted-issuer path
/// requires the caller wallet to be a linked account on a factory-deployed
/// identity.
error CallerNotLinkedToFactoryIdentity(address caller);

/// @notice Reverts when the claim's declared `issuer` field does not match the
/// identity the caller wallet resolves to. A trusted issuer cannot ship a
/// claim attributed to a different issuer.
error DeclaredIssuerMismatch(address declaredIssuer, address resolvedIdentity);

/// @notice Reverts when the resolved issuer identity is not of type
/// {IdentityTypes.CLAIM_ISSUER}. The trusted-issuer path is reserved for
/// identities deployed under the role-gated CLAIM_ISSUER type so a high
Expand Down
112 changes: 39 additions & 73 deletions contracts/modules/validators/ERC734Validator.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import { Bytes } from "@openzeppelin/contracts/utils/Bytes.sol";
import { ECDSA } from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
import { MessageHashUtils } from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol";
import { SignatureChecker } from "@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol";
import { InteroperableAddress } from "@openzeppelin/contracts/utils/draft-InteroperableAddress.sol";
import { EnumerableSet } from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";

import { IIdentityFactory } from "../../factory/IIdentityFactory.sol";
Expand Down Expand Up @@ -169,18 +168,15 @@ contract ERC734Validator is ERC7579Validator, IERC735 {
address indexed identity, uint256 topic, bytes signature, Structs.ClaimData data, address caller
);

/// @notice Factory used by {addClaimByTrustedIssuer} to resolve a caller wallet to
/// its issuer identity and to confirm that identity came from the factory.
/// @notice Factory whose type record identifies trusted claim issuer identities.
IIdentityFactory public immutable factory;

/// @notice Registry consulted by {addClaimByTrustedIssuer} to confirm the issuer
/// identity meets the global claim-add threshold.
/// @notice Registry consulted by the trusted-issuer gate for the global
/// claim-add threshold.
IReputationRegistry public immutable reputationRegistry;

/// @param identityFactory Factory used to resolve a caller wallet to its issuer
/// identity in {addClaimByTrustedIssuer}. Reverts on zero.
/// @param registry Reputation registry consulted by {addClaimByTrustedIssuer}.
/// Reverts on zero.
/// @param identityFactory Factory whose type record gates trusted issuers. Reverts on zero.
/// @param registry Reputation registry for the claim-add threshold. Reverts on zero.
constructor(address identityFactory, address registry) {
require(identityFactory != address(0), Errors.ZeroAddress());
require(registry != address(0), Errors.ZeroAddress());
Expand Down Expand Up @@ -655,9 +651,9 @@ contract ERC734Validator is ERC7579Validator, IERC735 {
Structs.ClaimData memory _data,
string memory _uri
) public returns (bytes32 claimRequestId) {
// CLAIM_SIGNER or CLAIM_ADDER can add a claim. Self-issued claims still need a real
// signature (checked by isClaimValid in _addClaim), so CLAIM_ADDER cannot fake
// self-attestations.
// CLAIM_SIGNER or CLAIM_ADDER can add a claim; so can a trusted issuer identity (see
// below). Self-issued claims still need a real signature (checked by isClaimValid in
// _addClaim), so CLAIM_ADDER cannot fake self-attestations.
address caller = _msgSender();
// If the caller is this module itself, the call came from {addClaimTo}: the module made
// the outbound call, so the target's fallback appended the module's own address as the
Expand All @@ -675,39 +671,17 @@ contract ERC734Validator is ERC7579Validator, IERC735 {
if (caller == address(this)) {
caller = _issuer;
}
_requireClaimKey(msg.sender, caller, false);
// Either a claim key on the target, or the caller is the declared issuer itself and
// passes the trusted-issuer gate. A key grant is the target's own authorization, so
// it is never overridden by the global gate.
if (!_hasClaimKey(msg.sender, caller, false)) {
require(caller == _issuer, Errors.SenderDoesNotHaveClaimSignerKey());
_requireTrustedIssuer(_issuer);
}
return _addClaim(msg.sender, _topic, _scheme, _issuer, _signature, _data, _uri, caller);
}

/// @notice Add a claim without holding a CLAIM_ADDER / CLAIM_SIGNER key on the target
/// identity, by proving the caller is a trusted issuer in the {ReputationRegistry}.
///
/// Trust check (see {_requireTrustedIssuer}):
/// 1. The caller's wallet resolves through the factory to a non-zero issuer
/// identity (i.e. the wallet is a linked account on a factory-deployed
/// identity).
/// 2. `_issuer` equals that resolved identity. A trusted issuer cannot ship a
/// claim attributed to a different issuer.
/// 3. The identity self-declares type CLAIM_ISSUER.
/// 4. The issuer's score in the registry meets the global claim-add threshold.
///
/// When all four hold the rest of the flow is identical to {addClaim}: the issuer's
/// `isClaimValid` confirms the signature, the storage write happens, and
/// `ClaimAdded` / `ClaimChanged` fires. Removal is not exposed via this path.
function addClaimByTrustedIssuer(
uint256 _topic,
uint256 _scheme,
address _issuer,
bytes memory _signature,
Structs.ClaimData memory _data,
string memory _uri
) public returns (bytes32 claimRequestId) {
address caller = _msgSender();
_requireTrustedIssuer(caller, _issuer);
claimRequestId = _addClaim(msg.sender, _topic, _scheme, _issuer, _signature, _data, _uri, caller);
}

/// @dev Shared write path for `addClaim` and `addClaimByTrustedIssuer`. The issuer-side
/// @dev Shared write path for `addClaim` and `addClaimTo`. The issuer-side
/// `isClaimValid` is called unconditionally; the storage write and event match the
/// standard `addClaim` body. Claim id is (issuer, topic); re-adding overwrites.
function _addClaim(
Expand Down Expand Up @@ -746,28 +720,15 @@ contract ERC734Validator is ERC7579Validator, IERC735 {
}
}

/// @dev Trusted-issuer gate. Four conditions, all required:
/// 1. The caller wallet resolves through the factory to a non-zero issuer identity
/// (i.e. it is a linked account on a factory-deployed identity).
/// 2. That identity equals the claim's declared issuer (issuer-bound rule).
/// 3. The factory's type record for that identity is `CLAIM_ISSUER`. Without this,
/// any identity that happens to be scored above the threshold (e.g. an
/// INDIVIDUAL elevated by the manager) could write claims silently.
/// 4. Its reputation in the registry meets the global claim-add threshold.
/// `reputationOf` already returns `0` for non-factory identities, so the score check
/// implicitly re-confirms factory membership.
function _requireTrustedIssuer(address caller, address expectedIssuer) internal view {
address callerIdentity = factory.getIdentity(InteroperableAddress.formatEvmV1(block.chainid, caller));
require(callerIdentity != address(0), Errors.CallerNotLinkedToFactoryIdentity(caller));
require(expectedIssuer == callerIdentity, Errors.DeclaredIssuerMismatch(expectedIssuer, callerIdentity));
require(
factory.identityTypeOf(callerIdentity) == IdentityTypes.CLAIM_ISSUER,
Errors.IdentityNotClaimIssuerType(callerIdentity)
);
/// @dev Trusted-issuer gate. The issuer must be recorded by the factory as type
/// CLAIM_ISSUER (the record is written once at deploy, so it also proves factory
/// membership) and its reputation must meet the global claim-add threshold.
function _requireTrustedIssuer(address issuer) internal view {
require(factory.identityTypeOf(issuer) == IdentityTypes.CLAIM_ISSUER, Errors.IdentityNotClaimIssuerType(issuer));
IReputationRegistry registry = reputationRegistry;
uint128 score = registry.reputationOf(callerIdentity);
uint128 score = registry.reputationOf(issuer);
uint128 threshold = registry.claimAddThreshold();
require(score >= threshold, Errors.ReputationBelowClaimAddThreshold(callerIdentity, score, threshold));
require(score >= threshold, Errors.ReputationBelowClaimAddThreshold(issuer, score, threshold));
}

/// @inheritdoc IERC735
Expand All @@ -776,14 +737,19 @@ contract ERC734Validator is ERC7579Validator, IERC735 {
function removeClaim(bytes32 _claimId) public returns (bool success) {
address account = msg.sender;
address caller = _msgSender();
// CLAIM_ADDER cannot remove; only CLAIM_SIGNER (or self-call) is accepted here.
_requireClaimKey(account, caller, true);

AccountRegistry storage s = _store().registries[account];
Structs.Claim storage c = s.claims[_claimId];
uint256 topic = c.topic;
require(topic != 0, Errors.ClaimNotRegistered(_claimId));

// CLAIM_ADDER cannot remove; CLAIM_SIGNER can. A trusted issuer can remove
// its own claims only, so it can never touch another issuer's claims.
if (!_hasClaimKey(account, caller, true)) {
require(caller == c.issuer, Errors.SenderDoesNotHaveClaimSignerKey());
_requireTrustedIssuer(c.issuer);
}

// Revoke the digest on both the holder's and the issuer's sets so _getClaimStatus (which
// reads the issuer's set) blocks re-adding the same bytes. Marking an already marked
// digest is fine: an outside issuer can accept the same claim again, and that one still
Expand Down Expand Up @@ -882,8 +848,10 @@ contract ERC734Validator is ERC7579Validator, IERC735 {
return _getClaimDigest(msg.sender, _identity, _topic, _data);
}

/// @notice Verify a claim then write it to the target identity via its `addClaim`. The target
/// must have granted the calling issuer identity a CLAIM_SIGNER or CLAIM_ADDER key.
/// @notice Verify a claim then write it to the target identity via its `addClaim`. The write
/// is accepted when the target granted the calling issuer identity a CLAIM_SIGNER or
/// CLAIM_ADDER key, or when the issuer passes the trusted-issuer gate (factory
/// CLAIM_ISSUER type record plus reputation threshold) — no per-target grant needed.
function addClaimTo(
uint256 _topic,
uint256 _scheme,
Expand Down Expand Up @@ -983,15 +951,13 @@ contract ERC734Validator is ERC7579Validator, IERC735 {
return IClaimIssuer.ClaimStatus.Valid;
}

/// @dev Require the off-chain caller to hold a claim key on `account`. CLAIM_SIGNER covers add
/// and remove; CLAIM_ADDER is accepted only when `onlyClaimSigner` is false (addClaim).
function _requireClaimKey(address account, address caller, bool onlyClaimSigner) internal view {
/// @dev Whether `caller` holds a claim key on `account`. CLAIM_SIGNER covers add and
/// remove; CLAIM_ADDER counts only when `onlyClaimSigner` is false (addClaim).
function _hasClaimKey(address account, address caller, bool onlyClaimSigner) internal view returns (bool) {
bytes32 keyHash = hashAddress(caller);

if (keyHasPurpose(account, keyHash, KeyPurposes.CLAIM_SIGNER)) return;
if (!onlyClaimSigner && keyHasPurpose(account, keyHash, KeyPurposes.CLAIM_ADDER)) return;

revert Errors.SenderDoesNotHaveClaimSignerKey();
if (keyHasPurpose(account, keyHash, KeyPurposes.CLAIM_SIGNER)) return true;
return !onlyClaimSigner && keyHasPurpose(account, keyHash, KeyPurposes.CLAIM_ADDER);
}

/// @dev Require the off-chain caller to hold MANAGEMENT on `account`.
Expand Down
11 changes: 5 additions & 6 deletions scripts/DeployOnchainID.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ contract DeployOnchainID is Script {
pure
returns (Structs.ModuleInstall[] memory installs)
{
installs = new Structs.ModuleInstall[](20);
installs = new Structs.ModuleInstall[](19);
installs[0] =
Structs.ModuleInstall({ moduleType: MODULE_TYPE_VALIDATOR, module: validator, initData: "", purpose: 0 });
installs[1] = Structs.ModuleInstall({
Expand All @@ -269,11 +269,10 @@ contract DeployOnchainID is Script {
installs[12] = _fallback(validator, IClaimIssuer.revokeClaimByDigest.selector);
installs[13] = _fallback(validator, IClaimIssuer.isDigestRevoked.selector);
installs[14] = _fallback(validator, IClaimIssuer.addClaimTo.selector);
installs[15] = _fallback(validator, ERC734Validator.addClaimByTrustedIssuer.selector);
installs[16] = _fallback(validator, IERC734.keyHasPurpose.selector);
installs[17] = _fallback(validator, IERC734.getKey.selector);
installs[18] = _fallback(validator, IERC734.getKeyPurposes.selector);
installs[19] = _fallback(validator, IERC734.getKeysByPurpose.selector);
installs[15] = _fallback(validator, IERC734.keyHasPurpose.selector);
installs[16] = _fallback(validator, IERC734.getKey.selector);
installs[17] = _fallback(validator, IERC734.getKeyPurposes.selector);
installs[18] = _fallback(validator, IERC734.getKeysByPurpose.selector);
}

function _fallback(address module, bytes4 selector) private pure returns (Structs.ModuleInstall memory) {
Expand Down
4 changes: 2 additions & 2 deletions test/Proxy.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ contract ProxyTest is OnchainIDSetup {
}

function test_revertBecauseBeaconIsZeroAddress() public {
vm.expectRevert(abi.encode(ERC1967Utils.ERC1967InvalidBeacon.selector, address(0)));
vm.expectRevert(abi.encodeWithSelector(ERC1967Utils.ERC1967InvalidBeacon.selector, address(0)));
new BeaconProxy(address(0), _initData(IdentityTypes.INDIVIDUAL));
}

Expand All @@ -99,7 +99,7 @@ contract ProxyTest is OnchainIDSetup {
}

function test_preventCreatingBeaconWithZeroImplementation() public {
vm.expectRevert(abi.encode(UpgradeableBeacon.BeaconInvalidImplementation.selector, address(0)));
vm.expectRevert(abi.encodeWithSelector(UpgradeableBeacon.BeaconInvalidImplementation.selector, address(0)));
new UpgradeableBeacon(address(0), address(this));
}

Expand Down
38 changes: 35 additions & 3 deletions test/claim-issuers/ClaimTo.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { Errors } from "contracts/libraries/Errors.sol";
import { KeyPurposes } from "contracts/libraries/KeyPurposes.sol";
import { KeyTypes } from "contracts/libraries/KeyTypes.sol";
import { ERC734Validator } from "contracts/modules/validators/ERC734Validator.sol";
import { ReputationRegistry } from "contracts/reputation/ReputationRegistry.sol";
import { Structs } from "contracts/storage/Structs.sol";

/// @notice Coverage for `addClaimTo`. The target's claim-key policy must be evaluated against the
Expand Down Expand Up @@ -81,12 +82,43 @@ contract ClaimToTest is OnchainIDSetup {
assertEq(topic, TOPIC);
}

/// @notice A target that granted nothing rejects the issuer.
function test_addClaimTo_reverts_whenTargetHasNoGrantForIssuer() public {
/// @notice A target that granted nothing still accepts the claim when the issuer passes the
/// trusted-issuer gate (factory CLAIM_ISSUER type + reputation threshold). No
/// per-target key grant is needed for a trusted issuer.
function test_addClaimTo_writesClaim_whenIssuerIsTrusted_noGrantOnTarget() public {
(bytes memory signature, Structs.ClaimData memory data) = _buildIssuerClaim(address(bobIdentity));

vm.prank(claimIssuerOwner);
vm.expectRevert(Errors.SenderDoesNotHaveClaimSignerKey.selector);
IClaimIssuer(address(claimIssuer)).addClaimTo(TOPIC, 1, signature, data, "uri", IIdentity(address(bobIdentity)));

bytes32 claimId = ClaimSignerHelper.computeClaimId(address(claimIssuer), TOPIC);
(uint256 topic,, address issuer,,,) = IIdentity(address(bobIdentity)).getClaim(claimId);
assertEq(topic, TOPIC);
assertEq(issuer, address(claimIssuer));
}

/// @notice A target that granted nothing rejects an issuer that fails the trusted-issuer
/// gate: with the claim-add threshold above the issuer's score, the keyless path
/// is refused.
function test_addClaimTo_reverts_whenTargetHasNoGrantAndIssuerNotTrusted() public {
ReputationRegistry registry = onchainidSetup.reputationRegistry;
address reputationManager = makeAddr("reputationManager");

bytes4[] memory selectors = new bytes4[](1);
selectors[0] = ReputationRegistry.setClaimAddThreshold.selector;
vm.startPrank(deployer);
onchainidSetup.accessManager.setTargetFunctionRole(address(registry), selectors, 1001);
onchainidSetup.accessManager.grantRole(1001, reputationManager, 0);
vm.stopPrank();
vm.prank(reputationManager);
registry.setClaimAddThreshold(1);

(bytes memory signature, Structs.ClaimData memory data) = _buildIssuerClaim(address(bobIdentity));

vm.prank(claimIssuerOwner);
vm.expectRevert(
abi.encodeWithSelector(Errors.ReputationBelowClaimAddThreshold.selector, address(claimIssuer), 0, 1)
);
IClaimIssuer(address(claimIssuer)).addClaimTo(TOPIC, 1, signature, data, "uri", IIdentity(address(bobIdentity)));
}

Expand Down
18 changes: 2 additions & 16 deletions test/helpers/IdentityHelper.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ library IdentityHelper {
pure
returns (Structs.ModuleInstall[] memory installs)
{
installs = new Structs.ModuleInstall[](16);
installs = new Structs.ModuleInstall[](15);
// ----- merged ERC734Validator: validator (holds the key registry) -----
// Empty initData -> onInstall does not seed a key; MANAGEMENT comes from `_keys`.
installs[0] = Structs.ModuleInstall({
Expand Down Expand Up @@ -193,13 +193,6 @@ library IdentityHelper {
initData: abi.encodePacked(IClaimIssuer.addClaimTo.selector),
purpose: 0
});
// ----- trusted-issuer add path served by the merged module via fallback -----
installs[15] = Structs.ModuleInstall({
moduleType: MODULE_TYPE_FALLBACK,
module: claimsModule,
initData: abi.encodePacked(ERC734Validator.addClaimByTrustedIssuer.selector),
purpose: 0
});
}

/// @notice Deploys an Identity through a standalone BeaconProxy and installs an
Expand Down Expand Up @@ -247,7 +240,7 @@ library IdentityHelper {
clientData: ""
});

Structs.ModuleInstall[] memory modules = new Structs.ModuleInstall[](16);
Structs.ModuleInstall[] memory modules = new Structs.ModuleInstall[](15);
// The validator install carries empty initData: the MANAGEMENT key is seeded from `keys`
// above (the account seeds every `_keys` entry into the enshrined module during
// `initialize`). The validator owns scoping now, so no account-level purpose is granted to
Expand Down Expand Up @@ -338,13 +331,6 @@ library IdentityHelper {
initData: abi.encodePacked(IERC734.getKeysByPurpose.selector),
purpose: 0
});
modules[15] = Structs.ModuleInstall({
moduleType: MODULE_TYPE_FALLBACK,
module: address(claimsModule),
initData: abi.encodePacked(ERC734Validator.addClaimByTrustedIssuer.selector),
purpose: 0
});

BeaconProxy proxy =
new BeaconProxy(address(b), abi.encodeCall(Identity.initialize, (identityType, keys, modules)));
identity = Identity(payable(address(proxy)));
Expand Down
Loading
Loading