fix: disable 'latest' and 'test' runner versions with friendly error message#1418
Open
cristiam86 wants to merge 12 commits intomainfrom
Open
fix: disable 'latest' and 'test' runner versions with friendly error message#1418cristiam86 wants to merge 12 commits intomainfrom
cristiam86 wants to merge 12 commits intomainfrom
Conversation
Replace "latest" with fixed version hash to prevent unexpected breaking changes during GenVM upgrades. Resolves DXP-740 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace `return "Already resolved"` with `raise gl.vm.UserError("Already resolved")`
for proper error handling in football_prediction_market contract.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR introduces runtime validation to prevent deployment of contracts with invalid runner versions (floating tags "latest"/"test"), adds user-friendly error messaging infrastructure for GenVM errors, updates GenVM initialization logic, and pins test contract dependencies from floating tags to specific version hashes for retrocompatibility testing. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/node/genvm/base.py`:
- Around line 184-195: The code directly indexes result_decoded["message"] which
can raise KeyError; change to safely obtain the original message (e.g., use
result_decoded.get("message", fallback) or derive a fallback like
str(result_decoded) or raw_error) before calling get_user_friendly_message and
constructing ExecutionError; update the references around result_decoded,
original_message, get_user_friendly_message, and ExecutionError so
original_message is always a string (not None) and raw_error is used as a
secondary fallback to avoid exceptions.
🧹 Nitpick comments (1)
.claude/skills/linear-issue-fixer/SKILL.md (1)
45-61: Consider adding language identifiers to fenced code blocks.Several code blocks in this documentation lack language specifiers (e.g., lines 45-61, 84-92, 98-110, etc.). While this doesn't affect functionality, adding identifiers like
textorplaintextimproves markdown linting compliance and may enable syntax highlighting in some viewers.
Comment on lines
+184
to
195
| # Get user-friendly message if available | ||
| original_message = result_decoded["message"] | ||
| friendly_message = get_user_friendly_message( | ||
| error_code, original_message, raw_error | ||
| ) | ||
|
|
||
| result = ExecutionError( | ||
| result_decoded["message"], | ||
| friendly_message, | ||
| res.result_kind, | ||
| error_code=error_code, | ||
| raw_error=raw_error if raw_error else None, | ||
| ) |
Contributor
There was a problem hiding this comment.
Potential KeyError if dict error lacks "message" key.
Line 185 directly accesses result_decoded["message"] without checking if the key exists. If GenVM returns a dict error without a "message" key, this will raise a KeyError.
🛡️ Proposed defensive fix
# Get user-friendly message if available
- original_message = result_decoded["message"]
+ original_message = result_decoded.get("message", str(result_decoded))
friendly_message = get_user_friendly_message(
error_code, original_message, raw_error
)🤖 Prompt for AI Agents
In `@backend/node/genvm/base.py` around lines 184 - 195, The code directly indexes
result_decoded["message"] which can raise KeyError; change to safely obtain the
original message (e.g., use result_decoded.get("message", fallback) or derive a
fallback like str(result_decoded) or raw_error) before calling
get_user_friendly_message and constructing ExecutionError; update the references
around result_decoded, original_message, get_user_friendly_message, and
ExecutionError so original_message is always a string (not None) and raw_error
is used as a secondary fallback to avoid exceptions.
- Remove --debug-mode flag only for contract deployments (is_init=True) - Keep --debug-mode for running existing contracts and getting schemas - Add INVALID_RUNNER error code for contracts using disallowed versions - Add user-friendly error messages for common GenVM errors When deploying a contract with py-genlayer:latest or py-genlayer:test, users now see a clear message: "Invalid runner version. The 'latest' and 'test' versions are not allowed. Please use a fixed version hash in your contract header." Resolves DXP-742 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
cristiam86
force-pushed
the
dxp-742-disable-latest-version
branch
from
ff27889 to
be1414e
Compare
Add custom linting rule INVALID_RUNNER_VERSION to detect and report when contracts use 'py-genlayer:latest' or 'py-genlayer:test' in their Depends header. This provides early feedback in the editor before deployment fails. - Add _check_invalid_runner_version method to ContractLinter - Include custom rule results in lint_contract output - Update test_linter_endpoint.py with invalid version test case - Add comprehensive unit tests for the new linting rule Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Reject contract upgrades that use 'latest' or 'test' versions in the Depends header, matching the validation added for new deployments. - Add regex check in admin_upgrade_contract_code - Return clear error message with example of valid version format - Add unit tests for the validation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/protocol_rpc/endpoints.py`:
- Around line 600-616: The current regex-based check using re.search on new_code
(creating invalid_version_match) is too brittle; instead parse the contract
header JSON blocks and inspect the "Depends" field to detect disallowed runner
versions. Locate the logic around invalid_version_match and JSONRPCError and
replace the fragile re.search approach with a JSON-parse of header blocks:
extract header JSON objects from new_code, read Depends (handling extra
fields/whitespace and multiple headers), and if Depends indicates
"py-genlayer:latest" or "py-genlayer:test" raise the same JSONRPCError
(including the invalid_version in data) to reject the submission.
Extract the validation logic to a shared function in contract_linter.py so that both the linter and contract upgrade endpoint use the same code. This ensures that changes to the validation are properly tested. - Add check_invalid_runner_version() shared function - Add INVALID_VERSION_ERROR_MESSAGE constant - Update endpoints.py to import and use shared validation - Update tests to use the shared function directly Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update all test contracts from 'latest' and 'test' versions to use the fixed version hash 1jb45aa8ynh2a9c9xn3b7qqh8sm5q93hwfp7jqmwsfhh8jpz09h6. Add Storage contracts for each historical py-genlayer version to enable retrocompatibility testing: - v0.1.0: 15qfivjvy80800rh998pcxmd2m8va1wq2qzqhz850n8ggcr4i9q0 - v0.1.3: 1j12s63yfjpva9ik2xgnffgrs6v44y1f52jvj9w7xvdn7qckd379 - v0.1.8: 132536jbnxkd1axfxg5rpfr5b60cr11adm2y4r90hgn0l59qsp9w - v0.2.1: 1jb45aa8ynh2a9c9xn3b7qqh8sm5q93hwfp7jqmwsfhh8jpz09h6 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/integration/icontracts/contracts/storage_versions/storage_v0_2_1.py`:
- Line 4: Replace the star import to satisfy Ruff F403/F405 by importing the
genlayer module explicitly and preserving existing `gl` references; change `from
genlayer import *` to `import genlayer as gl` (so existing usages like
`gl.Contract` and `gl.public` continue to work). Apply the same replacement
across all contract files in tests/integration/icontracts/contracts/ that
currently use `from genlayer import *`.
| # v0.2.1 | ||
| # { "Depends": "py-genlayer:1jb45aa8ynh2a9c9xn3b7qqh8sm5q93hwfp7jqmwsfhh8jpz09h6" } | ||
|
|
||
| from genlayer import * |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check Ruff configuration and genlayer import patterns
echo "=== Checking Ruff configuration files ==="
rg -n "ruff|per-file-ignores|extend-exclude|exclude" pyproject.toml setup.cfg .ruff.toml 2>/dev/null || echo "No Ruff config found or pattern not matched"
echo ""
echo "=== Checking genlayer imports in contracts directory ==="
rg -n "from genlayer import" tests/integration/icontracts/contractsRepository: genlayerlabs/genlayer-studio
Length of output: 1576
🏁 Script executed:
#!/bin/bash
# Check what's actually available in genlayer and how it's used
echo "=== Check genlayer module location ==="
fd "genlayer" --type d
echo ""
echo "=== Check for genlayer __init__.py or module ==="
find . -name "__init__.py" -o -name "genlayer.py" | grep -i genlayer
echo ""
echo "=== Sample usage in contract files ==="
head -20 tests/integration/icontracts/contracts/storage_versions/storage_v0_2_1.py
echo ""
echo "=== Check for explicit 'gl' usage ==="
rg "from genlayer import \*" -A 5 tests/integration/icontracts/contracts/storage_versions/storage_v0_2_1.pyRepository: genlayerlabs/genlayer-studio
Length of output: 756
🏁 Script executed:
#!/bin/bash
# Check Ruff configuration more comprehensively and check for per-file ignores
echo "=== List all config files ==="
ls -la | grep -E "\.(ruff|toml|cfg)$|pyproject"
echo ""
echo "=== Check pyproject.toml for any tool section ==="
cat pyproject.toml 2>/dev/null | head -100
echo ""
echo "=== Check if there's a pre-commit config ==="
cat .pre-commit-config.yaml 2>/dev/null | grep -A 20 ruff
echo ""
echo "=== Check if contracts are in any ignore/exclude patterns ==="
rg "contracts|integration" pyproject.toml .pre-commit-config.yamlRepository: genlayerlabs/genlayer-studio
Length of output: 514
🏁 Script executed:
#!/bin/bash
# Check the ruff.toml configuration
echo "=== Contents of ruff.toml ==="
cat ruff.toml
echo ""
echo "=== Check if contracts directory is excluded ==="
rg "contracts|integration" ruff.tomlRepository: genlayerlabs/genlayer-studio
Length of output: 1058
🏁 Script executed:
#!/bin/bash
# Verify what needs to be imported by checking all usages in contract files
echo "=== Check actual usage patterns in contracts ==="
rg "gl\." tests/integration/icontracts/contracts/ | head -30
echo ""
echo "=== Verify what gl is exported from genlayer ==="
rg "^gl = " tests/integration/icontracts/contracts/ || echo "No local gl definition found"
echo ""
echo "=== Check if any contract tries direct imports instead of star ==="
rg "from genlayer import" tests/integration/icontracts/contracts/ | grep -v "\*"Repository: genlayerlabs/genlayer-studio
Length of output: 3298
Remove star import to resolve Ruff F403/F405 violations.
The file uses from genlayer import * but only references gl (e.g., gl.Contract, gl.public). Ruff is configured to enforce Pyflakes rules (F), which flag star imports. Replace with explicit import:
-from genlayer import *
+from genlayer import glThis pattern applies to all contract files in tests/integration/icontracts/contracts/ (15+ files).
📝 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
| from genlayer import * | |
| from genlayer import gl |
🧰 Tools
🪛 Ruff (0.14.14)
[error] 4-4: from genlayer import * used; unable to detect undefined names
(F403)
🤖 Prompt for AI Agents
In `@tests/integration/icontracts/contracts/storage_versions/storage_v0_2_1.py` at
line 4, Replace the star import to satisfy Ruff F403/F405 by importing the
genlayer module explicitly and preserving existing `gl` references; change `from
genlayer import *` to `import genlayer as gl` (so existing usages like
`gl.Contract` and `gl.public` continue to work). Apply the same replacement
across all contract files in tests/integration/icontracts/contracts/ that
currently use `from genlayer import *`.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/integration/icontracts/contracts/storage_versions/storage_v0_2_5.py`:
- Around line 1-2: The storage version files v0.2.1, v0.2.4 and v0.2.5 all use
the identical dependency hash for py-genlayer which looks like a copy/paste
mistake; open the files for versions v0.2.1, v0.2.4 and v0.2.5 and either
replace the duplicated hash with the correct, unique dependency hashes for each
version (if different runners were used) or, if they truly share the same
runner, add a short clarifying comment in each file indicating the shared
dependency is intentional; ensure you update the dependency string on the top of
storage_v0_2_5.py (and the corresponding v0.2.1 and v0.2.4 files) to reflect the
correct hashes or the explicit documentation of shared usage.
Comment on lines
+1
to
+2
| # v0.2.5 | ||
| # { "Depends": "py-genlayer:1jb45aa8ynh2a9c9xn3b7qqh8sm5q93hwfp7jqmwsfhh8jpz09h6" } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd 'storage_v.*\.py$' tests/integration/icontracts/contracts/storage_versions/ --exec head -3 {}Repository: genlayerlabs/genlayer-studio
Length of output: 635
Multiple storage versions share the same dependency hash—likely a copy-paste error.
v0.2.1, v0.2.4, and v0.2.5 all declare py-genlayer:1jb45aa8ynh2a9c9xn3b7qqh8sm5q93hwfp7jqmwsfhh8jpz09h6, while v0.1.0, v0.1.3, and v0.1.8 each have distinct hashes. These version files should either have unique hashes reflecting their different versions, or if they genuinely share the same runner dependency, that should be explicitly documented. Verify and correct if unintentional.
🤖 Prompt for AI Agents
In `@tests/integration/icontracts/contracts/storage_versions/storage_v0_2_5.py`
around lines 1 - 2, The storage version files v0.2.1, v0.2.4 and v0.2.5 all use
the identical dependency hash for py-genlayer which looks like a copy/paste
mistake; open the files for versions v0.2.1, v0.2.4 and v0.2.5 and either
replace the duplicated hash with the correct, unique dependency hashes for each
version (if different runners were used) or, if they truly share the same
runner, add a short clarifying comment in each file indicating the shared
dependency is intentional; ensure you update the dependency string on the top of
storage_v0_2_5.py (and the corresponding v0.2.1 and v0.2.4 files) to reflect the
correct hashes or the explicit documentation of shared usage.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/node/genvm/error_codes.py (1)
93-110: Good coverage of all error codes.All
GenVMErrorCodemembers have corresponding user-friendly messages. The type annotation could be more precise for better static analysis and IDE support.🔧 Optional: Stronger typing for ERROR_CODE_MESSAGES
# User-friendly error messages for each error code -ERROR_CODE_MESSAGES: dict[str, str] = { +ERROR_CODE_MESSAGES: dict[GenVMErrorCode, str] = { GenVMErrorCode.INVALID_RUNNER: (This change would provide better IDE autocomplete and catch typos at type-check time.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/node/genvm/error_codes.py` around lines 93 - 110, ERROR_CODE_MESSAGES is currently typed as dict[str, str]; change its annotation to use the GenVMErrorCode enum as the key type (e.g., dict[GenVMErrorCode, str] or Mapping[GenVMErrorCode, str]) so IDEs and type checkers can validate keys and provide autocompletion for GenVMErrorCode members; update the type import if necessary and keep the same mapping values, referencing ERROR_CODE_MESSAGES and GenVMErrorCode to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/node/genvm/error_codes.py`:
- Around line 93-110: ERROR_CODE_MESSAGES is currently typed as dict[str, str];
change its annotation to use the GenVMErrorCode enum as the key type (e.g.,
dict[GenVMErrorCode, str] or Mapping[GenVMErrorCode, str]) so IDEs and type
checkers can validate keys and provide autocompletion for GenVMErrorCode
members; update the type import if necessary and keep the same mapping values,
referencing ERROR_CODE_MESSAGES and GenVMErrorCode to locate the change.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.claude/skills/linear-issue-fixer/SKILL.mdbackend/node/base.pybackend/node/genvm/base.pybackend/node/genvm/error_codes.pybackend/protocol_rpc/endpoints.pytests/integration/icontracts/contracts/intelligent_oracle.py
🚧 Files skipped from review as they are similar to previous changes (4)
- .claude/skills/linear-issue-fixer/SKILL.md
- backend/node/genvm/base.py
- backend/protocol_rpc/endpoints.py
- backend/node/base.py
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.
Summary
--debug-modeflag only for contract deployments (enforces version restrictions)--debug-modefor running existing contracts and getting schemas (allows flexibility)INVALID_RUNNERerror code for contracts using disallowed versionsWhat Changed
When deploying a new contract, GenVM now rejects contracts that use
py-genlayer:latestorpy-genlayer:testversions. Running existing contracts and getting schemas still work with any version.Users see a clear, actionable message when deploying with invalid versions:
Behavior
--debug-modelatest/testTest Plan
docker compose -f tests/db-sqlalchemy/docker-compose.yml run tests)py-genlayer:latestheader and verify the friendly error is shownFiles Changed
backend/node/base.py: Conditionally remove--debug-modeonly for deploymentsbackend/node/genvm/error_codes.py: AddINVALID_RUNNERerror code and user-friendly message mappingbackend/node/genvm/base.py: Useget_user_friendly_message()when creatingExecutionErrorFixes https://linear.app/genlayer-labs/issue/DXP-742/disable-latest-after-migrating-all-examples-and-docs
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Documentation