PR Review: refactor/PVM-080-Appendix-AB → 1012-update-to-v080

Summary

This PR implements the PVM changes for Gray Paper v0.8.0, including block-level gas accounting, host-call gas, grow_heap, opcode changes, and interpreter/recompiler integration. It is not ready to merge: the retained critical comments are consensus or availability defects introduced by this branch, or incomplete implementations of features explicitly updated by the PR. Findings that merely pre-date this branch, duplicate another comment, or are optional clean-up have been excluded.

Specification references: Gray Paper v0.8.0 Appendix A and Appendix B.

Files Reviewed

All 74 changed files were reviewed. The 14 files below contain retained comments; the other 60 have no retained comments.

Verification

• go test ./PVM/...: passed for packages available on Windows.
• go vet ./PVM/...: passed.
• gofmt -s -l PVM cmd: reported six changed Go files.
• Linux/amd64/cgo recompiler tests were not run locally because the Docker daemon was unavailable.
• The PR currently has no GitHub checks.

File: PVM/block_info.go

[CRITICAL] Lines 149-193: Reject a final basic block without a terminator

Problem:
The new decoder deliberately emits an open block and returns ExitContinue when the instruction data ends without a terminator. Gray Paper v0.8.0 Appendix A.2 defines v_blob to return false when the final instruction reaches the end and is not in T. The base branch rejected this case, so this is a regression introduced by the refactor rather than a feature.

Current code:

// If code ends without a terminator, the prefix is still emitted; bad PCs panic
// at execution. Mid-stream 𝔳_inst failures remain fatal.
// ...
if pc >= n {
	// Code ended mid-block (no terminator): keep the prefix block.
	if blockInstrStart < len(p.Instrs) {
		emitBlock(int(p.Instrs[len(p.Instrs)-1].PC))
	}
	return ExitContinue
}

Suggested fix:

if pc >= n {
	if pc == n && len(p.Instrs) > 0 &&
		IsBlockTerminator(p.Instrs[len(p.Instrs)-1].Opcode) {
		return ExitContinue
	}
	return ExitPanic
}

Keep emitting blocks only when a terminator is encountered. Add regression tests for a final non-terminator and a valid final terminator.

File: PVM/invocation.go

[CRITICAL] Lines 226-232: Charge the whole containing block for a fresh mid-block entry

Problem:
blockGasAtPC simulates only the suffix beginning at pc when execution starts in the middle of a basic block. Appendix A.4 instead charges gascostforblock(c, k, L(pc)), where L(pc) is the start of the containing block. deblob permits a valid instruction entry in the middle of a block, so this changes consensus gas for a legal initial invocation. PVM/recompiler/compiler.go:430-433 repeats the same suffix rule.

Current code:

func blockGasAtPC(prog *Program, pc ProgramCounter, block *BlockMeta) Gas {
	if pc == block.StartPC {
		return block.GasCost
	}
	return GasCostFromPC(prog, pc)
}

Suggested fix:

func blockGasAtPC(_ *Program, _ ProgramCounter, block *BlockMeta) Gas {
	return block.GasCost
}

The recompiler should likewise obtain BlockContaining(pc).GasCost rather than call GasCostFromPC. Resuming with gaschargedflag = true already prevents a second charge; it does not require suffix gas.

[CRITICAL] Lines 209-223: Preserve a charged flag after a fault at the first instruction

Problem:
The helper discards a stored gaschargedflag = true whenever the saved PC is a basic-block entry. That state is valid when the first instruction of a block faults after the block was pre-charged. Appendix A.4 preserves the flag for fault, and Appendix B.6 stores it in the integrated PVM. Clearing it makes the next invoke charge the same block again.

Current code:

func gasChargedForIntegratedResume(prog *Program, pc ProgramCounter, stored bool) bool {
	if !stored || prog == nil {
		return false
	}
	blockStart, ok := prog.StartOfBasicBlock(pc)
	if !ok {
		return false
	}
	if pc == blockStart {
		return false
	}
	return true
}

