Re-confirm unchanged stake, and never submit a partial total (#33)

People Chain now counts a staking duration only up to the last confirmation
of the stake plus MaxDataAge (fourteen days), so a stopped noter cannot leave
scores climbing on unchecked data. The bot submitted only on change, which
under that rule would freeze every account whose stake stays put.

confirm.js decides when unchanged stake is sent again: not while a
submission is pending (each one restarts the dispute window), not on
half-read data, and only once the last confirmation is CONFIRM_EVERY_BLOCKS
old (default three days). On a runtime without stakingScore.lastConfirmed
the bot behaves as before.

The direct-staking query failed silently into zeros, and a zero that lands
clears the account's stake clock on chain. It now reports the failure, and a
failure in either half skips the account for that scan.

.github/workflows/ci.yml

@@ -80,7 +80,7 @@ jobs:
       # Run inside the image, so the modules under test are the ones the image ships.
       - name: Unit tests
         if: matrix.bot == 'noter'
-        run: docker run --rm -v "$PWD/noter/test:/app/test:ro" bot node --test test/submit.test.mjs
+        run: docker run --rm -v "$PWD/noter/test:/app/test:ro" bot node --test test/submit.test.mjs test/confirm.test.mjs
 
       - name: The bot loads and stops at its missing mnemonic
         env:

noter/Dockerfile

@@ -10,6 +10,6 @@ WORKDIR /app
 COPY package.json package-lock.json ./
 RUN npm ci --omit=dev --omit=optional --no-audit --no-fund
 
-COPY bot.js submit.js ./
+COPY bot.js submit.js confirm.js ./
 
 CMD ["node", "bot.js"]

noter/bot.js

@@ -23,6 +23,7 @@ import { Keyring } from '@pezkuwi/keyring';
 import { cryptoWaitReady } from '@pezkuwi/util-crypto';
 import fs from 'fs';
 import { watchSubmission, isStale } from './submit.js';
+import { needsReconfirmation, DEFAULT_CONFIRM_EVERY_BLOCKS } from './confirm.js';
 
 // ========================================
 // CONFIGURATION
@@ -33,6 +34,9 @@ const ASSET_HUB_RPC = process.env.ASSET_HUB_RPC || 'wss://asset-hub-rpc.pezkuwic
 const PEOPLE_RPC   = process.env.PEOPLE_RPC   || 'wss://people-rpc.pezkuwichain.io';
 const SCAN_INTERVAL = parseInt(process.env.SCAN_INTERVAL_MS || '300000', 10); // 5 min
 const UNITS = BigInt('1000000000000'); // 10^12
+// How old a confirmation of unchanged stake may get before it is sent again (see confirm.js).
+const CONFIRM_EVERY_BLOCKS = parseInt(
+  process.env.CONFIRM_EVERY_BLOCKS || String(DEFAULT_CONFIRM_EVERY_BLOCKS), 10);
 
 // Liveness. A scan that throws is logged and retried, which is right — a chain can
 // be briefly unreachable. What is not right is doing that forever in silence: this
@@ -173,7 +177,7 @@ async function getAssetHubStakingData(assetHubApi, address) {
     return { stakedAmount, nominationsCount, unlockingChunksCount };
   } catch (err) {
     log('ERROR', `Failed to get AH staking data for ${address}`, { error: err.message });
-    return { stakedAmount: 0n, nominationsCount: 0, unlockingChunksCount: 0 };
+    return { stakedAmount: 0n, nominationsCount: 0, unlockingChunksCount: 0, queryFailed: true };
   }
 }
 
@@ -282,6 +286,17 @@ async function getPendingData(peopleApi, address, source) {
   }
 }
 
