Skip to content

Refactor instance tests around a network harness - #508

Open
samliok wants to merge 11 commits into
mainfrom
instance-test-refactor
Open

Refactor instance tests around a network harness#508
samliok wants to merge 11 commits into
mainfrom
instance-test-refactor

Conversation

@samliok

@samliok samliok commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Final PR of the cleanup-instance stack (4/4), on top of the transition listener PR.

  • All scaffolding moves to instance_testhelpers_test.go, rebuilt around a network harness: addNode, acceptNewBlock, waitUntilSealingBlock. The queue-based inMemNetwork is replaced by instanceComm, which delivers messages directly and re-parses blocks so recipients do not share mutable canoto state.
  • instance_test.go becomes short scenario tests. Epoch sealing tests now rely on production approval/aux info dissemination instead of hand-injecting approvals (removing the old TODO).
  • New coverage: validator set unchanged/decreased, a validator skipping an epoch, and signer-set assertions proving a promoted non-validator contributes its approval.
  • TestParseBlockSizeMatchesBytes moves verbatim to external_test.go next to external.go.
  • TestNonValidator_StaysNonValidator (replacing TestInstanceNonValidatorBootstraps) is skipped pending offline-node timeouts, and TestInstanceRestartAcrossEpochs is dropped (the node.restart() helper remains unused).

@samliok
samliok force-pushed the instance-test-refactor branch from 97c53ab to d147e3d Compare August 19, 2026 18:25
@samliok
samliok marked this pull request as ready for review August 20, 2026 21:53
@samliok samliok closed this Aug 21, 2026
@samliok
samliok force-pushed the instance-test-refactor branch from 4bb3bc9 to 5040547 Compare August 21, 2026 15:15
@samliok samliok reopened this Aug 21, 2026
Comment thread instance_test.go
// order to do so.
// We then check does so by checking they participated in signing
// Equivalent test as the previous TestInstanceMixedNodeType.
func TestNonValidator_BecomesValidator(t *testing.T) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

equivalent to the previous: TestInstanceMixedNodeType

Comment thread instance_test.go
// TestNonValidator_StaysNonValidator ensures that a non-validator does not restart when it is processing
// previous epoch changes.
// Equivalent to TestInstanceNonValidatorBootstraps
func TestNonValidator_StaysNonValidator(t *testing.T) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

previously was TestInstanceNonValidatorBootstraps

Comment thread instance_test.go
// then it is no longer a validator, and finally it is
// Equivalent to: TestInstanceValidatorSkipsAnEpoch. This also starts a validator when the tip is a sealing block so it covers an edge
// case previously caught from the logging test.
func TestInstanceValidatorSkipsAnEpoch(t *testing.T) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Equivalent to: TestInstanceValidatorSkipsAnEpoch. This also starts a validator when the tip is a sealing block so it covers an edge case previously caught from the logging test.