Suggested fix:

func gasChargedForIntegratedResume(prog *Program, pc ProgramCounter, stored bool) bool {
	return stored && prog != nil && prog.ValidInstructionAt(uint64(pc))
}

Add a test where the first instruction faults, the page is mapped, and the integrated machine resumes without another block charge.

[CRITICAL] Lines 298-307 and 432-436: Do not identify a taken self-loop by newPC != currentPC

Problem:
A valid jump or branch may target its own instruction. Both execution paths treat newPC == currentPC as ordinary fall-through, so a legal self-loop advances to the next instruction instead of looping and charging the block again. Whether control flow was produced by a terminator must be determined from the opcode, not by comparing the two PCs.

Current code:

if instr.PC != newPC {
	pc = newPC
	branchTaken = true
	break
}

if !branchTaken {
	last := &instrs[len(instrs)-1]
	pc = last.PC + ProgramCounter(last.SkipLen) + 1
}

if pc != newPC {
	return newPC, exitReason
}

pc += skipLength + 1

Suggested fix:

if IsBlockTerminator(instr.Opcode) {
	pc = newPC
	branchTaken = true
	break
}

Apply the equivalent opcode-based rule in ExecuteInstructions and DebugSingleStepInvoke. Add taken jump and taken branch tests whose target equals the terminator PC.

File: PVM/recompiler/compiler.go

[CRITICAL] Lines 376-380: Clear GasCharged only for CONTINUE or HOST_CALL

Problem:
The compiler clears GasCharged before every terminator executes. Appendix A.4 clears it only when a terminator produces CONTINUE or HOST_CALL; it remains true for PANIC, HALT, and faults. The interpreter applies the conditional rule, so this also creates backend divergence.

Current code:

for i := range instrs {
	instr := &instrs[i]
	if i == len(instrs)-1 && PVM.IsBlockTerminator(instr.Opcode) {
		emitGasCharged(a, false)
	}

	handler := opcodeHandlers[instr.Opcode]
	// ...
}

Suggested fix:
Remove the unconditional write from the compiler loop. Emit GasCharged = false in terminator paths that actually continue or exit as a host call, such as successful branch/jump/fall-through transfers and ecalli. Trap and halt paths must retain the charged flag.

// Example in a CONTINUE/HOST_CALL exit path:
emitGasCharged(a, false)
emitExitToPC(a, targetPC, reason)

Add backend-consistency assertions for the final flag after trap, halt, page fault, and host call.

File: PVM/gas_opcode.go

[CRITICAL] Lines 32-50: Treat branch targets beyond the code as zero-padded traps

Problem:
Appendix A defines instructions = c || [0, 0, ...]. Therefore, a branch target or fall-through beyond len(c) observes opcode zero (trap), and the Appendix A.10 branch cost is one cycle. isTrapOrUnlikely returns false for out-of-range PCs, causing a cost of 20 cycles instead.

Current code:

func isTrapOrUnlikely(p *Program, pc int) bool {
	if pc < 0 || pc >= len(p.InstructionData) {
		return false
	}
	op := p.InstructionData[pc]
	return op == 0 || op == 2
}

Suggested fix:

func isTrapOrUnlikely(p *Program, pc int) bool {
	if pc < 0 || pc >= len(p.InstructionData) {
		return true // zero-padded opcode is trap
	}
	op := p.InstructionData[pc]
	return op == 0 || op == 2
}

Add gas-vector tests for both an out-of-range taken target and an out-of-range fall-through target.

File: PVM/gas_sim.go

[CRITICAL] Lines 254-277: Never return a partial gas result at an arbitrary step limit

Problem:
The simulator returns the current cycle count after 100,000 state transitions even when the pipeline has not converged. A valid large block containing serial high-latency instructions can exceed this limit, so its block gas is silently undercharged. Appendix A.9 defines the result only at the converged final state.

Current code:

