Ensure Non-Validators and Validators Bootstrap Before Indexing and Verifying Blocks by samliok · Pull Request #573 · ava-labs/Simplex

samliok · 2026-09-01T22:56:52Z

Addresses #530 but for non-validator and validators.

Nodes will first need to complete bootstrapping before being able to index/verify blocks.

yacovm · 2026-09-09T19:51:37Z

Looks like this test demonstrates we have a liveness problem in case we miss the first broadcast we send via broadcastLatestEpoch:

// TestNonValidatorBootstrapsWhenStartupBroadcastIsLost asserts a non-validator still bootstraps when
// nobody receives the replication request it broadcasts on start. The non-validator joins an empty
// network, so that broadcast reaches no one, and the validator only comes online afterwards. Until it
// bootstraps, the non-validator drops every block and finalization the validator sends it, so the only
// way for it to recover is to ask again.
func TestNonValidatorBootstrapsWhenStartupBroadcastIsLost(t *testing.T) {
	validator := newNodeMapping(1)
	pChain := newTestPChain([]metadata.NodeBLSMapping{validator})
	network := newNetwork(t, pChain)

	// The non-validator starts alone, so the replication request it broadcasts on start is lost.
	nonValidator := newNodeMapping(2)
	node := network.addNode(nonValidator.NodeID[:])
	isValidator, bootstrapped := node.role()
	require.False(t, isValidator)
	require.False(t, bootstrapped)

	// The validator then comes online and commits the first block on its own,
	// broadcasting its finalization to the non-validator, which drops it.
	network.addNode(validator.NodeID[:]).sync()

	// The validator is there to answer, so the non-validator must eventually ask again and bootstrap.
	require.Eventually(t, func() bool {
		_, bootstrapped := node.role()
		return bootstrapped
	}, 20*time.Second, 100*time.Millisecond, "the non-validator never bootstrapped after its startup broadcast was lost")

	node.sync()
}

yacovm · 2026-09-09T20:00:35Z

In the issue #530 I wrote:

What we should do instead is when we bootstrap the node, we replicate only the sealing blocks backwards, until we reach the latest committed sealing block, and then we keep the chain of sealing blocks and use them and never replicate them by verifying quorum certificates.

The below test demonstrates that that's not what we do:

// TestNonValidatorRejectsSealingBlockOffTheHashChain asserts a bootstrapping non-validator only accepts
// sealing blocks that lie on the backward hash chain from the sealing block it bootstrapped on, however
// well they are signed. See https://github.com/ava-labs/Simplex/issues/530.
//
// The node has committed epoch 1's defining block. The current validator set tells it the latest sealing
// block is at seq 3, whose PrevSealingBlockHash names an honest sealing block at seq 2. A peer then serves
// a forged sealing block at seq 2 that carries a valid quorum certificate of epoch 1's validators but is
// not the block seq 3 points to. Epoch 1's keys may have leaked long ago, so the forged block must be
// rejected. Today the node validates it by its quorum certificate alone and indexes it.
func TestNonValidatorRejectsSealingBlockOffTheHashChain(t *testing.T) {
	validator := newNodeMapping(1)
	validators := metadata.NodeBLSMappings{validator}
	signers := []common.NodeID{validator.NodeID[:]}
	sigAggregator := &testutil.TestSignatureAggregator{N: len(validators)}

	pChain := newTestPChain(validators)
	storage, epochBlock := newChainStorage(t, validators)

	// A node outside the validator set whose ledger ends at epoch 1's defining block.
	nonValidator := newNodeMapping(2)
	node := newNetwork(t, pChain).addNodeWithConfig(nonValidator.NodeID[:], nodeConfig{storage: storage})
	isValidator, bootstrapped := node.role()
	require.False(t, isValidator)
	require.False(t, bootstrapped)

	// sealingBlock builds a sealing block at seq in the given epoch, keeping the same validator set.
	sealingBlock := func(seq, epoch uint64, prev, prevSealingBlockHash [32]byte, payload string) *ParsedBlock {
		timestamp := epochBlockTime.Add(time.Duration(seq) * time.Millisecond)
		return &ParsedBlock{StateMachineBlock: metadata.StateMachineBlock{
			InnerBlock: &testInnerBlock{Height_: seq, TS: timestamp, Payload: []byte(payload)},
			Metadata: metadata.StateMachineMetadata{
				Timestamp:               uint64(timestamp.UnixMilli()),
				SimplexProtocolMetadata: common.ProtocolMetadata{Epoch: epoch, Round: seq, Seq: seq, Prev: common.Digest(prev)},
				SimplexEpochInfo: metadata.SimplexEpochInfo{
					EpochNumber:          epoch,
					PrevSealingBlockHash: prevSealingBlockHash,
					BlockValidationDescriptor: &metadata.BlockValidationDescriptor{
						AggregatedMembership: metadata.AggregatedMembership{Members: validators},
					},
				},
			},
		}}
	}

	// The honest chain: seq 1 (in storage) <- honest seq 2 <- seq 3. The forged seq 2 is signed by the
	// same epoch 1 validators but is not what seq 3 points to.
	honestSealing2 := sealingBlock(2, 1, epochBlock.Digest(), epochBlock.Digest(), "honest sealing block 2")
	forgedSealing2 := sealingBlock(2, 1, epochBlock.Digest(), epochBlock.Digest(), "forged sealing block 2")
	require.NotEqual(t, honestSealing2.Digest(), forgedSealing2.Digest())
	sealing3 := sealingBlock(3, 2, honestSealing2.Digest(), honestSealing2.Digest(), "sealing block 3")

	honestFinalization2, _ := testutil.NewFinalizationRecord(t, sigAggregator, honestSealing2, signers)
	forgedFinalization2, _ := testutil.NewFinalizationRecord(t, sigAggregator, forgedSealing2, signers)
	finalization3, _ := testutil.NewFinalizationRecord(t, sigAggregator, sealing3, signers)

	// The current validator set reports seq 3 as the latest sealing block, which bootstraps the node.
	require.NoError(t, node.inst.HandleMessage(&common.Message{
		ReplicationResponse: &common.ReplicationResponse{
			LatestSeq: &common.QuorumRound{Block: sealing3, Finalization: &finalization3},
		},
	}, validator.NodeID[:]))
	_, bootstrapped = node.role()
	require.True(t, bootstrapped)

	// A peer serves the forged seq 2. Its quorum certificate checks out against epoch 1, but its digest
	// is not the PrevSealingBlockHash of seq 3, so it is off the hash chain and must not be indexed.
	require.NoError(t, node.inst.HandleMessage(&common.Message{
		ReplicationResponse: &common.ReplicationResponse{
			Data: []common.QuorumRound{{Block: forgedSealing2, Finalization: &forgedFinalization2}},
		},
	}, validator.NodeID[:]))
	require.Never(t, func() bool {
		_, _, err := node.storage.Retrieve(2)
		return err == nil
	}, 2*time.Second, 50*time.Millisecond, "indexed a sealing block that is not on the hash chain from the sealing block we bootstrapped on")

	// The honest seq 2 is what seq 3 points to, so it is accepted.
	require.NoError(t, node.inst.HandleMessage(&common.Message{
		ReplicationResponse: &common.ReplicationResponse{
			Data: []common.QuorumRound{{Block: honestSealing2, Finalization: &honestFinalization2}},
		},
	}, validator.NodeID[:]))
	committed := node.storage.WaitForBlockCommit(2)
	require.Equal(t, honestSealing2.Bytes(), committed.Bytes())
}

yacovm · 2026-09-09T20:02:49Z

+		return nil
+	}
+
+	n.Logger.Info("Bootstrapped, received a threshold of sealing block info for an epoch", zap.Stringer("Info", qr.Block.SealingBlockInfo()))


We should only consider ourselves as bootstrapped when we have replicated and committed all blocks from the last block in the ledger to the last known tip.

i think we can consider ourselves bootstrapped once we have validated the hash chain of sealing blocks, then we can start as normal syncing all the blocks in between

as to your test, i made a few non-validator tests to ensure we do the backwards hash validation first

TestNonValidator_BootstrapIgnoresSealingBlockOffChain && TestNonValidator_BootstrapWalksHashChain

yacovm

These are the comments I have so far, I'm not nearly done with the review.

yacovm · 2026-09-11T15:56:48Z

 }

+// Equal returns whether both hold the same nodes, ignoring order.
+func (nws Nodes) Equal(other Nodes) bool {


can we add a UT for this?

yacovm · 2026-09-11T15:57:40Z

 }

