Improve subscriber test reliability (#431)

* Improve subscriber test reliability

This tries to produce certificates at a reliable interval so we can
accurately measure how "good" our predictions are.

* Generate certificates on-demand

So we don't run out.

Author: Stebalien

certexchange/polling/common_test.go

@@ -58,19 +58,39 @@ func RandomPowerTable(backend signing.Backend, entries int64) gpbft.PowerEntries
 	return powerTable
 }
 
-func MakeCertificates(t *testing.T, rng *rand.Rand, backend signing.Backend) ([]*certs.FinalityCertificate, gpbft.PowerEntries) {
+func MakeCertificates(t *testing.T, rng *rand.Rand, backend signing.Backend) *CertificateGenerator {
 	powerTable := RandomPowerTable(backend, 10)
 	tableCid, err := certs.MakePowerTableCID(powerTable)
 	require.NoError(t, err)
 
 	tsg := sim.NewTipSetGenerator(rng.Uint64())
 	base := &gpbft.TipSet{Epoch: 0, Key: tsg.Sample(), PowerTable: tableCid}
 
-	certificates := make([]*certs.FinalityCertificate, 1000)
-	for i := range certificates {
-		cert := MakeCertificate(t, rng, tsg, backend, base, uint64(i), powerTable, powerTable)
-		base = cert.ECChain.Head()
-		certificates[i] = cert
+	return &CertificateGenerator{
+		PowerTable:   powerTable,
+		t:            t,
+		rng:          rng,
+		backend:      backend,
+		tsg:          tsg,
+		base:         base,
+		NextInstance: 0,
 	}
-	return certificates, powerTable
+}
+
+type CertificateGenerator struct {
+	PowerTable   gpbft.PowerEntries
+	NextInstance uint64
+
+	t       *testing.T
+	rng     *rand.Rand
+	backend signing.Backend
+	tsg     *sim.TipSetGenerator
+	base    *gpbft.TipSet
+}
+
+func (cg *CertificateGenerator) MakeCertificate() *certs.FinalityCertificate {
+	cert := MakeCertificate(cg.t, cg.rng, cg.tsg, cg.backend, cg.base, cg.NextInstance, cg.PowerTable, cg.PowerTable)
+	cg.base = cert.ECChain.Head()
+	cg.NextInstance++
+	return cert
 }

certexchange/polling/poller_test.go

@@ -20,7 +20,7 @@ func TestPoller(t *testing.T) {
 	backend := signing.NewFakeBackend()
 	rng := rand.New(rand.NewSource(1234))
 
-	certificates, powerTable := polling.MakeCertificates(t, rng, backend)
+	cg := polling.MakeCertificates(t, rng, backend)
 
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
@@ -37,12 +37,11 @@ func TestPoller(t *testing.T) {
 
 	serverDs := ds_sync.MutexWrap(datastore.NewMapDatastore())
 
-	serverCs, err := certstore.CreateStore(ctx, serverDs, 0, powerTable)
+	serverCs, err := certstore.CreateStore(ctx, serverDs, 0, cg.PowerTable)
 	require.NoError(t, err)
 
-	certificatesAdded := 10
-	for _, cert := range certificates[:certificatesAdded] {
-		require.NoError(t, serverCs.Put(ctx, cert))
+	for cg.NextInstance < 10 {
+		require.NoError(t, serverCs.Put(ctx, cg.MakeCertificate()))
 	}
 
 	server := certexchange.Server{
@@ -55,7 +54,7 @@ func TestPoller(t *testing.T) {
 	t.Cleanup(func() { require.NoError(t, server.Stop()) })
 
 	clientDs := ds_sync.MutexWrap(datastore.NewMapDatastore())
-	clientCs, err := certstore.CreateStore(ctx, clientDs, 0, powerTable)
+	clientCs, err := certstore.CreateStore(ctx, clientDs, 0, cg.PowerTable)
 	require.NoError(t, err)
 
 	client := certexchange.Client{
@@ -82,21 +81,21 @@ func TestPoller(t *testing.T) {
 		res, err := poller.Poll(ctx, serverHost.ID())
 		require.NoError(t, err)
 		require.Equal(t, polling.PollHit, res.Status)
-		require.Equal(t, uint64(certificatesAdded), poller.NextInstance)
+		require.Equal(t, cg.NextInstance, poller.NextInstance)
 	}
 
 	// If we put a certificate on the client, we should call it a _miss_
 	{
-		require.NoError(t, clientCs.Put(ctx, certificates[certificatesAdded]))
+		cert := cg.MakeCertificate()
+		require.NoError(t, clientCs.Put(ctx, cert))
 
 		res, err := poller.Poll(ctx, serverHost.ID())
 		require.NoError(t, err)
 		require.Equal(t, polling.PollMiss, res.Status)
-	}
 
-	// Add that cert to the server.
-	require.NoError(t, serverCs.Put(ctx, certificates[certificatesAdded]))
-	certificatesAdded++
+		// Add that cert to the server.
+		require.NoError(t, serverCs.Put(ctx, cert))
+	}
 
 	// And now it's a hit!
 	{
@@ -106,31 +105,31 @@ func TestPoller(t *testing.T) {
 	}
 
 	// Add more than the request maximum (up till the last cert)
-	for ; certificatesAdded < len(certificates)-1; certificatesAdded++ {
-		require.NoError(t, serverCs.Put(ctx, certificates[certificatesAdded]))
+	for cg.NextInstance < 500 {
+		require.NoError(t, serverCs.Put(ctx, cg.MakeCertificate()))
 	}
 
 	// We should poll multiple times and completely catch up.
 	{
 		res, err := poller.Poll(ctx, serverHost.ID())
 		require.NoError(t, err)
 		require.Equal(t, polling.PollHit, res.Status)
-		require.Equal(t, uint64(certificatesAdded), poller.NextInstance)
+		require.Equal(t, cg.NextInstance, poller.NextInstance)
 	}
 
 	// We catch evil servers!
 	{
-		lastCert := certificates[certificatesAdded]
-		lastCert.Signature = []byte("bad sig")
-		require.NoError(t, serverCs.Put(ctx, lastCert))
+		badCert := cg.MakeCertificate()
+		badCert.Signature = []byte("bad sig")
+		require.NoError(t, serverCs.Put(ctx, badCert))
 
 		res, err := poller.Poll(ctx, serverHost.ID())
 		require.NoError(t, err)
 		require.Equal(t, polling.PollIllegal, res.Status)
 
 		// And we don't store certificates from them!
-		require.Equal(t, uint64(certificatesAdded), poller.NextInstance)
-		_, err = clientCs.Get(ctx, lastCert.GPBFTInstance)
+		require.Equal(t, badCert.GPBFTInstance, poller.NextInstance)
+		_, err = clientCs.Get(ctx, badCert.GPBFTInstance)
 		require.ErrorIs(t, err, certstore.ErrCertNotFound)
 	}
 

certexchange/polling/subscriber_test.go

@@ -22,7 +22,7 @@ func TestSubscriber(t *testing.T) {
 	backend := signing.NewFakeBackend()
 	rng := rand.New(rand.NewSource(1234))
 
-	certificates, powerTable := polling.MakeCertificates(t, rng, backend)
+	cg := polling.MakeCertificates(t, rng, backend)
 
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
@@ -38,7 +38,7 @@ func TestSubscriber(t *testing.T) {
 		require.NoError(t, err)
 
 		ds := ds_sync.MutexWrap(datastore.NewMapDatastore())
-		cs, err := certstore.CreateStore(ctx, ds, 0, powerTable)
+		cs, err := certstore.CreateStore(ctx, ds, 0, cg.PowerTable)
 		require.NoError(t, err)
 
 		servers[i] = &certexchange.Server{
@@ -56,7 +56,7 @@ func TestSubscriber(t *testing.T) {
 	}
 
 	clientDs := ds_sync.MutexWrap(datastore.NewMapDatastore())
-	clientCs, err := certstore.CreateStore(ctx, clientDs, 0, powerTable)
+	clientCs, err := certstore.CreateStore(ctx, clientDs, 0, cg.PowerTable)
 	require.NoError(t, err)
 
 	client := certexchange.Client{
@@ -80,15 +80,29 @@ func TestSubscriber(t *testing.T) {
 	require.NoError(t, mocknet.ConnectAllButSelf())
 
 	liveServers := slices.Clone(servers)
-	for i := 0; len(liveServers) > 0; i++ {
-		for _, s := range liveServers {
-			require.NoError(t, s.Store.Put(ctx, certificates[i]))
+	lastPoll := time.Now()
+	i := 0
+	for len(liveServers) > 0 {
+		now := time.Now()
+		timeDelta := now.Sub(lastPoll)
+		certCount := timeDelta/subscriber.InitialPollInterval + 1
+		waitTime := certCount * subscriber.InitialPollInterval
+		for target := i + int(certCount); i < target; i++ {
+			cert := cg.MakeCertificate()
+			for _, s := range liveServers {
+				require.NoError(t, s.Store.Put(ctx, cert))
+			}
 		}
 
+		polling.MockClock.Add(waitTime)
+
 		require.Eventually(t, func() bool {
-			polling.MockClock.WaitForAllTimers()
 			latest := clientCs.Latest()
-			return latest != nil && latest.GPBFTInstance == uint64(i)
+			if latest != nil && latest.GPBFTInstance == uint64(i-1) {
+				return true
+			}
+			polling.MockClock.WaitForAllTimers()
+			return false
 		}, 10*time.Second, time.Millisecond)
 
 		// After we settle for a bit, every 4 instances, stop updating 20% of the