From c5639f47b81db9b2663ec81cec7521836505dce2 Mon Sep 17 00:00:00 2001 From: ascandone Date: Wed, 28 Oct 2026 11:43:37 +0100 Subject: [PATCH 1/3] fix(vm): fail closed when pulled funds are left in the queue Exec returns an InternalError when set_current_asset runs, or the run ends, while the source queue still holds a positive amount. Whether a capped send drains the queue depends on the balances, so this is not checked by Verify. --- bytecode-verifier.md | 12 ++++++ instruction-encoding.md | 2 +- internal/funds/funds.go | 12 ++++++ internal/vm/export_test.go | 6 ++- internal/vm/instruction.go | 2 + internal/vm/ir_test.go | 31 +++++++++++++++ internal/vm/vm.go | 21 +++++++--- internal/vm/vm_test.go | 78 ++++++++++++++++++++++++++++++++++++++ 8 files changed, 157 insertions(+), 7 deletions(-) diff --git a/bytecode-verifier.md b/bytecode-verifier.md index 8bfbdd2f..0244d2a4 100644 --- a/bytecode-verifier.md +++ b/bytecode-verifier.md @@ -83,6 +83,18 @@ verified, so `ErrNoOpenMark` and the INVARIANT documented on `MarkEnd` stay. it is a legitimate user-facing error (`InvalidUncappedSource`, "unbounded source is not allowed here"), not a malformed program. Deliberately left to run time. +### Funds left in the queue + +A pull debits its source at once; the posting only exists once a send drains the +queue. Funds still queued when `Op_SetCurrentAsset` runs would be posted under +the new asset, and funds still queued when the run ends are a debit that never +became a posting. Whether a capped send empties the queue depends on the +balances, so neither is decidable from the instruction stream: `Exec` checks +both at run time (`errSetAssetWithQueuedFunds`, `errEndWithQueuedFunds`, via +`runstate.HasQueuedFunds()`). Compiled numscript never trips them: every send +statement either pulls exactly what it sends (`Op_CheckEnoughFunds`) or sends +everything it pulled. + ### Anything semantic A verified program can still produce nonsense: allotment portions that don't sum diff --git a/instruction-encoding.md b/instruction-encoding.md index bd026985..31f5b545 100644 --- a/instruction-encoding.md +++ b/instruction-encoding.md @@ -27,7 +27,7 @@ Instructions are **4 bytes** wide: `[Opcode: 8] [A: 8] [B: 8] [C: 8]`. 00x00SET_CURRENT_ASSET asset-- - Sets the current asset (used by PULL_ACCOUNT / SEND_TO_ACCOUNT) from str_regs[A] + Sets the current asset (used by PULL_ACCOUNT / SEND_TO_ACCOUNT) from str_regs[A]. Traps while a mark is open, or while the source queue still holds funds pulled under the previous asset 10x01ASSERT_SAME_ASSET diff --git a/internal/funds/funds.go b/internal/funds/funds.go index ab26f93c..2cd68b80 100644 --- a/internal/funds/funds.go +++ b/internal/funds/funds.go @@ -244,6 +244,18 @@ func (s *RunState) GetAccountBalance(account, scope, asset, color string) (*big. return new(big.Int).Set(bal), nil } +// HasQueuedFunds reports whether a queued source still holds a positive amount. +// Zero-amount entries do not count: a Pull of a zero cap queues one, and a Send +// of a zero cap does not consume it. +func (s *RunState) HasQueuedFunds() bool { + for i := range s.sources { + if s.sources[i].amount.Sign() > 0 { + return true + } + } + return false +} + // Pull debits up to cap from src's (currentAsset, color) balance, queues the // pulled amount as a funding source tagged with color, and writes the amount made // available into out: diff --git a/internal/vm/export_test.go b/internal/vm/export_test.go index 77388479..c7a3ffed 100644 --- a/internal/vm/export_test.go +++ b/internal/vm/export_test.go @@ -1,3 +1,7 @@ package vm -var ErrSendWhileMarkOpen = errSendWhileMarkOpen +var ( + ErrSendWhileMarkOpen = errSendWhileMarkOpen + ErrSetAssetWithQueuedFunds = errSetAssetWithQueuedFunds + ErrEndWithQueuedFunds = errEndWithQueuedFunds +) diff --git a/internal/vm/instruction.go b/internal/vm/instruction.go index 5c4c0a52..a6e4fe7d 100644 --- a/internal/vm/instruction.go +++ b/internal/vm/instruction.go @@ -36,6 +36,8 @@ type Opcode byte // a category without renumbering. See instruction-encoding.md. const ( // --- state & assertions (0x00) --- + // InternalError while a mark is open or while the source queue still holds + // funds pulled under the previous asset Op_SetCurrentAsset Opcode = 0x00 Op_AssertSameAsset Opcode = 0x01 diff --git a/internal/vm/ir_test.go b/internal/vm/ir_test.go index 49214979..839abfa7 100644 --- a/internal/vm/ir_test.go +++ b/internal/vm/ir_test.go @@ -1401,3 +1401,34 @@ func TestIRPullDestAliasingItsBoundIsRejected(t *testing.T) { require.ErrorContains(t, vm.Verify(program), "aliases its cap or overdraft") }) } + +// Funds pulled under one asset and still queued when the asset changes would be +// posted under the new asset. Not statically decidable (a capped send drains the +// queue or not depending on the balances), so this is a run-time check only. +func TestIRSetCurrentAssetWithQueuedFundsIsAnInternalError(t *testing.T) { + execErr := runIRExpectingError(t, ` + $usd = "USD/2" + set_current_asset($usd) + $src = "src" + $dest = "dest" + $overdraft = 0 + $cap = 100 + $pulled = pull_account(account: $src, cap: $cap, overdraft: $overdraft) + $eur = "EUR/2" + set_current_asset($eur) + send_to_account(account: $dest) +`, balances(map[string]int64{"src": 100}), nil) + require.Equal(t, vm.InternalError{Err: vm.ErrSetAssetWithQueuedFunds}, execErr) +} + +func TestIREndWithQueuedFundsIsAnInternalError(t *testing.T) { + execErr := runIRExpectingError(t, ` + $asset = "USD/2" + set_current_asset($asset) + $src = "src" + $overdraft = 0 + $cap = 100 + $pulled = pull_account(account: $src, cap: $cap, overdraft: $overdraft) +`, balances(map[string]int64{"src": 100}), nil) + require.Equal(t, vm.InternalError{Err: vm.ErrEndWithQueuedFunds}, execErr) +} diff --git a/internal/vm/vm.go b/internal/vm/vm.go index 2001ffa8..c04317ad 100644 --- a/internal/vm/vm.go +++ b/internal/vm/vm.go @@ -30,6 +30,13 @@ var ( errSaveWhileMarkOpen = errors.New("save while a mark is open") ) +// Not statically decidable: whether a send drains the queue depends on the +// balances. +var ( + errSetAssetWithQueuedFunds = errors.New("set_current_asset with funds still queued") + errEndWithQueuedFunds = errors.New("run ended with funds still queued") +) + type Vm struct { Program Program runstate *funds.RunState @@ -156,9 +163,6 @@ func Exec[S Store]( switch Opcode(instr.Opcode) { // --- Domain-specific ops case Op_PullAccount: - // TODO crashes if this is the last instruction (the ext word is - // missing): instrs[pc] reads past the end. e.g. a program ending in a - // lone Op_PullAccount word. instrExt := instrs[pc] pc++ @@ -292,6 +296,10 @@ func Exec[S Store]( if runstate.HasOpenMark() { return funds.ExecutionResult{}, InternalError{Err: errSetAssetWhileMarkOpen} } + // funds queued under the previous asset would be posted under the new one + if runstate.HasQueuedFunds() { + return funds.ExecutionResult{}, InternalError{Err: errSetAssetWithQueuedFunds} + } currentAsset = stringsRegs[instr.A] runstate.SetCurrentAsset(currentAsset) @@ -426,8 +434,6 @@ func Exec[S Store]( portionsRegs[instr.A].Set(r) case Op_MetaMonetary: - // TODO crashes if this is the last instruction (the ext word carrying the - // amount destination is missing), same as Op_PullAccount. instrExt := instrs[pc] pc++ @@ -635,6 +641,11 @@ func Exec[S Store]( }) } + // a debit that never became a posting + if runstate.HasQueuedFunds() { + return funds.ExecutionResult{}, InternalError{Err: errEndWithQueuedFunds} + } + postings := runstate.GetPostings() for _, p := range postings { if !funds.ValidatePosting(p) { diff --git a/internal/vm/vm_test.go b/internal/vm/vm_test.go index 6d8922e7..788a84a5 100644 --- a/internal/vm/vm_test.go +++ b/internal/vm/vm_test.go @@ -537,3 +537,81 @@ func TestExecutionErrorsUnwrap(t *testing.T) { require.ErrorIs(t, InternalError{Err: cause}, cause) require.ErrorIs(t, StoreError{Wrapped: cause}, cause) } + +// --- malformed pull/send sequences ----------------------------------------- + +func TestSetCurrentAssetWithQueuedFundsIsAnInternalError(t *testing.T) { + // pull 100 USD/2 from @src, switch to EUR/2, send: the posting would be in + // EUR/2 while the debit was in USD/2 + prog := Program{ + Instructions: []Instruction{ + bc(Op_LoadStr, 0, 0), + bc(Op_LoadStr, 1, 1), + bc(Op_LoadStr, 2, 2), + bc(Op_LoadStr, 3, 3), + bc(Op_LoadInt, 1, 0), + bc(Op_LoadInt, 2, 1), + abc(Op_SetCurrentAsset, 0, nilReg, nilReg), + abc(Op_PullAccount, 0, 1, 1), + abc(0, 2, nilReg, nilReg), // ext + abc(Op_SetCurrentAsset, 3, nilReg, nilReg), + abc(Op_SendToAccount, 2, nilReg, nilReg), + }, + StringsPool: []string{"USD/2", "src", "dest", "EUR/2"}, + IntsPool: []big.Int{*big.NewInt(100), *big.NewInt(0)}, + } + store := mockStore{bal: map[funds.PairKey]int64{{Account: "src", Asset: "USD/2"}: 100}} + + require.NoError(t, Verify(fullBanks(prog))) + _, err := Exec(context.Background(), newTestVm(prog), nil, store) + require.Equal(t, InternalError{Err: errSetAssetWithQueuedFunds}, err) +} + +func TestEndWithQueuedFundsIsAnInternalError(t *testing.T) { + prog := Program{ + Instructions: []Instruction{ + bc(Op_LoadStr, 0, 0), + bc(Op_LoadStr, 1, 1), + bc(Op_LoadInt, 1, 0), + bc(Op_LoadInt, 2, 1), + abc(Op_SetCurrentAsset, 0, nilReg, nilReg), + abc(Op_PullAccount, 0, 1, 1), + abc(0, 2, nilReg, nilReg), // ext + }, + StringsPool: []string{"USD/2", "src"}, + IntsPool: []big.Int{*big.NewInt(100), *big.NewInt(0)}, + } + store := mockStore{bal: map[funds.PairKey]int64{{Account: "src", Asset: "USD/2"}: 100}} + + require.NoError(t, Verify(fullBanks(prog))) + _, err := Exec(context.Background(), newTestVm(prog), nil, store) + require.Equal(t, InternalError{Err: errEndWithQueuedFunds}, err) +} + +// `send [USD/2 0]` lowers to a zero-cap pull (which queues a zero-amount source) +// followed by a zero-cap send (which does not consume it): the queued zero must +// not count as funds, at the next asset switch or at the end of the run. +func TestQueuedZeroAmountIsNotQueuedFunds(t *testing.T) { + prog := Program{ + Instructions: []Instruction{ + bc(Op_LoadStr, 0, 0), + bc(Op_LoadStr, 1, 1), + bc(Op_LoadStr, 2, 2), + bc(Op_LoadInt, 0, 0), // 0 + abc(Op_SetCurrentAsset, 0, nilReg, nilReg), + abc(Op_PullAccount, 1, 1, 0), + abc(0, nilReg, nilReg, nilReg), // ext: unbounded overdraft + abc(Op_SendToAccount, 2, 0, nilReg), + abc(Op_SetCurrentAsset, 0, nilReg, nilReg), + abc(Op_PullAccount, 1, 1, 0), + abc(0, nilReg, nilReg, nilReg), // ext: unbounded overdraft + }, + StringsPool: []string{"USD/2", "world", "dest"}, + IntsPool: []big.Int{*big.NewInt(0)}, + } + + require.NoError(t, Verify(fullBanks(prog))) + res, err := Exec(context.Background(), newTestVm(prog), nil, mockStore{}) + require.Nil(t, err) + require.Empty(t, res.Postings) +} From 3b01c21b1f9799c3dbc6d7e58bbf74d9d92fc3c3 Mon Sep 17 00:00:00 2001 From: ascandone Date: Thu, 29 Oct 2026 09:42:18 +0100 Subject: [PATCH 2/3] prevent zero amounts in queue --- internal/funds/funds.go | 28 +++++++++++++--------------- internal/vm/vm_test.go | 6 +++--- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/internal/funds/funds.go b/internal/funds/funds.go index 2cd68b80..8b63b121 100644 --- a/internal/funds/funds.go +++ b/internal/funds/funds.go @@ -244,21 +244,15 @@ func (s *RunState) GetAccountBalance(account, scope, asset, color string) (*big. return new(big.Int).Set(bal), nil } -// HasQueuedFunds reports whether a queued source still holds a positive amount. -// Zero-amount entries do not count: a Pull of a zero cap queues one, and a Send -// of a zero cap does not consume it. +// HasQueuedFunds reports whether any source is still queued. Every queued +// source holds a positive amount: Pull and PullUncapped never queue a zero. func (s *RunState) HasQueuedFunds() bool { - for i := range s.sources { - if s.sources[i].amount.Sign() > 0 { - return true - } - } - return false + return len(s.sources) > 0 } // Pull debits up to cap from src's (currentAsset, color) balance, queues the -// pulled amount as a funding source tagged with color, and writes the amount made -// available into out: +// pulled amount as a funding source tagged with color (only when positive), and +// writes the amount made available into out: // // overdraft == nil -> unbounded: available = max(0, cap) // overdraft == b -> available = min(max(0, balance + max(0,b)), max(0, cap)) @@ -275,8 +269,10 @@ func (s *RunState) Pull(out *big.Int, src string, scope string, cap *big.Int, ov if out.Sign() < 0 { out.SetInt64(0) } - amt := new(big.Int).Set(out) - s.sources = append(s.sources, source{src, scope, amt, color}) + if out.Sign() > 0 { + amt := new(big.Int).Set(out) + s.sources = append(s.sources, source{src, scope, amt, color}) + } e := s.entryFor(PairKey{src, scope, s.currentAsset, color}) e.amount.Sub(&e.amount, out) return nil @@ -306,8 +302,10 @@ func (s *RunState) Pull(out *big.Int, src string, scope string, cap *big.Int, ov // an independent copy: out stays the caller's, while the queued amount is // mutated in place by compactAt/Send - amt := new(big.Int).Set(out) - s.sources = append(s.sources, source{src, scope, amt, color}) + if out.Sign() > 0 { + amt := new(big.Int).Set(out) + s.sources = append(s.sources, source{src, scope, amt, color}) + } currentBal.Sub(currentBal, out) return nil diff --git a/internal/vm/vm_test.go b/internal/vm/vm_test.go index 788a84a5..ac377b59 100644 --- a/internal/vm/vm_test.go +++ b/internal/vm/vm_test.go @@ -588,9 +588,9 @@ func TestEndWithQueuedFundsIsAnInternalError(t *testing.T) { require.Equal(t, InternalError{Err: errEndWithQueuedFunds}, err) } -// `send [USD/2 0]` lowers to a zero-cap pull (which queues a zero-amount source) -// followed by a zero-cap send (which does not consume it): the queued zero must -// not count as funds, at the next asset switch or at the end of the run. +// `send [USD/2 0]` lowers to a zero-cap pull followed by a zero-cap send, which +// never drains the queue: the zero pull must not count as queued funds, at the +// next asset switch or at the end of the run. func TestQueuedZeroAmountIsNotQueuedFunds(t *testing.T) { prog := Program{ Instructions: []Instruction{ From 9181f6f7245a04177208e1a9375a57c18f842ba5 Mon Sep 17 00:00:00 2001 From: ascandone Date: Thu, 29 Oct 2026 09:54:18 +0100 Subject: [PATCH 3/3] refactor: extract method --- internal/funds/funds.go | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/internal/funds/funds.go b/internal/funds/funds.go index 8b63b121..ec043698 100644 --- a/internal/funds/funds.go +++ b/internal/funds/funds.go @@ -269,10 +269,7 @@ func (s *RunState) Pull(out *big.Int, src string, scope string, cap *big.Int, ov if out.Sign() < 0 { out.SetInt64(0) } - if out.Sign() > 0 { - amt := new(big.Int).Set(out) - s.sources = append(s.sources, source{src, scope, amt, color}) - } + s.enqueue(src, scope, out, color) e := s.entryFor(PairKey{src, scope, s.currentAsset, color}) e.amount.Sub(&e.amount, out) return nil @@ -300,13 +297,7 @@ func (s *RunState) Pull(out *big.Int, src string, scope string, cap *big.Int, ov out.SetInt64(0) } - // an independent copy: out stays the caller's, while the queued amount is - // mutated in place by compactAt/Send - if out.Sign() > 0 { - amt := new(big.Int).Set(out) - s.sources = append(s.sources, source{src, scope, amt, color}) - } - + s.enqueue(src, scope, out, color) currentBal.Sub(currentBal, out) return nil } @@ -331,14 +322,20 @@ func (s *RunState) PullUncapped(out *big.Int, src string, scope string, overdraf out.SetInt64(0) } - if out.Sign() > 0 { - amt := new(big.Int).Set(out) - s.sources = append(s.sources, source{src, scope, amt, color}) - currentBal.Sub(currentBal, out) // debit in place; cache keeps the pointer - } + s.enqueue(src, scope, out, color) + currentBal.Sub(currentBal, out) // debit in place; cache keeps the pointer return nil } +// enqueue queues a copy of amount, so the caller keeps amount while the queued +// one is mutated in place by compactAt/Send. A zero amount is not queued. +func (s *RunState) enqueue(src, scope string, amount *big.Int, color string) { + if amount.Sign() > 0 { + out := new(big.Int).Set(amount) + s.sources = append(s.sources, source{src, scope, out, color}) + } +} + // Send drains queued funding sources in FIFO order until cap is satisfied or // eligible sources run out. Each emitted posting carries the *consumed source's* // own color.