Skip to content

chore(chain): surface V10 per-publish 1-wei allowance floor for operators (#871)#875

Merged
branarakic merged 3 commits into
mainfrom
investigate/871-curator-wallet-allowance-ghosting
Jun 1, 2026
Merged

chore(chain): surface V10 per-publish 1-wei allowance floor for operators (#871)#875
branarakic merged 3 commits into
mainfrom
investigate/871-curator-wallet-allowance-ghosting

Conversation

@branarakic
Copy link
Copy Markdown
Contributor

Summary

Investigation outcome for #871"rc.12: curator wallet's approve(KAV_Lifecycle, X) tx confirms but on-chain allowance stays at 1 wei".

Root cause: not a code bug. On-chain Approval-event replay proves the curator wallet's manual approve(KAV, 10 TRAC) succeeded and updated the allowance mapping at block 42270519 (tx 0x9f590889a06b14602efbb4cf475d081a84c7eac9eb99bb8e28d3f428d5559290, 2026-06-01T10:22:06Z), exactly like the alt-key and keystore wallets. Current on-chain allowance is 9999999999999999999 (= 10 TRAC − 1 wei consumed by one direct-spend publish since). The reported "ghosting" is a read-side artifact (the issue body itself lists a typo'd KnowledgeAssetsLifecycle address that points at an empty EOA, and any reproduction reading allowance against that EOA — or against the real KAV through a stale provider cache — would see exactly the symptom described).

The operator confusion is still real and worth closing: the daemon's intentional per-publish 1-wei allowance floor (the documented #720 workaround for the contract's transferFrom(..., 1n) minimum on zero-cost publishes) is silent, so operators who inspect allowance() and see "1 wei dust" reasonably mistake it for a stuck approval.

This PR adds one diagnostic console.warn in EVMChainAdapter.ensureV10ApproveTrac that fires only when the per-publish policy emits its 1-wei floor approval, explicitly identifying it as the #720 workaround:

[chain] V10 per-publish auto-approve floor: signer=0x… kav10=0x… target=1 wei (tokenAmount=0, currentAllowance=0). This is the #720 transferFrom-minimum workaround; not a stuck approval.

The full investigation — hypotheses, on-chain proofs, the spender-address typo finding, and a recommended next-step list for the issue reporter — lives at docs/investigations/871-curator-wallet-allowance-ghosting.md.

What changed

  • packages/chain/src/evm-adapter.ts — one console.warn inside the existing if (needsApprove) block when targetAllowance === V10_PUBLISH_ONCHAIN_MIN_ALLOWANCE && approvalPolicy.mode === 'per-publish'. No behaviour change; production code paths and invariants are untouched.
  • docs/investigations/871-curator-wallet-allowance-ghosting.md — full investigation note (~250 lines): hypotheses tested, on-chain Approval-event scan, current-state allowance reads, spender-typo analysis, recommended next steps for the reporter.
  • CHANGELOG.mdUnreleased entry under "Added — Operator observability".

Test plan

  • pnpm --filter @origintrail-official/dkg-chain test:unit132 passed (132) including the 5 cases in evm-adapter.unit.test.ts > ensureV10ApproveTrac that exercise the per-publish floor branch. The new log line fires exactly in those 5 cases, confirming both the trigger condition and that the existing tests still pin the floor invariant.
  • No new tests added — this is a pure observability change on a path already covered by the ensureV10ApproveTrac and computeApprovalAction unit suites; the log is a side-effect, not behaviour.
  • Investigation note + CHANGELOG entry self-contained.

Decision: not closing #871 in this PR

Closing the issue is the reporter's call once they re-test against the correct rc.12 KAV address (0x7558E9B30BeFA87F68C808c1f705b30507d19146) sourced from running daemon config rather than typed by hand, and confirm a fresh-provider read returns the expected 10 TRAC. I'm leaving #871 open with a comment linking to this PR and to the investigation note so the reporter can verify and close.

Made with Cursor

…tors (#871)

Investigation of #871 — "rc.12 curator wallet's approve(KAV, X) confirms but
allowance stays at 1 wei" — concluded that the user's manual approve did
update on-chain allowance to 10 TRAC (verified via Approval event replay at
block 42270519, tx 0x9f590889a0...). The reported "ghosting" is most likely a
read-side artifact: the issue body itself lists a typo'd KAV address pointing
at an empty EOA, and the surviving symptom (stale 1-wei reads) is consistent
with provider/RPC caching of `eth_call` for the curator's
`(owner, spender)` slot.

The 1-wei "dust" the user observed _before_ their manual approve is real: it's
the daemon's per-publish auto-approve floor (`V10_PUBLISH_ONCHAIN_MIN_ALLOWANCE`
= 1n), the documented #720 workaround for the contract's `transferFrom(..., 1n)`
minimum on zero-cost publishes. That floor is silent, so operators who inspect
on-chain allowance and see "1 wei" reasonably mistake it for a stuck approval.

