Skip to content

Commit 22d5435

Browse files
Branimir Rakiccursoragent
authored andcommitted
fix(verify): tighten SPEC_CG_MEMORY_MODEL gates; address Codex round-5
Five real findings from Codex round-5 on PR #595, plus two findings held as Hold-the-Line (backwards-compat we explicitly don't owe) and one stale finding already fixed in 6a58ae2. Findings 5 + 6 — Adapter-level fail-open removed - chain/src/chain-adapter.ts: make `accessPolicy` and `publishPolicy` required fields on CreateOnChainContextGraphParams so `{}` can no longer silently create a public + open CG (privilege widening). - chain/src/evm-adapter.ts + mock-adapter.ts: enforce the required fields at runtime; refuse with an actionable error when omitted. - chain/src/evm-adapter.ts: getMinimumRequiredSignatures() now throws when ParametersStorage isn't resolvable instead of returning a hardcoded 3. The agent+publisher fail-closed guards already throw on "missing probe" / "garbage value", but the adapter was silently returning the wrong quorum from a null contract ref. Closed. - Added parity-test coverage for both behaviours. Finding 8 — Proposer-eligibility math in VerifyCollector - VerifyCollector.collect() previously hardcoded `remoteRequired = requiredSignatures - 1`, assuming the proposer always self-counts. After the round-4 sharding-table filter, an edge node proposer with identityId=0 gets dropped, leaving 0 sigs on a `minimumRequiredSignatures=1` chain → spurious no_quorum. - New `proposerCountsTowardQuorum?: boolean` (default true) lets the agent flag ineligible proposers so the collector demands the FULL quorum from remote peers. - DKGAgent.verify() now computes `proposerEligible` BEFORE calling collect() and passes it through. - Tests pin all three branches (proposer counts, proposer doesn't count, mixed) and the still-passing self-sign 1-of-1 path. Finding 7 — Verify-proposal metadata leak - The collector was fanning verify proposals (payload includes contextGraphId / verifiedMemoryId / batchId / root entities) to EVERY connected peer — for curated CGs that leaked CG metadata to unrelated peers before the downstream signer filter ran. - DKGAgent now resolves `getParticipantPeers` via the existing CGMemberEnumerator (allowlist for curated CGs, topic-subscribers for public, empty for legacy unknown). VerifyCollector accepts an async getParticipantPeers signature for this. Finding 2 — Drop legacy DKG_PARTICIPANT_IDENTITY_ID verify filter - `getVerifyParticipantIdentityIds` read pre-LU2 `_meta` triples and enforced them as a HARD allowlist in verify, which could strand already-registered CGs after upgrade when the current sharding table doesn't match the old roster (and `registerContextGraph` early-returns when onChainId is set, so it can't be re-fixed). - Deleted the method entirely; sharding-table membership is now the only authoritative ACK gate. Trust-degradation now keys off "any remote sharding-table-eligible ACK" instead of the legacy participant distinction. - Simplified `resolveVerifyApprovalIdentityId` to drop the legacy candidate-set probe; modern responders stamp identityId in the approval payload. Stale / held-the-line - Finding 3 (sharding-table fail-open) was on commit 269e8a2 and was already fixed by `6a58ae2d` (no-method branch throws). - Findings 1 + 4 (hard-400 on deprecated request fields breaks legacy callers) — user explicitly declined backwards compat for internal rc.x clients; we're keeping the explicit deprecation error so old callers fail loudly rather than silently writing CGs with broken assumptions. Test-callsite ripple - 19 test call sites updated to pass `accessPolicy`+`publishPolicy` explicitly. Existing semantics preserved (public+open where the test used to publish openly, curated+curators-only where it used the strict path). No new functional behaviour, just explicit args. Suites green locally: - packages/chain mock-adapter-parity 15/15 - packages/publisher verify-collector 14/14 - packages/publisher workspace + sigcoll 93/93 - packages/agent e2e-publish-protocol 13/13 - packages/agent agent.test 115/115 - packages/agent swm-ack-quorum 31/31 - packages/cli daemon-http-behavior-extra 50/50 - swm-snapshot-sync flake (pre-existing) unrelated. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 6a58ae2 commit 22d5435