const maxSteps = 100000
for step := 0; step < maxSteps; step++ {
	// ...
	if b.Iota == iotaNone && b.robActiveCount() == 0 {
		return blockGasFromCycles(b.Cyc)
	}
	b.advanceCycle()
}
// Safety fallback for malformed simulation state.
return blockGasFromCycles(b.Cyc)

Suggested fix:

for {
	// existing decode/start/advance transitions
	if b.Iota == iotaNone && b.robActiveCount() == 0 {
		return blockGasFromCycles(b.Cyc)
	}
}

If an invariant guard is required, make it an explicit error or panic rather than a valid-looking gas value. Add a generated legal block that needs more than 100,000 simulator transitions and compare it with the reference model.

[CRITICAL] Lines 42-49 and 137-245: Bound the physical ROB storage instead of retaining every retired entry

Problem:
The implementation appends one robEntry per decoded instruction and only changes retired entries to robNone; it never removes or reuses them. Every subsequent decode, readiness check, and cycle scans the entire historical slice. A long valid basic block therefore makes deblob/gas precomputation quadratic in untrusted program size, before PVM gas can bound execution.

Current code:

func (b *BlockState) robActiveCount() int {
	n := 0
	for _, e := range b.ROB {
		if e.state != robNone {
			n++
		}
	}
	return n
}

b.ROB = append(b.ROB, robEntry{
	state:      robDEC,
	cyclesLeft: cost.Cycles,
	deps:       deps,
	regs:       dstSet,
	units:      cost.Units,
})

Suggested fix:
Represent the ROB as a fixed 32-slot ring or another bounded structure. Reuse retired slots, keep dependency identifiers stable until retirement, and ensure all operations are proportional to the maximum active ROB size rather than the total block length.

type BlockState struct {
	ROB [MaxROB]robEntry
	// head/tail or free-slot bookkeeping
}

Add a benchmark and a timeout-backed test for a near-maximum valid single block to prevent quadratic regressions.

File: PVM/host_call_general.go

[CRITICAL] Lines 482-508: Implement grow_heap's explicit gas outcomes

Problem:
grow_heap has exceptional gas rules in Appendix B.5. For a no-growth or invalid request it returns CONTINUE after subtracting only the constant cost, even if that produces negative signed gas. For a valid growth request with insufficient gas it returns OOG with the original gas unchanged. chargeGasAndCheck instead returns OOG in both low-gas cases and leaves the gas deducted.

Current code:

if n <= h || n > b {
	if result := chargeGasAndCheck(&input, HostGasGrowHeapConst); result != nil {
		input.VM.Registers[7] = h
		return *result
	}
	input.VM.Registers[7] = h
	return OmegaOutput{ExitReason: ExitContinue, Addition: input.Addition}
}

g := HostGasGrowHeapConst + Gas(n-h)*HostGasGrowHeapPage
if result := chargeGasAndCheck(&input, g); result != nil {
	input.VM.Registers[7] = h
	return *result
}

Suggested fix:

if n <= h || n > b {
	*input.VM.Gas -= HostGasGrowHeapConst
	input.VM.Registers[7] = h
	return OmegaOutput{ExitReason: ExitContinue, Addition: input.Addition}
}

g := HostGasGrowHeapConst + Gas(n-h)*HostGasGrowHeapPage
if *input.VM.Gas < g {
	input.VM.Registers[7] = h
	return OmegaOutput{ExitReason: ExitOOG, Addition: input.Addition}
}
*input.VM.Gas -= g
input.VM.Mem.GrowHeapTo(n)

Cover all three branches with gas below, equal to, and above the relevant cost.

[CRITICAL] Lines 1108-1146: Serialise the v0.8.0 fetch(0) constant sequence

Problem:
The sequence still contains four constants that existed in v0.7.2 but were removed from the v0.8.0 fetch definition: TicketsPerValidator, ValidatorsCount, ECBasicSize, and ECPiecesPerSegment. This adds 12 bytes and shifts every later field, so guests decode incorrect constants. This is an incomplete v0.8 host-call update, not a new extension.

Current code:

