Repository navigation
Conversation
Two pull/send shapes passed vm.Verify and produced wrong postings with no error: a pull whose destination register aliases its cap or overdraft (funds.Pull writes the destination before it reads the bounds, so the cap stops bounding and the overdraft doubles the balance), and a set_current_asset with funds still queued (the posting comes out under the new asset). A run ending with queued funds dropped the debit, and an unverified program ending in the first word of a two-word instruction panicked. Exec now returns an InternalError for all four; the verifier rejects the aliasing statically. Queue emptiness depends on the balances, so the two queue checks stay at run time.
NumaryBot
approved these changes
Oct 12, 2026
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
The required automated review completed with no remaining findings.
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.
Problem
Two pull/send shapes pass
vm.Verifyand produce wrong postings with no error:pull_accountwhose destination register is also its cap or overdraft.funds.Pullwrites the destination before it reads the bounds on the bounded path, so an aliased cap stops bounding the pull and an aliased overdraft doubles the balance. Balance 30, cap 100, overdraft 0 pulls 60, leaving the source at -30 with a zero overdraft.set_current_assetwhile funds are still queued. The debit was taken under the previous asset; the posting is emitted under the new one.Two more are silent but less severe: a run that ends with funds still queued drops the debit, and an unverified program ending in the first word of a two-word instruction panics on the ext-word read (the old TODO in
vm.go).Compiled numscript never emits any of these: the allocator never aliases a destination with an operand of the same instruction, and every send statement either pulls exactly what it sends (
check_enough_funds) or sends everything it pulled. They matter for bytecode that did not come out of this process's compiler, which is what the ledger replays.Fix
Execreturns anInternalErrorfor all four:errTruncatedInstructionon every two-word opcode (one compare before the ext-word read).errPullDestAliasesInputonOp_PullAccount(two byte compares).errSetAssetWithQueuedFundsanderrEndWithQueuedFunds, via a newfunds.RunState.HasQueuedFunds(). Zero-amount queue entries do not count:send [USD 0]lowers to a zero-cap pull that queues one and a zero-cap send that does not consume it.Verifyrejects the aliasing statically. The queue checks are not statically decidable (whether a capped send drains the queue depends on the balances), so they stay at run time;bytecode-verifier.mddocuments that under "What it does not check".instruction-encoding.mdand the opcode comments carry the new traps.Tests
go test ./..., the difftest module,-raceon the touched packages,TestCompiledCorpusPassesVerify, and 20s each ofFuzzExecandFuzzMutatedBytecodeare clean.Performance
BenchmarkCompiledVM*, 10 runs each, comparing minimums: identical allocations, timings within noise except the smallest script (BenchmarkCompiledVM, 630 → 648 ns/op), which is the twoHasQueuedFundscalls on an empty queue.Not changed
ir.Typecheckdoes not know the aliasing rule. Hand-written IR with it assembles, and is caught byVerifyone level down.