23 files changed

Lines changed: 368 additions & 170 deletions

packages/agent/src/dkg-agent.ts

Lines changed: 80 additions & 139 deletions
Original file line numberDiff line numberDiff line change
@@ -11717,6 +11717,56 @@ export class DKGAgent {
1171711717
const proposerAddress = ethers.computeAddress(signingKey.publicKey);
1171811718

1171911719
// 5. Collect M-of-N approvals
11720+
// SPEC_CG_MEMORY_MODEL §4.3: sharding-table membership is the only
11721+
// authoritative gate for who can ACK a VM publish. Adapters that
11722+
// don't implement the membership probe are a misconfiguration here
11723+
// (real EVM and the in-tree mock both implement it). Cache decisions
11724+
// per batch to avoid hammering the RPC for repeated approvers.
11725+
if (typeof this.chain.isShardingTableMember !== 'function') {
11726+
throw new Error(
11727+
'verify: chain adapter does not implement `isShardingTableMember()`. ' +
11728+
'Cannot enforce SPEC_CG_MEMORY_MODEL §4.3 sharding-table ACK eligibility — refusing fail-open.',
11729+
);
11730+
}
11731+
const shardingMembershipCache = new Map<string, boolean>();
11732+
const probeShardingTableMembership = async (identityId: bigint): Promise<boolean> => {
11733+
if (identityId <= 0n) return false;
11734+
const key = identityId.toString();
11735+
const cached = shardingMembershipCache.get(key);
11736+
if (cached !== undefined) return cached;
11737+
try {
11738+
const ok = await this.chain.isShardingTableMember!(identityId);
11739+
shardingMembershipCache.set(key, ok);
11740+
return ok;
11741+
} catch (err: any) {
11742+
this.log.warn(
11743+
ctx,
11744+
`[verify] isShardingTableMember(${identityId}) probe failed (${err?.message ?? err}); ` +
11745+
`dropping that signer's approval as fail-closed`,
11746+
);
11747+
shardingMembershipCache.set(key, false);
11748+
return false;
11749+
}
11750+
};
11751+
11752+
// Proposer eligibility computed BEFORE collect() so VerifyCollector
11753+
// can require the full `requiredSignatures` remote ACKs (instead of
11754+
// `requiredSignatures - 1`) when the proposer can't self-count.
11755+
// Edge nodes have identityId=0 and aren't in the sharding table, so
11756+
// they always need every ACK to come from a member peer.
11757+
const proposerEligible =
11758+
this.identityId > 0n && await probeShardingTableMembership(this.identityId);
11759+
11760+
// CG-visible peer set for verify-proposal fan-out:
11761+
// SPEC_CG_MEMORY_MODEL §4.4 — the verify proposal payload includes
11762+
// contextGraphId, verifiedMemoryId, batchId, and root entities; for
11763+
// curated CGs that's metadata only allowlisted members may see. Run
11764+
// every recipient through `getOrCreateCGMemberEnumerator()` so
11765+
// unrelated peers don't receive the proposal. Public CGs fall
11766+
// through to the topic-subscribers set; legacy CGs with no member
11767+
// signal return an empty set (verify just degrades to no-quorum).
11768+
const cgMemberEnumerator = this.getOrCreateCGMemberEnumerator();
11769+
1172011770
const collector = new VerifyCollector({
1172111771
// rc.9 PR-11: route through messenger.sendReliable so
1172211772
// /dkg/10.0.1/verify-proposal gets envelope wrap + sender-side
@@ -11731,13 +11781,10 @@ export class DKGAgent {
1173111781
}
1173211782
return sendResult.response;
1173311783
},
11734-
getParticipantPeers: (cgId?: string) => {
11735-
const allPeers = this.node.libp2p.getPeers().map(p => p.toString()).filter(id => id !== this.peerId);
11736-
// LU-2: per SPEC_CG_MEMORY_MODEL there is no per-CG hosting
11737-
// committee — any sharding-table member can ACK. We pass all
11738-
// connected peers; signer eligibility is enforced via
11739-
// signature recovery + identityId resolution downstream.
11740-
return allPeers;
11784+
getParticipantPeers: async (cgId?: string) => {
11785+
const targetCg = cgId ?? opts.contextGraphId;
11786+
const enumeration = await cgMemberEnumerator.enumerate(targetCg);
11787+
return enumeration.members;
1174111788
},
1174211789
log: (msg: string) => this.log.info(ctx, msg),
1174311790
});
@@ -11756,86 +11803,38 @@ export class DKGAgent {
1175611803
entities,
1175711804
proposerSignature: { r: ethers.getBytes(proposerSig.r), vs: ethers.getBytes(proposerSig.yParityAndS) },
1175811805
requiredSignatures,
11806+
proposerCountsTowardQuorum: proposerEligible,
1175911807
timeoutMs: opts.timeoutMs ?? 30 * 60 * 1000, // 30 min default; VerifyCollector also enforces this as its max.
1176011808
allowPartial: true,
1176111809
});
1176211810