getPtr(types.U32(types.MaxLookupAge)),                  // L
getPtr(types.U16(types.TicketsPerValidator)),           // N
getPtr(types.U16(types.AuthPoolMaxSize)),               // O
// ...
getPtr(types.U16(types.ValidatorsCount)),                // V
getPtr(types.U32(types.MaxIsAuthorizedCodeSize)),        // W_A
// ...
getPtr(types.U32(types.ECBasicSize)),                    // W_E
getPtr(types.U32(types.MaxImportCount)),                 // W_M
getPtr(types.U32(types.ECPiecesPerSegment)),             // W_P

Suggested fix:
Remove those four entries and add a byte-for-byte fixture for the complete Appendix B.5 sequence, rather than testing individual values only.

getPtr(types.U32(types.MaxLookupAge)),              // L
getPtr(types.U16(types.AuthPoolMaxSize)),           // O
// ...
getPtr(types.U32(types.MaxIsAuthorizedCodeSize)),   // W_A
getPtr(types.U32(types.MaxTotalSize)),              // W_B
getPtr(types.U32(types.MaxServiceCodeSize)),        // W_C
getPtr(types.U32(types.MaxImportCount)),            // W_M

File: PVM/guest_memory.go

[CRITICAL] Lines 84-102: Reserve the major guard zone in the grow_heap upper bound

Problem:
HeapMaxPages returns heapLimit / ZP, where heapLimit is stackStart. Appendix B.5 defines b one major zone below that point:

b = (2^32 - 3*ZZ - ZI - P(s)) / ZP

The initialiser's stackStart is 2^32 - 2*ZZ - ZI - P(s), so the current bound exposes 16 extra pages and allows the heap to consume the required guard zone. The recompiler implementation has the same error.

Current code:

func (p pagedGuestMemory) HeapMaxPages() uint64 {
	return p.mem.heapLimit / uint64(ZP)
}

Suggested fix:

func (p pagedGuestMemory) HeapMaxPages() uint64 {
	return (p.mem.heapLimit - uint64(ZZ)) / uint64(ZP)
}

Apply the same formula in PVM/recompiler/guest_memory.go, and add a cross-backend test that rejects b + 1 while accepting b.

File: PVM/host_call_refine.go

[CRITICAL] Lines 128-149: Check machine capacity first and return FULL

Problem:
Appendix B.6 gives the 63-machine capacity condition priority over reading the guest program and requires result FULL. The new limit check occurs after the memory access and returns HUH. A full machine map with an unreadable (po, pz) therefore panics instead of returning FULL, and a readable input returns the wrong code.

Current code:

if !input.VM.Mem.IsReadable(po, pz) {
	input.VM.Registers[7] = OOB
	return OmegaOutput{
		ExitReason: ExitPanic,
		Addition:   input.Addition,
	}
}

if uint64(len(input.Addition.IntegratedPVMMap)) >= 63 {
	input.VM.Registers[7] = HUH
	return OmegaOutput{ExitReason: ExitContinue, Addition: input.Addition}
}

Suggested fix:

if uint64(len(input.Addition.IntegratedPVMMap)) >= 63 {
	input.VM.Registers[7] = FULL
	return OmegaOutput{ExitReason: ExitContinue, Addition: input.Addition}
}
if !input.VM.Mem.IsReadable(po, pz) {
	return OmegaOutput{ExitReason: ExitPanic, Addition: input.Addition}
}

Add a test with 63 machines and an invalid outer-memory range to verify both precedence and the result code.

File: PVM/host_call_accumulate.go

[CRITICAL] Lines 439-507: Charge the complete transfer gas before mutating context

Problem:
The function charges HostGasTransfer, mutates balances, updates shared account state, and appends the deferred transfer before checking whether the remaining gas covers l. An OOG result can therefore return with a partially committed accumulation context, contrary to Appendix B's default rule that host state remains unchanged when gascounter < g. This PR changes transfer gas for v0.8.0, so the total M_T + l must be handled atomically.

Current code:

