Skip to content

feat: Planning parameter update system - #637

Open
claycuy wants to merge 1 commit into
mainfrom
feat/parameter
Open

claycuy wants to merge 1 commit into
mainfrom
feat/parameter

Conversation

@claycuy

@claycuy claycuy commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator

What did you change?

Change type

  • Fix (Bug/Patch)
  • Feature (New Feature)
  • Refactor (Code Polish)
  • Docs (Documentation)
  • Chore (Build/Maintenance)

Checklist

  • I have done tests on this change
  • The code is in accordance with the project style guide.
  • I have updated the documentation if necessary.

Link Issue (if any)

Summary by CodeRabbit

  • Documentation
    • Added an internal TODO comment; there are no user-facing changes.

@vercel

vercel Bot commented Sep 21, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
lightvm Building Building Preview Sep 21, 2026 1:18am UTC

@github-actions github-actions Bot added the enhancement New feature or request label Sep 21, 2026
@coderabbitai

coderabbitai Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: soteenstudio/lightvm/.coderabbit.yml

Review profile: CHILL

Plan: Advanced

Run ID: 11c90090-08c8-4c0a-8e59-8c5a4dde7a4d

📥 Commits

Reviewing files that changed from the base of the PR and between 5dc4168 and fd25261.

📒 Files selected for processing (1)
  • rust/src/lib.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

A TODO comment was added to rust/src/lib.rs after the license header. No functional or public API changes were made.

Changes

Comment Update

Layer / File(s) Summary
TODO comment annotation
rust/src/lib.rs
Added // TODO: Added params after the license header. Code behavior is unchanged.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~2 minutes

Change: Other

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title claims that the pull request adds a planning parameter update system, but the changeset only adds the comment // TODO: Added params. The title does not accurately describe the implemented … Update the title to describe the comment-only change, or add the planned parameter update system implementation so the current title matches the changeset.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
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.
Full details: Title check

Explanation

The title claims that the pull request adds a planning parameter update system, but the changeset only adds the comment // TODO: Added params. The title does not accurately describe the implemented change.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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

@claycuy

claycuy commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator Author

@coderabbitai Would it be appropriate if this VM had a parameter feature inspired by the Android parameter system where the parameters can be updated without the user having to download a new version?

@coderabbitai

coderabbitai Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

ast-grep outline rust/src/lib.rs --items all || true

printf '\n--- Rust source files ---\n'
fd -t f -e rs . rust | sort

printf '\n--- Relevant declarations and call sites ---\n'
rg -n -i -C 3 '\b(param(eter)?s?|config(uration)?|governance|upgrade|version|state|consensus|fee|limit|gas)\b' rust -g '*.rs' || true

printf '\n--- Package manifests ---\n'
fd -t f -a 'Cargo.toml' . -x sh -c 'echo "--- $1"; sed -n "1,220p" "$1"' sh {} \; || true

Length of output: 201010


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- lib.rs ---'
sed -n '1,260p' rust/src/lib.rs

printf '%s\n' '--- root modules ---'
find rust/src -maxdepth 2 -type f -name '*.rs' | sort | head -n 80

printf '%s\n' '--- network and cryptographic use (excluding license headers) ---'
rg -n -i --glob '*.rs' --glob '!**/*in.rs' -C 4 \
  '\b(ureq|https?::|get\(|post\(|ed25519|verifyingkey|signature|verify\(|public.?key|remote|download|update|manifest|parameter|config)\b' \
  rust/src || true

printf '%s\n' '--- VM types and constructors ---'
rg -n --glob '*.rs' -C 3 \
  '^(pub )?(struct|enum|trait)\s+(VM|Vm|VirtualMachine|Config|Settings|Runtime)|^(pub )?fn\s+(new|run|execute|eval|interpret)\b|impl\s+(VM|Vm|VirtualMachine)' \
  rust/src || true

printf '%s\n' '--- Limits and capacities outside generated instruction files ---'
rg -n --glob '*.rs' --glob '!**/*in.rs' -C 3 \
  '\b(stack|heap|memory|instruction|recursion|capacity|max(imum)?|limit)\b' \
  rust/src | head -n 800 || true

