Skip to content

container: fix envelope spoofing, token estimation, and job bookkeeping bugs - #19

Merged
ffaerber merged 1 commit into
mainfrom
claude/code-review-bugs-uft8tk
Aug 5, 2026
Merged

container: fix envelope spoofing, token estimation, and job bookkeeping bugs#19
ffaerber merged 1 commit into
mainfrom
claude/code-review-bugs-uft8tk

Conversation

@ffaerber

@ffaerber ffaerber commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Found during a code review of the container and contracts. Each fix carries a
regression test; the suite goes 139 → 168.

Authorization on PSS envelopes
Topics are derived from public on-chain addresses and the routing id is
keccak(jobs[jobId].requestHash), so any observer could forge an ack or a
delivery for someone else's job. A valid signature only proved the sender
existed, not that it was the provider we hired.

  • PssTransport now drops envelopes not addressed to selfAddress.
  • The gateway authorizes ack/deliver against jobMeta, which records the
    provider chosen at postJob time. Previously a forged job_deliver could
    resolve a caller's request with attacker-chosen content, or fail the
    decrypt — which dropped the pending slot and cleared the failure timers,
    stranding the escrow with no cancel or timeout left to fire.
  • The provider verifies a matching JobPosted exists before running
    inference. The JobPosted index is already filtered to our own address and
    keyed by the on-chain requestHash, so a hit is the proof; previously an
    unbacked notify bought a full inference run for free.

Prompt token estimation
estimatePromptTokens assumed content was a string. content: null (legal
on assistant turns carrying tool_calls) threw, turning a valid request into
a 502. Array content (multimodal parts) counted the number of parts, so a
40k-char prompt priced as the 256-token floor — the escrow was undersized
and the provider's claim got clipped to it, eating the difference. Content
is now normalised: text parts count characters, non-text parts bill a flat
rate rather than their base64 length.

Other correctness fixes

  • Manual selection read only listProviders(0, 1) and synthesized a stub on
    miss, so it worked solely when the manual provider was registry entry 0 —
    and the stub lacked the pssPublicKey the caller dereferences. Reads the
    provider row directly now.
  • The JobClaimed poller fell back to block 0 when its first getBlockNumber
    failed, then asked a public RPC for all logs since genesis forever, so
    actualPayment was never recorded. Anchors lazily instead.
  • The stale-job sweeper filtered listGatewayJobs, which windows to 7 days
    and orders newest-first — the old orphans it exists to cancel were the
    ones it could not see. Dedicated query, oldest-first.
  • JobPostedIndex anchored at head, so a JobPosted mined in the anchor block
    was never indexed and that job could be delivered but never claimed.
  • The admin price input rendered through the display formatter, which
    truncates at 6 decimals; the save handler parsed it straight back, so any
    finer price was rounded down on every save and anything below 1e-6 BZZ
    became free. Uses the exact formatter.
  • Gateway status showed "last success" from a single-row query it then
    filtered, so it was almost always blank.
  • Tx toasts build DOM nodes instead of splicing on-chain strings into
    innerHTML.
  • Declare @scure/bip39 and @scure/bip32, used directly by lib/wallet.ts but
    resolved only as transitive viem deps.
  • Log loudly when a provider boots registered-but-inactive: deactivate() is
    irreversible in ProviderRegistry, so the container would otherwise
    heartbeat forever while unroutable.

Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01F8LVHhaYxeCBdVQG1q2ju6

…ng bugs

Found during a code review of the container and contracts. Each fix carries a
regression test; the suite goes 139 → 168.

Authorization on PSS envelopes
  Topics are derived from public on-chain addresses and the routing id is
  keccak(jobs[jobId].requestHash), so any observer could forge an ack or a
  delivery for someone else's job. A valid signature only proved the sender
  existed, not that it was the provider we hired.

  - PssTransport now drops envelopes not addressed to `selfAddress`.
  - The gateway authorizes ack/deliver against `jobMeta`, which records the
    provider chosen at postJob time. Previously a forged job_deliver could
    resolve a caller's request with attacker-chosen content, or fail the
    decrypt — which dropped the pending slot and cleared the failure timers,
    stranding the escrow with no cancel or timeout left to fire.
  - The provider verifies a matching JobPosted exists before running
    inference. The JobPosted index is already filtered to our own address and
    keyed by the on-chain requestHash, so a hit is the proof; previously an
    unbacked notify bought a full inference run for free.

Prompt token estimation
  estimatePromptTokens assumed `content` was a string. `content: null` (legal
  on assistant turns carrying tool_calls) threw, turning a valid request into
  a 502. Array content (multimodal parts) counted the number of parts, so a
  40k-char prompt priced as the 256-token floor — the escrow was undersized
  and the provider's claim got clipped to it, eating the difference. Content
  is now normalised: text parts count characters, non-text parts bill a flat
  rate rather than their base64 length.

Other correctness fixes
  - Manual selection read only listProviders(0, 1) and synthesized a stub on
    miss, so it worked solely when the manual provider was registry entry 0 —
    and the stub lacked the pssPublicKey the caller dereferences. Reads the
    provider row directly now.
  - The JobClaimed poller fell back to block 0 when its first getBlockNumber
    failed, then asked a public RPC for all logs since genesis forever, so
    actualPayment was never recorded. Anchors lazily instead.
  - The stale-job sweeper filtered listGatewayJobs, which windows to 7 days
    and orders newest-first — the old orphans it exists to cancel were the
    ones it could not see. Dedicated query, oldest-first.
  - JobPostedIndex anchored at head, so a JobPosted mined in the anchor block
    was never indexed and that job could be delivered but never claimed.
  - The admin price input rendered through the display formatter, which
    truncates at 6 decimals; the save handler parsed it straight back, so any
    finer price was rounded down on every save and anything below 1e-6 BZZ
    became free. Uses the exact formatter.
  - Gateway status showed "last success" from a single-row query it then
    filtered, so it was almost always blank.
  - Tx toasts build DOM nodes instead of splicing on-chain strings into
    innerHTML.
  - Declare @scure/bip39 and @scure/bip32, used directly by lib/wallet.ts but
    resolved only as transitive viem deps.
  - Log loudly when a provider boots registered-but-inactive: deactivate() is
    irreversible in ProviderRegistry, so the container would otherwise
    heartbeat forever while unroutable.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F8LVHhaYxeCBdVQG1q2ju6
@ffaerber
ffaerber merged commit dce710b into main Aug 5, 2026
1 check passed
@ffaerber
ffaerber deleted the claude/code-review-bugs-uft8tk branch August 5, 2026 12:46
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.

2 participants