+/**
+ * The block this source's stake was last confirmed at on People Chain. `supported` is false on
+ * a runtime that does not keep confirmations yet, and the bot then never re-confirms.
+ */
+async function getLastConfirmed(peopleApi, address, source) {
+  const query = peopleApi.query.stakingScore.lastConfirmed;
+  if (!query) return { supported: false, at: null };
+  const result = await query(address, source);
+  return { supported: true, at: result.isNone ? null : result.unwrap().toNumber() };
+}
+
 /**
  * Check if staking data has changed compared to the most recent known value for this account -
  * whichever is more recent, an unmatured pending submission or (once nothing is pending) the
@@ -426,11 +441,37 @@ async function processAccount(relayApi, assetHubApi, peopleApi, noterKeypair, ad
   const ahStakingPending = await getPendingData(peopleApi, address, 'AssetHub');
   const ahStakingCached = ahStakingPending ?? await getCachedData(peopleApi, address, 'AssetHub');
 
-  if (hasDataChanged(combinedData, ahStakingCached)) {
-    // Skip update if pool query failed and we'd be downgrading a known stake to 0
-    if (poolData.queryFailed && ahStakingCached && ahStakingCached.stakedAmount > combinedData.stakedAmount) {
-      log('WARN', `Skipping update for ${address.slice(0, 8)}... — pool query failed, would downgrade stake`, {
-        cached: Number(ahStakingCached.stakedAmount / UNITS),
+  const changed = hasDataChanged(combinedData, ahStakingCached);
+  if (!changed) {
+    const confirmed = await getLastConfirmed(peopleApi, address, 'AssetHub');
+    const currentBlock = (await peopleApi.rpc.chain.getHeader()).number.toNumber();
+    if (needsReconfirmation({
+      supported: confirmed.supported,
+      changed,
+      pending: ahStakingPending !== null,
+      queryFailed: ahStakingData.queryFailed || poolData.queryFailed,
+      recordedStake: ahStakingCached ? ahStakingCached.stakedAmount : null,
+      lastConfirmedAt: confirmed.at,
+      currentBlock,
+      everyBlocks: CONFIRM_EVERY_BLOCKS,
+    })) {
+      updates.push({ address, source: 'AssetHub', data: combinedData });
+      log('INFO', `Re-confirming unchanged AH stake for ${address.slice(0, 8)}...`, {
+        lastConfirmedAt: confirmed.at,
+        currentBlock,
+      });
+    }
+  }
+
+  if (changed) {
+    // A total with a failed half in it is not a measurement. The direct-staking query used to
+    // fail silently into zeros, and a zero that lands clears the account's stake clock on
+    // chain -- a dropped RPC call cost a citizen the duration they had held. Either half
+    // failing skips the account this cycle; the next scan reads it again.
+    const failed = ahStakingData.queryFailed || poolData.queryFailed;
+    if (failed) {
+      log('WARN', `Skipping update for ${address.slice(0, 8)}... — a staking query failed, the total is partial`, {
+        cached: ahStakingCached ? Number(ahStakingCached.stakedAmount / UNITS) : null,
         wouldSubmit: Number(combinedData.stakedAmount / UNITS),
       });
     } else {

noter/confirm.js

@@ -0,0 +1,36 @@
+/**
+ * When unchanged stake has to be confirmed again.
+ *
+ * The People chain counts a staking duration only up to the last confirmation of the stake
+ * plus `MaxDataAge` (fourteen days): a noter that stops reporting must not leave scores
+ * climbing on data nobody checks. The other side of that rule is this one -- a healthy noter
+ * re-confirms stake that has not changed, well inside the allowance, so an account whose
+ * stake simply stays put never reaches it.
+ *
+ * Only resubmitting identical data is new; resubmitting it too often is the old bug
+ * (every submission restarts the dispute window). So: never while a submission is still
+ * pending, and only once the last confirmation is `everyBlocks` old.
+ */
+
+// Three days of six-second blocks: four or five confirmations inside the fourteen-day
+// allowance, so a missed day or two costs nothing.
+export const DEFAULT_CONFIRM_EVERY_BLOCKS = 3 * 24 * 600;
+
+/**
+ * @param {object} p
+ * @param {boolean} p.supported   the chain keeps confirmations (`stakingScore.lastConfirmed`)
+ * @param {boolean} p.changed     the fresh data differs from what is recorded
+ * @param {boolean} p.pending     a submission for this source is still in its dispute window
+ * @param {boolean} p.queryFailed part of the fresh data could not be read
+ * @param {bigint|null} p.recordedStake what the chain holds for this source, or null
+ * @param {number|null} p.lastConfirmedAt block of the last confirmation, or null if none
+ * @param {number} p.currentBlock
+ * @param {number} p.everyBlocks
+ */
+export function needsReconfirmation(p) {
+  if (!p.supported || p.changed || p.pending || p.queryFailed) return false;
+  if (p.recordedStake === null || p.recordedStake === 0n) return false;
+  // Stake recorded before the chain kept confirmations has none on record: confirm it.
+  if (p.lastConfirmedAt === null) return true;
+  return p.currentBlock - p.lastConfirmedAt >= p.everyBlocks;
+}

noter/test/confirm.test.mjs

@@ -0,0 +1,48 @@
+// node --test noter/test
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+import { needsReconfirmation, DEFAULT_CONFIRM_EVERY_BLOCKS } from '../confirm.js';
+
+const EVERY = DEFAULT_CONFIRM_EVERY_BLOCKS;
+const steady = {
+  supported: true, changed: false, pending: false, queryFailed: false,
+  recordedStake: 500n, lastConfirmedAt: 1_000, currentBlock: 1_000 + EVERY, everyBlocks: EVERY,
+};
+
+test('unchanged stake is confirmed again once the last confirmation is old enough', () => {
+  assert.equal(needsReconfirmation(steady), true);
+});
+
+test('not before then: every submission restarts the dispute window', () => {
+  assert.equal(needsReconfirmation({ ...steady, currentBlock: 1_000 + EVERY - 1 }), false);
+});
+
+test('never while a submission is still pending', () => {
+  assert.equal(needsReconfirmation({ ...steady, pending: true }), false);
+});
+
+test('a change is submitted as a change, not as a confirmation', () => {
+  assert.equal(needsReconfirmation({ ...steady, changed: true }), false);
+});
+
+test('half-read data is not confirmed', () => {
+  assert.equal(needsReconfirmation({ ...steady, queryFailed: true }), false);
+});
+
+test('there is nothing to confirm without a recorded stake', () => {
+  assert.equal(needsReconfirmation({ ...steady, recordedStake: null }), false);
+  assert.equal(needsReconfirmation({ ...steady, recordedStake: 0n }), false);
+});
+
+test('stake recorded before the chain kept confirmations is confirmed', () => {
+  assert.equal(needsReconfirmation({ ...steady, lastConfirmedAt: null, currentBlock: 5 }), true);
+});
+
+test('a chain that keeps no confirmations is left alone', () => {
+  assert.equal(needsReconfirmation({ ...steady, supported: false }), false);
+});
+
+test('the default leaves several tries inside the fourteen-day allowance', () => {
+  const allowance = 14 * 24 * 600;
+  assert.ok(allowance / EVERY >= 4);
+});