Skip to content

fix(uxf,payments): bundleCid determinism — lock envelope timestamp across attempts#362

Merged
vrogojin merged 1 commit into
mainfrom
fix/bundle-cid-determinism
May 30, 2026
Merged

fix(uxf,payments): bundleCid determinism — lock envelope timestamp across attempts#362
vrogojin merged 1 commit into
mainfrom
fix/bundle-cid-determinism

Conversation

@vrogojin

Copy link
Copy Markdown
Contributor

Blocks PR #358 (V6-RECOVER test gap). The Node 20 CI failure on #358 traced to a pre-existing bundleCid non-determinism bug in main, not to V6's content. This PR fixes the root cause; #358 will be rebased onto the updated main and re-run CI.

Summary

The CAR root CID of a UXF bundle is the dag-cbor SHA-256 of the envelope block. The envelope embeds createdAt and updatedAt — both sourced from Math.floor(Date.now() / 1000) at create/ingest/merge time. This makes the bundleCid non-deterministic across attempts: two UxfPackage.create({...}).ingestAll(...) sequences straddling a wall-clock second boundary (a few ms apart) produce different envelope bytes → different bundleCids.

How the bug surfaces

tests/integration/transfer/crash-recovery.test.ts:726 asserts:

expect(finalEntry!.bundleCid).toBe(persisted!.bundleCid)

On slow Node 20 CI runs the resume call lands in a different wall-clock second than the first attempt → assertion flakes. Today's PR #358 CI fail (Node 20) → re-run pass demonstrates the flake exactly.

What's broken (besides this one test)

bundleCid stability is a core invariant, not a test convenience. The bug silently degrades:

Fix

Public API (additive, fully back-compat)

UxfPackage.create({ description?, creator?, createdAt?, updatedAt? })
pkg.ingest(token, { updatedAt? })
pkg.ingestAll(tokens, { updatedAt? })

When the new fields are omitted, behaviour is identical to before. When provided, they lock the envelope timestamps.

Sender wiring

interface InstantSenderDeps {
  // ...existing fields
  readonly bundleCreatedAt?: number;  // unix ms
}
interface ConservativeSenderDeps {
  // ...existing fields
  readonly bundleCreatedAt?: number;  // unix ms
}

Inside both senders:

const now = deps.bundleCreatedAt ?? Date.now();         // lock once at top
const envelopeStamp = Math.floor(now / 1000);
const pkg = UxfPackage.create({ ..., createdAt: envelopeStamp });
pkg.ingestAll(tokens, { updatedAt: envelopeStamp });    // lock updatedAt too

Test update

tests/integration/transfer/crash-recovery.test.ts now passes bundleCreatedAt: persisted!.createdAt on each resume deps — what the test had always intended to verify. The 3 idempotency assertions there finally guarantee what they read.

New test file — forward regression guard

tests/unit/uxf/bundle-cid-determinism.test.ts (6 tests):

  • ✅ Two calls with the SAME createdAt produce identical CIDs
  • ✅ Two calls with DIFFERENT createdAt produce different CIDs (sanity: fix didn't strip the timestamp)
  • ✅ Explicit updatedAt independently controls envelope
  • ✅ Omitted updatedAt defaults to createdAt
  • Production scenario — clock advances across a second boundary, locked timestamps produce identical CIDs
  • Contrast — clock advances across the same boundary WITHOUT locks → different CIDs (proves the bug exists if locks are removed)

The contrast test is the regression guard: a future refactor that re-introduces unlocked Date.now() somewhere on the envelope path will fail this specific test, not just flake.

Production wiring (deferred)

PaymentsModule.dispatchUxfInstantSend / dispatchUxfConservativeSend should pass bundleCreatedAt from the resumed outbox entry's createdAt. First-attempt sends (no prior outbox entry) omit it and the sender computes a single locked timestamp internally. That's a follow-up — the infrastructure here is sufficient.

Test plan

  • 6 new determinism tests pass
  • All 4 tests/integration/transfer/crash-recovery.test.ts tests pass
  • All 463 tests/unit/uxf/ + 1491 tests/unit/payments/transfer/ tests pass
  • All 514 unit test files (8651 tests + 11 skipped) pass
  • tsc --noEmit clean

Related

…ross attempts

The CAR root CID of a UXF bundle is the dag-cbor SHA-256 of the
envelope block. The envelope embeds `createdAt` and `updatedAt`. Both
fields were sourced from `Math.floor(Date.now() / 1000)`:

  - `UxfPackage.create()` at uxf/UxfPackage.ts:75
  - `ingest()` at uxf/UxfPackage.ts:550
  - `ingestAll()` at uxf/UxfPackage.ts:686
  - `removeToken()` at uxf/UxfPackage.ts:719
  - `mergePkg()` at uxf/UxfPackage.ts:1011

This makes the bundleCid non-deterministic across attempts. Two
`UxfPackage.create({...}).ingestAll(...)` sequences straddling a
wall-clock second boundary produce DIFFERENT envelope bytes →
DIFFERENT bundleCids. The flake surfaces in
tests/integration/transfer/crash-recovery.test.ts:726 on slow Node
20 runs (5-min CI hits the boundary often enough to flake the
"bundleCid stable on resume" assertion).

The bug breaks every system keyed on bundleCid for idempotency:

  - replay-LRU at the recipient (T.3.A's dedup key)
  - IPFS pin reuse (republishing should hit the existing pin, not
    create a duplicate)
  - sender-side outbox dedup ("same retry, not a new send")
  - audit-#333 H3 mergePkg `targetExisting === sourceIncoming`
    fast-path

Fix:

  - `UxfPackage.create({ createdAt?, updatedAt? })` accepts caller-
    locked timestamps. Falls back to `Math.floor(Date.now() / 1000)`
    when omitted.

  - `UxfPackage.ingest(token, { updatedAt? })` and
    `ingestAll(tokens, { updatedAt? })` accept an explicit updatedAt
    override. Internal `ingest()`/`ingestAll()` functions widen to
    match.

  - InstantSenderDeps and ConservativeSenderDeps gain
    `bundleCreatedAt?: number` (ms). Senders lock `now =
    deps.bundleCreatedAt ?? Date.now()` once at the top, compute
    `envelopeStamp = Math.floor(now / 1000)`, and pass it to BOTH
    `UxfPackage.create({ createdAt: envelopeStamp })` and
    `pkg.ingestAll(..., { updatedAt: envelopeStamp })`. The outbox
    entry's `createdAt` already uses the same `now` (just at ms
    resolution).

  - tests/integration/transfer/crash-recovery.test.ts: each resume
    deps now passes `bundleCreatedAt: persisted!.createdAt` so the
    re-built bundle reproduces the original bytes — what the test
    has always intended to assert.

  - New tests/unit/uxf/bundle-cid-determinism.test.ts (6 tests):
    forward regression guard for the determinism contract. Includes
    a contrast test that verifies the bug exists when locks are
    omitted, so a future refactor that accidentally re-introduces
    the unlocked path will fail this file specifically.

Production wiring (deferred to a follow-up):
  - `PaymentsModule.dispatchUxfInstantSend` / `dispatchUxfConservativeSend`
    should pass `bundleCreatedAt` from the resumed outbox entry's
    `createdAt`. First-attempt sends (no prior outbox entry) omit it
    and the sender computes a single locked timestamp internally.

Tests: 6 new bundleCid determinism tests; existing
crash-recovery.test.ts now correctly verifies idempotency (was a
silent flake); 8651 unit/integration tests pass; tsc clean.

Unblocks: PR #358 (V6-RECOVER test gap) which inherited the Node 20
flake.
@vrogojin vrogojin merged commit f725fc5 into main May 30, 2026
3 checks passed
@vrogojin vrogojin deleted the fix/bundle-cid-determinism branch May 30, 2026 17:22
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