1176311811
// 6. Resolve identity IDs for each approver before on-chain submission.
11764-
const participantIdentityIds = await this.getVerifyParticipantIdentityIds(
11765-
opts.contextGraphId,
11766-
contextGraphIdOnChain,
11767-
);
11768-
const isEligibleParticipant = (identityId: bigint): boolean =>
11769-
participantIdentityIds === null || participantIdentityIds.has(identityId);
11770-
11771-
// Sharding-table eligibility: SPEC_CG_MEMORY_MODEL §4.3 promises
11772-
// that only sharding-table members can ACK a VM publish. Probe
11773-
// membership for every resolved signer and drop non-members
11774-
// fail-closed. Adapters that don't implement the probe are a
11775-
// misconfiguration in this code path (real EVM and the in-tree
11776-
// mock both implement it) — fail loudly instead of silently
11777-
// accepting any signer. Cache per batch to avoid repeated RPCs.
11778-
if (typeof this.chain.isShardingTableMember !== 'function') {
11779-
throw new Error(
11780-
'verify: chain adapter does not implement `isShardingTableMember()`. ' +
11781-
'Cannot enforce SPEC_CG_MEMORY_MODEL §4.3 sharding-table ACK eligibility — refusing fail-open.',
11782-
);
11783-
}
11784-
const shardingMembershipCache = new Map<string, boolean>();
11785-
const probeShardingTableMembership = async (identityId: bigint): Promise<boolean> => {
11786-
if (identityId <= 0n) return false;
11787-
const key = identityId.toString();
11788-
const cached = shardingMembershipCache.get(key);
11789-
if (cached !== undefined) return cached;
11790-
try {
11791-
const ok = await this.chain.isShardingTableMember!(identityId);
11792-
shardingMembershipCache.set(key, ok);
11793-
return ok;
11794-
} catch (err: any) {
11795-
this.log.warn(
11796-
ctx,
11797-
`[verify] isShardingTableMember(${identityId}) probe failed (${err?.message ?? err}); ` +
11798-
`dropping that signer's approval as fail-closed`,
11799-
);
11800-
shardingMembershipCache.set(key, false);
11801-
return false;
11802-
}
11803-
};
11804-
1180511812
const resolvedSignatures: Array<{ identityId: bigint; r: Uint8Array; vs: Uint8Array }> = [];
1180611813
const resolvedSignerAddresses: string[] = [];
11807-
if (
11808-
this.identityId > 0n
11809-
&& isEligibleParticipant(this.identityId)
11810-
&& await probeShardingTableMembership(this.identityId)
11811-
) {
11814+
if (proposerEligible) {
1181211815
resolvedSignatures.push({
1181311816
identityId: this.identityId,
1181411817
r: ethers.getBytes(proposerSig.r),
1181511818
vs: ethers.getBytes(proposerSig.yParityAndS),
1181611819
});
1181711820
resolvedSignerAddresses.push(proposerAddress);
1181811821
}
11819-
const participantResolvedRemoteAddresses: string[] = [];
1182011822
for (const a of result.approvals) {
11821-
let id = a.identityId || await this.resolveVerifyApprovalIdentityId(
11822-
a.approverAddress,
11823-
participantIdentityIds,
11824-
);
11823+
let id = a.identityId || await this.resolveVerifyApprovalIdentityId(a.approverAddress);
1182511824
if (!id || id === 0n) continue;
11826-
if (!isEligibleParticipant(id)) continue;
1182711825
if (!(await probeShardingTableMembership(id))) continue;
1182811826
resolvedSignatures.push({ identityId: id, r: a.signatureR, vs: a.signatureVS });
1182911827
resolvedSignerAddresses.push(a.approverAddress);
11830-
if (participantIdentityIds !== null && participantIdentityIds.has(id)) {
11831-
participantResolvedRemoteAddresses.push(a.approverAddress);
11832-
}
1183311828
}
1183411829
if (!result.quorumReached || resolvedSignatures.length < requiredSignatures) {
11835-
const trustLevel = participantResolvedRemoteAddresses.length > 0
11830+
// Trust degradation: any remote sharding-table-eligible ACK we
11831+
// collected (i.e. any signer past the proposer slot) lifts the
11832+
// batch to PartiallyVerified; otherwise it's self-attested.
11833+
const remoteCount = resolvedSignatures.length - (proposerEligible ? 1 : 0);
11834+
const trustLevel = remoteCount > 0
1183611835
? TrustLevel.PartiallyVerified
1183711836
: TrustLevel.SelfAttested;
11838-
const status = participantResolvedRemoteAddresses.length > 0 ? 'partial' : 'no_quorum';
11837+
const status = remoteCount > 0 ? 'partial' : 'no_quorum';
1183911838
await this.stampBatchTrustLevel(
1184011839
opts.contextGraphId,
1184111840
opts.batchId,
@@ -11845,8 +11844,8 @@ export class DKGAgent {
1184511844
this.log.info(
1184611845
ctx,
1184711846
`Verify batch ${opts.batchId} did not reach quorum ` +
11848-
`(${resolvedSignatures.length}/${requiredSignatures} identity-resolved signers, ` +
11849-
`${participantResolvedRemoteAddresses.length}/${result.requiredRemoteApprovals} participant remote approvals) — ` +
11847+
`(${resolvedSignatures.length}/${requiredSignatures} sharding-table-eligible signers, ` +
11848+
`${remoteCount}/${result.requiredRemoteApprovals} remote approvals) — ` +
1185011849
`stamped trustLevel=${trustLevel} without chain tx`,
1185111850
);
1185211851
return {
@@ -11908,81 +11907,23 @@ export class DKGAgent {
1190811907
};
1190911908
}
1191011909

11911-
private async resolveVerifyApprovalIdentityId(
11912-
approverAddress: string,
11913-
participantIdentityIds: Set<bigint> | null,
11914-
): Promise<bigint> {
11915-
if (typeof (this.chain as any).getIdentityIdForAddress === 'function') {
11916-
try {
11917-
const id = await (this.chain as any).getIdentityIdForAddress(approverAddress);
11918-
if (id && id !== 0n) return BigInt(id);
11919-
} catch {
11920-
// Fall through to participant-set probing below.
11921-
}
11922-
}
11923-
11924-
if (participantIdentityIds === null || participantIdentityIds.size === 0) return 0n;
11925-
if (typeof this.chain.verifyACKIdentity === 'function') {
11926-
for (const candidateIdentityId of participantIdentityIds) {
11927-
try {
11928-
if (await this.chain.verifyACKIdentity.call(this.chain, approverAddress, candidateIdentityId)) {
11929-
return candidateIdentityId;
11930-
}
11931-
} catch {
11932-
// Ignore individual lookup failures; another participant may still match.
11933-
}
11934-
}
11935-
}
11936-
11937-
if (typeof this.chain.isOperationalWalletRegistered !== 'function') return 0n;
11938-
for (const candidateIdentityId of participantIdentityIds) {
11939-
try {
11940-
if (await this.chain.isOperationalWalletRegistered.call(this.chain, candidateIdentityId, approverAddress)) {
11941-
return candidateIdentityId;
11942-
}
11943-
} catch {
11944-
// Ignore individual lookup failures; another participant may still match.
11945-
}
11946-
}
11947-
return 0n;
11948-
}
11949-
11950-
private async getVerifyParticipantIdentityIds(
11951-
contextGraphId: string,
11952-
contextGraphIdOnChain: bigint,
11953-
): Promise<Set<bigint> | null> {
11954-
// LU-2: on-chain CGs no longer carry per-CG hosting committees, so
11955-
// there is no `chain.getContextGraphParticipants()` to consult.
11956-
// Any sharding-table member can ACK; participant gating is removed.
11957-
// We still return locally-cached participant identity IDs (legacy
11958-
// `_meta` triples written before LU-2) when present, so existing
11959-
// CGs that pre-date this change continue to filter signers the same
11960-
// way until they're re-registered.
11961-
void contextGraphIdOnChain;
11962-
const metaGraph = assertSafeIri(contextGraphMetaGraphUri(contextGraphId));
11963-
const contextGraphUri = assertSafeIri(`did:dkg:context-graph:${contextGraphId}`);
11964-
const result = await this.store.query(
11965-
`SELECT ?identityId WHERE {
11966-
GRAPH <${metaGraph}> {
11967-
<${contextGraphUri}> <${DKG_ONTOLOGY.DKG_PARTICIPANT_IDENTITY_ID}> ?identityId
11968-
}
11969-
}`,
11970-
);
11971-
if (result.type !== 'bindings' || result.bindings.length === 0) {
11972-
return null;
11910+
private async resolveVerifyApprovalIdentityId(approverAddress: string): Promise<bigint> {
11911+
// Post-SPEC_CG_MEMORY_MODEL: identity resolution is whatever the
11912+
// chain adapter exposes via `getIdentityIdForAddress`. The legacy
11913+
// candidate-set probe against per-CG `participantIdentityId`
11914+
// triples was a pre-LU2 affordance and has been removed (Codex
11915+
// PR #595 round-5: stop using legacy roster as a verify filter).
11916+
// Modern responders that want to be counted MUST stamp their
11917+
// identityId in the VerifyApproval payload.
11918+
if (typeof (this.chain as any).getIdentityIdForAddress !== 'function') {
11919+
return 0n;
1197311920
}
11974-
const ids = new Set<bigint>();
11975-
for (const row of result.bindings as Record<string, string>[]) {
11976-
const raw = row.identityId?.replace(/^"|"$/g, '');
11977-
if (!raw) continue;
11978-
try {
11979-
const parsed = BigInt(raw);
11980-
if (parsed > 0n) ids.add(parsed);
11981-
} catch {
11982-
// Ignore malformed local metadata; it is not usable for trust elevation.
11983-
}
11921+
try {
11922+
const id = await (this.chain as any).getIdentityIdForAddress(approverAddress);
11923+
return id ? BigInt(id) : 0n;
11924+
} catch {
11925+
return 0n;
1198411926
}
11985-
return ids.size > 0 ? ids : null;
1198611927
}
1198711928

1198811929
private async promoteToVerifiedMemory(

packages/agent/test/e2e-chain.test.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,10 @@ describe('E2E: DKGAgent with real blockchain', () => {
112112
const chainAdapter = new EVMChainAdapter(
113113
makeAdapterConfig(ctx.rpcUrl, ctx.hubAddress, HARDHAT_KEYS.EXTRA1),
114114
);
115-
const cgResult = await chainAdapter.createOnChainContextGraph({});
115+
const cgResult = await chainAdapter.createOnChainContextGraph({
116+
accessPolicy: 0,
117+
publishPolicy: 1,
118+
});
116119
CONTEXT_GRAPH_ID = String(cgResult.contextGraphId);
117120

118121
await agents[0].createContextGraph({
@@ -222,7 +225,10 @@ describe('E2E: DKGAgent with real blockchain', () => {
222225
const chainAdapter = new EVMChainAdapter(
223226
makeAdapterConfig(ctx.rpcUrl, ctx.hubAddress, HARDHAT_KEYS.EXTRA1),
224227
);
225-
const cgResult = await chainAdapter.createOnChainContextGraph({});
228+
const cgResult = await chainAdapter.createOnChainContextGraph({
229+
accessPolicy: 0,
230+
publishPolicy: 1,
231+
});
226232
const secondCG = String(cgResult.contextGraphId);
227233

228234
await agents[0].createContextGraph({
@@ -315,7 +321,10 @@ describe('E2E: DKGAgent with real blockchain', () => {
315321
const chainAdapter = new EVMChainAdapter(
316322
makeAdapterConfig(ctx.rpcUrl, ctx.hubAddress, HARDHAT_KEYS.EXTRA1),
317323
);
318-
const cgResult = await chainAdapter.createOnChainContextGraph({});
324+
const cgResult = await chainAdapter.createOnChainContextGraph({
325+
accessPolicy: 0,
326+
publishPolicy: 1,
327+
});
319328
const gossipCG = String(cgResult.contextGraphId);
320329

321330
await agents[0].createContextGraph({

packages/agent/test/e2e-context-graph.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,10 @@ describe('E2E: context graph publish + finalization (shared chain)', () => {
7979
}, 15_000);
8080

8181
it('creates a context graph on the shared chain', async () => {
82-
const result = await nodeA.registerContextGraphOnChain({});
82+
const result = await nodeA.registerContextGraphOnChain({
83+
accessPolicy: 0,
84+
publishPolicy: 1,
85+
});
8386

8487
contextGraphId = result.contextGraphId;
8588
expect(contextGraphId).toBeDefined();

packages/agent/test/e2e-publish-protocol.test.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,10 @@ describe('E2E: Context graph publish with receiver + participant signatures', ()
255255
await sleep(1500);
256256

257257
// Both A and B are participants
258-
const result = await nodeA.registerContextGraphOnChain({});
258+
const result = await nodeA.registerContextGraphOnChain({
259+
accessPolicy: 0,
260+
publishPolicy: 1,
261+
});
259262
contextGraphId = result.contextGraphId;
260263
expect(Number(contextGraphId)).toBeGreaterThan(0);
261264
}, 20_000);
@@ -448,7 +451,10 @@ describe('E2E: Context graph registration rejected with insufficient participant
448451
nodeA.subscribeToContextGraph(CONTEXT_GRAPH);
449452

450453
// Context graph requires 2 signatures, but only 1 node available
451-
const cgResult = await nodeA.registerContextGraphOnChain({});
454+
const cgResult = await nodeA.registerContextGraphOnChain({
455+
accessPolicy: 0,
456+
publishPolicy: 1,
457+
});
452458
const contextGraphId = cgResult.contextGraphId;
453459

454460
await nodeA.share(CONTEXT_GRAPH, [
@@ -524,7 +530,10 @@ describe('E2E: Edge node participates in context graph governance', () => {
524530
const coreIdentity = await chainCore.getIdentityId();
525531
expect(coreIdentity).toBeGreaterThan(0n);
526532

527-
const cgResult = await coreNode.registerContextGraphOnChain({});
533+
const cgResult = await coreNode.registerContextGraphOnChain({
534+
accessPolicy: 0,
535+
publishPolicy: 1,
536+
});
528537
contextGraphId = cgResult.contextGraphId;
529538

530539
// Core writes data

packages/agent/test/e2e-security.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,7 @@ describe('Access protocol denial', () => {
288288
}), chainA);
289289

290290
const cgResult = await chainA.createOnChainContextGraph({
291+
accessPolicy: 0,
291292
publishPolicy: 1,
292293
});
293294

@@ -420,6 +421,7 @@ describe('Access protocol round-trip', () => {
420421
}), chainA);
421422

422423
const cgResult = await chainA.createOnChainContextGraph({
424+
accessPolicy: 0,
423425
publishPolicy: 1,
424426
});
425427
const onChainCgId = cgResult.contextGraphId.toString();

0 commit comments

Comments
 (0)