Use GetOpenReputerSubmissionWindows query for reputers - #96
Conversation
There was a problem hiding this comment.
Review completed against the latest diff
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
helder-moreira
left a comment
There was a problem hiding this comment.
Multi-Agent Review — PR #96
Reviewers: Kimi K3 + GPT-5.6-Sol
Verdict: REQUEST_CHANGES — critical backward-compatibility regression
The new GetOpenReputerSubmissionWindows query and block_height → nonce mapping are correct against the generated v10/v3 protos, the return type matches set[int], and 4/4 tests pass. However, the change breaks the event-driven submission fallback on older nodes.
Critical — query failure breaks event-driven submissions (GPT-5.6-Sol)
src/allora_sdk/worker/reputer.py line 123 — The old code returned set() (empty set), so EventReputerSubmissionWindowOpened could still trigger submissions via _maybe_submit_impl (line 685 of worker.py): the event nonce was added to the empty set and submitted. The new code makes an RPC call that throws UNIMPLEMENTED on older nodes. Since _maybe_submit_impl calls get_unfulfilled_nonces() before adding the event nonce, the exception aborts the entire submission — including the event-driven path that was previously working.
The polling loop catches this (except Exception in _polling_worker), so nothing crashes. But the event handler also catches it (except Exception in _handle_submission_window_opened_event), meaning the event nonce is silently dropped. On older nodes, reputer submission is completely disabled — worse than before, when at least events worked.
Fix: Catch the unsupported-method error and return an empty set so the event-driven path remains functional:
try:
resp = await self.client.emissions.query.get_open_reputer_submission_windows(...)
except Exception:
return set() # older nodes: fall back to event-driven onlyWarning — reputer_nonce may be None (Kimi K3)
src/allora_sdk/worker/reputer.py line 128 — In the generated v3 protos, ReputerRequestNonce.reputer_nonce is Nonce | None (optional message field). If a node returns an entry without reputer_nonce set, the comprehension raises AttributeError. Add a defensive filter:
return {x.reputer_nonce.block_height for x in resp.nonces.nonces if x.reputer_nonce is not None}Suggestion — clearer error message for unsupported nodes (Kimi K3)
src/allora_sdk/worker/reputer.py line 123 — The PR body acknowledges older nodes won't implement this RPC, but a raw gRPC UNIMPLEMENTED error in the logs is hard to diagnose. A wrapped error message (e.g. "node does not support GetOpenReputerSubmissionWindows; upgrade your node") would help operators.
Suggestion — missing regression test for unsupported-node fallback (GPT-5.6-Sol)
tests/test_reputer_submit.py — Add a test where get_open_reputer_submission_windows raises the older-node unsupported error and verify the fallback returns an empty set (after the fix above). Existing tests cover populated and empty responses correctly.
(Reviewed by Helder's Agent — Kimi K3 + GPT-5.6-Sol)
537e9d9 to
c4b29df
Compare
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
d34234a to
46198e4
Compare
46198e4 to
5800962
Compare
We had effectively disabled polling for reputers because the
GetUnfulfilledNoncescall returned too many nonces (it included epochs not ready for reputation yet). This reenables polling, but uses theGetOpenReputerSubmissionWindowsevent instead, which wasn't available in earlier protocol versions. Should result in a slight improvement of reputer reliability.Summary by cubic
Re-enables reputer polling by querying open submission windows and handles query failures without blocking submissions. Previously
get_unfulfilled_noncesreturned empty and relied only onEventReputerSubmissionWindowOpened; now it returns open window block heights, and explicit/event-driven submissions still run if polling fails.get_unfulfilled_noncesviaemissions.query.get_open_reputer_submission_windows(...); returns empty when no windows ornoncesisNone._maybe_submit_impl, log a warning on failure, and continue processing the providednonce.GetOpenReputerSubmissionWindowsfor polling; on older protocol versions, only event-driven submissions will run.Written for commit 5800962. Summary will update on new commits.