Never invent objective or action ids (finish what #61 started for claims)

scripts/verify-create-resolvers.js

@@ -0,0 +1,157 @@
+// Acceptance check for chain.js/resolveCreatedActionId and
+// resolveCreatedObjectiveId against a REAL node.
+//
+//   NODE_ENV=prod BLOCKCHAIN_URL=https://app.cambiatus.io \
+//   BLOCKCHAIN_COMMUNITY_CONTRACT=cambiatus.cm BLOCKCHAIN_TOKEN_CONTRACT=cambiatus.tk \
+//   node scripts/verify-create-resolvers.js
+//
+// Both resolvers answer "which id did the contract just generate?" with: the
+// smallest chain id for this parent that the DB does not have yet. So replay the
+// creation history of every parent — knownIds = the first k ids, ask for the
+// next — and require the resolver to name id k+1 every time. Then confirm an
+// exhausted parent throws rather than inventing an id.
+//
+// This is the check that would have caught the failure the serial fallback hid:
+// a truncated chain read makes a real id look missing, and the resolver hands
+// back an id that belongs to a different action.
+
+const config = require(`../src/config/${process.env.NODE_ENV || 'dev'}`)
+const { resolveCreatedActionId, resolveCreatedObjectiveId } = require('../src/chain')
+
+const CONTRACT = config.blockchain.contract.community
+
+async function check (label, ids, resolve) {
+  let pass = 0
+  const failures = []
+
+  // Replay the parent's creation order: with the first k ids known, the next
+  // create must resolve to ids[k].
+  for (let k = 0; k < ids.length; k++) {
+    const known = new Set(ids.slice(0, k))
+    let got
+    try {
+      got = await resolve(known)
+    } catch (e) {
+      got = `THREW: ${e.message}`
+    }
+    if (got === ids[k]) pass++
+    else failures.push({ k, want: ids[k], got })
+  }
+
+  // Everything known -> nothing left to create -> must throw.
+  let exhausted = 'did not throw'
+  try {
+    await resolve(new Set(ids))
+  } catch (e) {
+    exhausted = 'threw as required'
+  }
+
+  const ok = failures.length === 0 && exhausted === 'threw as required'
+  console.log(`  ${ok ? 'PASS' : 'FAIL'} ${label}: ${pass}/${ids.length} steps, exhausted-probe ${exhausted}`)
+  for (const f of failures.slice(0, 5)) {
+    console.log(`      step ${f.k}: want ${f.want} got ${f.got}`)
+  }
+  return ok
+}
+
+async function main () {
+  console.log(`node: ${config.blockchain.url}  contract: ${CONTRACT}`)
+
+  // Communities to exercise, read off chain so this is not hardcoded to muda.
+  const https = require('https')
+  const post = (path, body) => new Promise((resolve, reject) => {
+    const data = JSON.stringify(body)
+    const url = new URL(path, config.blockchain.url)
+    const req = https.request({
+      hostname: url.hostname,
+      port: 443,
+      path: url.pathname,
+      method: 'POST',
+      headers: { 'Content-Type': 'application/json', 'Content-Length': Buffer.byteLength(data) }
+    }, res => {
+      const chunks = []
+      res.on('data', c => chunks.push(c))
+      res.on('end', () => { try { resolve(JSON.parse(Buffer.concat(chunks).toString())) } catch (e) { reject(e) } })
+    })
+    req.on('error', reject)
+    req.write(data)
+    req.end()
+  })
+
+  const cmm = await post('/v1/chain/get_table_rows', {
+    json: true, code: CONTRACT, scope: CONTRACT, table: 'community', limit: 1000
+  })
+  const symbols = (cmm.rows || []).map(c => c.symbol)
+  console.log(`communities: ${symbols.length}`)
+
+  let allOk = true
+
+  // ---- objectives, per community -----------------------------------------
+  let objChecked = 0
+  for (const symbol of symbols) {
+    // The resolver has its own reader; to know the EXPECTED ids we read the
+    // community's objective scope independently here.
+    const raw = (() => {
+      const [precision, code] = symbol.split(',')
+      let v = BigInt(precision)
+      for (let i = 0; i < code.length; i++) v |= BigInt(code.charCodeAt(i)) << BigInt(8 * (i + 1))
+      return v.toString()
+    })()
+    const page = await post('/v1/chain/get_table_rows', {
+      json: true, code: CONTRACT, scope: raw, table: 'objective', limit: 1000
+    })
+    const ids = (page.rows || []).map(o => Number(o.id)).sort((a, b) => a - b)
+    if (ids.length === 0) continue
+    objChecked++
+    const ok = await check(
+      `objectives of ${symbol}`,
+      ids,
+      known => resolveCreatedObjectiveId(CONTRACT, symbol, known)
+    )
+    allOk = allOk && ok
+  }
+  console.log(`communities with objectives checked: ${objChecked}`)
+
+  // ---- actions, per objective --------------------------------------------
+  const actions = await post('/v1/chain/get_table_rows', {
+    json: true, code: CONTRACT, scope: CONTRACT, table: 'action', limit: 1000
+  })
+  // Page the rest (one call is never the whole table).
+  let all = actions.rows || []
+  let more = actions.more
+  let lb = actions.next_key
+  while (more) {
+    const p = await post('/v1/chain/get_table_rows', {
+      json: true, code: CONTRACT, scope: CONTRACT, table: 'action', lower_bound: Number(lb), limit: 1000
+    })
+    all = all.concat(p.rows || [])
+    more = p.more
+    lb = p.next_key
+  }
+  const byObjective = new Map()
+  for (const a of all) {
+    const o = Number(a.objective_id)
+    if (!byObjective.has(o)) byObjective.set(o, [])
+    byObjective.get(o).push(Number(a.id))
+  }
+  console.log(`actions: ${all.length} across ${byObjective.size} objectives`)
+
+  // Checking every objective means re-reading the action table once per step,
+  // which is slow; sample the busiest ones plus a spread of the rest.
+  const objectives = [...byObjective.entries()].sort((a, b) => b[1].length - a[1].length)
+  const sample = objectives.slice(0, 3).concat(objectives.slice(3).filter((_, i) => i % 20 === 0))
+  for (const [objectiveId, ids] of sample) {
+    ids.sort((a, b) => a - b)
+    const ok = await check(
+      `actions of objective ${objectiveId}`,
+      ids,
+      known => resolveCreatedActionId(CONTRACT, objectiveId, known)
+    )
+    allOk = allOk && ok
+  }
+
+  console.log(allOk ? '\nALL CHECKS PASS' : '\nFAILURES PRESENT')
+  process.exit(allOk ? 0 : 1)
+}
+
+main().catch(e => { console.error(e); process.exit(1) })

src/chain.js

@@ -42,15 +42,69 @@ class ResolveError extends Error {}
 // Measured against prod on 2026-08-07: the identical bounded `byaction` claim
 // query returned 31, 35, 38, 51, 60, 67 then 79 rows on seven consecutive calls
 // (the count tracks page-cache warmth, so it is not even deterministic), always
-// with `more: true`; an unbounded read with `limit: 5000` returned 27 rows.
+// with `more: true`; an unbounded read with `limit: 5000` returned 27 rows. The
+// 400-row `action` table takes 5 calls to read in full.
 //
-// So this guard is only safe for sets small enough that the node reliably walks
-// them in one budget — it converts truncation into a throw. Anything that needs
-// a COMPLETE set of a potentially large table must page until `more === false`
-// instead (see resolveClaimId).
-function assertComplete (res, table, context) {
-  if (!res || !Array.isArray(res.rows)) throw new Error(`get_table_rows(${table}) returned no rows ${context}`)
-  if (res.more) throw new Error(`get_table_rows(${table}) truncated ${context} (${res.rows.length} rows, more=true) — cannot page safely`)
+// So a single call is never proof of a complete set, at any table size. Read
+// every table that a resolver depends on through this pager, which follows
+// `next_key` until the node says `more: false`.
+//
+// PRIMARY index only. On a secondary index nodeos returns the secondary key as
+// `next_key` (a query bounded to action 389 returns `next_key: 389`), so it does
+// not advance and a truncated secondary read cannot be resumed at all. That is
+// why the callers below scan a table and filter client-side rather than asking
+// the node for "the rows matching key K".
+// One get_table_rows call, retried on a transient failure.
+//
+// Retrying matters more than it looks: the resolvers no longer fall back to a DB
+// serial, so a read that fails is a create the indexer skips until someone
+// reindexes. A dropped connection or a node hiccup should not cost that. Measured
+// against prod 2026-08-08, roughly 2 in 100 calls came back without a `rows`
+// array while paging the action table. Reads are pure, and every caller runs
+// before its first write, so a retry is free of side effects.
+//
+// The node's own message is carried into the final error — with Sentry not
+// running, the log line is the only diagnostic anyone gets.
+async function getTableRows (body, what, attempts = 3) {
+  let last
+  for (let attempt = 1; attempt <= attempts; attempt++) {
+    try {
+      const res = await post('/v1/chain/get_table_rows', body)
+      if (res && Array.isArray(res.rows)) return res
+      last = `node replied without a rows array: ${JSON.stringify(res).slice(0, 300)}`
+    } catch (e) {
+      last = e.message
+    }
+    if (attempt < attempts) await new Promise(r => setTimeout(r, 250 * attempt))
+  }
+  throw new Error(`get_table_rows for ${what} failed after ${attempts} attempts — ${last}`)
+}
+
+async function pageAllRows (params, what) {
+  const rows = []
+  let lowerBound = 0
+
+  for (let page = 0; page < 1000; page++) {
+    const res = await getTableRows({
+      json: true,
+      limit: 1000,
+      ...params,
+      lower_bound: lowerBound
+    }, what)
+    rows.push(...res.rows)
+
+    if (!res.more) return rows
+
+    const next = Number(res.next_key)
+    if (!Number.isFinite(next) || next <= lowerBound) {
+      throw new Error(
+        `paging ${what}: next_key (${res.next_key}) did not advance past ${lowerBound}`
+      )
+    }
+    lowerBound = next
+  }
+
+  throw new Error(`paging ${what} did not terminate`)
 }
 
 // One page of the `claim` table read through the PRIMARY index, ascending from
@@ -64,18 +118,14 @@ function assertComplete (res, table, context) {
 // resolution reads the primary index and filters client-side rather than asking
 // the node for "the claims of action N".
 async function claimPage (communityContract, lowerBound, limit) {
-  const res = await post('/v1/chain/get_table_rows', {
+  return getTableRows({
     json: true,
     code: communityContract,
     scope: communityContract,
     table: 'claim',
     lower_bound: lowerBound,
     limit
-  })
-  if (!res || !Array.isArray(res.rows)) {
-    throw new Error(`get_table_rows(claim) returned no rows from id ${lowerBound}`)
-  }
-  return res
+  }, `claims from id ${lowerBound}`)
 }
 
 // Resolve the real on-chain claim id for the claim a `claimaction` just created.
@@ -136,53 +186,42 @@ function symbolRaw (symbolString) {
   return raw.toString()
 }
 
-// Fetch every on-chain action for a single objective via the secondary index
-// on objective_id (index_position 2). Objectives hold a handful of actions
-// (measured on prod 2026-08-07: objective 93 -> 2 rows, `more: false`), well
-// inside one walk budget, so assertComplete's throw-on-`more` is the right
-// guard here. If an objective ever grows past what the node walks in one
-// budget this must move to primary-index paging like resolveClaimId — a
-// secondary index cannot be resumed.
+// Every on-chain action belonging to one objective.
+//
+// This reads the WHOLE `action` table through the primary index and filters
+// client-side, rather than using the `byobjective` secondary index. The
+// secondary index would look cheaper, but it truncates on the same time budget
+// as everything else and cannot be resumed (see pageAllRows), so a short read
+// silently looks like "this objective has fewer actions than it does" — and the
+// caller turns a missing id into the id of the action being created. The whole
+// table is 400 rows / 5 calls on prod (2026-08-08), and this runs only on the
+// create path, so scanning it is the cheaper mistake.
 async function actionsForObjective (communityContract, objectiveId) {
-  const res = await post('/v1/chain/get_table_rows', {
-    json: true,
-    code: communityContract,
-    scope: communityContract,
-    table: 'action',
-    index_position: 2,
-    key_type: 'i64',
-    lower_bound: objectiveId,
-    upper_bound: objectiveId,
-    limit: 5000
-  })
-  assertComplete(res, 'action', `for objective ${objectiveId}`)
-  return res.rows
+  const all = await pageAllRows(
+    { code: communityContract, scope: communityContract, table: 'action' },
+    `actions of objective ${objectiveId}`
+  )
+  return all.filter(a => Number(a.objective_id) === Number(objectiveId))
 }
 
-// Fetch every on-chain objective for a single community. Objectives live in a
-// per-community scope (the raw symbol value) under the primary index, so a
-// plain scan returns exactly this community's objectives. Same truncation
-// guard as above.
+// Every on-chain objective of one community. Objectives live in a per-community
+// scope (the raw symbol value) under the primary index, so paging that scope
+// returns exactly this community's objectives and nothing else.
 async function objectivesForCommunity (communityContract, communitySymbol) {
-  const res = await post('/v1/chain/get_table_rows', {
-    json: true,
-    code: communityContract,
-    scope: symbolRaw(communitySymbol),
-    table: 'objective',
-    limit: 2000
-  })
-  assertComplete(res, 'objective', `for community ${communitySymbol}`)
-  return res.rows
+  return pageAllRows(
+    { code: communityContract, scope: symbolRaw(communitySymbol), table: 'objective' },
+    `objectives of community ${communitySymbol}`
+  )
 }
 
 // Resolve the real on-chain id of the action being CREATED (upsertaction with
 // action_id = 0 — the contract generates the id and the payload doesn't carry
 // it). `knownIds` is the set of action ids already in the DB for this
 // objective. Blocks are processed in order, so the earliest chain id we don't
 // have yet is this create: the created action = the SMALLEST chain id not in
-// `knownIds`. Throws if every chain id is already known (chain/DB out of sync
-// — e.g. the create hasn't reached the node we query yet), so the block
-// retries instead of writing a wrong id.
+// `knownIds`. Throws if every chain id is already known (chain/DB out of sync —
+// e.g. the create hasn't reached the node we query yet); the caller turns that
+// into a ResolveError and skips the action rather than inventing an id.
 async function resolveCreatedActionId (communityContract, objectiveId, knownIds) {
   const chainIds = (await actionsForObjective(communityContract, objectiveId))
     .map(a => Number(a.id))
@@ -192,7 +231,7 @@ async function resolveCreatedActionId (communityContract, objectiveId, knownIds)
   if (created === undefined) {
     throw new Error(
       `resolveCreatedActionId: all ${chainIds.length} chain actions for objective ${objectiveId} ` +
-      'are already in the DB, nothing left to create. Retrying block.'
+      'are already in the DB, nothing left to create.'
     )
   }
   return created
@@ -210,7 +249,7 @@ async function resolveCreatedObjectiveId (communityContract, communitySymbol, kn
   if (created === undefined) {
     throw new Error(
       `resolveCreatedObjectiveId: all ${chainIds.length} chain objectives for community ${communitySymbol} ` +
-      'are already in the DB, nothing left to create. Retrying block.'
+      'are already in the DB, nothing left to create.'
     )
   }
   return created