Repository navigation
Interpreter - #4
Merged
Merged
Conversation
ascandone
marked this pull request as ready for review
August 1, 2024 17:47
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.
3 tasks done
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.
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
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.
No description provided.