+// HasTasks reports whether any task is still outstanding.
+func (t *TimeoutHandler[T]) HasTasks() bool {


Maybe call it something shorter like Empty() and just use ! Empty() ?

yacovm · 2026-09-11T16:56:48Z

 	parent, _, err := nonValidatorNode.storage.GetBlock(1)
 	require.NoError(t, err)

+	// The non-validator drops every message until it bootstraps. One peer reporting the block


I don't understand this statement.

If it drops every message how does it bootstrap? 🤡

ya i meant to say drops every message that doesn't help it bootstrap.

yacovm · 2026-09-11T18:58:02Z

 	// and it is in the validator set
 	TransitionToValidator func(epoch uint64, validators common.Nodes)
+
+	// Bootstrapped is set once every epoch from our tip up to the one a threshold of the latest


Why would we call that bootstrapped? Bootstrapped should just mean that we finished bootstrapping, exactly like we do in snowman. Which is that we have replicated all blocks we know are missing from the latest discovered tip down to the tip before bootstrapping.

yacovm · 2026-09-11T18:59:07Z

+		return nil
+	}
+
+	// No sealing block is missing, so every epoch from our tip to the highest is validated.


but we should still replicate all blocks in the last epoch that we know about before we declare that we have finished bootstrapping.

I think this comment is still relevant

yacovm · 2026-09-11T19:28:24Z

 	}

-	comm := newCommunication(i.Config.Sender, i.Config.Broadcaster, mappings.Nodes())
+	comm := newCommunication(i.Config.Sender, i.Config.Broadcaster, latestValidatorSet.Nodes())


unrelated to this PR, but... what updates the comm's validator set once we move through epochs after we bootstrap?

yea we need to change the non-validator comm to not hardcode its Validators() method. everytime it calls Validators the pchain should be queried or something

yacovm · 2026-09-11T19:36:15Z

+
+	// We have indexed the latest validator set, therefore we can skip bootstrapping and start as a validator.
+	// Note: this may not be the latest epoch, but a future PR will eventually notice we are behind and transition properly.
+	if latestIndexedEpochValidators.Equal(latestValidatorSet.Nodes()) && latestValidatorSet.Nodes().Contains(i.Config.ID) {


what if we're a validator and behind (old epoch) but somehow the latest on-chain validator set is equal to the P-chain validator set?

We won't know we're behind and will reject all messages because they will have a higher epoch.

Is #578 supposed to solve this?

yacovm · 2026-09-11T19:43:13Z

 	}

