Skip to content

feat(genvm): calls between different majors of genvm - #1716

Draft
kp2pml30 wants to merge 3 commits into
v0.123-devfrom
feat/v02-compat
Draft

feat(genvm): calls between different majors of genvm#1716
kp2pml30 wants to merge 3 commits into
v0.123-devfrom
feat/v02-compat

Conversation

@kp2pml30

@kp2pml30 kp2pml30 commented Jul 28, 2026

Copy link
Copy Markdown
Member

In general, tests pass locally, but I need to work a bit more on it

Summary by CodeRabbit

  • New Features

    • Added Studio-only executor version pinning for contract deployments.
    • Preserved executor routing settings across contract state, snapshots, and restarts.
    • Enabled cross-contract calls between contracts running on different executor versions.
    • Added validation to ensure routing pins use supported version formats and debug modes.
  • Bug Fixes

    • Improved executor connection recovery, retries, cleanup, and completion handling.
    • Updated GenVM assets to version v0.6.0-rc1.
  • Tests

    • Added comprehensive coverage for routing, persistence, nested execution, retries, and connection lifecycles.

@kp2pml30 kp2pml30 self-assigned this Jul 28, 2026
@github-actions
github-actions Bot changed the base branch from main to v0.123-dev July 28, 2026 13:25
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 21fe3dce-477e-42fc-9c7a-95ee55268b47

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds sim_config.reroute_to support for deploy-time executor pinning. The value is validated, persisted through contract state and snapshots, propagated into GenVM execution, and used for cross-major nested calls. GenVM manager communication and nested connection lifecycle handling are also reworked.

Changes

Executor rerouting

Layer / File(s) Summary
Contract pin propagation and persistence
backend/consensus/*, backend/domain/types.py, backend/database_handler/*, tests/db-sqlalchemy/*, tests/unit/consensus/*
Deploy transactions carry reroute_to into accepted contract data, CurrentState, snapshots, and serialized configuration.
Pin validation and executor assets
backend/protocol_rpc/endpoints.py, docker/scripts/download_genvm.sh, tests/unit/test_reroute_to_debug_mode.py
RPC validation gates pins by debug mode and executor-version syntax; asset installation verifies every pinned runner archive.
Manager protocol and run orchestration
backend/node/genvm/origin/*, tests/unit/test_genvm_*, tests/unit/test_manager_client_lifecycle.py
GenVM execution uses a websocket ManagerClient, structured consumed results, expanded protocol dispatch, retries, and terminal cleanup.
Nested executor routing and lifecycle
backend/node/base.py, backend/node/genvm/base.py, tests/unit/test_host_nested_connections.py, tests/unit/test_resolve_callcontract_executor.py
Nested connections remain active for cross-major calls, pinned ABI majors are resolved, and reroute overrides are passed into GenVM runs.
Cross-executor integration coverage
tests/integration/test_deploy_reroute_to.py, tests/integration/gltest_compat.py, third_party/genvm/version
Integration tests cover current/legacy message and synchronous call paths, including explicitly pinned callees.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

Suggested labels: run-tests

Suggested reviewers: cristiam86, muncleuscles

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description omits the required What, Why, Testing done, Decisions made, Checks, Reviewing tips, and release notes sections. Fill out the template sections, add an issue link, and summarize the change, rationale, testing, decisions, checks, tips, and release notes.
Docstring Coverage ⚠️ Warning Docstring coverage is 11.41% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change: adding GenVM calls across different major versions.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/v02-compat

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

This PR targeted main, which is only the default/static branch.

I retargeted it to v0.123-dev, the active development branch. Pushes to v0.123-dev automatically fast-forward main.

@kp2pml30

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🧹 Nitpick comments (5)
tests/db-sqlalchemy/contract_reroute_to_test.py (1)

1-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

File name doesn't follow the test_<feature>.py convention.

This new file is named contract_reroute_to_test.py rather than test_contract_reroute_to.py.

Based on path instructions for tests/**/*.py: "Place new backend tests in the closest scope folder and name them test_<feature>.py for Pytest auto-discovery."

#!/bin/bash
# Description: Check naming convention already used in tests/db-sqlalchemy/
fd . tests/db-sqlalchemy -e py
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/db-sqlalchemy/contract_reroute_to_test.py` around lines 1 - 63, Rename
the test module containing test_register_contract_persists_reroute_to and
related reroute tests from contract_reroute_to_test.py to
test_contract_reroute_to.py so it follows the required pytest discovery naming
convention.

Source: Path instructions

backend/node/genvm/origin/base_host.py (1)

34-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider sourcing the root-slot major offset from public_abi.

public_abi._RootOffsets.MAJOR = 0 already encodes this layout constant (generated from the ABI); duplicating it here as ROOT_OFFSET_MAJOR means a future ABI reshuffle silently desyncs. Reusing the generated value keeps one source of truth.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/node/genvm/origin/base_host.py` around lines 34 - 47, Replace the
duplicated ROOT_OFFSET_MAJOR constant with public_abi._RootOffsets.MAJOR
wherever the root-slot major offset is used, preserving the existing behavior
and documentation for UNDEPLOYED_MAJOR. Ensure the generated public_abi value
remains the single source of truth for this layout offset.
tests/unit/test_manager_client_lifecycle.py (1)

102-104: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

_NoopHandler.get_contract_major matches no host API.

The handler contract is IHost.storage_read (used by read_contract_major); this stub is never called since major=0 is passed explicitly, and it would not save the test if major were dropped. Renaming it to a storage_read stub keeps the fake honest.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/test_manager_client_lifecycle.py` around lines 102 - 104, Rename
_NoopHandler.get_contract_major to storage_read and preserve its async no-op
behavior, matching the IHost.storage_read contract used by read_contract_major.
Keep the existing return value and arguments unless required by that interface.
tests/unit/test_genvm_initial_time_units.py (1)

83-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assertion restates the implementation expression.

math.ceil(timeout or 10 * 60) is copied verbatim from run_genvm, so a change in that expression (including the or / * precedence) keeps the test green. Parametrize the expected value instead, e.g. (None, 600), (3.25, 4), (1200, 1200).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/test_genvm_initial_time_units.py` around lines 83 - 85, Replace
the implementation-derived assertion in the genvm initial-time test with
parametrized input and expected values, covering at least None → 600, 3.25 → 4,
and 1200 → 1200. Assert initial_time_units_allocation against the parametrized
expected value so the test independently verifies run_genvm behavior without
duplicating its expression.
backend/node/genvm/base.py (1)

713-724: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

No timeout guard when draining cancelled nested connection tasks.

close_connections cancels _connection_tasks and then awaits each one with no bound. If a nested host_loop_on task fails to observe cancellation promptly, this await (called from run_genvm_host's finally) can hang indefinitely, blocking the whole retry loop and delaying the next attempt/timeout response.

♻️ Suggested defensive timeout
         for task in self._connection_tasks:
             try:
-                await task
+                await asyncio.wait_for(task, timeout=5.0)
             except asyncio.CancelledError:
                 pass
+            except asyncio.TimeoutError:
+                if self._ctx is not None:
+                    self._ctx.logger.error("nested host connection did not cancel in time")
             except BaseException as e:
                 if self._ctx is not None:
                     self._ctx.logger.error("nested host connection failed", error=e)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/node/genvm/base.py` around lines 713 - 724, Update close_connections
to drain cancelled _connection_tasks under a bounded timeout, ensuring a task
that does not promptly observe cancellation cannot block run_genvm_host cleanup
indefinitely. Preserve logging for completed tasks and cancellation handling,
and clear _connection_tasks after the bounded drain completes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@backend/node/genvm/origin/base_host.py`:
- Around line 503-520: Update ConsumedResult.decode to treat empty raw payloads,
including empty bytes-like or list inputs, as an internal error before indexing
as_bytes[0]. Also protect gvm_calldata.decode from malformed payloads so decode
returns the established internal-error result instead of propagating decoding
exceptions.
- Around line 1200-1226: Ensure terminal_task is cleaned up when run_genvm
exits, including the host-task failure path: initialize it alongside
cancel_task, then in the existing cleanup/finally block cancel it and await it
under BaseException suppression, alongside cancel_task and host_task.

In `@backend/protocol_rpc/endpoints.py`:
- Around line 2138-2139: Update the validation flow around _validate_reroute_to
so reroute_to is accepted only for DEPLOY_CONTRACT transactions. Decode or
otherwise determine the transaction type before validating, then pass it into
_validate_reroute_to or reject sim_config.reroute_to for every other transaction
type, including RUN_CONTRACT.
- Around line 1864-1869: Update the reroute_to validation in the JSONRPC
endpoint to call _EXECUTOR_VERSION_RE.fullmatch(reroute_to) instead of match(),
ensuring trailing whitespace or newlines are rejected while valid executor
version pins remain accepted.

In `@docker/scripts/download_genvm.sh`:
- Line 226: Update the Runner pin confirmation echo to quote the INSTALL_DIR
expansion inside the RUNNER_TAR prefix-removal expression, ensuring it is
treated as a literal prefix rather than a glob pattern and resolving ShellCheck
SC2295.

In `@tests/integration/test_deploy_reroute_to.py`:
- Around line 159-168: Update the rpc_call function signature to add a type
annotation for params and an explicit return type annotation, using types that
accurately represent the JSON-RPC parameters and the value returned by
response.json().get("result").

In `@tests/unit/consensus/test_contract_reroute_to.py`:
- Around line 30-31: Add the return annotation -> None to
test_transaction_reroute_to_without_sim_config,
test_transaction_reroute_to_from_sim_config,
test_deploy_snapshot_carries_reroute_to, and
test_deploy_snapshot_without_reroute_to in
tests/unit/consensus/test_contract_reroute_to.py (lines 30-31, 34-38, 41-46, and
49-53), and to both newly added test methods in
tests/unit/consensus/test_decisions_accepted.py (lines 205-215). No other
behavior changes are needed.

---

Nitpick comments:
In `@backend/node/genvm/base.py`:
- Around line 713-724: Update close_connections to drain cancelled
_connection_tasks under a bounded timeout, ensuring a task that does not
promptly observe cancellation cannot block run_genvm_host cleanup indefinitely.
Preserve logging for completed tasks and cancellation handling, and clear
_connection_tasks after the bounded drain completes.

In `@backend/node/genvm/origin/base_host.py`:
- Around line 34-47: Replace the duplicated ROOT_OFFSET_MAJOR constant with
public_abi._RootOffsets.MAJOR wherever the root-slot major offset is used,
preserving the existing behavior and documentation for UNDEPLOYED_MAJOR. Ensure
the generated public_abi value remains the single source of truth for this
layout offset.

In `@tests/db-sqlalchemy/contract_reroute_to_test.py`:
- Around line 1-63: Rename the test module containing
test_register_contract_persists_reroute_to and related reroute tests from
contract_reroute_to_test.py to test_contract_reroute_to.py so it follows the
required pytest discovery naming convention.

In `@tests/unit/test_genvm_initial_time_units.py`:
- Around line 83-85: Replace the implementation-derived assertion in the genvm
initial-time test with parametrized input and expected values, covering at least
None → 600, 3.25 → 4, and 1200 → 1200. Assert initial_time_units_allocation
against the parametrized expected value so the test independently verifies
run_genvm behavior without duplicating its expression.

In `@tests/unit/test_manager_client_lifecycle.py`:
- Around line 102-104: Rename _NoopHandler.get_contract_major to storage_read
and preserve its async no-op behavior, matching the IHost.storage_read contract
used by read_contract_major. Keep the existing return value and arguments unless
required by that interface.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 04dbe263-5fb6-48bb-b737-02006b4eb1c1

📥 Commits

Reviewing files that changed from the base of the PR and between 4fca293 and 6a2cf8d.

📒 Files selected for processing (31)
  • backend/consensus/base.py
  • backend/consensus/decisions.py
  • backend/database_handler/contract_processor.py
  • backend/database_handler/contract_snapshot.py
  • backend/database_handler/migration/versions/c1d2e3f4a5b6_add_reroute_to_to_current_state.py
  • backend/database_handler/models.py
  • backend/database_handler/snapshot_manager.py
  • backend/domain/types.py
  • backend/node/base.py
  • backend/node/genvm/base.py
  • backend/node/genvm/origin/base_host.py
  • backend/node/genvm/origin/host_fns.py
  • backend/node/genvm/origin/manager_api.py
  • backend/node/genvm/origin/public_abi.py
  • backend/protocol_rpc/endpoints.py
  • docker/scripts/download_genvm.sh
  • tests/db-sqlalchemy/contract_reroute_to_test.py
  • tests/integration/gltest_compat.py
  • tests/integration/test_deploy_reroute_to.py
  • tests/unit/consensus/test_contract_reroute_to.py
  • tests/unit/consensus/test_decisions_accepted.py
  • tests/unit/test_genvm_initial_time_units.py
  • tests/unit/test_genvm_retry_integration.py
  • tests/unit/test_host_loop_fuel_widths.py
  • tests/unit/test_host_nested_connections.py
  • tests/unit/test_manager_client_lifecycle.py
  • tests/unit/test_reroute_to_debug_mode.py
  • tests/unit/test_resolve_callcontract_executor.py
  • tests/unit/test_run_genvm_host_state_copy.py
  • tests/unit/test_run_request_major.py
  • third_party/genvm/version

Comment on lines +503 to +520
@classmethod
def decode(cls, raw: typing.Any) -> "ConsumedResult":
if raw is None:
return cls.internal_error("no_result")
as_bytes = bytes(raw)
decoded = gvm_calldata.decode(as_bytes[1:])
if not isinstance(decoded, dict):
return cls.internal_error("result is not a mapping")
return cls(
execution_hash=decoded.get("execution_hash", b""),
result_kind=public_abi.ResultCode(as_bytes[0]),
result_data=decoded.get("data"),
result_fingerprint=decoded.get("fingerprint"),
result_storage_changes=decoded.get("storage_changes", []),
result_emissions=decoded.get("emissions", []),
result_nondet_results=decoded.get("nondet_results", []),
data_fees_remaining=decoded.get("data_fees_remaining", []),
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

decode crashes on an empty payload despite claiming defensive handling.

raw = b"" (or an empty artifact/list) passes the None guard, then as_bytes[0] raises IndexError out of the terminal-handling path instead of yielding an internal error. Guard on emptiness too, and consider wrapping the calldata decode.

🛡️ Proposed guard
-        if raw is None:
+        if raw is None:
             return cls.internal_error("no_result")
         as_bytes = bytes(raw)
+        if not as_bytes:
+            return cls.internal_error("empty_result")
-        decoded = gvm_calldata.decode(as_bytes[1:])
+        try:
+            decoded = gvm_calldata.decode(as_bytes[1:])
+        except Exception as exc:
+            return cls.internal_error(f"undecodable result: {exc}")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@classmethod
def decode(cls, raw: typing.Any) -> "ConsumedResult":
if raw is None:
return cls.internal_error("no_result")
as_bytes = bytes(raw)
decoded = gvm_calldata.decode(as_bytes[1:])
if not isinstance(decoded, dict):
return cls.internal_error("result is not a mapping")
return cls(
execution_hash=decoded.get("execution_hash", b""),
result_kind=public_abi.ResultCode(as_bytes[0]),
result_data=decoded.get("data"),
result_fingerprint=decoded.get("fingerprint"),
result_storage_changes=decoded.get("storage_changes", []),
result_emissions=decoded.get("emissions", []),
result_nondet_results=decoded.get("nondet_results", []),
data_fees_remaining=decoded.get("data_fees_remaining", []),
)
`@classmethod`
def decode(cls, raw: typing.Any) -> "ConsumedResult":
if raw is None:
return cls.internal_error("no_result")
as_bytes = bytes(raw)
if not as_bytes:
return cls.internal_error("empty_result")
try:
decoded = gvm_calldata.decode(as_bytes[1:])
except Exception as exc:
return cls.internal_error(f"undecodable result: {exc}")
if not isinstance(decoded, dict):
return cls.internal_error("result is not a mapping")
return cls(
execution_hash=decoded.get("execution_hash", b""),
result_kind=public_abi.ResultCode(as_bytes[0]),
result_data=decoded.get("data"),
result_fingerprint=decoded.get("fingerprint"),
result_storage_changes=decoded.get("storage_changes", []),
result_emissions=decoded.get("emissions", []),
result_nondet_results=decoded.get("nondet_results", []),
data_fees_remaining=decoded.get("data_fees_remaining", []),
)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/node/genvm/origin/base_host.py` around lines 503 - 520, Update
ConsumedResult.decode to treat empty raw payloads, including empty bytes-like or
list inputs, as an internal error before indexing as_bytes[0]. Also protect
gvm_calldata.decode from malformed payloads so decode returns the established
internal-error result instead of propagating decoding exceptions.

Comment thread backend/node/genvm/origin/base_host.py
Comment thread backend/protocol_rpc/endpoints.py Outdated
Comment on lines +2138 to +2139
_validate_reroute_to(sim_config)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Confirm whether sim_config.reroute_to is consumed anywhere for RUN_CONTRACT transactions.
rg -n 'reroute_to' backend/consensus backend/node --type=py -C3

Repository: genlayerlabs/genlayer-studio

Length of output: 9708


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== endpoints around reroute validation =="
sed -n '2080,2155p' backend/protocol_rpc/endpoints.py

echo
echo "== consensus validation/deployment helpers =="
sed -n '300,380p' backend/consensus/base.py
sed -n '2800,2860p' backend/consensus/base.py

echo
echo "== all reroute_to/readers refs =="
rg -n 'reroute_to|transaction_reroute_to|reroute_to_for|is_deploy' backend -g '*.py' -C2 | sed -n '1,220p'

Repository: genlayerlabs/genlayer-studio

Length of output: 23814


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== _validate_reroute_to implementation =="
sed -n '1838,1880p' backend/protocol_rpc/endpoints.py

echo
echo "== send_raw_transaction transaction body =="
sed -n '2140,2225p' backend/protocol_rpc/endpoints.py

echo
echo "== RunTransactionEffect / RegisterContractEffect consumers =="
rg -n 'class RunTransactionEffect|class RegisterContractEffect|run_transaction|register_contract|reroute_to' backend -g '*.py' -C3

echo
echo "== deploy contract path exact reroute usage =="
sed -n '380,435p' backend/consensus/decisions.py

Repository: genlayerlabs/genlayer-studio

Length of output: 32068


Reject reroute_to on non-deploy transactions

_validate_reroute_to currently runs before transaction decoding and only checks the runtime/debug-mode/version format. reroute_to is applied only from the contract deploy snapshot/decision; passing it with RUN_CONTRACT will be accepted and stored with the transaction while the value is not used. Pass the decoded transaction type into validation or reject sim_config.reroute_to when the transaction is not DEPLOY_CONTRACT.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/protocol_rpc/endpoints.py` around lines 2138 - 2139, Update the
validation flow around _validate_reroute_to so reroute_to is accepted only for
DEPLOY_CONTRACT transactions. Decode or otherwise determine the transaction type
before validating, then pass it into _validate_reroute_to or reject
sim_config.reroute_to for every other transaction type, including RUN_CONTRACT.

Comment thread docker/scripts/download_genvm.sh Outdated
Comment on lines +159 to +168
def rpc_call(method: str, params):
response = requests.post(
RPC_URL,
json={"jsonrpc": "2.0", "method": method, "params": params, "id": 1},
timeout=30,
)
result = response.json()
if "error" in result:
raise AssertionError(f"RPC error on {method}: {result['error']}")
return result.get("result")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add type hints to rpc_call.

params has no type annotation and the function has no return type hint.

🔧 Proposed fix
-def rpc_call(method: str, params):
+def rpc_call(method: str, params: dict | list) -> dict | list | None:
     response = requests.post(
         RPC_URL,
         json={"jsonrpc": "2.0", "method": method, "params": params, "id": 1},
         timeout=30,
     )
     result = response.json()
     if "error" in result:
         raise AssertionError(f"RPC error on {method}: {result['error']}")
     return result.get("result")

As per coding guidelines, "Include type hints in all Python code".

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def rpc_call(method: str, params):
response = requests.post(
RPC_URL,
json={"jsonrpc": "2.0", "method": method, "params": params, "id": 1},
timeout=30,
)
result = response.json()
if "error" in result:
raise AssertionError(f"RPC error on {method}: {result['error']}")
return result.get("result")
def rpc_call(method: str, params: dict | list) -> dict | list | None:
response = requests.post(
RPC_URL,
json={"jsonrpc": "2.0", "method": method, "params": params, "id": 1},
timeout=30,
)
result = response.json()
if "error" in result:
raise AssertionError(f"RPC error on {method}: {result['error']}")
return result.get("result")
🧰 Tools
🪛 ast-grep (0.45.0)

[warning] 159-163: Request-controlled URL passed to requests; validate against an allowlist to prevent SSRF.
Context: requests.post(
RPC_URL,
json={"jsonrpc": "2.0", "method": method, "params": params, "id": 1},
timeout=30,
)
Note: [CWE-918] Server-Side Request Forgery (SSRF).

(ssrf-requests)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/test_deploy_reroute_to.py` around lines 159 - 168, Update
the rpc_call function signature to add a type annotation for params and an
explicit return type annotation, using types that accurately represent the
JSON-RPC parameters and the value returned by response.json().get("result").

Source: Coding guidelines

Comment on lines +30 to +31
def test_transaction_reroute_to_without_sim_config():
assert transaction_reroute_to(_deploy_transaction(None)) is None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add -> None to all newly added test functions.

  • tests/unit/consensus/test_contract_reroute_to.py#L30-L31: annotate test_transaction_reroute_to_without_sim_config.
  • tests/unit/consensus/test_contract_reroute_to.py#L34-L38: annotate test_transaction_reroute_to_from_sim_config.
  • tests/unit/consensus/test_contract_reroute_to.py#L41-L46: annotate test_deploy_snapshot_carries_reroute_to.
  • tests/unit/consensus/test_contract_reroute_to.py#L49-L53: annotate test_deploy_snapshot_without_reroute_to.
  • tests/unit/consensus/test_decisions_accepted.py#L205-L215: annotate both new test methods.

As per coding guidelines, include type hints in all Python code.

📍 Affects 2 files
  • tests/unit/consensus/test_contract_reroute_to.py#L30-L31 (this comment)
  • tests/unit/consensus/test_contract_reroute_to.py#L34-L38
  • tests/unit/consensus/test_contract_reroute_to.py#L41-L46
  • tests/unit/consensus/test_contract_reroute_to.py#L49-L53
  • tests/unit/consensus/test_decisions_accepted.py#L205-L215
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/consensus/test_contract_reroute_to.py` around lines 30 - 31, Add
the return annotation -> None to test_transaction_reroute_to_without_sim_config,
test_transaction_reroute_to_from_sim_config,
test_deploy_snapshot_carries_reroute_to, and
test_deploy_snapshot_without_reroute_to in
tests/unit/consensus/test_contract_reroute_to.py (lines 30-31, 34-38, 41-46, and
49-53), and to both newly added test methods in
tests/unit/consensus/test_decisions_accepted.py (lines 205-215). No other
behavior changes are needed.

Source: Coding guidelines

Adapts base_host/run_genvm_host to the reworked manager (a persistent
ManagerClient replacing per-run HTTP) and wires resolve_callcontract_executor
so a contract pinned via reroute_to keeps its executor major across nested
calls. Run-start failures now surface as a classified ManagerRunNotStarted,
moving retry policy into the studio host rather than the shared host library.
The genvm runner tree is assembled from fixed-output derivations that
compile C to wasm (genvm-cpython-objs, genvm-ffi, bz2/xz/zlib, numpy,
PIL). Their identity is their output hash, and the nixos/nix build stage
runs with the sandbox off, so building them there picks up host state and
misses the pinned hashes -- which is why source-pinned builds fail. Every
other derivation in the build is input-addressed and unaffected.

Build `runners-all` on the runner instead, where the sandbox works, and
export its store closure into the Docker build context; the source stage
imports it rather than rebuilding it. `sandbox-fallback = false` keeps nix
from quietly falling back to an unsandboxed build and exporting exactly
the outputs this avoids.

Bake defaults to the Git context, which ignores files earlier steps wrote
into the workspace, so source-mode builds now pass `source: .`. Release
builds keep the Git context and are unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant