Skip to content

Use PeTTa as interpreter#3

Open
rmgaray wants to merge 3 commits into
milestone3from
rmgaray/execute-with-petta
Open

Use PeTTa as interpreter#3
rmgaray wants to merge 3 commits into
milestone3from
rmgaray/execute-with-petta

Conversation

@rmgaray
Copy link
Copy Markdown
Collaborator

@rmgaray rmgaray commented May 1, 2026

This PR adds execution of MeTTa programs using swipl and PeTTa. The output of the execution of MeTTa programs is captured by f1r3fly, parsed as JSON and then passed to the Rho program as a native data structure.

Example:

$ cargo run --bin rholang-cli -- --quiet rholang/examples/system-contract/swipl/01-compile-metta.rho
{"results" : [["Pair", 3, 1], ["Pair", "y", "x"]]}
Estimated deploy cost: Cost { value: 1269, operation: "subtraction" }

@rmgaray rmgaray requested a review from nalane May 1, 2026 16:49
@nalane nalane requested a review from nrutledge May 1, 2026 17:59
@rmgaray rmgaray closed this May 7, 2026
@rmgaray rmgaray reopened this May 7, 2026
@rmgaray
Copy link
Copy Markdown
Collaborator Author

rmgaray commented May 7, 2026

My bad - I closed this PR by accident.

@nrutledge
Copy link
Copy Markdown
Collaborator

@rmgaray I am still in the process of coming up to speed on everything. However, I have a moderate understanding now and was able to give this a review, relying heavily on Claude and Codex.

The following is everything flagged. I haven't been able to verify everything fully yet but it does seem like we need to at least address the non determinism, security concerns (even if just gating behind some clearly dangerous feature flag), and documentation/test gaps.


PR #3 Review — "Use PeTTa as interpreter" (commit 1670a734)

Summary

The PR swaps rho:petta:compilerho:petta:execute, has SWI Prolog actually run the PeTTa interpreter (via swipl subprocess), and parses results back into Rholang via a JSON bridge. The shape of the change matches the revised milestone scope (run PeTTa as the interpreter; drop the standalone rho:prolog:evaluate service).

Files touched (6):

M3-vs-M4 scope and downstream branches

This review evaluates 1670a734 against the M3 acceptance criteria. Some concerns it raises are explicitly M4 territory and should not block M3, and some are already addressed in commits in the follow-up branches but that aren't yet in this PR. The relevant context:

  • M4 owns: a per-execution working directory on disk (named by hash of the script, holds script + continuation files, deleted on termination); the petta_lib_snapshot injection (interrupt-and-resume); and replacing every impure MeTTa function with a {channel, arguments} JSON-on-stdout call that the host routes to a Rholang channel. The NamedTempFile in this PR is the M3 stepping stone toward that directory model.
  • On nearby follow-up branches (not in 1670a734):

Each issue below is tagged with one of:

  • [M3] — needs to be addressed in this PR (or before merging M3).
  • [M4 supersedes] — current approach is transitional; M4 will rewrite this. Don't over-engineer it in M3.

Ranked issues