Length of output: 135592


@claycuy Yes, this can be appropriate for LightVM.

Use it for application parameters, not for unrestricted VM changes. The VM should accept a versioned parameter bundle that applications can update without replacing the VM binary.

Do not use remote parameters to change opcode behavior, capability checks, or security limits by default. SecurityConfig controls execution limits such as ticks, stack size, imports, and allocations. A remote change to these limits can weaken the VM security boundary.

A safe design should include:

  • A fixed parameter schema in the VM version.
  • A signed parameter bundle.
  • An embedded public key or an application-supplied trust root.
  • A bundle ID, version, issue time, and expiry time.
  • Monotonic versions to reject rollback and replay.
  • Strict schema and range validation.
  • Atomic application between VM runs, never during execution.
  • A local cached last-known-good bundle.
  • Explicit fallback behavior when the bundle is missing, expired, or invalid.

The current Node binary-integrity code already uses Ed25519 verification in rust/src/lib.rs. The same verification model is a suitable base for signed parameter bundles.

I recommend two separate classes:

  1. Planning parameters: feature flags, defaults, tuning values, and application policy. These can update remotely.
  2. VM security policy: capabilities, SecurityConfig, and execution semantics. These should remain local and require explicit host approval.

The system cannot add new opcodes or alter existing opcode semantics without a VM update. It can only update values that the installed VM version already understands.

You are interacting with an AI system.

@claycuy

claycuy commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator Author

@coderabbitai Is it suitable to write the update parameter in the lightvm-parameter repo with Lua and then read it with mlua in this repo?

@coderabbitai

coderabbitai Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- Cargo manifests ---'
fd -t f -a '^Cargo\.toml$' . -x sh -c 'echo "--- $1"; rg -n -i -C 3 "\bmlua\b|\blua\b" "$1" || true' sh {} \;

printf '%s\n' '--- Rust files mentioning mlua or Lua ---'
rg -n -i -C 5 '\b(mlua|lua|Lua::new|StdLib|sandbox|set_memory_limit|set_hook|create_table|from_value)\b' rust -g '*.rs' || true

printf '%s\n' '--- Existing parameter/config/security declarations ---'
rg -n -i -C 4 '\b(parameter|config(uration)?|securityconfig|security|verify(ing)?|ed25519|signature|public.?key)\b' rust/src -g '*.rs' || true

Length of output: 76688


@claycuy The approach is possible, but mlua is not currently a dependency in this repository. Adding it would introduce a Lua interpreter and a new remote-code parsing boundary.

For planning parameters, I recommend a data format first: JSON, TOML, or CBOR with serde. The current code already uses serde_json at the native, Node, and WASM interfaces. This option has less implementation and security risk.

Use Lua only if lightvm-parameter needs Lua-specific authoring features. If you use Lua:

  • Sign the exact Lua source or a signed bundle that contains it.
  • Verify the signature before calling mlua.
  • Use a dedicated parameter-only mlua state.
  • Load a fixed return value, such as return { schema_version = 1, ... }.
  • Convert the returned table to a typed Rust PlanningParameters struct.
  • Reject functions, userdata, metatables, unknown keys, and values outside allowed ranges.
  • Enforce a strict memory limit and instruction limit for Lua evaluation.
  • Do not expose filesystem, process, network, module loading, or host callbacks.
  • Do not let the Lua result update SecurityConfig, capabilities, allowed imports, unsafe_mode, opcode behavior, or trust keys.

This restriction is important because SecurityConfig controls execution limits and import authorization in the current VM. Remote planning parameters must remain separate from that security policy.

A safe Lua parameter file can be limited to this shape:

return {
  schema_version = 1,
  bundle_version = 42,
  planning = {
    max_steps = 1000,
    feature_x_enabled = true,
  },
}

So: yes, it is suitable if Lua is only a signed, sandboxed, data-only parameter format. Otherwise, prefer a serde-supported data format.

You are interacting with an AI system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant