Skip to content

Commit 0eb8a4c

Browse files
Jurij89claude
andcommitted
fix(chain): carry structured authz facts on the error + cover the author-aware reroute probe (#1689)
Two review findings from PR #1971. PublisherNotAuthorizedForContextGraphError accepted the structured diagnostic details, rendered a message from them, then dropped several. A consumer asking the very next question -- "is this chain simply not rotated yet, or was the author checked and refused?" -- had to parse `error.message` for facts the adapter had already computed exactly once. This PR exists to remove that coupling, so re-introducing it one question later was self-defeating. The error now carries the full (frozen) details object; `code`, the message contract and the flattened convenience fields are unchanged, so publisher's classifier and existing consumers are unaffected. The author-aware `poolHasFundableSigner` call in `enrichInsufficientPublisherFundsError` had no test. Dropping the author argument there would leave every existing test green while silently turning a recoverable author-authorized reroute into a terminal NO_FUNDED_PUBLISHER_WALLET -- the same shape as the dkg-publisher.ts forwarding line the adversarial review caught. Now covered, with a payer-only negative control proving the legacy path is unchanged. Chain unit suite 853 -> 857 passing. Closure build clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent f37c634 commit 0eb8a4c

2 files changed

Lines changed: 131 additions & 1 deletion

File tree

packages/chain/src/evm-adapter-errors.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -434,23 +434,49 @@ export interface PublisherNotAuthorizedForCgDetails {
434434
* `authority_forbidden` instead of falling through to the retryable
435435
* `rpc_unavailable` default that made this failure loop to `maxRetries`. Same
436436
* shape and convention as {@link InsufficientPublisherFundsError}.
437+
*
438+
* Carries the FULL {@link PublisherNotAuthorizedForCgDetails} it was built from, not
439+
* just the fields the message happens to render. The gate computes those facts
440+
* exactly once at throw time; without them on the object, a consumer asking the very
441+
* next question — *"is this chain simply not rotated yet, or was the author checked
442+
* and refused?"* — would have to parse `message`. This PR exists to remove exactly
443+
* that coupling, so re-introducing it one question later would be self-defeating.
444+
* The message stays the human surface; `details` is the machine surface.
437445
*/
438446
export class PublisherNotAuthorizedForContextGraphError extends Error {
439447
readonly code = PUBLISHER_NOT_AUTHORIZED_FOR_CG_CODE;
448+
449+
/**
450+
* The exact facts the message was rendered from — frozen, so a consumer cannot
451+
* mutate one error's diagnosis and have it disagree with its own text.
452+
* `attestedAuthorConsidered` + `deployedLifecycleVersion` are the pair worth
453+
* reading: together they separate "author refused" from "author never weighed
454+
* because the lifecycle predates the capability (or its version was unreadable)".
455+
*/
456+
readonly details: PublisherNotAuthorizedForCgDetails;
457+
458+
// Flattened conveniences, retained so existing consumers keep compiling.
440459
readonly contextGraphId: bigint;
441460
readonly payerAddress: string;
442461
readonly payerPoolAddresses?: readonly string[];
443462
readonly attestedAuthorAddress?: string;
463+
444464
constructor(
445465
message: string,
446466
details: PublisherNotAuthorizedForCgDetails,
447467
options?: { cause?: unknown },
448468
) {
449469
super(message, options);
450470
this.name = 'PublisherNotAuthorizedForContextGraphError';
471+
this.details = Object.freeze({
472+
...details,
473+
payerPoolAddresses: details.payerPoolAddresses
474+
? Object.freeze([...details.payerPoolAddresses])
475+
: undefined,
476+
});
451477
this.contextGraphId = details.contextGraphId;
452478
this.payerAddress = details.payerAddress;
453-
this.payerPoolAddresses = details.payerPoolAddresses;
479+
this.payerPoolAddresses = this.details.payerPoolAddresses;
454480
this.attestedAuthorAddress = details.attestedAuthorAddress;
455481
}
456482
}

packages/chain/test/evm-adapter-publish-admission.unit.test.ts

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import { describe, it, expect } from 'vitest';
2727
import { ethers } from 'ethers';
2828
import {
2929
EVMChainAdapter,
30+
InsufficientPublisherFundsError,
3031
PublisherNotAuthorizedForContextGraphError,
3132
isPublisherNotAuthorizedForCgError,
3233
formatPublisherNotAuthorizedForCgMessage,
@@ -39,6 +40,7 @@ import { PUBLISHER_NOT_AUTHORIZED_FOR_CG_CODE } from '@origintrail-official/dkg-
3940

4041
const CG = 7n;
4142
const AUTHOR = '0x1111111111111111111111111111111111111111';
43+
const lc = (s: string) => s.toLowerCase();
4244
const PK_A = `0x${'1'.repeat(64)}`;
4345
const PK_B = `0x${'2'.repeat(64)}`;
4446

@@ -162,6 +164,48 @@ describe('#1689 publish admission — diagnostic accuracy [CH-1689-D]', () => {
162164
);
163165
expect(error.message).toContain(`deployed version is ${BELOW_THRESHOLD}`);
164166
});
167+
168+
// The facts are computed ONCE at throw time. If they live only in the rendered
169+
// message, the next consumer question — "not rotated yet, or author refused?" —
170+
// forces message-parsing, which is the exact coupling this PR removes. So the
171+
// thrown object must expose what the formatter was given, and the two must agree.
172+
it('the thrown error carries the structured facts its message was rendered from', async () => {
173+
const { a } = makeAdapter([AUTHOR.toLowerCase()], BELOW_THRESHOLD);
174+
const payer = a.signerPool[0].address;
175+
176+
const error = await caught(() => a.resolvePinnedPublisherSigner(CG, payer, AUTHOR));
177+
178+
// The two fields that answer the question without touching `message`.
179+
expect(error.details.attestedAuthorConsidered).toBe(false);
180+
expect(error.details.deployedLifecycleVersion).toBe(BELOW_THRESHOLD);
181+
expect(error.details.minLifecycleVersion).toBe(ATTESTED_AUTHOR_PUBLISH_AUTHZ_MIN_KAL_VERSION);
182+
expect(error.details.contextGraphId).toBe(CG);
183+
expect(lc(error.details.payerAddress)).toBe(lc(payer));
184+
expect(error.details.attestedAuthorAddress).toBe(AUTHOR);
185+
186+
// Facts and text must describe the same rejection — a message rendered from
187+
// different values than the object reports would be worse than either alone.
188+
expect(error.message).toBe(formatPublisherNotAuthorizedForCgMessage(error.details));
189+
190+
// Frozen: one error's diagnosis cannot be mutated into disagreeing with its text.
191+
expect(Object.isFrozen(error.details)).toBe(true);
192+
});
193+
194+
it('carries the pool facts on a signer-pool rejection, author consulted and refused', async () => {
195+
const { a, pool } = makeAdapter([], ATTESTED_AUTHOR_PUBLISH_AUTHZ_MIN_KAL_VERSION, {
196+
extraKeys: [PK_B],
197+
});
198+
199+
const error = await caught(() => a._authorizedPublisherSigners(pool, CG, AUTHOR));
200+
201+
// Author WAS weighed here (supported lifecycle) and refused — the opposite
202+
// diagnosis to the version-gated case above, distinguishable without parsing.
203+
expect(error.details.attestedAuthorConsidered).toBe(true);
204+
expect(error.details.deployedLifecycleVersion)
205+
.toBe(ATTESTED_AUTHOR_PUBLISH_AUTHZ_MIN_KAL_VERSION);
206+
expect(error.details.payerPoolAddresses).toEqual(pool.map((w) => w.address));
207+
expect(error.message).toBe(formatPublisherNotAuthorizedForCgMessage(error.details));
208+
});
165209
});
166210

