Skip to content

Commit a7adc8a

Browse files
Agilulfo1820claude
andauthored
fix: stop silently dropping citizens from the relayer work list (#13)
* fix: stop silently dropping citizens from the relayer work list Mainnet round 110 locked its relayer reward pool permanently: 8528 expected weighted actions vs 8128 completed, isRewardClaimable(110) == false, 4285 B3TR that no relayer can ever claim. Rounds 111 and 112 went the same way. The gap is exactly 100 navigator citizens x (3 vote + 1 claim). They were not ineligible and their navigator was healthy - it had set valid round-110 preferences with all 13 apps eligible and percentages summing to 10000. Simulating castNavigatorVote(citizen, 110) at historical revisions succeeds at every block from just after those preferences were set through 100 blocks before the deadline, a ~3.6 day window. The contract was ready; no relayer ever called it. The same cohort voted fine in rounds 108, 109 and 112. Four defects here can each produce that signature: 1. Thor's /accounts/* halts a batch at the first reverting clause and returns a SHORT array. batchSimulate iterated results.length, so one bad clause dropped every citizen after it in a 100-clause chunk, with no error and no log. The added regression test reproduces round 110's shape exactly: 120 citizens with one reverting clause at index 0 yielded 20 survivors before the fix, 119 after. simulateAllClauses now resumes past a revert so it only ever costs its own clause. 2. A failed citizen fetch left citizenUsers at 0, which is indistinguishable from "this network has no citizens", so both citizen voting phases were skipped for the whole cycle - and the error was not even logged when the message contained "reverted". The summary now carries citizenFetchFailed and always logs. The citizen phases are wrapped so a failure there cannot starve the claim phases that run after them. 3. ExitAnnounced removed all of a navigator's citizens on announcement. An exiting navigator is not dead: verified on mainnet, castNavigatorVote succeeded all round for a navigator with isExiting=true / isDeactivated=false, and that navigator accounts for 7 of round 110's missing citizens. Even for a genuinely deactivated navigator the citizens must stay, because the on-chain skip path is what reduces expected actions - dropping them means nobody ever makes the call. Citizens who really are undelegated are filtered out authoritatively by getNavigatorsForCitizens(citizens, snapshot). 4. CYCLE_RETRIES was nodePool.length, so any deployment setting NODE_URL got exactly one attempt with no backoff. Floored at 3. Verified with a DRY_RUN cycle against mainnet round 113: the citizen work list is 180 and getTotalDelegatedCitizensAtTimepoint(snapshot) is 180; auto-voters 2154 against 2154 on-chain. Two delegation-replay tests asserted the old removal behaviour as intentional "lazy invalidation". They are inverted here deliberately, with the reasoning recorded in the tests. Caveat: I cannot confirm the operator of the relayer that serves citizens runs this exact code, and I could not reproduce a truncated chunk against a live node today. These are reference-implementation defects consistent with the observed on-chain signature - hardening, not a proven root-cause fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016EvtKnHKXcJbRcLp9D7vk9 * fix: contain the citizen governance phase too The previous commit wrapped only the allocation cycle. A throw in runCitizenGovernanceVoteCycle still propagated out of runActiveRoundVotingCycles, aborted runAllCycles, and starved both claim phases — exactly the failure the containment exists to prevent, left half-open. Each citizen phase now has its own catch, so a failing allocation cycle doesn't cost the governance votes either, or vice versa. The two new tests fail without this change ("promise rejected 'governor unreachable' instead of resolving"). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016EvtKnHKXcJbRcLp9D7vk9 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 0aaf405 commit a7adc8a

9 files changed

Lines changed: 404 additions & 31 deletions

‎src/citizen-contracts.ts‎

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { ThorClient } from "@vechain/sdk-network"
22
import { ABIContract, Hex } from "@vechain/sdk-core"
33
import { LogFn } from "./types"
4+
import { simulateAllClauses } from "./simulate"
45

56
// ── Inline ABI fragments (contracts package not yet published with these) ──
67

@@ -170,8 +171,10 @@ async function batchSimulate<T>(
170171
data: fn.encodeData(encodeArgs(k)).toString(),
171172
}))
172173

173-
const results = await thor.transactions.simulateTransaction(clauses)
174-
for (let j = 0; j < results.length; j++) {
174+
// Thor truncates a batch at the first reverting clause; simulateAllClauses
175+
// resumes past it so a single revert can't silently drop the rest of the chunk.
176+
const results = await simulateAllClauses(thor, clauses)
177+
for (let j = 0; j < chunk.length; j++) {
175178
const val = decode(chunk[j], results[j])
176179
if (val !== undefined) result.set(chunk[j].toLowerCase(), val)
177180
}
@@ -427,12 +430,7 @@ async function collectEventLogs(
427430
return events
428431
}
429432

430-
function removeCitizensOfNavigator(nav: string): void {
431-
const lc = nav.toLowerCase()
432-
for (const [citizen, citizenNav] of citizenCache.delegations) {
433-
if (citizenNav === lc) citizenCache.delegations.delete(citizen)
434-
}
435-
}
433+
436434

437435
export async function getDelegatedCitizens(
438436
thor: ThorClient,
@@ -489,10 +487,22 @@ export async function getDelegatedCitizens(
489487
citizenCache.delegations.delete((event.args.citizen as string).toLowerCase())
490488
break
491489
case 'exit':
492-
removeCitizensOfNavigator(event.args.navigator as string)
493-
break
494490
case 'deactivated':
495-
removeCitizensOfNavigator(event.args.navigator as string)
491+
// Deliberately a no-op for the delegation map.
492+
//
493+
// A navigator announcing exit is NOT dead: verified on mainnet round 110,
494+
// castNavigatorVote succeeded for citizens of a navigator with
495+
// isExiting=true / isDeactivated=false at every block of the round. Dropping
496+
// them here meant they were never voted for AND never skipped, so the round's
497+
// expected actions stayed unreachable and the whole pool locked.
498+
//
499+
// Even for a genuinely dead navigator the citizens must stay: castNavigatorVote
500+
// takes the skip path and calls reduceUserAllocationVote, which is what keeps
501+
// the round unlockable. Only a relayer calling the contract can do that.
502+
//
503+
// Citizens who really are no longer delegated are filtered out authoritatively
504+
// by getNavigatorsForCitizens(citizens, snapshot) before any clause is built,
505+
// so this map is a candidate superset, not the final work list.
496506
break
497507
}
498508
}

‎src/contracts.ts‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { ThorClient } from "@vechain/sdk-network"
22
import { ABIContract, Hex } from "@vechain/sdk-core"
33
import { LogFn } from "./types"
4+
import { simulateAllClauses } from "./simulate"
45
import {
56
XAllocationVoting__factory,
67
VoterRewards__factory,
@@ -196,8 +197,9 @@ export async function getPreferredRelayersForUsers(
196197
data: fn.encodeData([user]).toString(),
197198
}))
198199

