Update README.md#2
Open
shahabsadat456-code wants to merge 1 commit into
Open
Conversation
rcarback
added a commit
that referenced
this pull request
May 14, 2026
P1 fixes (silent failures + missing test coverage): - substrate_client._coerce_receipt: raise instead of defaulting is_success to True when the attribute is missing — silently treating a failed extrinsic as ok increments proofs_submitted on a non-submission. - substrate_client._coerce_receipt: log on triggered_events parse failure rather than swallowing — callers downstream check for ProofAccepted. - substrate_miner_controller main-loop pending-task drain: narrow catch to CancelledError, log unexpected exceptions instead of swallowing. - substrate_miner_controller._await_handle_done: WARNING log on timeout — silent timeout re-emerges the cancel→clear→dispatch race the sentinel was added to prevent. - substrate_miner_controller._drain_handle: narrow except to queue.Empty; detect worker death via EOFError/BrokenPipeError/OSError and shut down. P1 tests (untested behaviors the MR rests on): - test_mining_snapshot_decode: pin get_mining_snapshot's +1 offset and parent_hash=at semantics (with at-None passthrough). - test_mine_work_item: assert work_item_done sentinel emission after a cancel — the controller's _await_handle_done depends on this. - test_substrate_miner_controller: dispatch-tracking pairs result with the handle's _dispatched context (not the controller's _current_context). - test_substrate_miner_controller: _handle_result raises RuntimeError on fatal receipt (BadSignature etc.). P2 fixes: - SubstrateMinerController.__init__: add `*` separator so topology_hash, on_proof_submitted, subscription_client become keyword-only (project ≤5 positional params rule). - SubstrateMinerController: add class docstring documenting subscription vs submission client contract. - _handle_head: parallelize per-handle cancel-ack wait (asyncio.gather) — sequential was N×0.5s, significant against 6s blocks. - _handle_head: track consecutive None snapshots, escalate to RuntimeError after _NONE_SNAPSHOT_FAIL_THRESHOLD. - _handle_result: include topology_hash in stale-context comparison. - _handle_result: catch encoder ValueError separately and raise RuntimeError — an encoder bug must not be logged identically to a transient RPC blip. - _handle_result: stash failed-submission error text on stats.last_submission_error. - _drain_handle: track per-handle miner_errors on stats. - _teardown: log each cleanup-step failure at WARNING rather than swallowing. - _subscribe_heads: stash crash cause on stats.last_submission_error. - Tests hoisted from inline imports to module-level (project rule). P2 tests: - subscription_client deadlock guard (rejects same client). - empty miner_handles rejection. - _verify_registered raises when miner not in QuantumPow.Miners. - topology-hash pin mismatch raises. - None-snapshot counter increments + escalates at threshold + resets on success. - classify_submission tests reframed in substrate-interface's actual Module(error=X, pallet=Y, index=N) wire format. P3 fixes: - New SubmissionOutcome str-Enum; classify_submission returns it instead of bare strings (typo-resistant comparisons). - _subscribe_heads uses self.shutdown() directly instead of loop.call_soon_threadsafe (already on the asyncio loop). - ControllerStats: PEP 585 lowercase generics throughout the module (`list`, `dict`, `tuple` instead of `typing.List`/`Dict`/`Tuple`). - _drain_handle: remove dead QueueFull handler (queue has no maxsize). - classify_submission: log unrecognized error text at classification time so operators see it on the same line as the fatal decision. P3 tests: - stale-drop by parent_hash only (forked-chain shape). - stale-drop when _current_context is None. - SubmissionOutcome compares equal to bare-string literals (locks the str-Enum compat contract). - Resets consecutive_none_snapshots on first success. Dismissed: - Result-loss-on-cancel race in main loop (code-reviewer #2): asyncio.Queue on Python 3.10+ correctly handles get() cancellation — items are not consumed unless delivered. Documenting via tests covers regression. - handle.get_stats() blocking while controller owns the handle (code-reviewer #11): documented via expanded comment in _drain_handle rather than refactored; Phase 5 will introduce a separate stats path. - mine_with_timeout fragile Empty detection (P4): out of scope, in legacy miner_worker code not touched by !79. - `_dispatch` malformed-payload test (P3 #14): defensive branch in substrate_client.py:346 is logged-warn-and-return; the upstream substrate-interface contract doesn't realistically produce this shape today, and adding a test would require extracting an internal closure. Verification: ruff clean on touched files; 24 unit tests pass in test_substrate_miner_controller.py; 7 in test_mining_snapshot_decode.py; sentinel emission test passes (113s, under 180s timeout). Pre-existing chain-integration failure in test_controller_submits_proof_end_to_end unchanged (scalecodec encoding issue against the docker chain, not a regression).
rcarback
added a commit
that referenced
this pull request
May 14, 2026
P1 fixes (silent failures + missing test coverage): - substrate_client._coerce_receipt: raise instead of defaulting is_success to True when the attribute is missing — silently treating a failed extrinsic as ok increments proofs_submitted on a non-submission. - substrate_client._coerce_receipt: log on triggered_events parse failure rather than swallowing — callers downstream check for ProofAccepted. - substrate_miner_controller main-loop pending-task drain: narrow catch to CancelledError, log unexpected exceptions instead of swallowing. - substrate_miner_controller._await_handle_done: WARNING log on timeout — silent timeout re-emerges the cancel→clear→dispatch race the sentinel was added to prevent. - substrate_miner_controller._drain_handle: narrow except to queue.Empty; detect worker death via EOFError/BrokenPipeError/OSError and shut down. P1 tests (untested behaviors the MR rests on): - test_mining_snapshot_decode: pin get_mining_snapshot's +1 offset and parent_hash=at semantics (with at-None passthrough). - test_mine_work_item: assert work_item_done sentinel emission after a cancel — the controller's _await_handle_done depends on this. - test_substrate_miner_controller: dispatch-tracking pairs result with the handle's _dispatched context (not the controller's _current_context). - test_substrate_miner_controller: _handle_result raises RuntimeError on fatal receipt (BadSignature etc.). P2 fixes: - SubstrateMinerController.__init__: add `*` separator so topology_hash, on_proof_submitted, subscription_client become keyword-only (project ≤5 positional params rule). - SubstrateMinerController: add class docstring documenting subscription vs submission client contract. - _handle_head: parallelize per-handle cancel-ack wait (asyncio.gather) — sequential was N×0.5s, significant against 6s blocks. - _handle_head: track consecutive None snapshots, escalate to RuntimeError after _NONE_SNAPSHOT_FAIL_THRESHOLD. - _handle_result: include topology_hash in stale-context comparison. - _handle_result: catch encoder ValueError separately and raise RuntimeError — an encoder bug must not be logged identically to a transient RPC blip. - _handle_result: stash failed-submission error text on stats.last_submission_error. - _drain_handle: track per-handle miner_errors on stats. - _teardown: log each cleanup-step failure at WARNING rather than swallowing. - _subscribe_heads: stash crash cause on stats.last_submission_error. - Tests hoisted from inline imports to module-level (project rule). P2 tests: - subscription_client deadlock guard (rejects same client). - empty miner_handles rejection. - _verify_registered raises when miner not in QuantumPow.Miners. - topology-hash pin mismatch raises. - None-snapshot counter increments + escalates at threshold + resets on success. - classify_submission tests reframed in substrate-interface's actual Module(error=X, pallet=Y, index=N) wire format. P3 fixes: - New SubmissionOutcome str-Enum; classify_submission returns it instead of bare strings (typo-resistant comparisons). - _subscribe_heads uses self.shutdown() directly instead of loop.call_soon_threadsafe (already on the asyncio loop). - ControllerStats: PEP 585 lowercase generics throughout the module (`list`, `dict`, `tuple` instead of `typing.List`/`Dict`/`Tuple`). - _drain_handle: remove dead QueueFull handler (queue has no maxsize). - classify_submission: log unrecognized error text at classification time so operators see it on the same line as the fatal decision. P3 tests: - stale-drop by parent_hash only (forked-chain shape). - stale-drop when _current_context is None. - SubmissionOutcome compares equal to bare-string literals (locks the str-Enum compat contract). - Resets consecutive_none_snapshots on first success. Dismissed: - Result-loss-on-cancel race in main loop (code-reviewer #2): asyncio.Queue on Python 3.10+ correctly handles get() cancellation — items are not consumed unless delivered. Documenting via tests covers regression. - handle.get_stats() blocking while controller owns the handle (code-reviewer #11): documented via expanded comment in _drain_handle rather than refactored; Phase 5 will introduce a separate stats path. - mine_with_timeout fragile Empty detection (P4): out of scope, in legacy miner_worker code not touched by !79. - `_dispatch` malformed-payload test (P3 #14): defensive branch in substrate_client.py:346 is logged-warn-and-return; the upstream substrate-interface contract doesn't realistically produce this shape today, and adding a test would require extracting an internal closure. Verification: ruff clean on touched files; 24 unit tests pass in test_substrate_miner_controller.py; 7 in test_mining_snapshot_decode.py; sentinel emission test passes (113s, under 180s timeout). Pre-existing chain-integration failure in test_controller_submits_proof_end_to_end unchanged (scalecodec encoding issue against the docker chain, not a regression).
rcarback
added a commit
that referenced
this pull request
May 15, 2026
P1 fixes (silent failures + missing test coverage): - substrate_client._coerce_receipt: raise instead of defaulting is_success to True when the attribute is missing — silently treating a failed extrinsic as ok increments proofs_submitted on a non-submission. - substrate_client._coerce_receipt: log on triggered_events parse failure rather than swallowing — callers downstream check for ProofAccepted. - substrate_miner_controller main-loop pending-task drain: narrow catch to CancelledError, log unexpected exceptions instead of swallowing. - substrate_miner_controller._await_handle_done: WARNING log on timeout — silent timeout re-emerges the cancel→clear→dispatch race the sentinel was added to prevent. - substrate_miner_controller._drain_handle: narrow except to queue.Empty; detect worker death via EOFError/BrokenPipeError/OSError and shut down. P1 tests (untested behaviors the MR rests on): - test_mining_snapshot_decode: pin get_mining_snapshot's +1 offset and parent_hash=at semantics (with at-None passthrough). - test_mine_work_item: assert work_item_done sentinel emission after a cancel — the controller's _await_handle_done depends on this. - test_substrate_miner_controller: dispatch-tracking pairs result with the handle's _dispatched context (not the controller's _current_context). - test_substrate_miner_controller: _handle_result raises RuntimeError on fatal receipt (BadSignature etc.). P2 fixes: - SubstrateMinerController.__init__: add `*` separator so topology_hash, on_proof_submitted, subscription_client become keyword-only (project ≤5 positional params rule). - SubstrateMinerController: add class docstring documenting subscription vs submission client contract. - _handle_head: parallelize per-handle cancel-ack wait (asyncio.gather) — sequential was N×0.5s, significant against 6s blocks. - _handle_head: track consecutive None snapshots, escalate to RuntimeError after _NONE_SNAPSHOT_FAIL_THRESHOLD. - _handle_result: include topology_hash in stale-context comparison. - _handle_result: catch encoder ValueError separately and raise RuntimeError — an encoder bug must not be logged identically to a transient RPC blip. - _handle_result: stash failed-submission error text on stats.last_submission_error. - _drain_handle: track per-handle miner_errors on stats. - _teardown: log each cleanup-step failure at WARNING rather than swallowing. - _subscribe_heads: stash crash cause on stats.last_submission_error. - Tests hoisted from inline imports to module-level (project rule). P2 tests: - subscription_client deadlock guard (rejects same client). - empty miner_handles rejection. - _verify_registered raises when miner not in QuantumPow.Miners. - topology-hash pin mismatch raises. - None-snapshot counter increments + escalates at threshold + resets on success. - classify_submission tests reframed in substrate-interface's actual Module(error=X, pallet=Y, index=N) wire format. P3 fixes: - New SubmissionOutcome str-Enum; classify_submission returns it instead of bare strings (typo-resistant comparisons). - _subscribe_heads uses self.shutdown() directly instead of loop.call_soon_threadsafe (already on the asyncio loop). - ControllerStats: PEP 585 lowercase generics throughout the module (`list`, `dict`, `tuple` instead of `typing.List`/`Dict`/`Tuple`). - _drain_handle: remove dead QueueFull handler (queue has no maxsize). - classify_submission: log unrecognized error text at classification time so operators see it on the same line as the fatal decision. P3 tests: - stale-drop by parent_hash only (forked-chain shape). - stale-drop when _current_context is None. - SubmissionOutcome compares equal to bare-string literals (locks the str-Enum compat contract). - Resets consecutive_none_snapshots on first success. Dismissed: - Result-loss-on-cancel race in main loop (code-reviewer #2): asyncio.Queue on Python 3.10+ correctly handles get() cancellation — items are not consumed unless delivered. Documenting via tests covers regression. - handle.get_stats() blocking while controller owns the handle (code-reviewer #11): documented via expanded comment in _drain_handle rather than refactored; Phase 5 will introduce a separate stats path. - mine_with_timeout fragile Empty detection (P4): out of scope, in legacy miner_worker code not touched by !79. - `_dispatch` malformed-payload test (P3 #14): defensive branch in substrate_client.py:346 is logged-warn-and-return; the upstream substrate-interface contract doesn't realistically produce this shape today, and adding a test would require extracting an internal closure. Verification: ruff clean on touched files; 24 unit tests pass in test_substrate_miner_controller.py; 7 in test_mining_snapshot_decode.py; sentinel emission test passes (113s, under 180s timeout). Pre-existing chain-integration failure in test_controller_submits_proof_end_to_end unchanged (scalecodec encoding issue against the docker chain, not a regression).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.