fix(vm): fail closed when pulled funds are left in the queue

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

instruction-encoding.md

@@ -27,7 +27,7 @@ Instructions are **4 bytes** wide: `[Opcode: 8] [A: 8] [B: 8] [C: 8]`.
     <tr>
       <td>0</td><td><code>0x00</code></td><td><strong>SET_CURRENT_ASSET</strong></td>
       <td>asset</td><td>-</td><td>-</td>
-      <td>Sets the current asset (used by <code>PULL_ACCOUNT</code> / <code>SEND_TO_ACCOUNT</code>) from <code>str_regs[A]</code></td>
+      <td>Sets the current asset (used by <code>PULL_ACCOUNT</code> / <code>SEND_TO_ACCOUNT</code>) from <code>str_regs[A]</code>. Traps while a mark is open, or while the source queue still holds funds pulled under the previous asset</td>
     </tr>
     <tr>
       <td>1</td><td><code>0x01</code></td><td><strong>ASSERT_SAME_ASSET</strong></td>

internal/funds/funds.go

@@ -244,9 +244,15 @@ func (s *RunState) GetAccountBalance(account, scope, asset, color string) (*big.
 	return new(big.Int).Set(bal), nil
 }
 
+// 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 {
+	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))
@@ -263,8 +269,7 @@ 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})
+		s.enqueue(src, scope, out, color)
 		e := s.entryFor(PairKey{src, scope, s.currentAsset, color})
 		e.amount.Sub(&e.amount, out)
 		return nil
@@ -292,11 +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
-	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
 }
@@ -321,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.

internal/vm/export_test.go

@@ -1,3 +1,7 @@
 package vm
 
-var ErrSendWhileMarkOpen = errSendWhileMarkOpen
+var (
+	ErrSendWhileMarkOpen       = errSendWhileMarkOpen
+	ErrSetAssetWithQueuedFunds = errSetAssetWithQueuedFunds
+	ErrEndWithQueuedFunds      = errEndWithQueuedFunds
+)

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

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)
+}

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) {

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 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{
+			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)
+}