Sitelet https://github.com/formancehq/numscript/pull/4
Skip to content

Interpreter - #4

Merged
ascandone merged 37 commits into
mainfrom
experiment/interpreter
Aug 1, 2024
Merged

ascandone merged 37 commits into
mainfrom
experiment/interpreter

Conversation

@ascandone

Copy link
Copy Markdown
Contributor

No description provided.

@ascandone
ascandone marked this pull request as ready for review August 1, 2024 17:47
@ascandone
ascandone merged commit 5874d05 into main Aug 1, 2024
@ascandone
ascandone deleted the experiment/interpreter branch August 1, 2024 17:48
ascandone added a commit that referenced this pull request Sep 18, 2026
DIVERGENCES.md and DIFFERENCES-BY-EXAMPLE.md covered the same four differences
at two levels of detail, and the second was written on top of the first rather
than replacing it, so both had to be kept in sync by hand.

One file now. 472 lines down to 244. Each difference gets a runnable script,
what all three engines do with it, why, and the open question. The vendoring
mechanics and the drift check move to the end, where they belong.

Section numbers become #1-#4, which is what the code comments now cite.
ascandone added a commit that referenced this pull request Sep 18, 2026
DIVERGENCES.md and DIFFERENCES-BY-EXAMPLE.md covered the same four differences
at two levels of detail, and the second was written on top of the first rather
than replacing it, so both had to be kept in sync by hand.

One file now. 472 lines down to 244. Each difference gets a runnable script,
what all three engines do with it, why, and the open question. The vendoring
mechanics and the drift check move to the end, where they belong.

Section numbers become #1-#4, which is what the code comments now cite.
ascandone added a commit that referenced this pull request Sep 18, 2026
…stead

The oracle carried a negative-amount guard on OP_TAKE that ledger does not
have. That made vm/machine.go a behaviour fork, and hid the cost: the oracle
silently stopped being ledger on every script with a negative send amount.

Copy ledger exactly instead, and absorb the difference in Compare under a named
tolerance. Both engines reject those scripts and no money moves either way;
only the error differs (`insufficient funds` vs `Cannot send negative amount`).

The tolerance is one-directional: the interpreter naming a negative amount
while the oracle blames the funds. The reverse is DIVERGENCES.md #4, where the
oracle rejects and the interpreter silently zeroes the clause and runs short,
and that must stay a mismatch. TestMissingFundsClassificationMismatchStillCaught
caught the symmetric version of this and is why it is not.

The sweep prints how often it fires -- 161 of 3000 scripts -- so the cost is
countable on every run instead of invisible. vm/machine.go is now
byte-identical to ledger apart from the vendored types, leaving `kept` as the
oracle's only behavioural divergence.

Also records in DIVERGENCES.md #4 why ledger PR #2079 was closed: clamping a
negative `max` needs a new opcode, and adding one to the machine is too
aggressive for what it buys.
ascandone added a commit that referenced this pull request Sep 25, 2026
… negative-max gap

Compare tolerated any one-sided runtime failure once both sides agreed on
missing-funds classification, not just the known negative `max` clause
divergence (DIVERGENCES.md #4). A real bug where one engine committed money
the other refused to move, for an unrelated reason, would have passed
through silently.

Add SideResult.NegativeMaxReject, set by run_oracle.go when the failure is
ledger's OP_TAKE_MAX guard rejecting a negative max clause (untyped upstream,
so matched by its fixed error text rather than by type), and require it
before tolerating the asymmetry. Any other one-sided failure is now a
mismatch. Renamed the tolerance from "one-sided runtime failure" to
"negative max clause" to match, and updated DIVERGENCES.md and the tests that
asserted the old name.
ascandone added a commit that referenced this pull request Sep 25, 2026
* test: differential testing against the legacy ledger machine

Runs generated programs through both the numscript interpreter and a vendored
copy of the ledger repo's `internal/machine`, and compares what moved.

- `internal/oracle/` is that vendored copy, in its own module so it cannot
  drift into the interpreter's dependency graph. `smoke_test.go` checks the
  vendoring itself did not change behaviour. `DIVERGENCES.md` records every
  difference from ledger main and why it is there; `DIFFERENCES-BY-EXAMPLE.md`
  is the same list as runnable scripts.
- `internal/difftest/` is the harness, also its own module: `Compare` (what
  counts as a real difference vs a posting-granularity one), the seeded
  `TestDifferentialSweep` gate, `TestKnownBugRepros`/`TestKnownOpenDivergences`,
  and `FuzzDiff` with its corpus.

CI gets a `Difftest` job, because both are separate modules and the root
`go test ./...` reaches neither. It runs the deterministic sweep, not the
fuzzer: `-fuzztime` expiry surfaces as a failure with no divergence behind it.

