-
Notifications
You must be signed in to change notification settings - Fork 10
feat(publisher): adopt already-minted KAs on KaIdAlreadyMinted revert #1878
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -272,6 +272,32 @@ export function enrichEvmError(err: unknown): string | null { | |
| * falls back to string matching on the message / shortMessage / reason / | ||
| * nested cause so a stringified or re-wrapped revert is still caught. | ||
| */ | ||
| /** | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 Nit: Do not split the allowance helper from its JSDoc Why it matters Suggestion |
||
| * Adopt-existing-mint (see dkg-publisher adoptExistingMintOrRethrow): decode a | ||
| * `KaIdAlreadyMinted(uint256 kaId)` custom-error revert and return the minted | ||
| * kaId. Unlike `isTooLowAllowanceError` there is deliberately NO string-matching | ||
| * fallback: adoption is state-changing and must cross-check the decoded kaId | ||
| * against the locally reserved id, so we require the structured decode that | ||
| * `enrichEvmError` stamps at `err.revert`. | ||
| */ | ||
| export function getKaIdAlreadyMintedKaId(err: unknown): bigint | undefined { | ||
| if (!err || typeof err !== 'object') return undefined; | ||
| // enrichEvmError is idempotent — call defensively in case no upstream layer | ||
| // (isRetryableRpcError / allowance recovery) enriched this error object yet. | ||
| enrichEvmError(err); | ||
| const e = err as { revert?: { name?: unknown; args?: unknown[] }; cause?: unknown }; | ||
| if (e.revert?.name === 'KaIdAlreadyMinted') { | ||
| const raw = e.revert.args?.[0]; | ||
| try { | ||
| return raw == null ? undefined : BigInt(raw as string | number | bigint); | ||
| } catch { | ||
| return undefined; | ||
| } | ||
| } | ||
| if (e.cause && typeof e.cause === 'object') return getKaIdAlreadyMintedKaId(e.cause); | ||
| return undefined; | ||
| } | ||
|
|
||
| export function isTooLowAllowanceError(err: unknown): boolean { | ||
| if (!err || typeof err !== 'object') return false; | ||
| const e = err as { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Bug: The real provenance verifier is untested
What's wrong
This PR's safety claim depends on the new adapter method refusing to adopt a kaId unless storage roots, context graph binding, and the mint event all match the sealed publish. The added publisher test only verifies that publish calls a stubbed hook with the expected arguments, so the most important adoption checks can regress while the new suite stays green.
Example
A regression that removed the root comparison at packages/chain/src/evm-adapter-base.ts:2748, skipped the CG mismatch check at 2772, or returned the first log without checking parsedArgs.merkleRoot at 2812 would still pass the new publisher tests because the provenance hook is stubbed to return a successful OnChainPublishResult.
Suggested direction
Keep the publisher tests for the handoff, but add unit coverage for the EVM adapter method itself so the security-critical chain-truth checks and log-recovery fallback are validated instead of stubbed away.
For Agents
Add focused chain-package tests around EVMChainAdapterBase.getMintedKnowledgeAssetProvenance. Stub readContract, getBlockTimestamp, resolveKaStorageDeployBlock, and queryEventLogsPage to prove: matching root+CG+event returns the synthesized result; empty/no logs returns null; no roots/root mismatch/multiple roots/CG mismatch/event-root mismatch throw the documented codes; non-collision scan failures return null.