This change adds one diagnostic log line in `EVMChainAdapter.ensureV10ApproveTrac`
when the per-publish policy emits its floor approval (`targetAllowance == 1n`
and `mode == 'per-publish'`), explicitly identifying the resulting 1-wei
allowance as the #720 workaround. No behaviour change in the production path —
only the log is new; the underlying floor logic, `effectivePublishAllowance`,
and `computeApprovalAction` invariants are unchanged. The existing
`ensureV10ApproveTrac` unit suite in `evm-adapter.unit.test.ts` (5 cases that
hit the per-publish floor branch) continues to pass.

Full investigation note with on-chain Approval event proofs, hypothesis-by-
hypothesis analysis, and the spender-address typo finding is at
`docs/investigations/871-curator-wallet-allowance-ghosting.md`.

Co-authored-by: Cursor <cursoragent@cursor.com>
`main` reset the Unreleased section to "_No changes yet._" after the
rc.12 cutover landed. Our new "Added — Operator observability" entry
stays; the empty `### Changed — Knowledge Collection -> Knowledge Asset
rename` heading is dropped (the rename is already documented in the
[10.0.0-rc.12] section below).

No code changes; this is just merging origin/main into the PR branch.

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread packages/chain/src/evm-adapter.ts Outdated
// allowance see "1 wei dust" persisting after every publish and
// misread it as a stuck or ghosted approval (#871).
if (
targetAllowance === V10_PUBLISH_ONCHAIN_MIN_ALLOWANCE &&
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Issue: This guard also matches a legitimate tokenAmount === 1n publish, so the new warning would incorrectly claim the approval was the #720 floor workaround when 1 wei was actually the real required allowance. Tighten the condition to the floored case (tokenAmount === 0n or tokenAmount < V10_PUBLISH_ONCHAIN_MIN_ALLOWANCE) so the diagnostic stays trustworthy.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in 646f298.

The guard now additionally requires tokenAmount === 0n, which is the only call shape where effectivePublishAllowance actually lifts targetAllowance above the caller's input (zero-cost / metadata-only publishes). For a legitimate tokenAmount === 1n publish, targetAllowance is still 1n, but it's the real publish cost — the warn no longer fires there.

Pinned both behaviours with two new cases in evm-adapter.unit.test.ts:

  • positive: tokenAmount === 0n, allowance < 1n → approve fires AND the #720 warn fires once.
  • negative: tokenAmount === 1n, allowance < 1n → approve fires (1 wei is the real cost) BUT the warn does NOT fire. This directly closes the false-positive you flagged.

Test count: 132 → 134 in the chain unit suite. Diff stays minimal: +30 / -11 in evm-adapter.ts (mostly the expanded comment explaining why tokenAmount === 0n matters and a literal tokenAmount=0 in the message), +54 / -0 in the test file.

…publishes (#875 review)

Codex review on PR #875 flagged a false-positive in the new
`ensureV10ApproveTrac` diagnostic warn: the prior guard fired whenever
`targetAllowance === V10_PUBLISH_ONCHAIN_MIN_ALLOWANCE` (= 1n) and
`policy.mode === 'per-publish'`, but `targetAllowance === 1n` is also the
correct outcome for a legitimate `tokenAmount === 1n` publish — where
1 wei is the real publish cost, not the #720 floor workaround. The warn
would have mislabelled that approval as the workaround.

Tightens the guard to additionally require `tokenAmount === 0n`, which
is the only path through `effectivePublishAllowance` where the floor
actually lifts `targetAllowance` above the caller's `tokenAmount`. The
warn message drops the `tokenAmount=…` interpolation in favour of a
literal `tokenAmount=0` since that's now invariant for the branch.

Pins both behaviours with two new cases in
`evm-adapter.unit.test.ts`:

- positive: `tokenAmount === 0n, allowance < 1n` → approve fires AND
  the warn fires exactly once with the `#720` wording.
- negative: `tokenAmount === 1n, allowance < 1n` → approve still fires
  (1 wei is the real cost) BUT the warn does NOT fire. Directly
  closes the Codex finding.

Both use `vi.spyOn(console, 'warn').mockImplementation(...)` matching
the existing pattern in `filter-error-silencer.test.ts`.

Test count: 132 → 134 in `packages/chain` unit suite. No behaviour
change beyond the tightened diagnostic guard.

Co-authored-by: Cursor <cursoragent@cursor.com>
it will let the next operator who sees a 1-wei floor approve in their
logs immediately understand the source.

We did **not** ship a regression test: there is no behavioural change in
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Nit: This note is now stale. The PR adds two unit tests in packages/chain/test/evm-adapter.unit.test.ts, so saying “We did not ship a regression test” will mislead future readers. Please update the document to reflect the new coverage.

@branarakic branarakic merged commit bc6fde8 into main Jun 1, 2026
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant