Good — that closes the ambiguity for me.
The combined invariant is the important part: every spend-controlling field belongs in the issued commitment, and every value the Guard consumes as proof-derived state must be constrained by the proof.
The missing adversarial vector also explains why the current suite stayed green: same proof + fresh nullifier is the case that actually exercises the broken binding.
The 7579 executor-module / 6900 direct-call clarification makes sense as well. Keeping coupling as a MUST while correcting the integration text is cleaner than weakening the security claim.
@mzf11125 @pipavlo82 the post 20 fix is in and I confirmed it on the PR, so this is a separate thing I hit while writing tests against the assets.
Row 16 of Test Cases promises that a collision at an issued index and one at an unissued index are distinguishable through highestIssuedIndex. That holds for one domain and inverts for two.
Line 75 says the index space is per pair:
`capabilityIndex` MUST be monotonically increasing per pair of `agentId` and `homeDomainId`
and the salt agrees, HKDF(issuerSecret, agentId || homeDomainId || capabilityIndex). But the ceiling that records it does not:
function highestIssuedIndex(uint256 agentId) external view returns (uint256);
and CoupledCredentialGuard.issue(bytes32, uint256 agentId, uint256 capabilityIndex) never receives a domain, so it could not key by one even if it wanted to.
Domain A issues indices 1 to 10 for an agent. Domain B has issued nothing. A clone then collides in domain B at index 4. The operator reads highestIssuedIndex[agentId] as 10, sees 4 <= 10, and classifies a live clone as their own reissue bug. That is the wrong answer in exactly the case the mechanism exists for.
Reproduced in Foundry against the assets on the PR head:
[FAIL] test_CollisionInASecondDomainIsMisclassified
domain B never issued index 4, but highestIssuedIndex[agentId] reads it as
issued because domain A raised the shared ceiling
The same scenario against a ceiling keyed [agentId][homeDomainId] passes, nothing else changed, so the failure is the keying rather than the test.
Same root cause, second place it shows: orchestrator is one immutable address for the whole Guard while line 173 says “the issuing orchestrator of the domain”. The Guard is domain-aware in execute and domain-blind in issuance.
Separately, and this is the bigger gap. Test Cases opens with “The reference implementation covers the following cases” and lists seventeen, and assets/erc-8380/ has six Solidity files and no tests. I would rather fix that than argue about it. I will write the suite and open a PR against your branch, with row 16 written per pair so it fails until the keying moves. Say if you would rather have the spec edit first.
Yes — this looks like a real second domain-separation bug, separate from the executor/expiry/nullifier binding issue.
If capabilityIndex is per (agentId, homeDomainId), then the issuance ceiling has to be scoped to the same namespace. A global highestIssuedIndex[agentId] lets issuance in one domain contaminate collision semantics in another, which makes row 16 invert exactly as you described.
The orchestrator point appears to be the same class of mismatch: execution is domain-aware, while issuance authority is currently global.
I’d prefer the failing suite first. Make row 16 explicit per pair and let it fail on the current implementation, then change the keying. That gives us a regression artifact proving the repair rather than just a prose correction.
And yes, the “reference implementation covers seventeen cases” claim should not remain stronger than the actual assets. Once the suite exists, we can make that sentence true mechanically.
Done in that order. The failing suite is on the PR.
Case 16 was test_16_KNOWNGAP_CollisionClassificationAcrossDomains and asserted 4 <= ceiling, which passes and only documents the gap. It is now test_16_CollisionClassificationIsPerDomain and asserts what row 16 promises, so it fails against the agent-keyed ceiling. 16 pass, 1 fail, against the assets on the PR head.
I ran the isolation rather than assuming it. Remove domain A’s issuance and the case turns green with nothing else changed, so the failure is the shared ceiling and not something else in the fixture.
One thing about the shape of the fix. Moving the ceiling to [agentId][homeDomainId] and passing a domain to issue are both interface changes, so this case gets rewritten in the same commit as the keying rather than surviving it. That is the ordinary red then green for an interface change, but I would rather say it than have the artifact look like it vanished.
On the seventeen cases line, agreed, and it is mechanically true for sixteen of them now. It stays overstated until the keying lands and case 16 goes green, so I would leave the sentence alone until then rather than weaken it and change it straight back.
Zidan has taken the proving relation repair as of today. The keying touches the same relation, so one pass is cheaper than two. I have said I will stay off the relation and rerun case 16 and the rest of the suite once the append-last change is in.
1 Like
Perfect — that is exactly the required red baseline. The isolation confirms that the shared agent-level ceiling causes the cross-domain contamination. Next gate: Zidan’s fix, followed by an exact-head 17/17 rerun.
@zexoverz @pipavlo82 case 16 is in, pushed directly to the branch: commit 7068f2a on erc-unclonable-agent-execution-credentials, visible on ethereum/ERCs#1953.
Same shape you both already agreed on. highestIssuedIndex moved to [agentId][homeDomainId], issue now takes homeDomainId. Orchestrator authorization moved into DomainRegistry alongside it, one address per domain rather than one for the whole Guard, since the single global orchestrator was the same domain-blind-issuance root cause showing up a second time.
One knock-on effect worth flagging: test_10_UnregisteredDomain broke under the fix, because issuance now needs a registered domain to resolve an orchestrator against, so issue itself reverts before execute’s own domain check ever runs. Rewrote it to arm the proof directly against the verifier rather than through issue, so it still exercises the domain check the row is actually about.
17/17 now, Faisal, so the suite you left at 16-pass-1-fail-on-purpose is green.
Confirmed — this is the right repair shape.
Scoping both the issuance ceiling and the issuing authority by homeDomainId removes the same domain-blind ambiguity at both points, rather than fixing only the observable symptom in highestIssuedIndex.
The test_10 rewrite also looks right: once issuance itself requires a registered domain, the unregistered-domain vector has to arm the verifier directly if the intended subject is execute()’s own domain gate.
Case 16 now matches the intended per-(agentId, homeDomainId) semantics.
1 Like
I have one question around reorgs and finality.
As I understand it, the at-most-once guarantee is relative to canonical chain state. A transaction emitting NullifierBurned could disappear in a reorg, and the credential could then be consumed again on the new canonical chain.
That is fine from the contract’s point of view, but an off-chain system may already have acted on the original event — for example, triggered an irreversible external action.
Would it be worth stating explicitly that ERC-8380 guarantees at-most-once consumption on canonical chain state, not physical-world exactly-once execution, and that off-chain consumers need their own finality policy before treating NullifierBurned as irreversible?
I think this also matters for monitoring: a second canonical consumption after a reorg should not automatically be treated as evidence that the credential was cloned.
This distinction looks worth making explicit.
The protocol’s at-most-once property is relative to canonical chain state. If a block containing NullifierBurned is reorged out, both the burn and any fused on-chain execution revert with that history; from the new canonical chain’s perspective the capability was not consumed there.
The separate hazard is an irreversible off-chain side effect triggered before the burn is final. That cannot be rolled back by consensus, so any consumer using NullifierBurned as authorization for an external irreversible action needs its own declared finality policy.
The monitoring consequence follows too: a later canonical consumption after a reorg is not, by itself, clone evidence. Clone classification must be relative to finalized/canonical history rather than to an observation that may have been orphaned.
This belongs in Security Considerations / observability rather than changing the nullifier construction itself.
1 Like
I’ve been running a soulbound identity layer for agents on Polygon mainnet since March — non-transferable Keys, epoch-based signals and attestations, no rebind path once minted.
Reading the executor-binding discussion here — the fix that puts executor and expiry into the commitment preimage so a clone can’t name itself executor — I want to check whether my design sits on the other side of the same line you’re drawing.
My Key enforces the opposite failure mode from the clone problem: executor is permanently fixed at mint —
if (from != address(0) && to != address(0)) revert Soulbound();
— so there’s no path for the credential to ever point at a different executor, even the agent’s own signer once it exists. An agent that starts out steward-minted stays steward-bound forever, or abandons the record and starts a fresh one.
Given the fix you’re converging on binds executor into the commitment permanently, is a rebind path something 8380 rules out by design, or just out of scope for v1? If it’s ruled out, that’s useful for me to know now rather than after I document it as a limitation.