167211
describe('#1689 publish admission — fails closed on EVERY path [CH-1689-F]', () => {
@@ -251,6 +295,66 @@ describe('#1689 publish admission — one condition, one error contract [CH-1689
251295
});
252296
});
253297

298+
describe('#1689 publish admission — author-aware NO_FUNDED enrichment [CH-1689-E]', () => {
299+
// `enrichInsufficientPublisherFundsError` decides whether a funding failure is the
300+
// TERMINAL `NO_FUNDED_PUBLISHER_WALLET` ("no wallet is a viable reroute") or a
301+
// recoverable wrong-pick. It answers that with `poolHasFundableSigner`, which must
302+
// be asked on the SAME two-principal rule the publish is held to: an attested
303+
// author the CG authorizes makes every funded wallet admissible.
304+
//
305+
// Nothing else guards the author argument at that call site. Drop it and the
306+
// payer-only NO_FUNDED tests still pass, while a recoverable reroute is
307+
// misreported as terminal — a misclassification in the same family as the bug
308+
// this PR fixes. Mutation-proven: removing the argument fails exactly this test.
309+
it('an authorized author keeps a fundable pool wallet a viable reroute (NOT terminal NO_FUNDED)', async () => {
310+
const { a, pool } = makeAdapter(
311+
// ONLY the attested author is authorized — no wallet in the pool is.
312+
[AUTHOR.toLowerCase()],
313+
ATTESTED_AUTHOR_PUBLISH_AUTHZ_MIN_KAL_VERSION,
314+
{ extraKeys: [PK_B] },
315+
);
316+
const [pinned, other] = pool;
317+
// The pinned payer is short on TRAC; the other pool wallet can cover the cost.
318+
a.getWalletFunding = async (address: string) => (
319+
address.toLowerCase() === pinned.address.toLowerCase()
320+
? { native: 10n ** 18n, trac: 0n }
321+
: { native: 10n ** 18n, trac: 10n ** 18n }
322+
);
323+
a.isWalletPublishFundable = async (address: string) =>
324+
address.toLowerCase() !== pinned.address.toLowerCase();
325+
a.snapshotPublisherWalletBalances = async () => [];
326+
327+
// A TRAC shortfall surfaces as a funds-shaped transferFrom revert.
328+
const original = new Error('ERC20: transfer amount exceeds balance');
329+
const enriched = await a.enrichInsufficientPublisherFundsError(
330+
original, pinned, CG, 1_000n, AUTHOR,
331+
);
332+
333+
expect(enriched).not.toBeInstanceOf(InsufficientPublisherFundsError);
334+
// The original error is preserved so a retry can reroute to the funded wallet.
335+
expect(enriched).toBe(original);
336+
void other;
337+
});
338+
339+
it('with no author, an unauthorized-and-unfunded pool is still terminal NO_FUNDED (unchanged)', async () => {
340+
// The payer-only behaviour this must not disturb: nothing admissible is
341+
// fundable, so the whole-pool diagnosis is correct and stays terminal.
342+
const { a, pool } = makeAdapter([], ATTESTED_AUTHOR_PUBLISH_AUTHZ_MIN_KAL_VERSION, {
343+
extraKeys: [PK_B],
344+
});
345+
const [pinned] = pool;
346+
a.getWalletFunding = async () => ({ native: 10n ** 18n, trac: 0n });
347+
a.isWalletPublishFundable = async () => false;
348+
a.snapshotPublisherWalletBalances = async () => [];
349+
350+
const enriched = await a.enrichInsufficientPublisherFundsError(
351+
new Error('ERC20: transfer amount exceeds balance'), pinned, CG, 1_000n,
352+
);
353+
354+
expect(enriched).toBeInstanceOf(InsufficientPublisherFundsError);
355+
});
356+
});
357+
254358
describe('#1689 publish admission — cross-package version coupling [CH-1689-V]', () => {
255359
// The client threshold and `KnowledgeAssetsLifecycle._VERSION` are two literals in
256360
// two packages maintained by different people. A mismatch fails NOTHING: the

0 commit comments

Comments
 (0)