@samliok
samliok force-pushed the instance-test-refactor branch 2 times, most recently from 7ff7591 to ed0e6d5 Compare August 21, 2026 15:43
@samliok
samliok force-pushed the instance-test-refactor branch from ed0e6d5 to ac05817 Compare August 21, 2026 15:56
@samliok
samliok force-pushed the instance-test-refactor branch from ac05817 to 070a524 Compare August 21, 2026 16:01
@samliok samliok linked an issue Aug 21, 2026 that may be closed by this pull request
@samliok
samliok force-pushed the instance-test-refactor branch from 070a524 to 4ac8236 Compare August 24, 2026 21:33
@samliok
samliok force-pushed the instance-test-refactor branch from 4ac8236 to 569e39c Compare August 26, 2026 14:13
@samliok
samliok force-pushed the instance-test-refactor branch from 569e39c to 274e913 Compare August 26, 2026 14:20
@samliok
samliok force-pushed the instance-test-refactor branch from dbea945 to 6287af1 Compare August 26, 2026 21:17
@samliok
samliok force-pushed the instance-test-refactor branch from 6287af1 to f933854 Compare August 27, 2026 20:09
@samliok
samliok force-pushed the instance-test-refactor branch from f933854 to d4522b2 Compare August 31, 2026 14:04
Base automatically changed from transition-listener to main August 31, 2026 14:29
Replace the queue-based inMemNetwork with a network harness whose instanceComm
delivers messages directly and re-parses blocks so recipients do not share
canoto state. Scenario tests shrink to short flows against the harness, and
epoch sealing tests rely on production approval dissemination instead of
injecting approvals. Move TestParseBlockSizeMatchesBytes next to external.go.
@yacovm
yacovm force-pushed the instance-test-refactor branch from d4522b2 to 2826e2b Compare August 31, 2026 14:29
Comment thread instance_test.go
var id [20]byte
rand.Read(id[:])
nodeID := common.NodeID(id[:])
// TestInstance_OfflineDuringTransition asserts an epoch transition completes while a

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2026-08-31T14:40:37.4305288Z --- PASS: TestValidator_ValidatorSetDecreased (3.02s)
2026-08-31T14:40:37.4306101Z === RUN   TestInstance_OfflineDuringTransition
2026-08-31T14:40:37.4307634Z [08-31|14:30:53.602] INFO Simplex/instance.go:554 Last block is a sealing block, garbage collecting all WALs preceding it to start a new epoch
2026-08-31T14:40:37.4309724Z [08-31|14:30:53.602] INFO simplex/epoch.go:243 Starting Simplex Epoch {"ID": "0100000000000000", "nodes": "[0100000000000000 0200000000000000 0300000000000000 0400000000000000]"}
2026-08-31T14:40:37.4311664Z [08-31|14:30:53.603] INFO Simplex/instance.go:554 Last block is a sealing block, garbage collecting all WALs preceding it to start a new epoch
2026-08-31T14:40:37.4313525Z [08-31|14:30:53.603] INFO simplex/epoch.go:243 Starting Simplex Epoch {"ID": "0200000000000000", "nodes": "[0100000000000000 0200000000000000 0300000000000000 0400000000000000]"}
2026-08-31T14:40:37.4315668Z [08-31|14:30:53.604] INFO Simplex/instance.go:554 Last block is a sealing block, garbage collecting all WALs preceding it to start a new epoch
2026-08-31T14:40:37.4317770Z [08-31|14:30:53.604] INFO simplex/epoch.go:243 Starting Simplex Epoch {"ID": "0300000000000000", "nodes": "[0100000000000000 0200000000000000 0300000000000000 0400000000000000]"}
2026-08-31T14:40:37.4319304Z [08-31|14:30:56.605] INFO simplex/epoch.go:2851 It is time to build a block {"round": 2}
2026-08-31T14:40:37.4320605Z [08-31|14:30:56.606] INFO simplex/epoch.go:2851 It is time to build a block {"round": 2}
2026-08-31T14:40:37.4321576Z panic: test timed out after 10m0s
2026-08-31T14:40:37.4322204Z 	running tests:
2026-08-31T14:40:37.4322937Z 		TestInstance_OfflineDuringTransition (9m44s)
2026-08-31T14:40:37.4323445Z 
2026-08-31T14:40:37.4323634Z goroutine 2635 [running]:
2026-08-31T14:40:37.4324308Z testing.(*M).startAlarm.func1()

…broadcast

pendingBlockSignal.broadcast was edge-triggered: it closed the current channel
and installed a fresh one, so a VM that reached wait after the broadcast never
saw it. acceptNewBlock broadcasts once per block while each node calls
WaitForPendingBlock repeatedly, so whether a node was inside wait at that
instant was a scheduling race.

When the node that missed it was the round leader it never proposed, and the
round-2 wait is the one that has no deadline (the parent is a finalized sealing
block, so msm takes the blockBuildingDecider path rather than the bounded
MaxBlockBuildingWaitTime path). With one validator offline the remaining two
could not reach the 3-of-4 quorum needed to time the round out, so nothing
cancelled the leader's context and the test hung until the 10m alarm.

Give the signal a generation counter and each VM the last generation it
consumed, so a broadcast reaches every VM exactly once whether or not it was
already waiting.
… generation

The previous commit gave each VM a cursor into the broadcast generations but
started every cursor at zero, so a node added after an earlier acceptNewBlock
saw gen > 0 on its very first WaitForPendingBlock and returned immediately.
That spurious "time to build a block" during the epoch transition broke
TestNonValidator_BecomesValidator outright, and it reached CI.

Seed the cursor from the signal's current generation instead, so a VM joining
mid-test waits for the next broadcast rather than treating the ones it missed
as one pending block.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test flake in TestInstanceRestartAcrossEpochs

2 participants