diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2027b4b..a190558 100644 --- a/.github/workflows/ci.yml +++ b/.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: diff --git a/noter/Dockerfile b/noter/Dockerfile index c40ad70..6ea75e4 100644 --- a/noter/Dockerfile +++ b/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"] diff --git a/noter/bot.js b/noter/bot.js index 68aa34a..283a9df 100644 --- a/noter/bot.js +++ b/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 { diff --git a/noter/confirm.js b/noter/confirm.js new file mode 100644 index 0000000..e0492c8 --- /dev/null +++ b/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; +} diff --git a/noter/test/confirm.test.mjs b/noter/test/confirm.test.mjs new file mode 100644 index 0000000..1718eac --- /dev/null +++ b/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); +});