if result := chargeGasAndCheck(&input, HostGasTransfer); result != nil {
	return *result
}

// ... balance and deferred-transfer mutations ...

if uint64(*input.VM.Gas) < l {
	*input.VM.Gas = 0
	return OmegaOutput{
		ExitReason: ExitOOG,
		Addition:   input.Addition,
	}
}
*input.VM.Gas -= Gas(l)

Suggested fix:

d, a, l, o := input.VM.Registers[7], input.VM.Registers[8],
	input.VM.Registers[9], input.VM.Registers[10]

cost := addGas(HostGasTransfer, gasFromUint64(l))
if result := chargeGasAndCheck(&input, cost); result != nil {
	return *result
}

// Perform validation and context mutation only after the total gas check.

Add an OOG regression test that snapshots every affected account and deferred-transfer collection and asserts no mutation.

File: PVM/backend_consistency_test.go

[CRITICAL] Lines 26-50: Commit or deterministically generate the required blob corpus

Problem:
The new Linux/amd64/cgo test unconditionally reads PVM/testdata/psi_a_consistency/blobs and requires at least 30 decodable blobs, but that directory is absent from the PR. Consequently, the test fails on every supported environment before comparing either backend.

Current code:

codes, err := loadProgramCodes(backendConsistencyBlobDir)
if err != nil {
	t.Fatal(err)
}
if len(codes) < backendConsistencyMinBlobs {
	t.Fatalf("need >= %d decodable program blobs, found %d in %s",
		backendConsistencyMinBlobs, len(codes), backendConsistencyBlobDir)
}

Suggested fix:
Commit a reviewed deterministic corpus under the referenced path, or add a deterministic test-fixture generation step whose inputs are present in the repository. Do not skip or silently weaken the minimum: this test is intended to protect a consensus-critical backend boundary.

[SUGGESTION] Lines 55-64: Fail when both backends panic

Problem:
When both backends panic, the case is logged and treated as passing. This establishes only that both implementations share a crash, not that they agree on a valid PVM result. A common decoder or host-call defect can therefore make the complete corpus pass without testing gas or exit results.

Current code:

if panicI != panicR {
	t.Fatalf("panic mismatch\n  interpreter: %v\n  recompiler:  %v", panicI, panicR)
}
if panicI != "" {
	t.Logf("both panicked: %v", panicI)
	return
}

Suggested improvement:

if panicI != "" || panicR != "" {
	t.Fatalf("backend panic\n  interpreter: %q\n  recompiler:  %q", panicI, panicR)
}

If deliberately malformed fixtures are needed, classify them separately and assert a protocol-level PANIC result rather than a Go runtime panic.

File: PVM/gas_vectors_test.go

[SUGGESTION] Lines 52-64 and 181-217: Execute multistep vector actions instead of checking block metadata only

Problem:
The harness parses only run and assert, omits vector actions such as memory mapping/writes, and TestGasModelMultistepVectors merely calls assertBlockGasCosts. It therefore does not execute the ecalli/page-fault/resume sequences that the multistep vectors are designed to validate, including gaschargedflag behaviour.

Current code:

Steps []struct {
	Run    *struct{} `json:"run"`
	Assert *struct {
		Gas    uint64 `json:"gas"`
		Status string `json:"status"`
	} `json:"assert"`
} `json:"steps"`

func TestGasModelMultistepVectors(t *testing.T) {
	runGasModelProgramPrefix(t, "multistep_")
}

Suggested improvement:
Model every action used by the upstream schema, maintain VM state across steps, execute each run, apply map/write operations, and assert status, PC, gas, registers, memory, and charged-flag state at each assertion.

for _, step := range vec.Steps {
	switch {
	case step.Map != nil:
		applyMap(&state, *step.Map)
	case step.Write != nil:
		applyWrite(&state, *step.Write)
	case step.Run != nil:
		runUntilExit(&state)
	case step.Assert != nil:
		assertState(t, state, *step.Assert)
	}
}

File: PVM/execution_backend.go

[SUGGESTION] Lines 7-40: Avoid a process-global temporary backend override

Problem:
WithExecutionBackend mutates and later restores a process-global string without synchronisation. Concurrent tests or invocations can race, observe another goroutine's temporary backend, and restore stale values in the wrong order. Adding atomics alone would remove the data race but not the cross-talk.

Current code:

func WithExecutionBackend(backend string, fn func()) error {
	prev := ExecutionBackend
	if err := SetExecutionBackend(backend); err != nil {
		return err
	}
	defer func() { ExecutionBackend = prev }()
	fn()
	return nil
}

Suggested improvement:
Pass the selected backend explicitly to an invocation/runner object and dispatch directly, so backend choice is scoped to one call. If the global selector must remain for process configuration, reserve SetExecutionBackend for startup and do not provide a temporary global override for concurrent tests.

runner, err := NewRunner(BackendRecompiler)
if err != nil {
	return err
}
result := runner.PsiM(args)

File: PVM/recompiler/guest_memory.go

[SUGGESTION] Lines 242-255: Do not report successful heap growth after mprotect fails

Problem:
Every SetPageAccess error is discarded and the heap pointer is updated regardless. If mprotect fails, grow_heap reports success for pages that remain inaccessible, and the JIT page-permission map may disagree with the returned heap size.

Current code:

if newHP > uint64(oldBound) {
	for addr := uint32(oldHP); addr < uint32(newBound); addr += PVM.ZP {
		_ = ctx.SetPageAccess(addr/PVM.ZP, unix.PROT_READ|unix.PROT_WRITE)
	}
}
ctx.WriteHeapPointer(newHP)

Suggested improvement:
Change GrowHeapTo to return an error, propagate failures through the shared GuestMemory interface, and update the heap pointer only after all requested page protections succeed.

func (g jitGuestMemory) GrowHeapTo(targetPage uint64) error {
	// ...
	if err := ctx.SetPageAccess(page, unix.PROT_READ|unix.PROT_WRITE); err != nil {
		return err
	}
	// ...
	ctx.WriteHeapPointer(newHP)
	return nil
}

File: PVM/docs/4_HostCall_Integration.md

[SUGGESTION] Lines 175-215 and 274-279: Remove the obsolete sbrk execution path

Problem:
This PR removes the sbrk opcode and its recompiler handler, but the modified integration document still describes sbrk as an internal sentinel, includes a dedicated handling section, and lists 0xFF in the exit table. Related stale references remain in 1_Recompiler_Workflow.md, 2_x86_Assembler.md, 6_PVMtrace.md, and docs/TODO.md. These descriptions now contradict both v0.8.0 and the implementation.

Current code:

switch exitReason:
  CONTINUE → next block
  sbrk/djump → handled internally
  other → return to host

## 6. Special handling for sbrk

sbrk is handled through `SbrkCallID = 0xFF` rather than omega dispatch.

Suggested improvement:
Remove the sbrk section and sentinel row, update the flow diagrams to contain only the remaining djump miss path, and search all modified PVM documentation for stale sbrk, opcode 101, and per-instruction gas descriptions.

Keep unchanged:

1. Mid-block gas (suffix): When resuming mid-block with gaschargedflag = ⊥, we charge GasCostFromPC(ι) for the remaining suffix, not the full containing basic block. A literal “whole-block” charge would double-bill instructions already executed before the interrupt/resume and diverge from the koute gas vectors. It also matches the recompiler, which compiles and pre-charges only the suffix from the resume PC.
2. Charge the complete transfer gas before mutating context
   GP defined g = M_T + t. Our implementation charges M_T first, then t, as required by the v0.7.2 conformance tests.
3. File: PVM/backend_consistency_test.go
   The blobs are legacy 0.7.2 programs used only for best-effort interpreter/recompiler consistency under 0.8.0. Shared Go panics there are expected until official 0.8.0 vectors land; we’ll tighten this check then.

@TwEricShen Thanks for the response. I re-checked these points against the immutable Gray Paper v0.8.0 source, the v0.7.2 tag, the Koute vectors, and the PolkaVM implementation which generated those vectors.

1. Mid-block gas: the suffix rule is not supported

This remains a blocking correctness issue.

A.4 explicitly charges:

gascostforblock(c, k, L(i))

and states that no instruction may execute until the gas cost for the entire basic block has been charged. A.9 likewise defines gascostforblock for an instruction index i ∈ basicblocks; L(i) normalises an interior PC to that block entry.

The double-billing argument does not hold because a conforming interruption after executing part of a block does not produce gaschargedflag = false:

• page faults preserve true;
• ecalli is not a terminator and preserves true;
• a terminator clears the flag only when its continuation enters the next basic block;
• an OOG pre-charge failure executes no instruction.

A legal mid-block/false state does exist when machine creates an inner PVM at an arbitrary valid instruction, but A.4 requires the full containing-block cost in that case.

The Koute vector gas_start_execution_in_the_middle_of_block.json does not distinguish these interpretations: its only block has full cost 3, while starting at its interior PC also happens to produce suffix cost 3. The multistep page-fault/ecalli vectors preserve the already-charged state and therefore do not charge a suffix. PolkaVM's resolve_arbitrary_jump/extract_target_and_gas scans back to the containing block's charge handler and retrieves the full block cost:
https://github.com/paritytech/polkavm/blob/c1b00b81cdee1c536b760df369e6e2936bb3f450/crates/polkavm/src/interpreter.rs#L970-L1034

The newly added host-call resume tests manually reset GasCharged = false after ecalli and describe the flag as cleared, which is contrary to A.4 and to the implementation's own normal continuation. They therefore test an artificial state against GasCostFromPC, rather than validating the GP rule.

Please make both blockGasAtPC and recompiler blockGasCostAt use the containing BlockMeta.GasCost whenever the flag is false. Compiling only the suffix is correct for execution; it does not imply charging only the suffix.

2. transfer: the ordering rationale is incorrect, but I am revising the severity

The v0.7.2 tag already specifies g = 10 + t, with t = 0 for PANIC/WHO/LOW/CASH and t = l only for OK. It also applies the accumulate-function default that OOG leaves registers, memory, X, and Y unchanged. v0.8.0 has the same structure with M_T + t.

The current code deducts M_T, mutates the sender account and deferred-transfer list, and only then checks l. This does not implement either version's Ω_T transition, and conformance vectors do not require the gas deduction to be split in that order.

However, after tracing the complete Ψ_A composition, I am downgrading this from consensus-critical to an important correctness suggestion: on OOG, C selects the separate Y-context, and R saturates negative remaining gas with max(gas, 0). Thus I have not established a final consensus-state or gas-used difference through normal Ψ_A, although the raw host result/context and traces remain non-conformant.

The precise fix is not to charge M_T + l unconditionally at entry, since failure cases have t = 0. It should be: classify c,t without mutation, check/deduct M_T + t, and commit the account/deferred-transfer changes only after that succeeds.

3. Backend corpus: original Critical resolved; panic handling remains a suggestion

I accept the missing-corpus point as resolved now that a clean checkout skips this local-only test.

A shared Go runtime panic is still not a valid consistency result. Protocol PANIC is an ordinary PVM exit value; recover() here catches a host-process panic. The v0.7.2 host-ID shift can explain why legacy programs enter unsuitable handlers, but it does not make a shared process crash equivalent to backend agreement. Since the test is best-effort and skipped in CI, I consider this a coverage/robustness suggestion rather than a merge blocker.

Separate blockers in the update

Two independent issues remain in addition to the mid-block gas defect:

1. PVM/review_fixes_test.go duplicates seven test functions and newGrowHeapTestMem already added to the individual test files. The package currently fails to compile with redeclaration errors.
2. gas_sim still has an arbitrary 100,000-transition limit. Replacing the partial result with panic removes silent undercharging but turns it into an unrecovered process panic during deblob. A valid block containing about 1,613 serial DIV/REM instructions plus a terminator is only about 5.5 KB and exceeds this limit, far below both W_A = 64,000 and W_C = 4,000,000. The cap is therefore reachable by valid protocol input and cannot be treated as a logic invariant.

trimLeadingNone does resolve the separate retained-ROB O(N²) issue.

Current merge-blocking conclusions are therefore: full containing-block charging for fresh mid-block entry, removal/correct handling of the reachable gas-simulator cap, and the duplicate-test compile failure.

@YCC3741 Thanks for the follow-up — really helpful, especially the re-check against GP / PolkaVM / the Koute vectors.

1. Mid-block gas — Fixed. Interpreter and recompiler now always charge the full containing block (L(ι)). Also removed the resume tests that artificially cleared GasCharged after ecalli.

2. Transfer — Charge M_T first; on OK only, charge t = l before mutating state.

3. Backend corpus / panic — Skip is fine. Clarification on the earlier wording: our local dual-backend rerun showed matching protocol PANIC exits, not shared Go recover() crashes — sorry for the imprecise language.

Separately, we built a small real Ψ_A corpus from 0.7.2 jam-conformance fuzz traces. During STF replay we dump cases that actually enter accumulate / Psi_M (not synthetic empty arguments). In practice most accumulates exit with protocol PANIC, so we only kept and pushed a small set of workable cases (code blob + serialized Ψ_A argument / input under PVM/consistent-testdata/). These are exercised by TestInterpreterVsRecompilerPsiADump in PVM/backend_consistency_test.go (interpreter vs recompiler via Psi_M_OnBackend, comparing Gas + ReasonOrBytes).

How to run (linux/amd64 + CGO):

make test-backend-consistency
# or
CGO_ENABLED=1 go test -count=1 -v ./PVM/ -run TestInterpreterVsRecompilerPsiADump

On that dump-driven suite, both backends agree on every case; with the current minimal host-call fixture the observed PVM exits are again protocol PANIC. (In the original STF dump path several of these accumulates had completed with Halt, including two with a non-nil accumulate Result; the dual-backend test is intentionally a lightweight Psi_M consistency check, not a full STF host-state replay.)

4. review_fixes_test.go — Removed (duplicate declarations).

5. gas_sim cap — Removed maxSteps; sim runs to convergence. Added a long serial-DIV regression. Keeping trimLeadingNone for the ROB O(N²) case.

@TwEricShen Thanks — I re-checked the latest head and the earlier transfer OOG/commit-order issue is resolved: M_T is charged first, l is charged only on the OK path, and state is mutated afterwards.

There is one separate transfer conformance issue remaining, but I want to be explicit that it is pre-existing and was not introduced by this PR.

The destination register d is 64-bit, while types.ServiceID is 32-bit. The current lookup and deferred-transfer construction convert it before validation:

ServiceAccounts[types.ServiceID(d)]
ReceiverID: types.ServiceID(d)

Consequently, if service 7 exists, d = 2^32 + 7 aliases it and may return OK/commit a transfer to service 7. GP v0.8.0 instead takes the full register value as dest and returns WHO when dest ∉ keys(accounts); no modulo/truncation conversion is specified.

I traced this historically. The cast first appeared in the original accumulate host-call implementation in a343d4bf, merged through #377 in March 2025. At that time ServiceId was already U32, the PVM register was 64-bit, and GP v0.6.4 already specified WHO for d ∉ keys(accounts). The direct cast appears to have been a mechanical conversion needed for the Go map key; I found no review discussion or specification basis for wrapping. It then survived the HostCallArgs refactor, file split, and ServiceId → ServiceID rename unchanged.

Suggested fix: before any types.ServiceID(d) conversion, return WHO when d > math.MaxUint32; then perform the lookup/construction with the validated value. A regression test should use an existing low-32-bit destination and d = 2^32 + destination to ensure it returns WHO and leaves state/transfers unchanged.

So I am not attributing this as a regression to #1046, but it does prevent the touched transfer implementation from being fully GP v0.8.0-conformant.