The sweep is green -- 0 divergence classes and 0 tolerated over 3000 seeds.
Getting there meant changing the oracle's `kept` to consume its funding, the
way the interpreter does and unlike ledger. That is a deliberate loss of
independence on `kept` and is written up as DIVERGENCES.md §2 ②.

* refactor(gen): trim comments and unexport the internal-only helpers

Drops the running commentary about the generator's Haskell ancestry
(Gen.hs/Numscript.hs/Utils.hs, QuickCheck's `frequency`, `Ratio Int`) — none of
it is a fact about this code — along with comments that restate the field they
sit on and rationale that had grown into prose.

Kept: what cannot be read off the code — why `max`-clause amounts are never
negative, why both pools are sorted before being indexed, why the var-collision
shapes are biased for.

`GenerateScriptAST`, `ToBuilder` and `ToBuilderScript` were exported but used
only inside the package; `GenerateScript` and `RandFromBytes` are the entry
points. Unexported.

Also removes references to DIFFTEST_HANDOFF.md, which is not in the repo, and
deletes a superseded duplicate doc comment on genPresetMetadata.

internal/gen landed in #194 before this cleanup was ready, hence the separate
commit.

* docs(oracle): merge the two divergence docs into one

DIVERGENCES.md and DIFFERENCES-BY-EXAMPLE.md covered the same four differences
at two levels of detail, and the second was written on top of the first rather
than replacing it, so both had to be kept in sync by hand.

One file now. 472 lines down to 244. Each difference gets a runnable script,
what all three engines do with it, why, and the open question. The vendoring
mechanics and the drift check move to the end, where they belong.

Section numbers become #1-#4, which is what the code comments now cite.

* oracle: drop the OP_TAKE guard, tolerate the difference in Compare instead

The oracle carried a negative-amount guard on OP_TAKE that ledger does not
have. That made vm/machine.go a behaviour fork, and hid the cost: the oracle
silently stopped being ledger on every script with a negative send amount.

Copy ledger exactly instead, and absorb the difference in Compare under a named
tolerance. Both engines reject those scripts and no money moves either way;
only the error differs (`insufficient funds` vs `Cannot send negative amount`).

The tolerance is one-directional: the interpreter naming a negative amount
while the oracle blames the funds. The reverse is DIVERGENCES.md #4, where the
oracle rejects and the interpreter silently zeroes the clause and runs short,
and that must stay a mismatch. TestMissingFundsClassificationMismatchStillCaught
caught the symmetric version of this and is why it is not.

The sweep prints how often it fires -- 161 of 3000 scripts -- so the cost is
countable on every run instead of invisible. vm/machine.go is now
byte-identical to ledger apart from the vendored types, leaving `kept` as the
oracle's only behavioural divergence.

Also records in DIVERGENCES.md #4 why ledger PR #2079 was closed: clamping a
negative `max` needs a new opcode, and adding one to the machine is too
aggressive for what it buys.

* docs(oracle): replace the "rarely produces" claim about #3 with measurements

The doc said the generator rarely produces a save-overdraw followed by a
bounded-overdraft draw on the same account. Measured over the same 3000 seeds
it produces the ingredients constantly and the combination never: 686 saves,
2284 bounded overdrafts, 130 on a shared account, 7 in order, 2 that also
overdraw, 0 complete.

The reason is structural, so say it: statements are sampled independently, and
a shape needing two of them to hit the same (account, asset) in order is
quadratically suppressed.

* improve generator

* fix review issues

* apply bot suggestions

* difftest: narrow the one-sided-runtime-failure tolerance to the known negative-max gap

Compare tolerated any one-sided runtime failure once both sides agreed on
missing-funds classification, not just the known negative `max` clause
divergence (DIVERGENCES.md #4). A real bug where one engine committed money
the other refused to move, for an unrelated reason, would have passed
through silently.

Add SideResult.NegativeMaxReject, set by run_oracle.go when the failure is
ledger's OP_TAKE_MAX guard rejecting a negative max clause (untyped upstream,
so matched by its fixed error text rather than by type), and require it
before tolerating the asymmetry. Any other one-sided failure is now a
mismatch. Renamed the tolerance from "one-sided runtime failure" to
"negative max clause" to match, and updated DIVERGENCES.md and the tests that
asserted the old name.

* made the err more precise
ascandone added a commit that referenced this pull request Sep 25, 2026
Reverts the funds-engine rewrite's neg-max-dest change. A negative cap on a
destination max clause now behaves like the source-side one
(tryTakingUpTo/NonNeg): clamped to zero rather than NegativeAmountErr, matching
ledger's own gap on this shape (oracle/DIVERGENCES.md #4) and restoring what
internal/difftest's TestDestinationSideNegativeMaxTolerated already expects.
ascandone added a commit that referenced this pull request Sep 25, 2026
* refactor: extract the funds engine into internal/funds

Moves the funds queue, the balance cache, the value validators/parsers and the
asset-scaling helpers out of internal/interpreter into a standalone
internal/funds package, behind a RunState type. The interpreter becomes a
consumer of it rather than the owner. This is the shared substrate the
forthcoming bytecode VM needs: it must execute the same funds semantics without
depending on the tree-walking interpreter.

  internal/funds/funds.go    RunState: the source queue, the balance cache,
                             mark/rewind, postings
  internal/funds/values.go   account/asset/color/scope validators and parsers
  internal/funds/scaling.go  was interpreter/asset_scaling.go
  internal/funds/metadata.go AccountsMetadata

Behavior changes, each pinned by a spec in this commit:

  neg-max-dest        a negative amount in a destination max clause now fails
                      with NegativeAmountErr instead of emitting a posting.

  scaling-with-oneof  a oneof branch that falls short now discards its
                      scaled-swap postings, via RunState's mark/rewind.

  midscript-balance-after-credit
                      a mid-script balance() read is no longer masked by an
                      earlier write to the same account. The old
                      InternalBalances.has() could not tell "fetched from the
                      store" from "created by a write", so crediting @alice and
                      then reading balance(@alice) skipped the fetch and
                      reported only what the script sent, dropping the starting
                      balance. funds.balanceEntry now carries baseLoaded
                      separately from the amount: Has() reports false for an
                      entry holding only a write delta, and Prewarm folds the
                      fetched base into that delta instead of overwriting it.

The balance() change is not breaking: mid-script calls are gated behind
experimental-mid-script-function-call. balance() inside a vars block is
unaffected, since vars resolve before execution and no write can mask them.

Ledger implementors will see one store-call difference: the balance read is now
a lazy fetch where there was none, so GetBalancesCalls in TestMidscriptBalance
goes from nil to a single fetch.

internal/specs_format exports four helpers that were package-private
(MergeBalances, EndBalances, GetMovements, CompareMovements). They have no
consumer in this commit; the differential-testing harness will use them.

* fix: keep real balance values during dependency resolution

The injected getBalance for dependency resolution returned zero, on the
stated assumption that a balance() yields a Monetary and so cannot name an
account. get_amount breaks that assumption: it turns a balance into a number,
and an interpolated account can be built from that number.

So for

  vars {
    monetary $b = balance(@treasury, USD)
    number $id = get_amount($b)
  }
  send [USD 1] (
    source = @user:$id allowing unbounded overdraft
    destination = @out
  )

resolution reported a write to user:0 while execution posted from user:100 —
a ledger pre-locking or prefetching from the resolved set would act on the
wrong account. main is unaffected: there, resolution and execution shared one
getBalance that returned the fetched value.

Returns the fetched amount instead, and adds the case to
TestResolveDependenciesCoversRuntime, which asserts resolution covers what
execution touches. Confirmed the case fails without the fix.

The differential harness cannot catch this class: internal/gen emits no
account interpolation.

* fix: fail closed on a negative asset-scaling conversion

ForcePosting treated any non-positive amount as a no-op. FindScalingSolution
can return a negative conversion when the source holds a negative balance at
one scale, and dropping that leg while posting the positive ones moves money
one way without the compensating move back.

For @ACC1 holding EUR/2 -100 and EUR/1 20:

  send [EUR 1] (
    source = @ACC1 with scaling through @swap
    destination = @DesT
  )

the solution is the pair (-100, 20), netting the 1 EUR asked for. Discarding
the negative leg left acc1 paying EUR/1 20 — 2 EUR — to move 1 EUR, with
@swap keeping the difference, and the script reported success.

main fails this script: the negative conversion became a negative posting and
checkPostingInvariants rejected it. That check still exists here, but the
posting no longer reaches it, because ForcePosting discarded it first.

ForcePosting now returns ErrNegativePosting for a negative amount (zero stays
a no-op), and the interpreter maps it to the same InternalError, so the
user-visible behavior matches main exactly, error message included.

This is a fail-closed path, not a computed result: the underlying question of
whether FindScalingSolution should produce such a solution at all is left
alone deliberately — answering it would change the scaling semantics rather
than restore the invariant.

* fix: clamp a negative destination max clause to zero instead of erroring

Reverts the funds-engine rewrite's neg-max-dest change. A negative cap on a
destination max clause now behaves like the source-side one
(tryTakingUpTo/NonNeg): clamped to zero rather than NegativeAmountErr, matching
ledger's own gap on this shape (oracle/DIVERGENCES.md #4) and restoring what
internal/difftest's TestDestinationSideNegativeMaxTolerated already expects.

* fix: keep the scaling prefetch from masking other assets' balances

AccountBalances folded the base of every tracked entry of the account,
against the interpreter's zero-backed store. A delta-only entry for an
asset outside the scaled family got stamped base-loaded with a zero
base, so a later mid-script balance() skipped its fetch and reported
only the script's own writes. Filter to the base-asset family the
scaling prefetch actually covers; pinned by the
scaling-prefetch-midscript-balance fixture.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant