Port studio to genvm-manager (v0.3)#1697
Conversation
|
This PR targeted I retargeted it to |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughIntroduces GenVM ChangesGenVM Debug Mode, ABI Offsets, and Host Wiring
gl.contract API Migration Across Contracts
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/base.py`:
- Around line 77-85: _align _genvm_debug_mode() with the expected genvm-manager
debug_mode values: the current return values in _genvm_debug_mode() use
"unsafe"/"safe", but the documented and tested modes are "safe-unbounded" and
"disabled". Update the function’s ternary so the true branch returns the
unbounded debug mode and the false branch returns the disabled mode, and make
sure the docstring stays consistent with the values used by the tests and
production behavior._
In `@examples/contracts/tip_jar.py`:
- Around line 8-15: The TipJar constructor is missing the explicit return type
annotation required by the project’s typing convention. Update the __init__
method in TipJar to use a full type-hinted signature like the other methods in
this file and llm_erc20.py, and add the None return annotation without changing
its initialization logic.
In `@tests/integration/icontracts/contracts/company_naming.py`:
- Line 68: The call in company_naming’s contract test uses an invalid SDK
symbol, so revert the rename from run_nondet_default back to the supported gl.vm
API. Update the invocation in the test to use the existing gl.vm.run_nondet or
gl.vm.run_nondet_unsafe symbol that matches the target SDK, and keep the
leader_fn/validator_fn wiring unchanged.
🪄 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
Run ID: 9c496360-2c8b-46a6-be0c-4140dda4327e
📒 Files selected for processing (41)
backend/node/base.pybackend/node/genvm/__init__.pybackend/node/genvm/base.pybackend/node/genvm/origin/base_host.pybackend/node/genvm/origin/public_abi.pybackend/protocol_rpc/fees.pydocker/Dockerfile.backenddocker/Dockerfile.consensus-workerexamples/contracts/_hello_world.pyexamples/contracts/faucet.pyexamples/contracts/football_prediction_market.pyexamples/contracts/llm_erc20.pyexamples/contracts/log_indexer.pyexamples/contracts/storage.pyexamples/contracts/tip_jar.pyexamples/contracts/user_storage.pyexamples/contracts/wizard_of_coin.pytests/common/request.pytests/direct/contracts/error_llm_contract_direct.pytests/direct/contracts/error_web_contract_direct.pytests/direct/storage_read_bench.pytests/integration/icontracts/contracts/company_naming.pytests/integration/icontracts/contracts/error_execution_contract.pytests/integration/icontracts/contracts/error_llm_contract.pytests/integration/icontracts/contracts/error_web_contract.pytests/integration/icontracts/contracts/faucet.pytests/integration/icontracts/contracts/genvm_smoke_v1.pytests/integration/icontracts/contracts/intelligent_oracle.pytests/integration/icontracts/contracts/intelligent_oracle_factory.pytests/integration/icontracts/contracts/multi_file_contract/__init__.pytests/integration/icontracts/contracts/multi_file_contract/other.pytests/integration/icontracts/contracts/multi_read_erc20.pytests/integration/icontracts/contracts/multi_tenant_storage.pytests/integration/icontracts/contracts/payable_escrow.pytests/integration/icontracts/contracts/read_erc20.pytests/integration/icontracts/contracts/utf8_roundtrip_contract.pytests/integration/icontracts/tests/test_utf8_roundtrip_contract.pytests/integration/test_upgrade_contract.pytests/load/contracts/counter.pytests/test_linter_endpoint.pytests/unit/test_genvm_debug_mode_gate.py
| def _genvm_debug_mode() -> str: | ||
| """genvm-manager `debug_mode` level for the run request. | ||
|
|
||
| `unsafe` (dev/stg default) captures unbounded output and enables the | ||
| `:latest`/`:test` runner aliases that studio's bundled contracts depend | ||
| on; only `unsafe`/`unsafe-tracing` resolve those floating aliases. Prd | ||
| sets GENVM_DEBUG_MODE=false -> `disabled` so the aliases fail fast. | ||
| """ | ||
| return ["--debug-mode"] if _env_bool("GENVM_DEBUG_MODE", default=True) else [] | ||
| return "unsafe" if _env_bool("GENVM_DEBUG_MODE", default=True) else "safe" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Align _genvm_debug_mode() with the tested manager modes.
Line 85 returns "unsafe"/"safe", but the new unit tests and the docstring expect "safe-unbounded"/"disabled". With GENVM_DEBUG_MODE=false, production would still send "safe" instead of disabling debug mode.
🐛 Proposed fix
- return "unsafe" if _env_bool("GENVM_DEBUG_MODE", default=True) else "safe"
+ return (
+ "safe-unbounded"
+ if _env_bool("GENVM_DEBUG_MODE", default=True)
+ else "disabled"
+ )📝 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.
| def _genvm_debug_mode() -> str: | |
| """genvm-manager `debug_mode` level for the run request. | |
| `unsafe` (dev/stg default) captures unbounded output and enables the | |
| `:latest`/`:test` runner aliases that studio's bundled contracts depend | |
| on; only `unsafe`/`unsafe-tracing` resolve those floating aliases. Prd | |
| sets GENVM_DEBUG_MODE=false -> `disabled` so the aliases fail fast. | |
| """ | |
| return ["--debug-mode"] if _env_bool("GENVM_DEBUG_MODE", default=True) else [] | |
| return "unsafe" if _env_bool("GENVM_DEBUG_MODE", default=True) else "safe" | |
| def _genvm_debug_mode() -> str: | |
| """genvm-manager `debug_mode` level for the run request. | |
| `unsafe` (dev/stg default) captures unbounded output and enables the | |
| `:latest`/`:test` runner aliases that studio's bundled contracts depend | |
| on; only `unsafe`/`unsafe-tracing` resolve those floating aliases. Prd | |
| sets GENVM_DEBUG_MODE=false -> `disabled` so the aliases fail fast. | |
| """ | |
| return ( | |
| "safe-unbounded" | |
| if _env_bool("GENVM_DEBUG_MODE", default=True) | |
| else "disabled" | |
| ) |
🤖 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/base.py` around lines 77 - 85, _align _genvm_debug_mode() with
the expected genvm-manager debug_mode values: the current return values in
_genvm_debug_mode() use "unsafe"/"safe", but the documented and tested modes are
"safe-unbounded" and "disabled". Update the function’s ternary so the true
branch returns the unbounded debug mode and the false branch returns the
disabled mode, and make sure the docstring stays consistent with the values used
by the tests and production behavior._
| class TipJar(gl.contract.Contract): | ||
| owner: Address | ||
| total_tips: u256 | ||
|
|
||
| def __init__(self): | ||
| self.owner = gl.message.sender_address | ||
| self.total_tips = u256(0) | ||
| self.total_tips = 0 | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Missing return type hint on __init__.
Every other method in this file (and llm_erc20.py's __init__) has a full type-hint signature; this __init__ lacks -> None.
🔧 Proposed fix
- def __init__(self):
+ def __init__(self) -> None:
self.owner = gl.message.sender_address
self.total_tips = 0As 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.
| class TipJar(gl.contract.Contract): | |
| owner: Address | |
| total_tips: u256 | |
| def __init__(self): | |
| self.owner = gl.message.sender_address | |
| self.total_tips = u256(0) | |
| self.total_tips = 0 | |
| class TipJar(gl.contract.Contract): | |
| owner: Address | |
| total_tips: u256 | |
| def __init__(self) -> None: | |
| self.owner = gl.message.sender_address | |
| self.total_tips = 0 |
🤖 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 `@examples/contracts/tip_jar.py` around lines 8 - 15, The TipJar constructor is
missing the explicit return type annotation required by the project’s typing
convention. Update the __init__ method in TipJar to use a full type-hinted
signature like the other methods in this file and llm_erc20.py, and add the None
return annotation without changing its initialization logic.
Source: Coding guidelines
| ) | ||
|
|
||
| analysis = gl.vm.run_nondet(leader_fn, validator_fn) | ||
| analysis = gl.vm.run_nondet_default(leader_fn, validator_fn) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
genlayer SDK v0.3 changelog run_nondet_default gl.vm
💡 Result:
In the GenLayer SDK, there is no documented function named run_nondet_default. The GenVM SDK documentation and API references consistently define two primary functions for executing non-deterministic blocks within the gl.vm module: run_nondet and run_nondet_unsafe [1][2]. Regarding the GenLayer SDK v0.3 changelog, the public record identifies v0.3.0 as having been released on June 4, 2025 [3]. A key feature noted in the v0.3.0 changelog is the ability to decode new leader receipt data [3]. The gl.vm module acts as the core interface for interacting with the GenVM execution environment [4][5][6]. It provides the mechanisms to manage consensus-critical non-deterministic tasks [7][1]. Developers typically use run_nondet_unsafe to execute custom leader/validator logic, which allows for full control over validator error handling and classification, whereas run_nondet is primarily used by built-in convenience functions to automatically handle and compare errors during consensus [1].
Citations:
- 1: https://docs.genlayer.com/developers/intelligent-contracts/equivalence-principle
- 2: https://sdk.genlayer.com/v0.2.14/api/index.html
- 3: https://github.com/genlayerlabs/genlayer-py/blob/main/CHANGELOG.md
- 4: https://sdk.genlayer.com/main/python-sdk/index.html
- 5: https://sdk.genlayer.com/main/api/index.html
- 6: https://sdk.genlayer.com/main/api/genlayer.html
- 7: https://docs.genlayer.com/developers/intelligent-contracts/features/non-determinism
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Repo files mentioning run_nondet variants:\n'
rg -n --hidden --glob '!**/.git/**' 'run_nondet(_default|_unsafe)?' .
printf '\nRelevant file excerpt:\n'
sed -n '1,140p' tests/integration/icontracts/contracts/company_naming.pyRepository: genlayerlabs/genlayer-studio
Length of output: 1942
gl.vm.run_nondet_default is not a valid SDK symbol
gl.vm exposes run_nondet and run_nondet_unsafe; this rename should be reverted unless the target SDK added a new alias elsewhere. tests/integration/icontracts/contracts/company_naming.py:68
🤖 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/icontracts/contracts/company_naming.py` at line 68, The
call in company_naming’s contract test uses an invalid SDK symbol, so revert the
rename from run_nondet_default back to the supported gl.vm API. Update the
invocation in the test to use the existing gl.vm.run_nondet or
gl.vm.run_nondet_unsafe symbol that matches the target SDK, and keep the
leader_fn/validator_fn wiring unchanged.
c0af4cd to
145cb7c
Compare
|
|
||
|
|
||
| @allow | ||
| @gl.allow |
There was a problem hiding this comment.
should be @gl.storage.allow
|
Re: code slot / v0.2↔v0.3 interop — read your notes, here's the decision. First: yes, deployed contracts on hosted studio (v0.2.16) have to survive the upgrade. That's the constraint everything below follows from. Your per-contract metadata suggestion is the right call, and I'd make it explicit rather than inferred from storage. Sniffing the layout can't work: Concretely:
Since v0.3 storage self-describes via the Also 👍 on deriving the slot from |
c380c29 to
4f9f164
Compare
4f9f164 to
3697c6a
Compare
|
Note on This test and the rest of When run manually with |
|


Depends-On: genlayerlabs/genvm-manager#1
Depends-On: https://github.com/genlayerlabs/genlayer-node/pull/1502
Depends-On: https://github.com/genlayerlabs/genlayer-dev-env/pull/95
Depends-On: https://github.com/genlayerlabs/genlayer-e2e/pull/619
Depends-On: #1697
Depends-On: genlayerlabs/genlayer-py#96
Depends-On: genlayerlabs/genlayer-js#192
Depends-On: genlayerlabs/genlayer-testing-suite#98
Ports studio to genvm-manager (v0.3).
NOTE: I did not touch linters, gltest and other stuff
Compatibility with old stuff
"method"key became""so that it's value is prefix of binary calldataa. v0.2.16 has a hard remaping for exposing to contracts "method" and giving back only
""b. currently manager if sees
"method"in input calldata will remap it to""automatically and print an errorc. migration of genlayer-js and genlayer-py is needed! migration in node and studio is more or less handled, I believe
a. you may be interested in https://github.com/genlayerlabs/genvm-manager/blob/02741163aa89b10331f1585d9ea42868a816945f/docs/website/src/impl-spec/appendix/manager-api.yaml#L256
b. maybe for real db we can add a field for each contract alongisde storage or whatever
Other notes
there will be hash breaking commit to v0.3 with changing runners for allocating from balance
I have not yet supported builds releasesd and other stuff, as we are not merged into core and not even near that. For that reason I uploaded a hardcoded x86_64 artifact for you builded form commit for convenience
I ran some tests, deployed UI and verified basic functionality working. However, more complicated stuff may not work, it is a draft of changes