199-
const results = await thor.transactions.simulateTransaction(clauses)
200-
for (let j = 0; j < results.length; j++) {
200+
// Thor truncates a batch at the first reverting clause; resume past it.
201+
const results = await simulateAllClauses(thor, clauses, log)
202+
for (let j = 0; j < chunk.length; j++) {
201203
const sim = results[j]
202204
if (!sim || sim.reverted || !sim.data || sim.data === "0x") continue
203205

@@ -462,6 +464,7 @@ export async function fetchSummary(
462464

463465
// Citizen / navigator data (graceful fallback if contracts not deployed)
464466
let citizenUsers = 0
467+
let citizenFetchFailed = false
465468
let activeProposalCount = 0
466469
const navAddr = config.navigatorRegistryAddress
467470
const govAddr = config.b3trGovernorAddress
@@ -472,7 +475,12 @@ export async function fetchSummary(
472475
citizenUsers = citizenMap.size
473476
} catch (err) {
474477
const msg = err instanceof Error ? err.message : String(err)
475-
if (!msg.includes("reverted")) console.error(`Warning: citizen fetch failed: ${msg}`)
478+
// Never downgrade this to silence. Reporting a failed fetch as "0 citizens" is
479+
// indistinguishable from "this network has no citizens", and the caller then skips
480+
// both citizen voting phases for the whole cycle. That is how citizens go a full
481+
// round without a vote or a skip, which locks the round's reward pool.
482+
citizenFetchFailed = true
483+
console.error(`ERROR: citizen fetch failed, citizen count unknown: ${msg}`)
476484
}
477485
}
478486
if (govAddr && govAddr !== ZERO) {
@@ -565,6 +573,7 @@ export async function fetchSummary(
565573
latestBlock,
566574
autoVotingUsers,
567575
citizenUsers,
576+
citizenFetchFailed,
568577
activeProposals: activeProposalCount,
569578
totalVoters,
570579
totalVotes,

‎src/index.ts‎

Lines changed: 39 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ function logRaw(msg: string) {
105105
console.log(msg)
106106
}
107107

108-
async function runActiveRoundVotingCycles(
108+
export async function runActiveRoundVotingCycles(
109109
thor: ThorClient,
110110
config: NetworkConfig,
111111
walletAddress: string,
@@ -123,17 +123,37 @@ async function runActiveRoundVotingCycles(
123123
const voteResult = await runCastVoteCycle(thor, config, walletAddress, privateKey, batchSize, dryRun, log)
124124
renderCycleResult(voteResult).forEach(log)
125125

126-
if (summary.citizenUsers === 0) return
126+
// Only skip when we actually know there are no citizens. If the fetch failed the count
127+
// is unknown, so still run the cycles — they re-derive the citizen set themselves and
128+
// may well succeed where the summary call didn't.
129+
if (summary.citizenUsers === 0 && !summary.citizenFetchFailed) return
130+
if (summary.citizenFetchFailed) {
131+
log(chalk.yellow("Citizen count unknown (fetch failed) - running citizen cycles anyway"))
132+
}
127133

128-
logRaw("")
129-
logRaw(logSectionHeader("citizen-vote", summary.currentRoundId))
130-
const citizenVoteResult = await runCitizenAllocationVoteCycle(thor, config, walletAddress, privateKey, batchSize, dryRun, log)
131-
renderCycleResult(citizenVoteResult).forEach(log)
134+
// Both citizen phases are contained, and contained separately.
135+
//
136+
// A throw that escapes here aborts runAllCycles and starves the claim phases that run
137+
// after it — every cycle, for as long as the failure persists. Catching per phase also
138+
// means a failing allocation cycle doesn't cost us the governance votes, or vice versa.
139+
// Fail loudly, but lose only the phase that actually broke.
140+
try {
141+
logRaw("")
142+
logRaw(logSectionHeader("citizen-vote", summary.currentRoundId))
143+
const citizenVoteResult = await runCitizenAllocationVoteCycle(thor, config, walletAddress, privateKey, batchSize, dryRun, log)
144+
renderCycleResult(citizenVoteResult).forEach(log)
145+
} catch (err) {
146+
log(chalk.red(`ERROR: citizen-vote cycle failed: ${err instanceof Error ? err.message : String(err)}`))
147+
}
132148

133-
logRaw("")
134-
logRaw(logSectionHeader("citizen-governance", summary.currentRoundId))
135-
const citizenGovResults = await runCitizenGovernanceVoteCycle(thor, config, walletAddress, privateKey, batchSize, dryRun, log)
136-
for (const r of citizenGovResults) renderCycleResult(r).forEach(log)
149+
try {
150+
logRaw("")
151+
logRaw(logSectionHeader("citizen-governance", summary.currentRoundId))
152+
const citizenGovResults = await runCitizenGovernanceVoteCycle(thor, config, walletAddress, privateKey, batchSize, dryRun, log)
153+
for (const r of citizenGovResults) renderCycleResult(r).forEach(log)
154+
} catch (err) {
155+
log(chalk.red(`ERROR: citizen-governance cycle failed: ${err instanceof Error ? err.message : String(err)}`))
156+
}
137157
}
138158

139159
async function runAllCycles(
@@ -216,7 +236,10 @@ async function main() {
216236
process.on("SIGINT", shutdown)
217237
process.on("SIGTERM", shutdown)
218238

219-
const CYCLE_RETRIES = nodePool.length
239+
// At least 3 attempts even with a single node. Previously this was nodePool.length,
240+
// so any deployment setting NODE_URL (and all of testnet-staging) got exactly one
241+
// attempt with no backoff: one transient RPC blip lost the whole cycle.
242+
const CYCLE_RETRIES = Math.max(3, nodePool.length)
220243
const CYCLE_RETRY_MS = 3000
221244

222245
function refreshScreen(summary: Awaited<ReturnType<typeof fetchSummary>>) {
@@ -269,4 +292,8 @@ async function main() {
269292
}
270293
}
271294

272-
main()
295+
// Only auto-start when run as the CLI entry point, so the cycle functions above can be
296+
// imported and exercised by tests without booting a relayer.
297+
if (require.main === module) {
298+
main()
299+
}

‎src/simulate.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import { ThorClient } from "@vechain/sdk-network"
2+
import { LogFn } from "./types"
3+
4+
export interface SimulateResult {
5+
reverted: boolean
6+
data: string
7+
}
8+
9+
/**
10+
* Simulate a batch of clauses, returning exactly one result per clause.
11+
*
12+
* Thor's `/accounts/*` stops executing a batch at the first clause that reverts
13+
* and returns a SHORT array: the reverting clause is the last entry, and every
14+
* clause after it is simply absent. Callers that iterate `results.length` therefore
15+
* silently lose every key past the first revert — for a 100-clause chunk of
16+
* citizens, one bad clause at the head drops the other 99 with no error and no log.
17+
*
18+
* We resume from where the node stopped until every clause has a result, so a
19+
* revert only ever costs its own clause.
20+
*/
21+
export async function simulateAllClauses(
22+
thor: ThorClient,
23+
clauses: { to: string; value: string; data: string }[],
24+
log?: LogFn,
25+
): Promise<SimulateResult[]> {
26+
const out: SimulateResult[] = []
27+
let resumes = 0
28+
29+
while (out.length < clauses.length) {
30+
const remaining = clauses.slice(out.length)
31+
const res = (await thor.transactions.simulateTransaction(remaining)) as SimulateResult[]
32+
33+
if (!res || res.length === 0) {
34+
// Node returned nothing for a non-empty batch. Treat the head clause as
35+
// reverted so we make progress instead of looping forever.
36+
out.push({ reverted: true, data: "0x" })
37+
resumes++
38+
continue
39+
}
40+
41+
out.push(...res)
42+
if (out.length < clauses.length) resumes++
43+
}
44+
45+
if (resumes > 0) {
46+
log?.(`Simulate: batch truncated ${resumes}x by reverting clauses, resumed to cover all ${clauses.length}`)
47+
}
48+
49+
return out
50+
}

‎src/types.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ export interface RelayerSummary {
2929

3030
autoVotingUsers: number
3131
citizenUsers: number
32+
/** True when the citizen fetch errored, so citizenUsers is unknown rather than genuinely 0. */
33+
citizenFetchFailed: boolean
3234
activeProposals: number
3335
totalVoters: number
3436
totalVotes: bigint
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import { describe, it, expect, vi } from "vitest"
2+
import { getNavigatorsForCitizens } from "../src/citizen-contracts"
3+
import { addr } from "./helpers/builders"
4+
5+
const NAV = addr("9a71")
6+
const REGISTRY = addr("4e9157")
7+
8+
/** ABI-encode an address as a single 32-byte return word. */
9+
function encodeAddress(a: string): string {
10+
return "0x" + a.slice(2).toLowerCase().padStart(64, "0")
11+
}
12+
13+
/**
14+
* Mock thor that mimics the real node: execution halts at the first reverting clause and
15+
* the response is truncated there. Clause N's calldata ends with the citizen address, so
16+
* we can map each clause back to its citizen.
17+
*/
18+
function thorRevertingFor(citizens: string[], revertFor: Set<string>) {
19+
const simulateTransaction = vi.fn(async (sent: any[]) => {
20+
const out: any[] = []
21+
for (const clause of sent) {
22+
const who = citizens.find((c) => clause.data.toLowerCase().includes(c.slice(2).toLowerCase()))!
23+
if (revertFor.has(who)) {
24+
out.push({ reverted: true, data: "0x" })
25+
return out // node stops executing the rest of the batch
26+
}
27+
out.push({ reverted: false, data: encodeAddress(NAV) })
28+
}
29+
return out
30+
})
31+
return { transactions: { simulateTransaction } } as any
32+
}
33+
34+
describe("getNavigatorsForCitizens — one reverting clause must not drop the chunk", () => {
35+
it("keeps every other citizen when the FIRST clause reverts", async () => {
36+
// Round 110's failure shape: 181 citizens, and only the 81-citizen remainder chunk
37+
// came back. Before the fix, a revert at clause 0 lost all 100 of the first chunk.
38+
const citizens = Array.from({ length: 120 }, (_, i) => addr(`c17e${i.toString(16)}`))
39+
const thor = thorRevertingFor(citizens, new Set([citizens[0]]))
40+
41+
const result = await getNavigatorsForCitizens(thor, REGISTRY, citizens, 25520917)
42+
43+
expect(result.size).toBe(119)
44+
expect(result.has(citizens[0].toLowerCase())).toBe(false)
45+
for (const c of citizens.slice(1)) {
46+
expect(result.get(c.toLowerCase())).toBe(NAV.toLowerCase())
47+
}
48+
})
49+
50+
it("drops only the reverting citizens, wherever they sit in the batch", async () => {
51+
const citizens = Array.from({ length: 10 }, (_, i) => addr(`c17e${i.toString(16)}`))
52+
const bad = new Set([citizens[3], citizens[4], citizens[9]])
53+
const thor = thorRevertingFor(citizens, bad)
54+
55+
const result = await getNavigatorsForCitizens(thor, REGISTRY, citizens, 1000)
56+
57+
expect(result.size).toBe(7)
58+
for (const c of citizens) {
59+
expect(result.has(c.toLowerCase())).toBe(!bad.has(c))
60+
}
61+
})
62+
63+
it("returns everyone when nothing reverts", async () => {
64+
const citizens = Array.from({ length: 250 }, (_, i) => addr(`c17e${i.toString(16)}`))
65+
const thor = thorRevertingFor(citizens, new Set())
66+
67+
const result = await getNavigatorsForCitizens(thor, REGISTRY, citizens, 1000)
68+
69+
expect(result.size).toBe(250)
70+
})
71+
})

0 commit comments

Comments
 (0)