+	if !n.Bootstrapped && msg.ReplicationResponse == nil {
+		n.Logger.Debug("Dropping message received while bootstrapping, we only accept replication responses", zap.Any("Message", msg), zap.Stringer("From", from))


we assume here that the message can only have a single field inside of it.

But it can have at least a single field, not up to a single field.

Shouldn't we re-order the switch a few lines below to have msg.ReplicationResponse != nil: first?

If we're bootstrapping but replication response is not nil, we can still process block messages. Can we re-order so we have msg.ReplicationResponse != nil: first?

yacovm · 2026-09-11T20:41:09Z

 	for _, md := range e {
 		if bytes.Equal(md.prevSealingBlockHash[:], digest[:]) {
 			// We have validated the next epoch, and the next epoch has a backward pointer to this one
 			return true


what if the block is already indexed?

We initialize epochs with the latest indexed sealing block so we should be good.

switch { case n.epochs.canValidate(block): // The sealing block in the backwards hash chain n.validateSealingBlock(qr, from) case n.highestEpochCollector.collectedSealingBlockInfo(sealingInfo, bh, from): n.Logger.Info("A threshold of validators reported a sealing block", zap.Uint64("Seq", bh.Seq), zap.Stringer("Info", sealingInfo)) n.sealingBlockTimeouts.RemoveTask(startBroadcastTask) if !n.isIndexed(bh.Seq) { n.validateSealingBlock(qr, from) } default: return nil }

If we get a block we've already indexed we may think we can validate it and then we will get into validateSealingBlock even though we don't need to.

We can just add a check here:

switch { case n.epochs.canValidate(block): // The sealing block in the backwards hash chain if !n.isIndexed(bh.Seq) { n.validateSealingBlock(qr, from) } case n.highestEpochCollector.collectedSealingBlockInfo(sealingInfo, bh, from): n.Logger.Info("A threshold of validators reported a sealing block", zap.Uint64("Seq", bh.Seq), zap.Stringer("Info", sealingInfo)) n.sealingBlockTimeouts.RemoveTask(startBroadcastTask) if !n.isIndexed(bh.Seq) { n.validateSealingBlock(qr, from) } default: return nil }

no?

yacovm · 2026-09-11T21:16:50Z

+	bh := block.BlockHeader()
+	sealingInfo := block.SealingBlockInfo()
+
+	if sealingInfo == nil {


what if we have the epoch in the ledger?

so i think we still need this in the case for these scenarios

TestNonValidatorBootstrapLatestKnownEpoch and TestNonValidatorBootstrapRequestsSealingBlock

I am not sure i understand. If we have the epoch in the ledger why do we need to fetch it again?

yacovm

Made another pass

yacovm · 2026-09-15T17:13:28Z

+	}
+	nv.sealingBlockTimeouts = common.NewTimeoutHandler(config.Logger, "sealing block replication", config.StartTime, simplex.DefaultReplicationRequestTimeout, nv.requestMissingSealingBlocks)
+	if !config.Bootstrapped {
+		nv.sealingBlockTimeouts.AddTask(startBroadcastTask)


nv.sealingBlockTimeouts.AddTask(0) - I don't understand what we're trying to do here.

yacovm · 2026-09-15T17:40:55Z

+// The finalization has not been verified yet. Storing tells the replicator a valid sequence exists
+// and its validity is checked when the round is processed.
+func (n *NonValidator) validateSealingBlock(qr *common.QuorumRound, from common.NodeID) {
+	n.maybeValidateNextEpoch(qr.Block, from)


It looks like we maybe validate and then anyway store the QR?

but maybeValidateNextEpoch can return early in many cases. Is that intentional?

I guess the purpose of validateSealingBlock is to kickstart validation of the previous sealing block? If so, is this the right name for the method?

yacovm · 2026-09-15T17:58:17Z

 	epochResponses[string(from)] = digest

 	// check if we have a threshold of responses
 	counts := make(map[common.Digest]uint64)


Not really relevant to this PR, but - if validators changes between two invocations of this function, then we have a problem.

Consider two invocations collectedSealingBlockInfo() in t1 and collectedSealingBlockInfo() in t2, and the first one sampled validators = [v1, v2, v3, v4] and the second once sampled v2, v3, v4, v5]'.

The first one got a vote from v1 and the second one got a vote from v2 but v1 is not in the second.

We reached the threshold f+1 but with an illegal count.

yacovm · 2026-09-15T17:59:59Z

+		if !n.isIndexed(bh.Seq) {
+			n.validateSealingBlock(qr, from)
+		}
+	default:


if we have a default case that returns nil then how do we proceed below?

We don't, which means this is some kind of catch-all case in which we don't want to proceed.

Can we add some comments to each if case that explain better what we're trying to do?

yacovm · 2026-09-15T18:02:53Z

+	// No sealing block is missing, so every epoch from our tip to the highest is validated.
+	if !n.sealingBlockTimeouts.Empty() {
+		n.finishBootstrap()
+		// If the highest epoch is already indexed, nothing more gets indexed to trigger the transition.


Don't we need to replicate the blocks between the sealing blocks?

yacovm · 2026-09-15T18:32:41Z

+		return nil
+	}
+
+	// No sealing block is missing, so every epoch from our tip to the highest is validated.


I think this comment is still relevant

yacovm · 2026-09-15T18:36:25Z

 	}

+	if !n.Bootstrapped && msg.ReplicationResponse == nil {
+		n.Logger.Debug("Dropping message received while bootstrapping, we only accept replication responses", zap.Any("Message", msg), zap.Stringer("From", from))


If we're bootstrapping but replication response is not nil, we can still process block messages. Can we re-order so we have msg.ReplicationResponse != nil: first?

yacovm · 2026-09-15T18:37:45Z

+	bh := block.BlockHeader()
+	sealingInfo := block.SealingBlockInfo()
+
+	if sealingInfo == nil {


I am not sure i understand. If we have the epoch in the ledger why do we need to fetch it again?

yacovm · 2026-09-15T19:06:40Z

+	t.lock.Lock()
+	defer t.lock.Unlock()
+
+	return len(t.tasks) > 0


Shouldn't this be == instead of > ?

yacovm · 2026-09-15T19:18:59Z

 	for _, md := range e {
 		if bytes.Equal(md.prevSealingBlockHash[:], digest[:]) {
 			// We have validated the next epoch, and the next epoch has a backward pointer to this one
 			return true


switch { case n.epochs.canValidate(block): // The sealing block in the backwards hash chain n.validateSealingBlock(qr, from) case n.highestEpochCollector.collectedSealingBlockInfo(sealingInfo, bh, from): n.Logger.Info("A threshold of validators reported a sealing block", zap.Uint64("Seq", bh.Seq), zap.Stringer("Info", sealingInfo)) n.sealingBlockTimeouts.RemoveTask(startBroadcastTask) if !n.isIndexed(bh.Seq) { n.validateSealingBlock(qr, from) } default: return nil }

If we get a block we've already indexed we may think we can validate it and then we will get into validateSealingBlock even though we don't need to.

We can just add a check here:

switch { case n.epochs.canValidate(block): // The sealing block in the backwards hash chain if !n.isIndexed(bh.Seq) { n.validateSealingBlock(qr, from) } case n.highestEpochCollector.collectedSealingBlockInfo(sealingInfo, bh, from): n.Logger.Info("A threshold of validators reported a sealing block", zap.Uint64("Seq", bh.Seq), zap.Stringer("Info", sealingInfo)) n.sealingBlockTimeouts.RemoveTask(startBroadcastTask) if !n.isIndexed(bh.Seq) { n.validateSealingBlock(qr, from) } default: return nil }

no?

samliok self-assigned this Sep 1, 2026

samliok force-pushed the bootstrap-first branch 3 times, most recently from 6be0d9c to 23d9c67 Compare September 1, 2026 23:55

samliok linked an issue Sep 2, 2026 that may be closed by this pull request

Validate sealing blocks only via backward hash chain validation on bootstrap instead of also via QC #530

Open

samliok force-pushed the bootstrap-first branch 2 times, most recently from 9c76fa7 to 9501164 Compare September 2, 2026 16:18

samliok force-pushed the bootstrap-first branch from 9501164 to a1bb893 Compare September 2, 2026 18:11

samliok force-pushed the bootstrap-first branch from 9d1886a to b4bfaba Compare September 3, 2026 23:02

Base automatically changed from instance-test-refactor to main September 4, 2026 17:58

yacovm force-pushed the bootstrap-first branch from b4bfaba to 4b6b2f5 Compare September 4, 2026 17:58

samliok force-pushed the bootstrap-first branch 6 times, most recently from a1a3fb9 to b507b64 Compare September 9, 2026 18:30

yacovm reviewed Sep 9, 2026

View reviewed changes

Comment thread nonvalidator/epochs.go Outdated

yacovm reviewed Sep 9, 2026

View reviewed changes

samliok force-pushed the bootstrap-first branch from b04bddb to c2d3997 Compare September 10, 2026 01:20

samliok marked this pull request as draft September 10, 2026 14:25

samliok marked this pull request as ready for review September 10, 2026 18:45

yacovm reviewed Sep 11, 2026

View reviewed changes

samliok force-pushed the bootstrap-first branch from d7defc8 to 8fa6844 Compare September 14, 2026 19:49

samliok mentioned this pull request Sep 14, 2026

Drop messages delivered to an Instance before Start #593

Merged

samliok force-pushed the bootstrap-first branch from a189c1f to 2a76d39 Compare September 14, 2026 21:15

samliok added 9 commits September 15, 2026 12:08

bootstrap before verifying and indexing blocks

8d11ae9

bootstrapping reworked

8f236e6

lint

747af45

merge conflicts

ef521d7

reduce diff

b257799

reduce more diff

de0d564

add offline test

d714f9b

lint

0aedf2d

comments

a02c097

samliok force-pushed the bootstrap-first branch from 2a76d39 to a02c097 Compare September 15, 2026 16:08

yacovm reviewed Sep 15, 2026

View reviewed changes

Uh oh!

Conversation

samliok commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

yacovm commented Sep 9, 2026

Uh oh!

yacovm commented Sep 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yacovm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samliok Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yacovm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samliok commented Sep 1, 2026 •

edited

Loading

samliok Sep 11, 2026 •

edited

Loading