refactor: dedupe the 1_000_000 issue-ID bound in cli/issue_commands/helpers.py

gittensor/cli/issue_commands/helpers.py

@@ -783,9 +783,11 @@ def _read_issues_from_child_storage(substrate, contract_addr: str, verbose: bool
     if verbose:
         console.print(f'[dim]Debug: next_issue_id from contract = {next_issue_id}[/dim]')
 
-    # Sanity check: next_issue_id should be reasonable (< 1 million for any real deployment)
-    MAX_REASONABLE_ISSUE_ID = 1_000_000
-    if next_issue_id > MAX_REASONABLE_ISSUE_ID:
+    # Sanity check: next_issue_id should not exceed the same upper bound used to
+    # validate user-supplied IDs in `validate_issue_id`. Using the existing
+    # module-level constant keeps the two checks in sync — a future bound change
+    # can't accidentally raise the input cap without raising the storage cap too.
+    if next_issue_id > MAX_ISSUE_ID:
         console.print(f'[yellow]Warning: next_issue_id ({next_issue_id}) is unreasonably large.[/yellow]')
         console.print('[yellow]This may indicate a storage format mismatch. Check contract version.[/yellow]')
         return []

tests/cli/test_read_issues_sanity_bound.py

@@ -0,0 +1,64 @@
+# The MIT License (MIT)
+# Copyright © 2025 Entrius
+
+"""Regression test for the deduplicated 1_000_000 issue-ID bound in
+`gittensor.cli.issue_commands.helpers._read_issues_from_child_storage`.
+
+The function shares its sanity ceiling with `validate_issue_id` via the
+single module-level constant `MAX_ISSUE_ID`. This file pins the
+behaviour at the two boundary values (the bound itself, and one above)
+so a future change to the constant updates both checks together.
+"""
+
+from unittest.mock import MagicMock, patch
+
+from gittensor.cli.issue_commands import helpers
+from gittensor.cli.issue_commands.helpers import MAX_ISSUE_ID
+
+
+def test_read_issues_warns_and_returns_empty_when_next_issue_id_exceeds_bound():
+    """next_issue_id > MAX_ISSUE_ID is unreasonable — return [] without iterating."""
+    fake_substrate = MagicMock()
+    with (
+        patch(
+            'gittensor.cli.issue_commands.helpers.get_contract_child_storage_key',
+            return_value='0xchildkey',
+        ),
+        patch(
+            'gittensor.cli.issue_commands.helpers._read_contract_packed_storage',
+            return_value={'next_issue_id': MAX_ISSUE_ID + 1},
+        ),
+    ):
+        result = helpers._read_issues_from_child_storage(fake_substrate, '5xContract')
+
+    assert result == []
+    # No per-issue childstate_getStorage call should have happened.
+    fake_substrate.rpc_request.assert_not_called()
+
+
+def test_read_issues_proceeds_when_next_issue_id_at_bound():
+    """next_issue_id == MAX_ISSUE_ID is the inclusive boundary — proceeds past the sanity gate."""
+    fake_substrate = MagicMock()
+    fake_substrate.rpc_request.return_value = {'result': None}
+
+    with (
+        patch(
+            'gittensor.cli.issue_commands.helpers.get_contract_child_storage_key',
+            return_value='0xchildkey',
+        ),
+        patch(
+            'gittensor.cli.issue_commands.helpers._read_contract_packed_storage',
+            return_value={'next_issue_id': MAX_ISSUE_ID},
+        ),
+        patch(
+            'gittensor.cli.issue_commands.helpers.compute_ink5_lazy_key',
+            return_value='0xlazykey',
+        ),
+    ):
+        result = helpers._read_issues_from_child_storage(fake_substrate, '5xContract')
+
+    # All storage reads returned None, so the result is empty — but the function
+    # got past the sanity gate (proven by rpc_request being called for
+    # individual issue lookups).
+    assert result == []
+    assert fake_substrate.rpc_request.called