From 68e8cca29aa23795a329e047f1e9c0af08043914 Mon Sep 17 00:00:00 2001 From: kokialgo Date: Tue, 11 Aug 2026 11:22:50 -0300 Subject: [PATCH 1/4] fix(contracts): restructure keeper phase-gate to allow drawing-phase entry without minStakingTime --- contracts/scripts/keeperBot.ts | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/contracts/scripts/keeperBot.ts b/contracts/scripts/keeperBot.ts index 84052485c..02e1bea9e 100644 --- a/contracts/scripts/keeperBot.ts +++ b/contracts/scripts/keeperBot.ts @@ -635,7 +635,7 @@ async function main() { const minStakingTime = await sortition.minStakingTime(); const blockTime = await getBlockTime(); return await sortition.lastPhaseChange().then((lastPhaseChange) => { - return toBigInt(blockTime) - lastPhaseChange > minStakingTime; + return toBigInt(blockTime) - lastPhaseChange >= minStakingTime; }); }; @@ -643,7 +643,7 @@ async function main() { const maxDrawingTime = await sortition.maxDrawingTime(); const blockTime = await getBlockTime(); return await sortition.lastPhaseChange().then((lastPhaseChange) => { - return toBigInt(blockTime) - lastPhaseChange > maxDrawingTime; + return toBigInt(blockTime) - lastPhaseChange >= maxDrawingTime; }); }; @@ -708,7 +708,17 @@ async function main() { } logger.info(`Disputes needing more jurors: ${disputesWithoutJurors.map((dispute) => dispute.id)}`); - if ((await hasMinStakingTimePassed()) && disputesWithoutJurors.length > 0) { + + // Phase-aware dispatch: + // - If already in drawing phase (e.g. advanced by an external actor or resumed after a keeper + // restart mid-draw), enter the drawing loop directly — no minStakingTime gate required. + // - Otherwise, require minStakingTime to have elapsed (>= to match contract semantics) before + // advancing from staking through generating into drawing. + const enterDrawingBlock = + (disputesWithoutJurors.length > 0 && (await isPhaseDrawing())) || + ((await hasMinStakingTimePassed()) && disputesWithoutJurors.length > 0); + + if (enterDrawingBlock) { // ----------------------------------------------- // // DRAWING ATTEMPT // // ----------------------------------------------- // @@ -726,6 +736,7 @@ async function main() { await passPhase(); } if (await isPhaseDrawing()) { + let drawIterationsTotal = 0; let maxDrawingTimePassed = await hasMaxDrawingTimePassed(); for (const dispute of disputesWithoutJurors) { if (maxDrawingTimePassed) { @@ -746,11 +757,21 @@ async function main() { logger.error(`Failed to draw jurors for dispute #${dispute.id}, skipping it`); break; } + drawIterationsTotal += drawIterations; await delay(ITERATIONS_COOLDOWN_PERIOD); // To avoid spiking the gas price maxDrawingTimePassed = await hasMaxDrawingTimePassed(); numberOfMissingJurors = await getMissingJurors(dispute); } while (!(numberOfMissingJurors === 0n) && !maxDrawingTimePassed); } + // Warn if the drawing run completed with zero draws but disputes still need jurors. + // This indicates a stall (no eligible jurors staked, RNG issue, or all draws failing). + if (drawIterationsTotal === 0 && disputesWithoutJurors.length > 0) { + const pendingIds = disputesWithoutJurors.map((d) => d.id).join(", "); + logger.warn( + `Drawing phase run completed with zero draws for ${disputesWithoutJurors.length} ` + + `dispute(s) still needing jurors: [${pendingIds}]` + ); + } // At this point, either all disputes are fully drawn or max drawing time has passed } } From 0691c2b2a02031c15db65f4a2adc36c0572cb0e2 Mon Sep 17 00:00:00 2001 From: kokialgo Date: Tue, 11 Aug 2026 16:12:06 -0300 Subject: [PATCH 2/4] fix(contracts): allow generating-phase entry without minStakingTime gate minStakingTime only gates the staking -> generating transition per SortitionModule.sol. The generating -> drawing transition only requires RNG readiness. A keeper that restarts (or resumes after an external actor advanced the phase) while phase is already generating was wrongly held behind the minStakingTime check, skipping the RNG-readiness check entirely. Addresses code review feedback on PR #2575. --- contracts/scripts/keeperBot.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/contracts/scripts/keeperBot.ts b/contracts/scripts/keeperBot.ts index 02e1bea9e..2d0fe1011 100644 --- a/contracts/scripts/keeperBot.ts +++ b/contracts/scripts/keeperBot.ts @@ -710,12 +710,14 @@ async function main() { logger.info(`Disputes needing more jurors: ${disputesWithoutJurors.map((dispute) => dispute.id)}`); // Phase-aware dispatch: - // - If already in drawing phase (e.g. advanced by an external actor or resumed after a keeper - // restart mid-draw), enter the drawing loop directly — no minStakingTime gate required. - // - Otherwise, require minStakingTime to have elapsed (>= to match contract semantics) before - // advancing from staking through generating into drawing. + // - If already in generating or drawing phase (e.g. advanced by an external actor or resumed + // after a keeper restart mid-cycle), enter the block directly — no minStakingTime gate + // required. minStakingTime only governs the staking -> generating transition + // (SortitionModule.sol); generating -> drawing only requires RNG readiness. + // - Otherwise (phase is staking), require minStakingTime to have elapsed (>= to match contract + // semantics) before advancing from staking through generating into drawing. const enterDrawingBlock = - (disputesWithoutJurors.length > 0 && (await isPhaseDrawing())) || + (disputesWithoutJurors.length > 0 && ((await isPhaseGenerating()) || (await isPhaseDrawing()))) || ((await hasMinStakingTimePassed()) && disputesWithoutJurors.length > 0); if (enterDrawingBlock) { From 586beda2808ce7fae876670b075c70bc4b737555 Mon Sep 17 00:00:00 2001 From: kokialgo Date: Tue, 11 Aug 2026 16:20:00 -0300 Subject: [PATCH 3/4] fix(contracts): count actual jurors drawn instead of requested iterations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit drawJurors() returns true once its transaction confirms, not based on how many jurors it actually drew — its pre-flight probe checks a much larger simulated horizon (iterations * MAX_DRAW_CALLS_WITHOUT_JURORS) than the real batch it submits, so a confirmed tx can still draw zero new jurors. Track actual draws via getMissingJurors() deltas instead of accumulating requested drawIterations, so the zero-draw stall warning can no longer be silently suppressed by transactions that succeed without drawing anyone. Also re-query which disputes are still unresolved before building the warning message. Verified against KlerosCore.sol: nbVotes is immutable during the draw loop (draw() requires Period.evidence, the only nbVotes-changing path via appeal requires Period.appeal — mutually exclusive) and drawnJurors is push-only within draw(), so the getMissingJurors() delta is always >= 0 and always equals actual new jurors drawn in that window. Addresses code review feedback on PR #2575. --- contracts/scripts/keeperBot.ts | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/contracts/scripts/keeperBot.ts b/contracts/scripts/keeperBot.ts index 2d0fe1011..45b899eca 100644 --- a/contracts/scripts/keeperBot.ts +++ b/contracts/scripts/keeperBot.ts @@ -738,7 +738,13 @@ async function main() { await passPhase(); } if (await isPhaseDrawing()) { - let drawIterationsTotal = 0; + // Actual jurors newly drawn across the run, measured via getMissingJurors() deltas — + // NOT the requested `drawIterations` count. drawJurors() returns true once its transaction + // confirms, regardless of how many jurors it actually drew (its pre-flight probe checks a + // much larger simulated horizon than the real batch it submits), so counting requested + // iterations would let a fully-stalled run (transactions confirm, nobody gets drawn) + // silently suppress the stall warning below. + let actualJurorsDrawn = 0; let maxDrawingTimePassed = await hasMaxDrawingTimePassed(); for (const dispute of disputesWithoutJurors) { if (maxDrawingTimePassed) { @@ -759,20 +765,27 @@ async function main() { logger.error(`Failed to draw jurors for dispute #${dispute.id}, skipping it`); break; } - drawIterationsTotal += drawIterations; await delay(ITERATIONS_COOLDOWN_PERIOD); // To avoid spiking the gas price maxDrawingTimePassed = await hasMaxDrawingTimePassed(); + const missingBefore = numberOfMissingJurors; numberOfMissingJurors = await getMissingJurors(dispute); + actualJurorsDrawn += getNumber(missingBefore) - getNumber(numberOfMissingJurors); } while (!(numberOfMissingJurors === 0n) && !maxDrawingTimePassed); } - // Warn if the drawing run completed with zero draws but disputes still need jurors. + // Warn if the drawing run completed with zero actual draws but disputes still need jurors. // This indicates a stall (no eligible jurors staked, RNG issue, or all draws failing). - if (drawIterationsTotal === 0 && disputesWithoutJurors.length > 0) { - const pendingIds = disputesWithoutJurors.map((d) => d.id).join(", "); - logger.warn( - `Drawing phase run completed with zero draws for ${disputesWithoutJurors.length} ` + - `dispute(s) still needing jurors: [${pendingIds}]` - ); + // Re-query which disputes are still unresolved rather than trusting the pre-loop snapshot. + if (actualJurorsDrawn === 0 && disputesWithoutJurors.length > 0) { + const stillPending = await filterAsync(disputesWithoutJurors, async (dispute) => { + return !(await isDisputeFullyDrawn(dispute)); + }); + if (stillPending.length > 0) { + const pendingIds = stillPending.map((d) => d.id).join(", "); + logger.warn( + `Drawing phase run completed with zero draws for ${stillPending.length} ` + + `dispute(s) still needing jurors: [${pendingIds}]` + ); + } } // At this point, either all disputes are fully drawn or max drawing time has passed } From 6995bad32cb0dd2a1af460e663224c636c9dbb74 Mon Sep 17 00:00:00 2001 From: kokialgo Date: Tue, 11 Aug 2026 17:41:54 -0300 Subject: [PATCH 4/4] fix(contracts): guard zero-iteration draw call in keeper drawing loop The drawing do-while loop could enter with numberOfMissingJurors already at 0 (dispute fully drawn externally, e.g. by another keeper instance, between the pre-loop snapshot and this iteration), calling drawJurors(dispute, 0) and logging a misleading 'Failed to draw jurors' error for a non-error condition. Mirrors the existing zero-iterations guard already used in the executeRepartitions loop. Found by Judgment Day dual review of the keeper phase-gate fix. --- contracts/scripts/keeperBot.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/contracts/scripts/keeperBot.ts b/contracts/scripts/keeperBot.ts index 45b899eca..dd1474d3e 100644 --- a/contracts/scripts/keeperBot.ts +++ b/contracts/scripts/keeperBot.ts @@ -758,6 +758,13 @@ async function main() { } do { const drawIterations = Math.min(MAX_DRAW_ITERATIONS, getNumber(numberOfMissingJurors)); + if (drawIterations === 0) { + // Dispute was fully drawn externally (e.g. another keeper instance) between the + // pre-loop snapshot and this iteration. Nothing left to draw; avoid a wasted + // drawJurors(dispute, 0) call and the misleading "Failed to draw jurors" log it + // would otherwise produce. + break; + } logger.info( `Drawing ${drawIterations} out of ${numberOfMissingJurors} jurors needed for dispute #${dispute.id}` );