fix(keeper): heal every board's top each tick, drop cron settlement - #19
fix(keeper): heal every board's top each tick, drop cron settlement#19tnrdd wants to merge 3 commits into
Conversation
The keeper rescanned BackerUpdated logs from the factory deploy block for each board before moving to the next, so with 1.5M blocks of history it never finished inside the 300s function limit and boards late in the list were never healed. A drained top stayed enforced while a rival streamed. Read every board's topMarkee and live #1 in one multicall and claimTop wherever they disagree, isolating per-board failures. Remove settle from the cron: backers claim their own RevNet share from ClaimModal. Delete the log scan and STREAMING_FROM_BLOCK dependency, raise the RPC timeout for the multicall, and add fake-client tests for the keeper.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
🚀 Staging: https://2staging.markee.xyz |
There was a problem hiding this comment.
Gaston Review
Verdict: Approved
Score: ████████░░ 8/10
Pull Request Summary
This PR strips the keeper's settle responsibility entirely — backers now claim their own RevNet share from the UI via ClaimModal. The remaining claimTop logic is refactored from per-board sequential reads into a single batched multicall (two reads per board: topMarkee + getTopMarkees), with a 60s RPC timeout to accommodate the bigger payload. logScan.ts is deleted since there's no more BackerUpdated log scanning, and STREAMING_FROM_BLOCK is no longer needed. New unit tests cover the happy path, dry-run, and error recovery.
Review Summary
🔵 Suggestion — No test for continuation after a mid-run claimTop failure
The 'failed claimTop' test only has one board that drifts, so it doesn't verify that the loop continues to process subsequent boards after a failure. A test with two drifted boards where the first write fails would confirm the error isolation works end-to-end. Not a blocker — the code clearly continues via the try/catch inside the for-loop — but it'd be a higher-confidence test.
🔵 Suggestion — Large multicall may hit RPC limits at scale
All boards are read in a single multicall (2 calls per board). If the factory grows to hundreds of boards, this could exceed RPC provider response-size or gas-estimation limits. Not a problem today, but worth keeping in mind — chunking the multicall (e.g. 100 boards at a time) would be a straightforward future fix if needed.
Clean, well-scoped simplification. The multicall batching is the right call for the read side, error handling per-board is solid with allowFailure: true, and the tests cover the important paths. The deletion of logScan.ts and STREAMING_FROM_BLOCK leaves no orphaned references. Production-ready.
📌 1 inline comment
⚪ Nitpick: 1
🔍 Reviewed by Gaston
|
🚀 Staging: https://2staging.markee.xyz |
There was a problem hiding this comment.
Gaston Review
Verdict: Approved
Score: ████████░░ 8/10
Pull Request Summary
This PR removes the keeper's settle responsibility entirely (backers now claim their own RevNet share from the UI) and refactors the claimTop reads from sequential per-board calls into a single batched multicall with allowFailure: true. It deletes logScan.ts and the STREAMING_FROM_BLOCK env var since there's no more BackerUpdated log scanning. New unit tests cover the happy path, dry-run, and error recovery.
Second pass — nothing new to flag. The cleanup is thorough: no orphaned references to the deleted code, the multicall batching is well-structured, per-board error isolation works correctly, and the tests cover the important paths. The two suggestions from my first review (multi-board failure continuation test, and future multicall chunking) still apply but aren't blockers. Production-ready.
🔍 Reviewed by Gaston
getTopMarkees is O(n^2) in a board's markee count and every board in one eth_call shares the RPC gas cap, so one oversized board could fail the reads for its whole chunk. Read ten boards per multicall and re-read any failed board on its own so it only ever loses its own heal.
|
🚀 Staging: https://2staging.markee.xyz |
There was a problem hiding this comment.
Gaston Review
Verdict: Approved
Score: ████████░░ 8/10
Pull Request Summary
This PR removes the keeper's settle responsibility (backers now claim from the UI) and refactors claimTop reads into chunked multicalls of 10 boards, with per-board retry when a shared chunk fails. logScan.ts is deleted, and five new unit tests cover the happy path, dry-run, write failures, chunk-level retry, and per-board read failures.
Third pass. The latest commit adds the multicall chunking I suggested in my first review — chunks of 10 boards with single-board retry on failure. The implementation is clean: readTopChunk handles the multicall + result parsing, readTops does the chunking + retry, and healTops walks the results. The retry gate (chunk.length > 1) is correct — a board that fails alone stays failed. The new tests (especially the failMultiBoardChunks one that verifies the [2, 1, 1] multicall pattern) give good confidence. Both of my original suggestions are now addressed. Production-ready.
🔍 Reviewed by Gaston
Problem
The streaming keeper cron has never healed a board in production.
500 no signer configuredbecauseKEEPER_PRIVATE_KEYis not set in the Vercel env.BackerUpdatedlogs from the factory deploy block (now ~1.5M blocks, ~170 sequentialgetLogsper board across 24 boards) before healing the next board, so the run blew past the 300s function limit. A dry run against production hung for over two minutes with no response.Concretely, board
0x8673…8123hastopRate 0(its top backer's stream is gone) while0x854C…eeC6streams 1.52e9 wei/s and should hold the title.Change
topMarkeeandgetTopMarkees(1)in a single multicall and callsclaimTopwherever they disagree. Per-board read or tx failures are reported and do not abort the run.ClaimModal, which callssettle([backer]).STREAMING_FROM_BLOCKdependency are deleted; nothing reads that var any more.frontend/tests/keeper.test.ts(npm run test:keeper) with fake clients: drained top promoted while a healthy board is untouched, dry run signs nothing, a failedclaimTopis reported without stopping the run.Verification
test:keeper3/3,test:streaming-campaign8/8,tsc --noEmitclean,next buildgreen.claimTop(0x854C…eeC6)on0x8673…8123).claimTopconfirmed,topMarkeeflipped to0x854C…eeC6withtopRate 1522070014, and a second run was a no-op.Ops follow-up
Set
KEEPER_PRIVATE_KEYin the Vercel production env to a throwaway wallet with a little Base ETH. Until then the cron keeps returning 500.STREAMING_FROM_BLOCKcan be removed.