Critical / Blocking

  1. [M3] Consensus break: SWIPL_EXECUTE_PETTA is missing from non_deterministic_ops() (system_processes.rs:261-271).
    The handler at system_processes.rs:1814-1817 does if is_replay { produce(&previous_output, ack).await?; return Ok(previous_output); }, but previous_output is only persisted for body refs that the dispatcher classifies as non-deterministic (dispatch.rs:73,109-117, reduce.rs:469-475). Because the SWIPL_EXECUTE_PETTA body ref (the i64 = 37 declared in BodyRefs) isn't in the set, the original run records nothing; on replay previous_output is empty, so the contract emits an empty Par instead of the original result → tuple-space divergence → invalid block. GPT4 / DALLE3 use the same code shape but are in non_deterministic_ops; petta_execute must be added there.
    Note: this omission is inherited from the milestone3 base — the prior SWIPL_COMPILE_PETTA body ref was also absent from the set. The compile-only path may have tolerated it in practice because its output was a deterministic string transform, but the new execute path absolutely requires the non-deterministic marking. (The existing chroma_* body refs appear to share the same omission — worth flagging but out of scope for this PR.)

  2. [M3] No timeout / no resource bound on the swipl subprocess (swi_prolog_service.rs:52-60).
    MeTTa-via-PeTTa is Turing-complete and user-supplied via a system contract. Command::new("swipl")…output() is blocking and is being called from inside an async handler; any deploy containing !(<infinite loop>) halts the rholang runtime, and the synchronous call also blocks the Tokio worker. No CPU/memory caps, no kill on deadline, and no gas accounting tied to swipl execution. This is a denial-of-service primitive accessible to anyone who can deploy.
    Worth landing in M3 even though M4 makes long-running computation a feature — M4's snapshot interval makes cancellation more important, not less, because it introduces interruptible-but-resumable computation that needs a controlled shutdown path. At minimum here, switch to tokio::process::Command + tokio::time::timeout with kill-on-drop, and cap stdout size.

  3. [M3] n.as_i64().unwrap() panics on non-integer numbers (swi_prolog_service.rs:77).
    PeTTa returns numeric MeTTa results as JSON numbers, including floats and arbitrary-precision integers (the snapshot example produces the millionth Fibonacci number — gigantic). as_i64() returns None for floats and out-of-range integers, and the bare .unwrap() will panic the interpreter thread on otherwise-valid MeTTa output.
    RhoTypes.proto has only g_int (proto line 196) — no float/double field — so the right fix is to either return InterpreterError::SwiplError for non-integer/out-of-range numbers, or string-encode them.

  4. [M3, security] PeTTa exposes dangerous side-effecting predicates that M3 makes deployer-reachable. petta_execute runs user-supplied MeTTa through load_metta_file(...) (swi_prolog_service.rs:45-50). PeTTa registers — and exposes to MeTTa code — predicates including:

    • py-call (metta.pl:188-189) — arbitrary Python invocation. This is effectively RCE on any node that actually executes the deploy (the proposer / original executor). Replaying validators don't re-execute — they read previous_output from rspace once issue 1 is fixed — so the RCE surface is "any node that ever runs this as the original execution," not literally every validator on every block.
    • import_prolog_function / callPredicate / assertaPredicate / assertzPredicate / retractPredicate (metta.pl:237-239,285) — these give MeTTa code the ability to register arbitrary Prolog functions, call arbitrary Prolog predicates, and mutate the Prolog clause database. That's effectively full host-Prolog capability from inside a "MeTTa program," so many host / standard-library predicates can become reachable — including (depending on what's actually loaded / autoloaded) things like shell/1, process_create/3, open/3, http_open/3, socket/3.
    • git-import! (lib_import.pl:53-57) — clones arbitrary git repos onto the node filesystem. Lives in lib/lib_import.pl, not metta.pl, and is reachable once the importing library is loaded. (import! itself currently fails because working_dir/1 is unset — issue 11 — but that's a transient gap, not a security boundary.)
    • random-int / random-float / current-time (metta.pl:89-92,184) — non-determinism sources. With issue 1 fixed these get recorded as NonDeterministicCall output and replayed deterministically, so they don't by themselves break consensus. The remaining concerns are (a) different proposers running the same deploy compute different "original" outputs and can race / produce divergent proposed blocks before one is finalized, and (b) the recorded output is whatever the lucky proposer rolled, which is fine for liveness but means any contract that branches on random-int is effectively trusting whoever proposed first. Still worth removing under M4's channel-redirect.
    • readln! / exists_file — filesystem and stdin access.
      M4's channel-redirect (replace impure functions with {channel, arguments} JSON writes) is the design that closes these off, but M4 isn't merged. Until then, the M3 PR opens the most dangerous of these surfaces to any deployer. Mitigation options for M3, in increasing investment:
    • Don't merge rho:petta:execute to a public-deploy chain until M4 lands (gate behind a feature flag or sysauth token).
    • Or block the worst predicates at the Prolog level: prepend the goal with retractions/redefinitions of py-call, git-import!, shell/*, process_create/*, http_open/*, etc., so they fail closed.
    • Or run swipl under SWI's library(sandbox) for the user-code phase. (Won't compose cleanly with PeTTa's existing assertz usage; needs care.)
      At minimum this needs an explicit M3 decision rather than being silently inherited from "M4 will fix it."

High

  1. [M3 / partly M4] JSON serialization of arbitrary Prolog terms is fragile (swi_prolog_service.rs:45-50).
    process_form for runnable forms produces findall(EV, Conj, Out) results (PeTTa filereader.pl:31-35, translator.pl:113-114). For programs with free MeTTa variables that don't get bound, EV is an unbound Prolog variable; SWI's json_write_dict/2 expects JSON-shaped Prolog data (SWI JSON docs) and errors on unbound vars and on non-list compound terms (unless serialize_unknown(true) is passed). The Rust side then sees empty stdout (or partial output + stderr noise) and returns a generic "Can't parse JSON output…".
    Two-phase fix: in M3, wrap the json call in a catch/3 in the Prolog goal and emit a structured error or swrite/2-converted fallback so the boundary is "well-formed JSON in, structured error out." In M4 the {channel, arguments} output schema strips the remaining "arbitrary Prolog term in stdout" surface, because every emit is shaped by the host.

  2. [M3] output.status and stderr are ignored (swi_prolog_service.rs:52-71).
    On any swipl failure (missing PeTTa file, MeTTa syntax error, type error, json error) the user gets the generic "Can't parse JSON output from PeTTa execution" with no diagnostic. Include stderr in InterpreterError::SwiplError and fail explicitly when !output.status.success(). As hardening, also pass -q/--quiet and set_prolog_flag(verbose, silent) so library-load notices / a possible welcome banner don't contaminate stdout.

Medium

  1. [M3] Goal interpolates the tempfile path inside Prolog single-quotes without escaping (swi_prolog_service.rs:45-50).
    format!("'{metta_file_path}'") will produce malformed Prolog (or arbitrary goal injection) if the path contains a single quote, backslash, or newline. Tempfile paths from tempfile::NamedTempFile are normally safe on macOS/Linux, but this isn't enforced. Either validate the path, escape it, or pass it to Prolog via current_prolog_flag(argv, [Path|_]) rather than substituting into source. (Becomes especially important under M4's hash-named session directories where the file path is more structured and persistent.)

  2. [M3] No unit/integration tests added. The milestone explicitly calls for "tests and example Rholang scripts to demonstrate that the … execution is occurring." Only an example is present; no Rust-side test exercises petta_execute, no cargo test coverage of value_to_par, and crucially no replay test — which is what would have caught the missing non_deterministic_ops() entry (issue 1).

  3. [M3] PETTA_PATH default ./PeTTa is CWD-relative (swi_prolog_service.rs:37). A node launched from anywhere other than the repo root will fail with "Can't find PeTTa." Not superseded by M4 — the M4 snapshot commits (097e7d29, 0978f6ff) introduce hash-named session directories for user MeTTa programs but leave the env::var("PETTA_PATH").unwrap_or("./PeTTa") lookup for the PeTTa-installation path unchanged. Fix in M3: resolve to an absolute path at startup, or document the CWD constraint in DEVELOPER.md.

  4. [M3, packaging] swipl binary and the PeTTa submodule are not pinned or provisioned. No flake.nix entry for swi-prolog, no Docker image change, no DEVELOPER.md note about SWI-Prolog being a runtime dependency. Three consequences:

    • No swipl in the runtime pathwhich swipl returns "not found" in the dev environment as of this branch; the example contract fails immediately on any clean checkout.
    • The Docker image doesn't ship PeTTanode/Dockerfile COPYs the Rust workspace members and later runtime resources, but not the PeTTa/ submodule. Even with swipl installed in the image, the default ./PeTTa/src/metta.pl won't resolve. A COPY PeTTa/ ./PeTTa/ (or wherever PETTA_PATH resolves to) plus a swipl RUN apt-get install (or Nix equivalent) is needed.
    • Cross-node version drift — proposer-race / liveness concern, not a consensus safety break. Once issue 1 is fixed, replay reads previous_output from rspace and never re-runs swipl, so validators verifying a finalized block converge regardless of which swipl they have locally. The remaining version-drift risk is at proposal time: if multiple proposers run the same deploy on different SWI-Prolog builds (json formatter, predicate semantics, integer-overflow behavior, etc.), they record divergent outputs into competing blocks. Only one wins finalization; the others get orphaned. That's wasted work / fork churn / surprising "whoever proposed first wins" semantics for contracts that branch on the output, not invalid finalized state. Still worth pinning a specific swipl version in flake.nix / the Docker image for proposer-race hygiene, reproducibility, and operator sanity. M4 doesn't change this picture; the snapshot library is just another version-sensitive component on top. Long-term, WASM-bundled-swipl (binary ships in the node artifact, no drift possible) is the cleanest answer here.
  5. [M3] import! will fail because working_dir/1 is not asserted. PeTTa's import! predicate calls working_dir(Base) to resolve the relative path of .metta and .py modules (metta.pl:249-251). The PR's Prolog goal does not assert working_dir/1, so any MeTTa program containing import! calls will fail (or behave unpredictably). The M4 snapshot commit 0978f6ff explicitly adds assertz(working_dir('{session_path}')) for this reason. As written, PR Use PeTTa as interpreter #3 only supports simple, no-import MeTTa programs. Either pre-assert working_dir/1 here or document the limitation.

  6. [M4 supersedes] Output shape is {"results": [...]} (01-swap.rho:3-5). The example treats it as a single value (for(@ok <- retCh) { stdout!(ok) }), so it'll print the whole map; cosmetically awkward. M4 standardizes the schema as { "channel": "...", "arguments": [...] } with the host routing the arguments to the named channel, which supersedes the current ad-hoc shape. Transitional: consider returning the inner results list in M3 to keep the example clean, but don't over-design here.

  7. [M3] evaluate(_swi_prolog_code) -> Ok(()) is dead code (swi_prolog_service.rs:16-18). Leftover from the dropped rho:prolog:evaluate service. Remove it.

  8. [M3 minor] Two silent/1 clauses end up in the database. filereader.pl:3-4 asserts silent(false) at load time (since argv won't contain silent), then the Rust goal asserts silent(true). Both clauses are now true. The current code only ever queries silent(true) -> …, so it still works, but anyone later writing silent(false) -> … would be surprised. Safer to assert the flag via nb_setval / :- dynamic + retractall, or pass it as a swipl -g arg.

Low

  1. [M3] 01-swap.metta uses (Pair 1 2) but 01-swap.rho uses (Pair 1 3). Same logical example; the data drift looks unintentional.

  2. [M3] metta_file.write(...) not write_all (swi_prolog_service.rs:24-26). For small buffers on disk this is almost always a single write, but Write::write is allowed to be partial; should be write_all for correctness.

  3. [M3] env::var(...).unwrap_or(...) masks non-Unicode env var errors. Minor; the distinction-preserving fix is env::var_os (or an explicit match env::var(...)); .ok().unwrap_or_else(...) would also discard the VarError.

  4. [M3] Minor stylistic: let output = vec![output]; shadows the variable for no reason — vec![petta_execute(&metta_code)?] would be clearer. And the inconsistent &ack vs ack between the replay and non-replay produce calls is cosmetic.

Documentation

  1. [M3, may block strict reviewers] Documentation is below the codebase's peer pattern. The PR doesn't reach the bar a strict reviewer is likely to expect. None of the new public items have /// doc comments (compare chromadb_service.rs:21 which uses /// on its types). CLAUDE.md explicitly calls out a "Documentation-First Development methodology" with docs/requirements/, docs/specifications/, docs/architecture/ expected.

Concrete gaps:

  • No /// doc comments on any new item: the four new public items (petta_execute, swipl_execute_petta, FixedChannels::swipl_execute_petta, BodyRefs::SWIPL_EXECUTE_PETTA) plus the private value_to_par helper.
  • No spec for the rho:petta:execute URN — arity, expected arg types, return shape, error conditions are all implicit. The example file is the only de-facto spec, and the example misuses the return shape (issue 12).
  • No documentation of the JSON output schema. The {"results": [...]} envelope is invented in swi_prolog_service.rs with no comment explaining the shape, and the JSON-Value → Par mapping in value_to_par (all keys stringified, JSON numbers must fit i64, etc.) is implicit. Forcing this to be written down will also force a decision on issue 12 (and a clean alignment with the M4 {channel, arguments} shape).
  • No DEVELOPER.md entry explaining swipl + PeTTa as runtime dependencies, install instructions, or PETTA_PATH. Overlaps with issue 10.
  • No README in rholang/examples/system-contract/swipl/. The peer chroma-db/ folder has 7 numbered examples plus a "unified example"; the swipl folder has two files and no narrative.
  • No CHANGELOG entry despite the repo tracking CHANGELOG.md.

Suggested must-fix list before M3 merge

Code / behaviour:

  • Add BodyRefs::SWIPL_EXECUTE_PETTA to non_deterministic_ops() (issue 1).
  • Switch to tokio::process::Command + tokio::time::timeout with kill-on-drop and an stdout size cap (issue 2).
  • Replace the .unwrap() on n.as_i64() with an explicit InterpreterError (or tagged-string fallback) for non-integer / out-of-range numbers (issue 3).
  • Make an explicit M3 decision on PeTTa side-effects exposure — gate the service behind sysauth or a feature flag, block dangerous predicates at the Prolog level, or accept the RCE-via-deploy risk in writing (issue 4). Do not merge to a public-deploy chain in its current form.
  • Surface stderr and check output.status; add -q / set_prolog_flag(verbose, silent) as hardening (issue 6).
  • Pin swipl in flake.nix / Docker, and COPY PeTTa/ into the Docker image (issue 10).
  • Pre-assert working_dir/1 (or document the no-import limitation) so the service supports MeTTa with imports (issue 11).
  • Add at least one happy-path integration test and one replay test (issue 8) — the replay test is what catches regressions of issue 1.

Documentation (issue 19) — roughly an afternoon's work, no architectural changes:

  • Add /// doc comments to the four new public items (petta_execute, swipl_execute_petta, FixedChannels::swipl_execute_petta, BodyRefs::SWIPL_EXECUTE_PETTA) and the private value_to_par. Two to four sentences each, covering inputs, return shape, side-effects, error conditions.
  • Add a module-level header comment in swi_prolog_service.rs describing the module's responsibility, the JSON envelope shape, and the assumed runtime (swipl + PeTTa).
  • Add a 1-paragraph entry in DEVELOPER.md listing swipl and the PeTTa/ submodule as runtime dependencies (also satisfies the documentation half of issue 10).
  • Add a README.md in rholang/examples/system-contract/swipl/ describing what the example demonstrates and the contract's I/O shape (forces a concrete decision on issue 12).
  • Add a CHANGELOG line.

Issue 12 is explicitly M4 supersedes — flag it, but don't rebuild here. Issue 5 is the next one to dig into after the must-fix list; it needs a catch/3 in the Prolog goal and (M3) a structured error path or (M4) the {channel, arguments} schema.

@rmgaray
Copy link
Copy Markdown
Collaborator Author

rmgaray commented May 13, 2026

  • My analysis of the review:
    • Legitimate issues that need to be solved by M3:
      • Consensus break: SWIPL_EXECUTE_PETTA is missing from non_deterministic_ops()
        • NOTE: I'm not sure of how serious this issue actually is, but I'll spend some time to understand how non-deterministic ops work. I can try to reproduce the issue described.
      • No timeout / no resource bound on the swipl subprocess
      • n.as_i64().unwrap() panics on non-integer numbers
        • NOTE: I was aware of this at some point, but forgot to fix it before opening for review.
      • output.status and stderr are ignored
        • NOTE: I agree with the review here that our error reporting will increase significantly by inspecting this information and bubbling it up.
      • No unit/integration tests added
      • PETTA_PATH default ./PeTTa is CWD-relative
        • NOTE: We should document this in DEVELOPER.md.
      • swipl binary and the PeTTa submodule are not pinned or provisioned
      • evaluate(_swi_prolog_code) -> Ok(()) is dead code
      • Documentation is below the codebase's peer pattern
    • All other issues are, in my opinion, either:
      • Security related: we already discussed that M3 should not be used in production and we should wait for M4.
      • Addressed (or will be addressed) in M4.

@nrutledge
Copy link
Copy Markdown
Collaborator

I've added 3 new issues referenced above for tracking things flagged in this PR review that we will be addressing later (whether planned or requiring further consideration).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants