fhevm: a verified parent is findable, whoever built it

The in-flight block set was written in exactly one place — BuildBlock — so a
block arriving from a peer was never findable by id and its child failed
Verify with "parent not found". That is the ordinary shape whenever more than
one block is in flight, so a follower could not follow.

Both paths now register through trackVerified: the block we build, and the
block we verify from a peer. The read lock a Verify holds cannot be upgraded,
so the checks finish under it and the registration takes the write lock on its
own — pendingBlocks keeps one owner, which is the property this VM already has
a test for.

trackVerified also prunes, because nothing else would. The engine may drop a
block it never accepts and never rejects, so a set that only ever grew would
leak the way an unreleased mempool claim did; anything at or below the last
accepted height is decided or orphaned, which bounds it to what is actually in
flight.

This fixes the parent lookup and not the layer under it: Verify reads nonces
from committed state, so two blocks in flight from the SAME payer still cannot
both verify. Doing that needs a per-block state overlay. keyvm has the same
limit, and the test says so rather than implying otherwise.

fhevm/block.go

@@ -101,15 +101,34 @@ func (b *Block) Verify(ctx context.Context) error {
 			ErrInvalidBlock, len(b.transactions), MaxBlockTxs)
 	}
 
-	b.vm.stateLock.RLock()
-	defer b.vm.stateLock.RUnlock()
+	if err := func() error {
+		b.vm.stateLock.RLock()
+		defer b.vm.stateLock.RUnlock()
 
-	batch := newBatch(b.vm)
-	for _, tx := range b.transactions {
-		if err := batch.admit(tx); err != nil {
-			return err
+		batch := newBatch(b.vm)
+		for _, tx := range b.transactions {
+			if err := batch.admit(tx); err != nil {
+				return err
+			}
 		}
+		return nil
+	}(); err != nil {
+		return err
 	}
+
+	// A block that verifies is one the engine may build on, so it has to be
+	// findable by id — including one parsed from a peer rather than built here.
+	// Tracking only self-built blocks meant a follower verified b1 and then
+	// failed b2 with "verify parent: not found", which is the ordinary shape
+	// whenever more than one block is in flight.
+	//
+	// The read lock above cannot be upgraded, so the checks finish under it and
+	// the registration takes the write lock on its own. pendingBlocks keeps ONE
+	// owner — reaching it under a second mutex is the bug this VM already has a
+	// test against.
+	b.vm.stateLock.Lock()
+	b.vm.trackVerified(b)
+	b.vm.stateLock.Unlock()
 	return nil
 }
 

fhevm/regression_test.go

@@ -714,3 +714,64 @@ func blockBytesWithBlobPadding(b *Block, pad []byte) []byte {
 	ob.FinishAsRoot()
 	return bld.Finish()
 }
+
+// M3: a follower could not verify a block whose parent it had only parsed. The
+// in-flight block set was written in exactly one place — BuildBlock — so a block
+// arriving from a peer was never findable by id, and its child failed with
+// "verify parent: not found" no matter what it contained.
+//
+// The parent lookup is what this fixes. Note what it does NOT fix: Verify checks
+// nonces against COMMITTED state, so two blocks in flight from the SAME payer
+// still cannot both verify — the second reads a nonce the first has not yet
+// committed. Verifying against the parent's resulting state needs a per-block
+// overlay and is a separate change; keyvm has the same limit.
+func TestM3_AFollowerCanVerifyAgainstAParentItOnlyParsed(t *testing.T) {
+	first, second := newTestKey(t), newTestKey(t)
+	committee, _ := newCommittee(t, 1)
+	proposer := newTestVM(t, fundAll(first, second), committee, 1)
+	follower := newTestVM(t, fundAll(first, second), committee, 1)
+
+	build := func(k testKey, handle string) *Block {
+		_, err := proposer.SubmitTx(registerTx(t, k, testScheme, digestOf(handle), 1))
+		require.NoError(t, err)
+		blk, err := proposer.BuildBlock(context.Background())
+		require.NoError(t, err)
+		return blk.(*Block)
+	}
+
+	b1 := build(first, "one")
+	require.NoError(t, b1.Accept(context.Background()))
+	b2 := build(second, "two") // built on b1, still in flight
+
+	// The follower accepts neither. b2 must still verify: its parent is a block
+	// the follower has only parsed.
+	p1, err := follower.ParseBlock(context.Background(), b1.Bytes())
+	require.NoError(t, err)
+	require.NoError(t, p1.Verify(context.Background()))
+
+	p2, err := follower.ParseBlock(context.Background(), b2.Bytes())
+	require.NoError(t, err)
+	require.NoError(t, p2.Verify(context.Background()),
+		"a verified parent must be findable, whoever built it")
+}
+
+// M3: the tracker must not become the leak that an unreleased claim was. The
+// engine may drop a block it never accepts and never rejects, so nothing else
+// removes it — anything at or below the last accepted height is decided or
+// orphaned and must not be retained.
+func TestM3_TheInFlightSetIsBounded(t *testing.T) {
+	k := newTestKey(t)
+	committee, _ := newCommittee(t, 1)
+	vm := newTestVM(t, fundAll(k), committee, 1)
+
+	for nonce := uint64(1); nonce <= 4; nonce++ {
+		acceptOne(t, vm, registerTx(t, k, testScheme, digestOf(string(rune('a'+nonce))), nonce))
+	}
+
+	vm.stateLock.RLock()
+	defer vm.stateLock.RUnlock()
+	for id, blk := range vm.pendingBlocks {
+		require.Greater(t, blk.height, vm.height,
+			"block %s at height %d is at or below the accepted height %d", id, blk.height, vm.height)
+	}
+}

fhevm/vm.go

@@ -765,7 +765,7 @@ func (vm *VM) BuildBlock(ctx context.Context) (chain.Block, error) {
 		vm:           vm,
 	}
 	blk.id = blk.computeID()
-	vm.pendingBlocks[blk.id] = blk
+	vm.trackVerified(blk)
 	return blk, nil
 }
 
@@ -831,6 +831,30 @@ func (vm *VM) getBlockLocked(blockID ids.ID) (*Block, error) {
 	return parseBlock(vm, b)
 }
 
+// trackVerified makes a block findable by id while it is in flight, so a child
+// can resolve it as a parent. Both the block we build and a block we verify
+// from a peer go through here — tracking only our own left a follower able to
+// verify b1 and unable to verify b2.
+//
+// It also prunes, because nothing else will: the engine may drop a block it
+// never accepts and never rejects, so a tracker that only ever grew would leak
+// exactly the way an unreleased mempool claim did. Anything at or below the
+// last accepted height is already decided or orphaned, which bounds this to the
+// blocks actually in flight above it.
+//
+// Caller holds stateLock for writing.
+func (vm *VM) trackVerified(b *Block) {
+	if b.height <= vm.height {
+		return
+	}
+	for id, blk := range vm.pendingBlocks {
+		if blk.height <= vm.height {
+			delete(vm.pendingBlocks, id)
+		}
+	}
+	vm.pendingBlocks[b.id] = b
+}
+
 // ---- ChainVM lifecycle / misc ----
 
 func (vm *VM) SetState(ctx context.Context, state uint32) error { return nil }