From 726b49db761a92087db89d4634676a87cac008fe Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Wed, 3 Jul 2024 01:08:51 +0200 Subject: [PATCH 01/26] check registrations by poet address --- activation/nipost.go | 16 +++++-- sql/localsql/nipost/poet_registration.go | 48 ++++++++++++++++++- sql/localsql/nipost/poet_registration_test.go | 8 ++++ 3 files changed, 67 insertions(+), 5 deletions(-) diff --git a/activation/nipost.go b/activation/nipost.go index 3ee0b17b65..1049a80aee 100644 --- a/activation/nipost.go +++ b/activation/nipost.go @@ -86,8 +86,7 @@ func NewNIPostBuilder( opts ...NIPostBuilderOption, ) (*NIPostBuilder, error) { b := &NIPostBuilder{ - localDB: db, - + localDB: db, postService: postService, logger: lg, poetCfg: poetCfg, @@ -230,11 +229,20 @@ func (nb *NIPostBuilder) BuildNIPost( ) // Phase 0: Submit challenge to PoET services. - count, err := nipost.PoetRegistrationCount(nb.localDB, signer.NodeID()) + addresses := make([]string, 0) + for _, client := range nb.poetProvers { + addresses = append(addresses, client.Address()) + } + + // check, if already registered for some PoET services with given addresses + count, err := nipost.PoetRegistrationCount(nb.localDB, signer.NodeID(), addresses...) if err != nil { return nil, fmt.Errorf("failed to get poet registration count: %w", err) } + if count == 0 { + // no registrations + now := time.Now() // Deadline: start of PoET round for publish epoch. PoET won't accept registrations after that. if poetRoundStart.Before(now) { @@ -252,7 +260,7 @@ func (nb *NIPostBuilder) BuildNIPost( if err != nil { return nil, fmt.Errorf("submitting to poets: %w", err) } - count, err := nipost.PoetRegistrationCount(nb.localDB, signer.NodeID()) + count, err := nipost.PoetRegistrationCount(nb.localDB, signer.NodeID(), addresses...) if err != nil { return nil, fmt.Errorf("failed to get poet registration count: %w", err) } diff --git a/sql/localsql/nipost/poet_registration.go b/sql/localsql/nipost/poet_registration.go index 50aa83ac10..4a8b137248 100644 --- a/sql/localsql/nipost/poet_registration.go +++ b/sql/localsql/nipost/poet_registration.go @@ -2,6 +2,7 @@ package nipost import ( "fmt" + "strings" "time" "github.com/spacemeshos/go-spacemesh/common/types" @@ -37,7 +38,7 @@ func AddPoetRegistration( return nil } -func PoetRegistrationCount(db sql.Executor, nodeID types.NodeID) (int, error) { +/*func PoetRegistrationCount(db sql.Executor, nodeID types.NodeID) (int, error) { var count int enc := func(stmt *sql.Statement) { stmt.BindBytes(1, nodeID.Bytes()) @@ -52,6 +53,51 @@ func PoetRegistrationCount(db sql.Executor, nodeID types.NodeID) (int, error) { return 0, fmt.Errorf("get poet registration count for node id %s: %w", nodeID.ShortString(), err) } return count, nil +}*/ + +func PoetRegistrationCount(db sql.Executor, nodeID types.NodeID, addresses ...string) (int, error) { + var ( + count int + enc func(stmt *sql.Statement) + query string + ) + + if len(addresses) > 0 { + enc = func(stmt *sql.Statement) { + stmt.BindBytes(1, nodeID.Bytes()) + for i, addr := range addresses { + stmt.BindText(i+2, addr) // Start binding addresses at position 2 + } + } + + placeholders := make([]string, len(addresses)) + for i := range placeholders { + placeholders[i] = fmt.Sprintf("?%d", i+2) // Generate placeholders starting from ?2 + } + // Construct the query with IN clause for addresses + query = fmt.Sprintf(`select count(*) from poet_registration where id = ?1 and address IN (%s);`, strings.Join(placeholders, ", ")) + + } else { + enc = func(stmt *sql.Statement) { + stmt.BindBytes(1, nodeID.Bytes()) + } + + query = `select count(*) from poet_registration where id = ?1;` + } + + dec := func(stmt *sql.Statement) bool { + count = int(stmt.ColumnInt64(0)) + return true + } + + _, err := db.Exec(query, enc, dec) + if err != nil { + if len(addresses) > 0 { + return 0, fmt.Errorf("get poet registration count for node id %s and addresses %v: %w", nodeID.ShortString(), addresses, err) + } + return 0, fmt.Errorf("get poet registration count for node id %s: %w", nodeID.ShortString(), err) + } + return count, nil } func ClearPoetRegistrations(db sql.Executor, nodeID types.NodeID) error { diff --git a/sql/localsql/nipost/poet_registration_test.go b/sql/localsql/nipost/poet_registration_test.go index 9d130043ae..4c37532c1f 100644 --- a/sql/localsql/nipost/poet_registration_test.go +++ b/sql/localsql/nipost/poet_registration_test.go @@ -42,6 +42,14 @@ func Test_AddPoetRegistration(t *testing.T) { require.NoError(t, err) require.Equal(t, 2, count) + count, err = PoetRegistrationCount(db, nodeID, "address2") + require.NoError(t, err) + require.Equal(t, 1, count) + + count, err = PoetRegistrationCount(db, nodeID, "address2", "address1") + require.NoError(t, err) + require.Equal(t, 2, count) + registrations, err := PoetRegistrations(db, nodeID) require.NoError(t, err) require.Len(t, registrations, 2) From 5bba175c935d3990405bd94a3e1b3e4e8765eedb Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Wed, 10 Jul 2024 13:04:35 +0200 Subject: [PATCH 02/26] added: - new tests - registration to missing poet services --- activation/nipost.go | 100 +++-- activation/nipost_test.go | 350 +++++++++++++++++- sql/localsql/nipost/poet_registration.go | 53 +-- sql/localsql/nipost/poet_registration_test.go | 56 ++- 4 files changed, 493 insertions(+), 66 deletions(-) diff --git a/activation/nipost.go b/activation/nipost.go index 1049a80aee..dd1b78c090 100644 --- a/activation/nipost.go +++ b/activation/nipost.go @@ -207,9 +207,9 @@ func (nb *NIPostBuilder) BuildNIPost( // WE ARE HERE PROOF BECOMES ATX PUBLICATION // AVAILABLE DEADLINE - poetRoundStart := nb.layerClock.LayerToTime((postChallenge.PublishEpoch - 1).FirstLayer()). + curPoetRoundStart := nb.layerClock.LayerToTime((postChallenge.PublishEpoch - 1).FirstLayer()). Add(nb.poetCfg.PhaseShift) - poetRoundEnd := nb.layerClock.LayerToTime(postChallenge.PublishEpoch.FirstLayer()). + curPoetRoundEnd := nb.layerClock.LayerToTime(postChallenge.PublishEpoch.FirstLayer()). Add(nb.poetCfg.PhaseShift). Add(-nb.poetCfg.CycleGap) @@ -222,8 +222,8 @@ func (nb *NIPostBuilder) BuildNIPost( poetProofDeadline := publishEpochEnd.Add(-nb.poetCfg.CycleGap) logger.Info("building nipost", - zap.Time("poet round start", poetRoundStart), - zap.Time("poet round end", poetRoundEnd), + zap.Time("poet round start", curPoetRoundStart), + zap.Time("poet round end", curPoetRoundEnd), zap.Time("publish epoch end", publishEpochEnd), zap.Uint32("publish epoch", postChallenge.PublishEpoch.Uint32()), ) @@ -235,37 +235,59 @@ func (nb *NIPostBuilder) BuildNIPost( } // check, if already registered for some PoET services with given addresses - count, err := nipost.PoetRegistrationCount(nb.localDB, signer.NodeID(), addresses...) + existingRegistrations, err := nipost.PoetRegistrations(nb.localDB, signer.NodeID(), addresses...) if err != nil { - return nil, fmt.Errorf("failed to get poet registration count: %w", err) + return nil, fmt.Errorf("failed to get poet registrations from db: %w", err) } - if count == 0 { - // no registrations + // check if some registrations missing + existingRegistrationsMap := make(map[string]bool) + for _, er := range existingRegistrations { + existingRegistrationsMap[er.Address] = true + } - now := time.Now() - // Deadline: start of PoET round for publish epoch. PoET won't accept registrations after that. - if poetRoundStart.Before(now) { - return nil, fmt.Errorf( - "%w: poet round has already started at %s (now: %s)", - ErrATXChallengeExpired, - poetRoundStart, - now, - ) + missingRegistrationsPoETAddresses := make(map[string]bool) + for _, addr := range addresses { + if _, ok := existingRegistrationsMap[addr]; !ok { + missingRegistrationsPoETAddresses[addr] = true } + } - submitCtx, cancel := context.WithDeadline(ctx, poetRoundStart) - defer cancel() - err := nb.submitPoetChallenges(submitCtx, signer, poetProofDeadline, challenge.Bytes()) - if err != nil { - return nil, fmt.Errorf("submitting to poets: %w", err) - } - count, err := nipost.PoetRegistrationCount(nb.localDB, signer.NodeID(), addresses...) - if err != nil { - return nil, fmt.Errorf("failed to get poet registration count: %w", err) - } - if count == 0 { - return nil, &PoetSvcUnstableError{msg: "failed to submit challenge to any PoET", source: submitCtx.Err()} + if len(missingRegistrationsPoETAddresses) > 0 { + now := time.Now() + + // Deadline: start of PoET round: we will not accept registrations after that + if !curPoetRoundStart.Before(now) { + // send registrations to missing addresses + submitCtx, cancel := context.WithDeadline(ctx, curPoetRoundStart) + defer cancel() + + if err := nb.submitPoetChallengesToAddresses( + submitCtx, + signer, + poetProofDeadline, + missingRegistrationsPoETAddresses, challenge.Bytes()); err != nil { + return nil, fmt.Errorf("submitting to poets: %w", err) + } + + existingRegistrationsCount, err := nipost.PoetRegistrationCount(nb.localDB, signer.NodeID(), addresses...) + if err != nil { + return nil, fmt.Errorf("failed to get poet registration count: %w", err) + } + if existingRegistrationsCount == 0 { + return nil, + &PoetSvcUnstableError{msg: "failed to submit challenge to any PoET", source: submitCtx.Err()} + } + } else { + if len(missingRegistrationsPoETAddresses) == len(addresses) { + // no registrations and deadline passed -> drop existing challenge + return nil, fmt.Errorf( + "%w: poet round has already started at %s (now: %s)", + ErrATXChallengeExpired, // drop current registration challenge + curPoetRoundStart, + now, + ) + } } } @@ -288,7 +310,7 @@ func (nb *NIPostBuilder) BuildNIPost( ) } - events.EmitPoetWaitProof(signer.NodeID(), postChallenge.PublishEpoch, poetRoundEnd) + events.EmitPoetWaitProof(signer.NodeID(), postChallenge.PublishEpoch, curPoetRoundEnd) poetProofRef, membership, err = nb.getBestProof(ctx, signer.NodeID(), challenge, postChallenge.PublishEpoch) if err != nil { return nil, &PoetSvcUnstableError{msg: "getBestProof failed", source: err} @@ -396,23 +418,27 @@ func (nb *NIPostBuilder) submitPoetChallenge( }) } -// Submit the challenge to all registered PoETs. -func (nb *NIPostBuilder) submitPoetChallenges( +// Submit the challenge to registered PoETs with given addresses. +func (nb *NIPostBuilder) submitPoetChallengesToAddresses( ctx context.Context, signer *signing.EdSigner, deadline time.Time, + addresses map[string]bool, challenge []byte, ) error { signature := signer.Sign(signing.POET, challenge) prefix := bytes.Join([][]byte{signer.Prefix(), {byte(signing.POET)}}, nil) nodeID := signer.NodeID() g, ctx := errgroup.WithContext(ctx) - errChan := make(chan error, len(nb.poetProvers)) + errChan := make(chan error, len(addresses)) + for _, client := range nb.poetProvers { - g.Go(func() error { - errChan <- nb.submitPoetChallenge(ctx, nodeID, deadline, client, prefix, challenge, signature) - return nil - }) + if _, ok := addresses[client.Address()]; ok { + g.Go(func() error { + errChan <- nb.submitPoetChallenge(ctx, nodeID, deadline, client, prefix, challenge, signature) + return nil + }) + } } g.Wait() close(errChan) diff --git a/activation/nipost_test.go b/activation/nipost_test.go index a351345d9f..e60f6105dc 100644 --- a/activation/nipost_test.go +++ b/activation/nipost_test.go @@ -807,6 +807,344 @@ func TestNIPSTBuilder_PoetUnstable(t *testing.T) { }) } +// TestNIPoSTBuilder_PoETConfigChange checks if +// it properly detects added/deleted PoET services and re-registers if needed. +func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { + t.Parallel() + + sig, err := signing.NewEdSigner() + require.NoError(t, err) + + challenge := types.NIPostChallenge{ + PublishEpoch: postGenesisEpoch + 2, + } + + const ( + poetProverAddr = "http://localhost:9999" + poetProverAddr2 = "http://localhost:9988" + ) + + t.Run("1 poet deleted BEFORE round started -> continue get proof from known poets", func(t *testing.T) { + db := localsql.InMemory() + + require.NoError(t, nipost.AddChallenge(db, sig.NodeID(), &challenge)) + challengeHash := wire.NIPostChallengeToWireV1(&challenge).Hash() + + ctrl := gomock.NewController(t) + poetProver := defaultPoetServiceMock(t, ctrl, poetProverAddr) + poetProver.EXPECT().Proof(gomock.Any(), "1").AnyTimes().Return( + &types.PoetProof{}, []types.Hash32{ + challengeHash, + types.RandomHash(), + types.RandomHash(), + }, nil, + ) + + mclock := defaultLayerClockMock(ctrl) + + postClient := NewMockPostClient(ctrl) + nonce := types.VRFPostIndex(1) + postClient.EXPECT().Proof(gomock.Any(), gomock.Any()).Return( + &types.Post{ + Indices: []byte{1, 2, 3}, + }, &types.PostInfo{ + Nonce: &nonce, + }, nil, + ) + + postService := NewMockpostService(ctrl) + postService.EXPECT().Client(sig.NodeID()).Return(postClient, nil).AnyTimes() + + // successfully registered to 2 poets + err = nipost.AddPoetRegistration(db, sig.NodeID(), nipost.PoETRegistration{ + ChallengeHash: challengeHash, + Address: poetProverAddr, + RoundID: "1", + RoundEnd: time.Now().Add(10 * time.Second), + }) + require.NoError(t, err) + + err = nipost.AddPoetRegistration(db, sig.NodeID(), nipost.PoETRegistration{ + ChallengeHash: challengeHash, + Address: poetProverAddr2, + RoundID: "1", + RoundEnd: time.Now().Add(10 * time.Second), + }) + + // successful post exec + nb, err := NewNIPostBuilder( + db, + postService, + zaptest.NewLogger(t), + PoetConfig{}, + mclock, + nil, + WithPoetServices(poetProver), // add only 1 poet prover + ) + require.NoError(t, err) + + nipost, err := nb.BuildNIPost(context.Background(), sig, challengeHash, &types.NIPostChallenge{PublishEpoch: 7}) + require.NoError(t, err) + require.NotNil(t, nipost) + }) + + t.Run("1 poet added BEFORE round started -> register to missing poet, get proof from both", func(t *testing.T) { + db := localsql.InMemory() + + require.NoError(t, nipost.AddChallenge(db, sig.NodeID(), &challenge)) + challengeHash := wire.NIPostChallengeToWireV1(&challenge).Hash() + + ctrl := gomock.NewController(t) + poetProver := defaultPoetServiceMock(t, ctrl, poetProverAddr) + poetProver.EXPECT().Proof(gomock.Any(), "1").AnyTimes().Return( + &types.PoetProof{}, []types.Hash32{ + challengeHash, + types.RandomHash(), + types.RandomHash(), + }, nil, + ) + + addedPoetProver := defaultPoetServiceMock(t, ctrl, poetProverAddr2) + addedPoetProver.EXPECT().Proof(gomock.Any(), "").AnyTimes().Return( + &types.PoetProof{}, []types.Hash32{ + challengeHash, + types.RandomHash(), + types.RandomHash(), + }, nil, + ) + mclock := defaultLayerClockMock(ctrl) + + postClient := NewMockPostClient(ctrl) + nonce := types.VRFPostIndex(1) + postClient.EXPECT().Proof(gomock.Any(), gomock.Any()).Return( + &types.Post{ + Indices: []byte{1, 2, 3}, + }, &types.PostInfo{ + Nonce: &nonce, + }, nil, + ) + + postService := NewMockpostService(ctrl) + postService.EXPECT().Client(sig.NodeID()).Return(postClient, nil).AnyTimes() + + // successfully registered to 1 poet + err = nipost.AddPoetRegistration(db, sig.NodeID(), nipost.PoETRegistration{ + ChallengeHash: challengeHash, + Address: poetProverAddr, + RoundID: "1", + RoundEnd: time.Now().Add(10 * time.Second), + }) + require.NoError(t, err) + + // successful post exec + nb, err := NewNIPostBuilder( + db, + postService, + zaptest.NewLogger(t), + PoetConfig{}, + mclock, + nil, + WithPoetServices(poetProver, addedPoetProver), // add both poet provers + ) + require.NoError(t, err) + + nipost, err := nb.BuildNIPost(context.Background(), sig, challengeHash, &types.NIPostChallenge{PublishEpoch: 7}) + require.NoError(t, err) + require.NotNil(t, nipost) + }) + + t.Run("completely changed poet service BEFORE round started -> register new poet", func(t *testing.T) { + db := localsql.InMemory() + + require.NoError(t, nipost.AddChallenge(db, sig.NodeID(), &challenge)) + challengeHash := wire.NIPostChallengeToWireV1(&challenge).Hash() + + ctrl := gomock.NewController(t) + addedPoetProver := defaultPoetServiceMock(t, ctrl, poetProverAddr2) + addedPoetProver.EXPECT().Proof(gomock.Any(), "").AnyTimes().Return( + &types.PoetProof{}, []types.Hash32{ + challengeHash, + types.RandomHash(), + types.RandomHash(), + }, nil, + ) + mclock := defaultLayerClockMock(ctrl) + + postClient := NewMockPostClient(ctrl) + nonce := types.VRFPostIndex(1) + postClient.EXPECT().Proof(gomock.Any(), gomock.Any()).Return( + &types.Post{ + Indices: []byte{1, 2, 3}, + }, &types.PostInfo{ + Nonce: &nonce, + }, nil, + ) + + postService := NewMockpostService(ctrl) + postService.EXPECT().Client(sig.NodeID()).Return(postClient, nil).AnyTimes() + + // successfully registered to removed poet + err = nipost.AddPoetRegistration(db, sig.NodeID(), nipost.PoETRegistration{ + ChallengeHash: challengeHash, + Address: poetProverAddr, + RoundID: "1", + RoundEnd: time.Now().Add(10 * time.Second), + }) + require.NoError(t, err) + + // successful post exec + nb, err := NewNIPostBuilder( + db, + postService, + zaptest.NewLogger(t), + PoetConfig{}, + mclock, + nil, + WithPoetServices(addedPoetProver), // add new poet + ) + require.NoError(t, err) + + nipost, err := nb.BuildNIPost(context.Background(), sig, challengeHash, &types.NIPostChallenge{PublishEpoch: 7}) + require.NoError(t, err) + require.NotNil(t, nipost) + }) + + t.Run("1 poet added AFTER round started -> too late to register to added poet, get proof from known", + func(t *testing.T) { + db := localsql.InMemory() + const publishEpoch = 5 + challenge := types.NIPostChallenge{ + PublishEpoch: publishEpoch, + } + challengeHash := wire.NIPostChallengeToWireV1(&challenge).Hash() + + ctrl := gomock.NewController(t) + + currLayer := types.EpochID(publishEpoch - 1).FirstLayer() + genesis := time.Now().Add(-time.Duration(currLayer) * layerDuration) + + require.NoError(t, nipost.AddChallenge(db, sig.NodeID(), &challenge)) + + poetProver := defaultPoetServiceMock(t, ctrl, poetProverAddr) + poetProver.EXPECT().Proof(gomock.Any(), "1").AnyTimes().Return( + &types.PoetProof{}, []types.Hash32{ + challengeHash, + types.RandomHash(), + types.RandomHash(), + }, nil, + ) + + mclock := defaultLayerClockMock(ctrl) + mclock.EXPECT().LayerToTime(gomock.Any()).DoAndReturn( + func(got types.LayerID) time.Time { + return genesis.Add(layerDuration * time.Duration(got)) + }, + ).AnyTimes() + + postClient := NewMockPostClient(ctrl) + nonce := types.VRFPostIndex(1) + postClient.EXPECT().Proof(gomock.Any(), gomock.Any()).Return( + &types.Post{ + Indices: []byte{1, 2, 3}, + }, &types.PostInfo{ + Nonce: &nonce, + }, nil, + ) + + postService := NewMockpostService(ctrl) + postService.EXPECT().Client(sig.NodeID()).Return(postClient, nil).AnyTimes() + + // successfully registered to 2 poets + err = nipost.AddPoetRegistration(db, sig.NodeID(), nipost.PoETRegistration{ + ChallengeHash: challengeHash, + Address: poetProverAddr, + RoundID: "1", + RoundEnd: time.Now().Add(10 * time.Second), + }) + require.NoError(t, err) + + err = nipost.AddPoetRegistration(db, sig.NodeID(), nipost.PoETRegistration{ + ChallengeHash: challengeHash, + Address: poetProverAddr2, + RoundID: "1", + RoundEnd: time.Now().Add(10 * time.Second), + }) + require.NoError(t, err) + + // successful post exec + nb, err := NewNIPostBuilder( + db, + postService, + zaptest.NewLogger(t), + PoetConfig{}, + mclock, + nil, + WithPoetServices(poetProver), + ) + require.NoError(t, err) + + nipost, err := nb.BuildNIPost(context.Background(), sig, challengeHash, + &types.NIPostChallenge{PublishEpoch: publishEpoch}) + require.NoError(t, err) + require.NotNil(t, nipost) + }) + + t.Run("completely changed poet service AFTER round started -> fail, too late to register again", + func(t *testing.T) { + const publishEpoch = 5 + challenge := types.NIPostChallenge{ + PublishEpoch: publishEpoch, + } + + ctrl := gomock.NewController(t) + currLayer := types.EpochID(publishEpoch - 1).FirstLayer() + genesis := time.Now().Add(-time.Duration(currLayer) * layerDuration) + + poetProver := NewMockPoetService(ctrl) + poetProver.EXPECT().Address().Return(poetProverAddr).AnyTimes() + + mclock := NewMocklayerClock(ctrl) + mclock.EXPECT().LayerToTime(gomock.Any()).DoAndReturn( + func(got types.LayerID) time.Time { + return genesis.Add(layerDuration * time.Duration(got)) + }, + ).AnyTimes() + + db := localsql.InMemory() + + require.NoError(t, nipost.AddChallenge(db, sig.NodeID(), &challenge)) + challengeHash := wire.NIPostChallengeToWireV1(&challenge).Hash() + + // successfully registered to removed poet + err = nipost.AddPoetRegistration(db, sig.NodeID(), nipost.PoETRegistration{ + ChallengeHash: challengeHash, + Address: poetProverAddr2, + RoundID: "1", + RoundEnd: time.Now().Add(10 * time.Second), + }) + + require.NoError(t, err) + postService := NewMockpostService(ctrl) + + nb, err := NewNIPostBuilder( + localsql.InMemory(), + postService, + zaptest.NewLogger(t), + PoetConfig{}, + mclock, + nil, + WithPoetServices(poetProver), + ) + require.NoError(t, err) + + nipost, err := nb.BuildNIPost(context.Background(), sig, types.RandomHash(), + &types.NIPostChallenge{PublishEpoch: currLayer.GetEpoch()}) + require.ErrorIs(t, err, ErrATXChallengeExpired) + require.ErrorContains(t, err, "poet round has already started") + require.Nil(t, nipost) + }) +} + // TestNIPoSTBuilder_StaleChallenge checks if // it properly detects that the challenge is stale and the poet round has already started. func TestNIPoSTBuilder_StaleChallenge(t *testing.T) { @@ -818,12 +1156,14 @@ func TestNIPoSTBuilder_StaleChallenge(t *testing.T) { sig, err := signing.NewEdSigner() require.NoError(t, err) + const poetAddr = "http://localhost:9999" + // Act & Verify t.Run("no requests, poet round started", func(t *testing.T) { ctrl := gomock.NewController(t) mclock := NewMocklayerClock(ctrl) poetProver := NewMockPoetService(ctrl) - poetProver.EXPECT().Address().Return("http://localhost:9999") + poetProver.EXPECT().Address().Return(poetAddr).AnyTimes() mclock.EXPECT().LayerToTime(gomock.Any()).DoAndReturn( func(got types.LayerID) time.Time { return genesis.Add(layerDuration * time.Duration(got)) @@ -852,7 +1192,7 @@ func TestNIPoSTBuilder_StaleChallenge(t *testing.T) { ctrl := gomock.NewController(t) mclock := NewMocklayerClock(ctrl) poetProver := NewMockPoetService(ctrl) - poetProver.EXPECT().Address().Return("http://localhost:9999") + poetProver.EXPECT().Address().Return(poetAddr).AnyTimes() mclock.EXPECT().LayerToTime(gomock.Any()).DoAndReturn( func(got types.LayerID) time.Time { return genesis.Add(layerDuration * time.Duration(got)) @@ -879,7 +1219,7 @@ func TestNIPoSTBuilder_StaleChallenge(t *testing.T) { // successfully registered to at least one poet err = nipost.AddPoetRegistration(db, sig.NodeID(), nipost.PoETRegistration{ ChallengeHash: challengeHash, - Address: "http://poet1.com", + Address: poetAddr, RoundID: "1", RoundEnd: time.Now().Add(10 * time.Second), }) @@ -894,7 +1234,7 @@ func TestNIPoSTBuilder_StaleChallenge(t *testing.T) { ctrl := gomock.NewController(t) mclock := NewMocklayerClock(ctrl) poetProver := NewMockPoetService(ctrl) - poetProver.EXPECT().Address().Return("http://localhost:9999") + poetProver.EXPECT().Address().Return(poetAddr).AnyTimes() mclock.EXPECT().LayerToTime(gomock.Any()).DoAndReturn( func(got types.LayerID) time.Time { return genesis.Add(layerDuration * time.Duration(got)) @@ -921,7 +1261,7 @@ func TestNIPoSTBuilder_StaleChallenge(t *testing.T) { // successfully registered to at least one poet err = nipost.AddPoetRegistration(db, sig.NodeID(), nipost.PoETRegistration{ ChallengeHash: challengeHash, - Address: "http://poet1.com", + Address: poetAddr, RoundID: "1", RoundEnd: time.Now().Add(10 * time.Second), }) diff --git a/sql/localsql/nipost/poet_registration.go b/sql/localsql/nipost/poet_registration.go index 4a8b137248..7254c6a0f0 100644 --- a/sql/localsql/nipost/poet_registration.go +++ b/sql/localsql/nipost/poet_registration.go @@ -38,23 +38,6 @@ func AddPoetRegistration( return nil } -/*func PoetRegistrationCount(db sql.Executor, nodeID types.NodeID) (int, error) { - var count int - enc := func(stmt *sql.Statement) { - stmt.BindBytes(1, nodeID.Bytes()) - } - dec := func(stmt *sql.Statement) bool { - count = int(stmt.ColumnInt64(0)) - return true - } - query := `select count(*) from poet_registration where id = ?1;` - _, err := db.Exec(query, enc, dec) - if err != nil { - return 0, fmt.Errorf("get poet registration count for node id %s: %w", nodeID.ShortString(), err) - } - return count, nil -}*/ - func PoetRegistrationCount(db sql.Executor, nodeID types.NodeID, addresses ...string) (int, error) { var ( count int @@ -75,7 +58,8 @@ func PoetRegistrationCount(db sql.Executor, nodeID types.NodeID, addresses ...st placeholders[i] = fmt.Sprintf("?%d", i+2) // Generate placeholders starting from ?2 } // Construct the query with IN clause for addresses - query = fmt.Sprintf(`select count(*) from poet_registration where id = ?1 and address IN (%s);`, strings.Join(placeholders, ", ")) + query = fmt.Sprintf(`select count(*) from poet_registration where id = ?1 and address IN (%s);`, + strings.Join(placeholders, ", ")) } else { enc = func(stmt *sql.Statement) { @@ -93,7 +77,8 @@ func PoetRegistrationCount(db sql.Executor, nodeID types.NodeID, addresses ...st _, err := db.Exec(query, enc, dec) if err != nil { if len(addresses) > 0 { - return 0, fmt.Errorf("get poet registration count for node id %s and addresses %v: %w", nodeID.ShortString(), addresses, err) + return 0, fmt.Errorf("get poet registration count for node id %s and addresses %v: %w", + nodeID.ShortString(), addresses, err) } return 0, fmt.Errorf("get poet registration count for node id %s: %w", nodeID.ShortString(), err) } @@ -110,11 +95,18 @@ func ClearPoetRegistrations(db sql.Executor, nodeID types.NodeID) error { return nil } -func PoetRegistrations(db sql.Executor, nodeID types.NodeID) ([]PoETRegistration, error) { +func PoetRegistrations(db sql.Executor, nodeID types.NodeID, addresses ...string) ([]PoETRegistration, error) { var registrations []PoETRegistration + enc := func(stmt *sql.Statement) { stmt.BindBytes(1, nodeID.Bytes()) + if len(addresses) > 0 { + for i, address := range addresses { + stmt.BindText(i+2, address) + } + } } + dec := func(stmt *sql.Statement) bool { registration := PoETRegistration{ Address: stmt.ColumnText(1), @@ -125,10 +117,29 @@ func PoetRegistrations(db sql.Executor, nodeID types.NodeID) ([]PoETRegistration registrations = append(registrations, registration) return true } - query := `select hash, address, round_id, round_end from poet_registration where id = ?1;` + + var query string + if len(addresses) > 0 { + placeholders := make([]string, len(addresses)) + for i := range addresses { + placeholders[i] = fmt.Sprintf("?%d", i+2) + } + query = fmt.Sprintf(` + SELECT hash, address, round_id, round_end + FROM poet_registration + WHERE id = ?1 AND address IN (%s);`, + strings.Join(placeholders, ", ")) + } else { + query = ` + SELECT hash, address, round_id, round_end + FROM poet_registration + WHERE id = ?1;` + } + _, err := db.Exec(query, enc, dec) if err != nil { return nil, fmt.Errorf("get poet registrations for node id %s: %w", nodeID.ShortString(), err) } + return registrations, nil } diff --git a/sql/localsql/nipost/poet_registration_test.go b/sql/localsql/nipost/poet_registration_test.go index 4c37532c1f..9f155cc55f 100644 --- a/sql/localsql/nipost/poet_registration_test.go +++ b/sql/localsql/nipost/poet_registration_test.go @@ -42,6 +42,47 @@ func Test_AddPoetRegistration(t *testing.T) { require.NoError(t, err) require.Equal(t, 2, count) + registrations, err := PoetRegistrations(db, nodeID) + require.NoError(t, err) + require.Len(t, registrations, 2) + require.Equal(t, reg1, registrations[0]) + require.Equal(t, reg2, registrations[1]) + + err = ClearPoetRegistrations(db, nodeID) + require.NoError(t, err) + + count, err = PoetRegistrationCount(db, nodeID) + require.NoError(t, err) + require.Equal(t, 0, count) +} + +func Test_PoetRegistrations_and_PoetRegistrationCount(t *testing.T) { + db := localsql.InMemory() + + nodeID := types.RandomNodeID() + reg1 := PoETRegistration{ + ChallengeHash: types.RandomHash(), + Address: "address1", + RoundID: "round1", + RoundEnd: time.Now().Round(time.Second), + } + reg2 := PoETRegistration{ + ChallengeHash: types.RandomHash(), + Address: "address2", + RoundID: "round2", + RoundEnd: time.Now().Round(time.Second), + } + + err := AddPoetRegistration(db, nodeID, reg1) + require.NoError(t, err) + + err = AddPoetRegistration(db, nodeID, reg2) + require.NoError(t, err) + + count, err := PoetRegistrationCount(db, nodeID) + require.NoError(t, err) + require.Equal(t, 2, count) + count, err = PoetRegistrationCount(db, nodeID, "address2") require.NoError(t, err) require.Equal(t, 1, count) @@ -56,12 +97,21 @@ func Test_AddPoetRegistration(t *testing.T) { require.Equal(t, reg1, registrations[0]) require.Equal(t, reg2, registrations[1]) - err = ClearPoetRegistrations(db, nodeID) + registrations, err = PoetRegistrations(db, nodeID, "address1") require.NoError(t, err) + require.Len(t, registrations, 1) + require.Equal(t, reg1, registrations[0]) - count, err = PoetRegistrationCount(db, nodeID) + registrations, err = PoetRegistrations(db, nodeID, "address2") require.NoError(t, err) - require.Equal(t, 0, count) + require.Len(t, registrations, 1) + require.Equal(t, reg2, registrations[0]) + + registrations, err = PoetRegistrations(db, nodeID, "address2", "address1") + require.NoError(t, err) + require.Len(t, registrations, 2) + require.Equal(t, reg1, registrations[0]) + require.Equal(t, reg2, registrations[1]) } func Test_AddPoetRegistration_NoDuplicates(t *testing.T) { From 8080ed0814b02c38692b3741c0affec3bd23fc27 Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Fri, 12 Jul 2024 00:08:16 +0200 Subject: [PATCH 03/26] fixed: - minor issues - tests made refactoring --- activation/nipost.go | 161 +++++++++--------- activation/nipost_test.go | 70 +++----- cmd/merge-nodes/internal/merge_action_test.go | 6 +- sql/localsql/nipost/poet_registration.go | 107 +++++------- sql/localsql/nipost/poet_registration_test.go | 44 ++--- 5 files changed, 161 insertions(+), 227 deletions(-) diff --git a/activation/nipost.go b/activation/nipost.go index dd1b78c090..a8f2227537 100644 --- a/activation/nipost.go +++ b/activation/nipost.go @@ -229,66 +229,15 @@ func (nb *NIPostBuilder) BuildNIPost( ) // Phase 0: Submit challenge to PoET services. - addresses := make([]string, 0) - for _, client := range nb.poetProvers { - addresses = append(addresses, client.Address()) - } + // Deadline: start of PoET round: we will not accept registrations after that + submitedRegistrations, err := nb.submitPoetChallenges( + ctx, + signer, + poetProofDeadline, + curPoetRoundStart, challenge.Bytes()) - // check, if already registered for some PoET services with given addresses - existingRegistrations, err := nipost.PoetRegistrations(nb.localDB, signer.NodeID(), addresses...) if err != nil { - return nil, fmt.Errorf("failed to get poet registrations from db: %w", err) - } - - // check if some registrations missing - existingRegistrationsMap := make(map[string]bool) - for _, er := range existingRegistrations { - existingRegistrationsMap[er.Address] = true - } - - missingRegistrationsPoETAddresses := make(map[string]bool) - for _, addr := range addresses { - if _, ok := existingRegistrationsMap[addr]; !ok { - missingRegistrationsPoETAddresses[addr] = true - } - } - - if len(missingRegistrationsPoETAddresses) > 0 { - now := time.Now() - - // Deadline: start of PoET round: we will not accept registrations after that - if !curPoetRoundStart.Before(now) { - // send registrations to missing addresses - submitCtx, cancel := context.WithDeadline(ctx, curPoetRoundStart) - defer cancel() - - if err := nb.submitPoetChallengesToAddresses( - submitCtx, - signer, - poetProofDeadline, - missingRegistrationsPoETAddresses, challenge.Bytes()); err != nil { - return nil, fmt.Errorf("submitting to poets: %w", err) - } - - existingRegistrationsCount, err := nipost.PoetRegistrationCount(nb.localDB, signer.NodeID(), addresses...) - if err != nil { - return nil, fmt.Errorf("failed to get poet registration count: %w", err) - } - if existingRegistrationsCount == 0 { - return nil, - &PoetSvcUnstableError{msg: "failed to submit challenge to any PoET", source: submitCtx.Err()} - } - } else { - if len(missingRegistrationsPoETAddresses) == len(addresses) { - // no registrations and deadline passed -> drop existing challenge - return nil, fmt.Errorf( - "%w: poet round has already started at %s (now: %s)", - ErrATXChallengeExpired, // drop current registration challenge - curPoetRoundStart, - now, - ) - } - } + return nil, fmt.Errorf("submitting to poets: %w", err) } // Phase 1: query PoET services for proofs @@ -311,7 +260,7 @@ func (nb *NIPostBuilder) BuildNIPost( } events.EmitPoetWaitProof(signer.NodeID(), postChallenge.PublishEpoch, curPoetRoundEnd) - poetProofRef, membership, err = nb.getBestProof(ctx, signer.NodeID(), challenge, postChallenge.PublishEpoch) + poetProofRef, membership, err = nb.getBestProof(ctx, signer.NodeID(), challenge, submitedRegistrations) if err != nil { return nil, &PoetSvcUnstableError{msg: "getBestProof failed", source: err} } @@ -419,23 +368,70 @@ func (nb *NIPostBuilder) submitPoetChallenge( } // Submit the challenge to registered PoETs with given addresses. -func (nb *NIPostBuilder) submitPoetChallengesToAddresses( +func (nb *NIPostBuilder) submitPoetChallenges( ctx context.Context, signer *signing.EdSigner, - deadline time.Time, - addresses map[string]bool, + poetProofDeadline time.Time, + curPoetRoundStartDeadline time.Time, challenge []byte, -) error { +) ([]nipost.PoETRegistration, error) { + // check if some registrations missing + existingRegistrations, err := nipost.PoetRegistrationsByNodeId(nb.localDB, signer.NodeID()) + if err != nil { + return nil, fmt.Errorf("failed to get poet registrations from db: %w", err) + } + + var i int + addresses := make([]string, len(nb.poetProvers)) + for _, client := range nb.poetProvers { + addresses[i] = client.Address() + i++ + } + + existingRegistrationsMap := make(map[string]struct{}) + for _, er := range existingRegistrations { + existingRegistrationsMap[er.Address] = struct{}{} + } + + missingRegistrationsPoETAddresses := make(map[string]struct{}) + for _, addr := range addresses { + if _, ok := existingRegistrationsMap[addr]; !ok { + missingRegistrationsPoETAddresses[addr] = struct{}{} + } + } + + if len(missingRegistrationsPoETAddresses) == 0 { + return existingRegistrations, nil + } + + now := time.Now() + if curPoetRoundStartDeadline.Before(now) { + if len(missingRegistrationsPoETAddresses) == len(addresses) { + return nil, fmt.Errorf( + "%w: poet round has already started at %s (now: %s)", + ErrATXChallengeExpired, // no existing registration, drop current registration challenge + curPoetRoundStartDeadline, + now, + ) + } + return existingRegistrations, nil + } + + // send registrations to missing addresses signature := signer.Sign(signing.POET, challenge) prefix := bytes.Join([][]byte{signer.Prefix(), {byte(signing.POET)}}, nil) nodeID := signer.NodeID() - g, ctx := errgroup.WithContext(ctx) - errChan := make(chan error, len(addresses)) + + submitCtx, cancel := context.WithDeadline(ctx, curPoetRoundStartDeadline) + defer cancel() + + g, ctx := errgroup.WithContext(submitCtx) + errChan := make(chan error, len(missingRegistrationsPoETAddresses)) for _, client := range nb.poetProvers { - if _, ok := addresses[client.Address()]; ok { + if _, ok := missingRegistrationsPoETAddresses[client.Address()]; ok { g.Go(func() error { - errChan <- nb.submitPoetChallenge(ctx, nodeID, deadline, client, prefix, challenge, signature) + errChan <- nb.submitPoetChallenge(ctx, nodeID, poetProofDeadline, client, prefix, challenge, signature) return nil }) } @@ -457,18 +453,19 @@ func (nb *NIPostBuilder) submitPoetChallengesToAddresses( } if allInvalid { nb.logger.Warn("all poet submits were too late. ATX challenge expires", log.ZShortStringer("smesherID", nodeID)) - return ErrATXChallengeExpired + return nil, ErrATXChallengeExpired } - return nil -} -func (nb *NIPostBuilder) getPoetService(ctx context.Context, address string) PoetService { - for _, service := range nb.poetProvers { - if address == service.Address() { - return service - } + existingRegistrations, err = nipost.PoetRegistrationsByNodeIdAndAddresses(nb.localDB, signer.NodeID(), addresses) + if err != nil { + return nil, fmt.Errorf("failed to get poet registration count: %w", err) } - return nil + + if len(existingRegistrations) == 0 { + return nil, &PoetSvcUnstableError{msg: "failed to submit challenge to any PoET", source: ctx.Err()} + } + + return existingRegistrations, nil } // membersContainChallenge verifies that the challenge is included in proof's members. @@ -485,16 +482,12 @@ func (nb *NIPostBuilder) getBestProof( ctx context.Context, nodeID types.NodeID, challenge types.Hash32, - publishEpoch types.EpochID, + registrations []nipost.PoETRegistration, ) (types.PoetProofRef, *types.MerkleProof, error) { type poetProof struct { poet *types.PoetProof membership *types.MerkleProof } - registrations, err := nipost.PoetRegistrations(nb.localDB, nodeID) - if err != nil { - return types.PoetProofRef{}, nil, fmt.Errorf("getting poet registrations: %w", err) - } proofs := make(chan *poetProof, len(registrations)) var eg errgroup.Group @@ -505,11 +498,13 @@ func (nb *NIPostBuilder) getBestProof( zap.String("poet_address", r.Address), zap.String("round", r.RoundID), ) - client := nb.getPoetService(ctx, r.Address) - if client == nil { + + client, ok := nb.poetProvers[r.Address] + if !ok { logger.Warn("poet client not found") continue } + round := r.RoundID waitDeadline := proofDeadline(r.RoundEnd, nb.poetCfg.CycleGap) eg.Go(func() error { diff --git a/activation/nipost_test.go b/activation/nipost_test.go index e60f6105dc..6233dd5a52 100644 --- a/activation/nipost_test.go +++ b/activation/nipost_test.go @@ -832,12 +832,8 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { ctrl := gomock.NewController(t) poetProver := defaultPoetServiceMock(t, ctrl, poetProverAddr) - poetProver.EXPECT().Proof(gomock.Any(), "1").AnyTimes().Return( - &types.PoetProof{}, []types.Hash32{ - challengeHash, - types.RandomHash(), - types.RandomHash(), - }, nil, + poetProver.EXPECT().Proof(gomock.Any(), "1").Return( + &types.PoetProof{}, []types.Hash32{challengeHash}, nil, ) mclock := defaultLayerClockMock(ctrl) @@ -860,7 +856,7 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { ChallengeHash: challengeHash, Address: poetProverAddr, RoundID: "1", - RoundEnd: time.Now().Add(10 * time.Second), + RoundEnd: time.Now().Add(1 * time.Second), }) require.NoError(t, err) @@ -868,7 +864,7 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { ChallengeHash: challengeHash, Address: poetProverAddr2, RoundID: "1", - RoundEnd: time.Now().Add(10 * time.Second), + RoundEnd: time.Now().Add(1 * time.Second), }) // successful post exec @@ -896,22 +892,15 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { ctrl := gomock.NewController(t) poetProver := defaultPoetServiceMock(t, ctrl, poetProverAddr) - poetProver.EXPECT().Proof(gomock.Any(), "1").AnyTimes().Return( - &types.PoetProof{}, []types.Hash32{ - challengeHash, - types.RandomHash(), - types.RandomHash(), - }, nil, + poetProver.EXPECT().Proof(gomock.Any(), "1").Return( + &types.PoetProof{}, []types.Hash32{challengeHash}, nil, ) addedPoetProver := defaultPoetServiceMock(t, ctrl, poetProverAddr2) - addedPoetProver.EXPECT().Proof(gomock.Any(), "").AnyTimes().Return( - &types.PoetProof{}, []types.Hash32{ - challengeHash, - types.RandomHash(), - types.RandomHash(), - }, nil, + addedPoetProver.EXPECT().Proof(gomock.Any(), "").Return( + &types.PoetProof{}, []types.Hash32{challengeHash}, nil, ) + mclock := defaultLayerClockMock(ctrl) postClient := NewMockPostClient(ctrl) @@ -932,7 +921,7 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { ChallengeHash: challengeHash, Address: poetProverAddr, RoundID: "1", - RoundEnd: time.Now().Add(10 * time.Second), + RoundEnd: time.Now().Add(1 * time.Second), }) require.NoError(t, err) @@ -961,13 +950,10 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { ctrl := gomock.NewController(t) addedPoetProver := defaultPoetServiceMock(t, ctrl, poetProverAddr2) - addedPoetProver.EXPECT().Proof(gomock.Any(), "").AnyTimes().Return( - &types.PoetProof{}, []types.Hash32{ - challengeHash, - types.RandomHash(), - types.RandomHash(), - }, nil, + addedPoetProver.EXPECT().Proof(gomock.Any(), "").Return( + &types.PoetProof{}, []types.Hash32{challengeHash}, nil, ) + mclock := defaultLayerClockMock(ctrl) postClient := NewMockPostClient(ctrl) @@ -988,7 +974,7 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { ChallengeHash: challengeHash, Address: poetProverAddr, RoundID: "1", - RoundEnd: time.Now().Add(10 * time.Second), + RoundEnd: time.Now().Add(1 * time.Second), }) require.NoError(t, err) @@ -1026,15 +1012,13 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { require.NoError(t, nipost.AddChallenge(db, sig.NodeID(), &challenge)) poetProver := defaultPoetServiceMock(t, ctrl, poetProverAddr) - poetProver.EXPECT().Proof(gomock.Any(), "1").AnyTimes().Return( - &types.PoetProof{}, []types.Hash32{ - challengeHash, - types.RandomHash(), - types.RandomHash(), - }, nil, + poetProver.EXPECT().Proof(gomock.Any(), "1").Return( + &types.PoetProof{}, []types.Hash32{challengeHash}, nil, ) - mclock := defaultLayerClockMock(ctrl) + addedPoetProver := defaultPoetServiceMock(t, ctrl, poetProverAddr2) + + mclock := NewMocklayerClock(ctrl) mclock.EXPECT().LayerToTime(gomock.Any()).DoAndReturn( func(got types.LayerID) time.Time { return genesis.Add(layerDuration * time.Duration(got)) @@ -1054,20 +1038,12 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { postService := NewMockpostService(ctrl) postService.EXPECT().Client(sig.NodeID()).Return(postClient, nil).AnyTimes() - // successfully registered to 2 poets + // successfully registered to 1 poet err = nipost.AddPoetRegistration(db, sig.NodeID(), nipost.PoETRegistration{ ChallengeHash: challengeHash, Address: poetProverAddr, RoundID: "1", - RoundEnd: time.Now().Add(10 * time.Second), - }) - require.NoError(t, err) - - err = nipost.AddPoetRegistration(db, sig.NodeID(), nipost.PoETRegistration{ - ChallengeHash: challengeHash, - Address: poetProverAddr2, - RoundID: "1", - RoundEnd: time.Now().Add(10 * time.Second), + RoundEnd: time.Now().Add(1 * time.Second), }) require.NoError(t, err) @@ -1079,7 +1055,7 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { PoetConfig{}, mclock, nil, - WithPoetServices(poetProver), + WithPoetServices(poetProver, addedPoetProver), ) require.NoError(t, err) @@ -1120,7 +1096,7 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { ChallengeHash: challengeHash, Address: poetProverAddr2, RoundID: "1", - RoundEnd: time.Now().Add(10 * time.Second), + RoundEnd: time.Now().Add(1 * time.Second), }) require.NoError(t, err) diff --git a/cmd/merge-nodes/internal/merge_action_test.go b/cmd/merge-nodes/internal/merge_action_test.go index f2fdc11135..84be92650b 100644 --- a/cmd/merge-nodes/internal/merge_action_test.go +++ b/cmd/merge-nodes/internal/merge_action_test.go @@ -332,7 +332,7 @@ func Test_MergeDBs_Successful_Existing_Node(t *testing.T) { require.NoError(t, err) require.Equal(t, sig1Post, *post) - poet1, err := nipost.PoetRegistrations(dstDB, sig1.NodeID()) + poet1, err := nipost.PoetRegistrationsByNodeId(dstDB, sig1.NodeID()) require.NoError(t, err) require.Equal(t, poet1[0], sig1Poet1) require.Equal(t, poet1[1], sig1Poet2) @@ -345,7 +345,7 @@ func Test_MergeDBs_Successful_Existing_Node(t *testing.T) { require.NoError(t, err) require.Equal(t, sig2Post, *post) - poet2, err := nipost.PoetRegistrations(dstDB, sig2.NodeID()) + poet2, err := nipost.PoetRegistrationsByNodeId(dstDB, sig2.NodeID()) require.NoError(t, err) require.Equal(t, poet2[0], sig2Poet1) require.Equal(t, poet2[1], sig2Poet2) @@ -432,7 +432,7 @@ func Test_MergeDBs_Successful_Empty_Dir(t *testing.T) { require.NoError(t, err) require.Equal(t, sigPost, *post) - poet, err := nipost.PoetRegistrations(dstDB, sig.NodeID()) + poet, err := nipost.PoetRegistrationsByNodeId(dstDB, sig.NodeID()) require.NoError(t, err) require.Equal(t, poet[0], sigPoet1) require.Equal(t, poet[1], sigPoet2) diff --git a/sql/localsql/nipost/poet_registration.go b/sql/localsql/nipost/poet_registration.go index 7254c6a0f0..3e9f4f9696 100644 --- a/sql/localsql/nipost/poet_registration.go +++ b/sql/localsql/nipost/poet_registration.go @@ -38,72 +38,55 @@ func AddPoetRegistration( return nil } -func PoetRegistrationCount(db sql.Executor, nodeID types.NodeID, addresses ...string) (int, error) { - var ( - count int - enc func(stmt *sql.Statement) - query string - ) - - if len(addresses) > 0 { - enc = func(stmt *sql.Statement) { - stmt.BindBytes(1, nodeID.Bytes()) - for i, addr := range addresses { - stmt.BindText(i+2, addr) // Start binding addresses at position 2 - } - } - - placeholders := make([]string, len(addresses)) - for i := range placeholders { - placeholders[i] = fmt.Sprintf("?%d", i+2) // Generate placeholders starting from ?2 - } - // Construct the query with IN clause for addresses - query = fmt.Sprintf(`select count(*) from poet_registration where id = ?1 and address IN (%s);`, - strings.Join(placeholders, ", ")) +func ClearPoetRegistrations(db sql.Executor, nodeID types.NodeID) error { + enc := func(stmt *sql.Statement) { + stmt.BindBytes(1, nodeID.Bytes()) + } + if _, err := db.Exec(`delete from poet_registration where id = ?1;`, enc, nil); err != nil { + return fmt.Errorf("clear poet registrations for %s: %w", nodeID.ShortString(), err) + } + return nil +} - } else { - enc = func(stmt *sql.Statement) { - stmt.BindBytes(1, nodeID.Bytes()) - } +func PoetRegistrationsByNodeId(db sql.Executor, nodeID types.NodeID) ([]PoETRegistration, error) { + var registrations []PoETRegistration - query = `select count(*) from poet_registration where id = ?1;` + enc := func(stmt *sql.Statement) { + stmt.BindBytes(1, nodeID.Bytes()) } dec := func(stmt *sql.Statement) bool { - count = int(stmt.ColumnInt64(0)) + registration := PoETRegistration{ + Address: stmt.ColumnText(1), + RoundID: stmt.ColumnText(2), + RoundEnd: time.Unix(stmt.ColumnInt64(3), 0), + } + stmt.ColumnBytes(0, registration.ChallengeHash[:]) + registrations = append(registrations, registration) return true } + query := `SELECT hash, address, round_id, round_end FROM poet_registration WHERE id = ?1;` + _, err := db.Exec(query, enc, dec) if err != nil { - if len(addresses) > 0 { - return 0, fmt.Errorf("get poet registration count for node id %s and addresses %v: %w", - nodeID.ShortString(), addresses, err) - } - return 0, fmt.Errorf("get poet registration count for node id %s: %w", nodeID.ShortString(), err) + return nil, fmt.Errorf("get poet registrations for node id %s: %w", nodeID.ShortString(), err) } - return count, nil -} -func ClearPoetRegistrations(db sql.Executor, nodeID types.NodeID) error { - enc := func(stmt *sql.Statement) { - stmt.BindBytes(1, nodeID.Bytes()) - } - if _, err := db.Exec(`delete from poet_registration where id = ?1;`, enc, nil); err != nil { - return fmt.Errorf("clear poet registrations for %s: %w", nodeID.ShortString(), err) - } - return nil + return registrations, nil } -func PoetRegistrations(db sql.Executor, nodeID types.NodeID, addresses ...string) ([]PoETRegistration, error) { +func PoetRegistrationsByNodeIdAndAddresses(db sql.Executor, nodeID types.NodeID, addresses []string) ([]PoETRegistration, error) { var registrations []PoETRegistration + if len(addresses) == 0 { + return nil, fmt.Errorf("addresses list is empty") + } + enc := func(stmt *sql.Statement) { stmt.BindBytes(1, nodeID.Bytes()) - if len(addresses) > 0 { - for i, address := range addresses { - stmt.BindText(i+2, address) - } + for i, address := range addresses { + stmt.BindText(i+2, address) } } @@ -118,27 +101,21 @@ func PoetRegistrations(db sql.Executor, nodeID types.NodeID, addresses ...string return true } - var query string - if len(addresses) > 0 { - placeholders := make([]string, len(addresses)) - for i := range addresses { - placeholders[i] = fmt.Sprintf("?%d", i+2) - } - query = fmt.Sprintf(` - SELECT hash, address, round_id, round_end - FROM poet_registration - WHERE id = ?1 AND address IN (%s);`, - strings.Join(placeholders, ", ")) - } else { - query = ` - SELECT hash, address, round_id, round_end - FROM poet_registration - WHERE id = ?1;` + placeholders := make([]string, len(addresses)) + for i := range addresses { + placeholders[i] = fmt.Sprintf("?%d", i+2) } + query := fmt.Sprintf( + `SELECT hash, address, round_id, round_end + FROM poet_registration + WHERE id = ?1 AND address IN (%s);`, + strings.Join(placeholders, ", "), + ) + _, err := db.Exec(query, enc, dec) if err != nil { - return nil, fmt.Errorf("get poet registrations for node id %s: %w", nodeID.ShortString(), err) + return nil, fmt.Errorf("get poet registrations for node id %s and addresses %v: %w", nodeID.ShortString(), addresses, err) } return registrations, nil diff --git a/sql/localsql/nipost/poet_registration_test.go b/sql/localsql/nipost/poet_registration_test.go index 9f155cc55f..d71a029bc9 100644 --- a/sql/localsql/nipost/poet_registration_test.go +++ b/sql/localsql/nipost/poet_registration_test.go @@ -31,18 +31,14 @@ func Test_AddPoetRegistration(t *testing.T) { err := AddPoetRegistration(db, nodeID, reg1) require.NoError(t, err) - count, err := PoetRegistrationCount(db, nodeID) + registrations, err := PoetRegistrationsByNodeId(db, nodeID) require.NoError(t, err) - require.Equal(t, 1, count) + require.Len(t, registrations, 1) err = AddPoetRegistration(db, nodeID, reg2) require.NoError(t, err) - count, err = PoetRegistrationCount(db, nodeID) - require.NoError(t, err) - require.Equal(t, 2, count) - - registrations, err := PoetRegistrations(db, nodeID) + registrations, err = PoetRegistrationsByNodeId(db, nodeID) require.NoError(t, err) require.Len(t, registrations, 2) require.Equal(t, reg1, registrations[0]) @@ -51,12 +47,14 @@ func Test_AddPoetRegistration(t *testing.T) { err = ClearPoetRegistrations(db, nodeID) require.NoError(t, err) - count, err = PoetRegistrationCount(db, nodeID) + registrations, err = PoetRegistrationsByNodeId(db, nodeID) require.NoError(t, err) - require.Equal(t, 0, count) + require.Len(t, registrations, 0) } func Test_PoetRegistrations_and_PoetRegistrationCount(t *testing.T) { + t.Parallel() + db := localsql.InMemory() nodeID := types.RandomNodeID() @@ -79,35 +77,23 @@ func Test_PoetRegistrations_and_PoetRegistrationCount(t *testing.T) { err = AddPoetRegistration(db, nodeID, reg2) require.NoError(t, err) - count, err := PoetRegistrationCount(db, nodeID) - require.NoError(t, err) - require.Equal(t, 2, count) - - count, err = PoetRegistrationCount(db, nodeID, "address2") - require.NoError(t, err) - require.Equal(t, 1, count) - - count, err = PoetRegistrationCount(db, nodeID, "address2", "address1") - require.NoError(t, err) - require.Equal(t, 2, count) - - registrations, err := PoetRegistrations(db, nodeID) + registrations, err := PoetRegistrationsByNodeId(db, nodeID) require.NoError(t, err) require.Len(t, registrations, 2) require.Equal(t, reg1, registrations[0]) require.Equal(t, reg2, registrations[1]) - registrations, err = PoetRegistrations(db, nodeID, "address1") + registrations, err = PoetRegistrationsByNodeIdAndAddresses(db, nodeID, []string{"address1"}) require.NoError(t, err) require.Len(t, registrations, 1) require.Equal(t, reg1, registrations[0]) - registrations, err = PoetRegistrations(db, nodeID, "address2") + registrations, err = PoetRegistrationsByNodeIdAndAddresses(db, nodeID, []string{"address2"}) require.NoError(t, err) require.Len(t, registrations, 1) require.Equal(t, reg2, registrations[0]) - registrations, err = PoetRegistrations(db, nodeID, "address2", "address1") + registrations, err = PoetRegistrationsByNodeIdAndAddresses(db, nodeID, []string{"address2", "address1"}) require.NoError(t, err) require.Len(t, registrations, 2) require.Equal(t, reg1, registrations[0]) @@ -128,14 +114,14 @@ func Test_AddPoetRegistration_NoDuplicates(t *testing.T) { err := AddPoetRegistration(db, nodeID, reg) require.NoError(t, err) - count, err := PoetRegistrationCount(db, nodeID) + registrations, err := PoetRegistrationsByNodeId(db, nodeID) require.NoError(t, err) - require.Equal(t, 1, count) + require.Len(t, registrations, 1) err = AddPoetRegistration(db, nodeID, reg) require.ErrorIs(t, err, sql.ErrObjectExists) - count, err = PoetRegistrationCount(db, nodeID) + registrations, err = PoetRegistrationsByNodeId(db, nodeID) require.NoError(t, err) - require.Equal(t, 1, count) + require.Len(t, registrations, 1) } From 7a371a298653c96e6e7b6039ad62a50335bce839 Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Fri, 12 Jul 2024 00:14:15 +0200 Subject: [PATCH 04/26] fix lint --- activation/nipost.go | 1 - sql/localsql/nipost/poet_registration.go | 11 ++++++++--- sql/localsql/nipost/poet_registration_test.go | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/activation/nipost.go b/activation/nipost.go index a8f2227537..986bafbff2 100644 --- a/activation/nipost.go +++ b/activation/nipost.go @@ -235,7 +235,6 @@ func (nb *NIPostBuilder) BuildNIPost( signer, poetProofDeadline, curPoetRoundStart, challenge.Bytes()) - if err != nil { return nil, fmt.Errorf("submitting to poets: %w", err) } diff --git a/sql/localsql/nipost/poet_registration.go b/sql/localsql/nipost/poet_registration.go index 3e9f4f9696..4abed7ca06 100644 --- a/sql/localsql/nipost/poet_registration.go +++ b/sql/localsql/nipost/poet_registration.go @@ -1,6 +1,7 @@ package nipost import ( + "errors" "fmt" "strings" "time" @@ -76,11 +77,14 @@ func PoetRegistrationsByNodeId(db sql.Executor, nodeID types.NodeID) ([]PoETRegi return registrations, nil } -func PoetRegistrationsByNodeIdAndAddresses(db sql.Executor, nodeID types.NodeID, addresses []string) ([]PoETRegistration, error) { +func PoetRegistrationsByNodeIdAndAddresses( + db sql.Executor, + nodeID types.NodeID, + addresses []string) ([]PoETRegistration, error) { var registrations []PoETRegistration if len(addresses) == 0 { - return nil, fmt.Errorf("addresses list is empty") + return nil, errors.New("addresses list is empty") } enc := func(stmt *sql.Statement) { @@ -115,7 +119,8 @@ func PoetRegistrationsByNodeIdAndAddresses(db sql.Executor, nodeID types.NodeID, _, err := db.Exec(query, enc, dec) if err != nil { - return nil, fmt.Errorf("get poet registrations for node id %s and addresses %v: %w", nodeID.ShortString(), addresses, err) + return nil, fmt.Errorf("get poet registrations for node id %s and addresses %v: %w", + nodeID.ShortString(), addresses, err) } return registrations, nil diff --git a/sql/localsql/nipost/poet_registration_test.go b/sql/localsql/nipost/poet_registration_test.go index d71a029bc9..12cf5ed7a5 100644 --- a/sql/localsql/nipost/poet_registration_test.go +++ b/sql/localsql/nipost/poet_registration_test.go @@ -49,7 +49,7 @@ func Test_AddPoetRegistration(t *testing.T) { registrations, err = PoetRegistrationsByNodeId(db, nodeID) require.NoError(t, err) - require.Len(t, registrations, 0) + require.Empty(t, registrations) } func Test_PoetRegistrations_and_PoetRegistrationCount(t *testing.T) { From 89696b171059552da8577e72a51f6859f1feb28a Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Fri, 12 Jul 2024 00:21:22 +0200 Subject: [PATCH 05/26] fix lint2 --- sql/localsql/nipost/poet_registration.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sql/localsql/nipost/poet_registration.go b/sql/localsql/nipost/poet_registration.go index 4abed7ca06..55ae978c44 100644 --- a/sql/localsql/nipost/poet_registration.go +++ b/sql/localsql/nipost/poet_registration.go @@ -80,7 +80,8 @@ func PoetRegistrationsByNodeId(db sql.Executor, nodeID types.NodeID) ([]PoETRegi func PoetRegistrationsByNodeIdAndAddresses( db sql.Executor, nodeID types.NodeID, - addresses []string) ([]PoETRegistration, error) { + addresses []string, +) ([]PoETRegistration, error) { var registrations []PoETRegistration if len(addresses) == 0 { From 189aa4c11625b664e68ff9de112ac5b536016621 Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Mon, 15 Jul 2024 00:15:05 +0200 Subject: [PATCH 06/26] - added new test - fixed review issues --- README.md | 2 +- activation/activation_errors.go | 4 + activation/nipost.go | 80 +++++++++++------- activation/nipost_test.go | 83 +++++++++++++++++-- cmd/merge-nodes/internal/merge_action_test.go | 6 +- sql/localsql/nipost/poet_registration.go | 54 +----------- sql/localsql/nipost/poet_registration_test.go | 58 ++----------- 7 files changed, 141 insertions(+), 146 deletions(-) diff --git a/README.md b/README.md index 3f2bfde5a2..2f3fb38e7c 100644 --- a/README.md +++ b/README.md @@ -202,7 +202,7 @@ the build folder you need to ensure that you have the gpu setup dynamic library binary. The simplest way to do this is just copy the library file to be in the same directory as the go-spacemesh binary. Alternatively you can modify your system's library search paths (e.g. LD_LIBRARY_PATH) to ensure that the -library is found._ +library is found. go-spacemesh is p2p software which is designed to form a decentralized network by connecting to other instances of go-spacemesh running on remote computers. diff --git a/activation/activation_errors.go b/activation/activation_errors.go index 601541e83c..e4aa928577 100644 --- a/activation/activation_errors.go +++ b/activation/activation_errors.go @@ -12,6 +12,10 @@ var ( ErrPoetServiceUnstable = &PoetSvcUnstableError{} // ErrPoetProofNotReceived is returned when no poet proof was received. ErrPoetProofNotReceived = errors.New("builder: didn't receive any poet proof") + // ErrNoRegistrationForGivenPoetFound is returned when there are exising registrations for given node id + // in current poet round, but for other poet services and poet round has already started. + // So poet proof will not be fetched. + ErrNoRegistrationForGivenPoetFound = errors.New("builder: no registration found for given poet set") ) // PoetSvcUnstableError means there was a problem communicating diff --git a/activation/nipost.go b/activation/nipost.go index 986bafbff2..849df5202e 100644 --- a/activation/nipost.go +++ b/activation/nipost.go @@ -5,6 +5,7 @@ import ( "context" "errors" "fmt" + "golang.org/x/exp/maps" "math/rand/v2" "time" @@ -207,7 +208,7 @@ func (nb *NIPostBuilder) BuildNIPost( // WE ARE HERE PROOF BECOMES ATX PUBLICATION // AVAILABLE DEADLINE - curPoetRoundStart := nb.layerClock.LayerToTime((postChallenge.PublishEpoch - 1).FirstLayer()). + poetRoundStart := nb.layerClock.LayerToTime((postChallenge.PublishEpoch - 1).FirstLayer()). Add(nb.poetCfg.PhaseShift) curPoetRoundEnd := nb.layerClock.LayerToTime(postChallenge.PublishEpoch.FirstLayer()). Add(nb.poetCfg.PhaseShift). @@ -222,7 +223,7 @@ func (nb *NIPostBuilder) BuildNIPost( poetProofDeadline := publishEpochEnd.Add(-nb.poetCfg.CycleGap) logger.Info("building nipost", - zap.Time("poet round start", curPoetRoundStart), + zap.Time("poet round start", poetRoundStart), zap.Time("poet round end", curPoetRoundEnd), zap.Time("publish epoch end", publishEpochEnd), zap.Uint32("publish epoch", postChallenge.PublishEpoch.Uint32()), @@ -234,7 +235,7 @@ func (nb *NIPostBuilder) BuildNIPost( ctx, signer, poetProofDeadline, - curPoetRoundStart, challenge.Bytes()) + poetRoundStart, challenge.Bytes()) if err != nil { return nil, fmt.Errorf("submitting to poets: %w", err) } @@ -341,7 +342,7 @@ func (nb *NIPostBuilder) submitPoetChallenge( client PoetService, prefix, challenge []byte, signature types.EdSignature, -) error { +) (nipost.PoETRegistration, error) { logger := nb.logger.With( log.ZContext(ctx), zap.String("poet", client.Address()), @@ -355,18 +356,26 @@ func (nb *NIPostBuilder) submitPoetChallenge( round, err := client.Submit(submitCtx, deadline, prefix, challenge, signature, nodeID) if err != nil { - return &PoetSvcUnstableError{msg: "failed to submit challenge to poet service", source: err} + return nipost.PoETRegistration{}, &PoetSvcUnstableError{msg: "failed to submit challenge to poet service", source: err} } logger.Info("challenge submitted to poet proving service", zap.String("round", round.ID)) - return nipost.AddPoetRegistration(nb.localDB, nodeID, nipost.PoETRegistration{ + + registration := nipost.PoETRegistration{ ChallengeHash: types.Hash32(challenge), Address: client.Address(), RoundID: round.ID, RoundEnd: round.End, - }) + } + + if err := nipost.AddPoetRegistration(nb.localDB, nodeID, registration); err != nil { + return nipost.PoETRegistration{}, err + } + + return registration, err } -// Submit the challenge to registered PoETs with given addresses. +// submitPoetChallenges submit the challenge to registered PoETs +// if some registrations are missing and PoET round didn't start. func (nb *NIPostBuilder) submitPoetChallenges( ctx context.Context, signer *signing.EdSigner, @@ -374,41 +383,50 @@ func (nb *NIPostBuilder) submitPoetChallenges( curPoetRoundStartDeadline time.Time, challenge []byte, ) ([]nipost.PoETRegistration, error) { - // check if some registrations missing - existingRegistrations, err := nipost.PoetRegistrationsByNodeId(nb.localDB, signer.NodeID()) + // check if some registrations missing or were removed + nodeID := signer.NodeID() + registrations, err := nipost.PoetRegistrations(nb.localDB, nodeID) if err != nil { return nil, fmt.Errorf("failed to get poet registrations from db: %w", err) } - var i int - addresses := make([]string, len(nb.poetProvers)) - for _, client := range nb.poetProvers { - addresses[i] = client.Address() - i++ - } - - existingRegistrationsMap := make(map[string]struct{}) - for _, er := range existingRegistrations { - existingRegistrationsMap[er.Address] = struct{}{} + registrationsMap := make(map[string]nipost.PoETRegistration) + for _, reg := range registrations { + registrationsMap[reg.Address] = reg } + existingRegistrationsMap := make(map[string]nipost.PoETRegistration) missingRegistrationsPoETAddresses := make(map[string]struct{}) - for _, addr := range addresses { - if _, ok := existingRegistrationsMap[addr]; !ok { + + for addr := range nb.poetProvers { + if val, ok := registrationsMap[addr]; ok { + existingRegistrationsMap[addr] = val + } else { missingRegistrationsPoETAddresses[addr] = struct{}{} } } + existingRegistrations := maps.Values(existingRegistrationsMap) if len(missingRegistrationsPoETAddresses) == 0 { return existingRegistrations, nil } now := time.Now() + if curPoetRoundStartDeadline.Before(now) { - if len(missingRegistrationsPoETAddresses) == len(addresses) { + if len(existingRegistrations) == 0 { + if len(registrations) == 0 { + // no existing registration at all, drop current registration challenge + err = ErrATXChallengeExpired + } else { + // no existing registration for given poets set + err = ErrNoRegistrationForGivenPoetFound + nb.logger.Warn("revert poet configuration to previous is needed immediately", + zap.Error(err), log.ZShortStringer("smesherID", nodeID)) + } return nil, fmt.Errorf( "%w: poet round has already started at %s (now: %s)", - ErrATXChallengeExpired, // no existing registration, drop current registration challenge + err, curPoetRoundStartDeadline, now, ) @@ -419,18 +437,22 @@ func (nb *NIPostBuilder) submitPoetChallenges( // send registrations to missing addresses signature := signer.Sign(signing.POET, challenge) prefix := bytes.Join([][]byte{signer.Prefix(), {byte(signing.POET)}}, nil) - nodeID := signer.NodeID() submitCtx, cancel := context.WithDeadline(ctx, curPoetRoundStartDeadline) defer cancel() g, ctx := errgroup.WithContext(submitCtx) errChan := make(chan error, len(missingRegistrationsPoETAddresses)) + submittedRegistrations := make([]nipost.PoETRegistration, 0) for _, client := range nb.poetProvers { if _, ok := missingRegistrationsPoETAddresses[client.Address()]; ok { g.Go(func() error { - errChan <- nb.submitPoetChallenge(ctx, nodeID, poetProofDeadline, client, prefix, challenge, signature) + registration, err := nb.submitPoetChallenge(ctx, nodeID, poetProofDeadline, client, prefix, challenge, signature) + if err == nil { + submittedRegistrations = append(submittedRegistrations, registration) + } + errChan <- err return nil }) } @@ -455,11 +477,7 @@ func (nb *NIPostBuilder) submitPoetChallenges( return nil, ErrATXChallengeExpired } - existingRegistrations, err = nipost.PoetRegistrationsByNodeIdAndAddresses(nb.localDB, signer.NodeID(), addresses) - if err != nil { - return nil, fmt.Errorf("failed to get poet registration count: %w", err) - } - + existingRegistrations = append(existingRegistrations, submittedRegistrations...) if len(existingRegistrations) == 0 { return nil, &PoetSvcUnstableError{msg: "failed to submit challenge to any PoET", source: ctx.Err()} } diff --git a/activation/nipost_test.go b/activation/nipost_test.go index 6233dd5a52..85bbb3c804 100644 --- a/activation/nipost_test.go +++ b/activation/nipost_test.go @@ -867,7 +867,6 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { RoundEnd: time.Now().Add(1 * time.Second), }) - // successful post exec nb, err := NewNIPostBuilder( db, postService, @@ -1065,6 +1064,80 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { require.NotNil(t, nipost) }) + t.Run("1 poet removed AFTER round started -> too late to register to added poet, get proof from known", + func(t *testing.T) { + db := localsql.InMemory() + const publishEpoch = 5 + challenge := types.NIPostChallenge{ + PublishEpoch: publishEpoch, + } + challengeHash := wire.NIPostChallengeToWireV1(&challenge).Hash() + + ctrl := gomock.NewController(t) + + currLayer := types.EpochID(publishEpoch - 1).FirstLayer() + genesis := time.Now().Add(-time.Duration(currLayer) * layerDuration) + + require.NoError(t, nipost.AddChallenge(db, sig.NodeID(), &challenge)) + + poetProver := defaultPoetServiceMock(t, ctrl, poetProverAddr) + poetProver.EXPECT().Proof(gomock.Any(), "1").Return( + &types.PoetProof{}, []types.Hash32{challengeHash}, nil, + ) + + mclock := NewMocklayerClock(ctrl) + mclock.EXPECT().LayerToTime(gomock.Any()).DoAndReturn( + func(got types.LayerID) time.Time { + return genesis.Add(layerDuration * time.Duration(got)) + }, + ).AnyTimes() + + postClient := NewMockPostClient(ctrl) + nonce := types.VRFPostIndex(1) + postClient.EXPECT().Proof(gomock.Any(), gomock.Any()).Return( + &types.Post{ + Indices: []byte{1, 2, 3}, + }, &types.PostInfo{ + Nonce: &nonce, + }, nil, + ) + + postService := NewMockpostService(ctrl) + postService.EXPECT().Client(sig.NodeID()).Return(postClient, nil).AnyTimes() + + // successfully registered to 2 poets + err = nipost.AddPoetRegistration(db, sig.NodeID(), nipost.PoETRegistration{ + ChallengeHash: challengeHash, + Address: poetProverAddr, + RoundID: "1", + RoundEnd: time.Now().Add(1 * time.Second), + }) + require.NoError(t, err) + + err = nipost.AddPoetRegistration(db, sig.NodeID(), nipost.PoETRegistration{ + ChallengeHash: challengeHash, + Address: poetProverAddr2, + RoundID: "1", + RoundEnd: time.Now().Add(1 * time.Second), + }) + + nb, err := NewNIPostBuilder( + db, + postService, + zaptest.NewLogger(t), + PoetConfig{}, + mclock, + nil, + WithPoetServices(poetProver), + ) + require.NoError(t, err) + + nipost, err := nb.BuildNIPost(context.Background(), sig, challengeHash, + &types.NIPostChallenge{PublishEpoch: publishEpoch}) + require.NoError(t, err) + require.NotNil(t, nipost) + }) + t.Run("completely changed poet service AFTER round started -> fail, too late to register again", func(t *testing.T) { const publishEpoch = 5 @@ -1087,8 +1160,8 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { ).AnyTimes() db := localsql.InMemory() - require.NoError(t, nipost.AddChallenge(db, sig.NodeID(), &challenge)) + challengeHash := wire.NIPostChallengeToWireV1(&challenge).Hash() // successfully registered to removed poet @@ -1098,12 +1171,12 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { RoundID: "1", RoundEnd: time.Now().Add(1 * time.Second), }) - require.NoError(t, err) + postService := NewMockpostService(ctrl) nb, err := NewNIPostBuilder( - localsql.InMemory(), + db, postService, zaptest.NewLogger(t), PoetConfig{}, @@ -1115,7 +1188,7 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { nipost, err := nb.BuildNIPost(context.Background(), sig, types.RandomHash(), &types.NIPostChallenge{PublishEpoch: currLayer.GetEpoch()}) - require.ErrorIs(t, err, ErrATXChallengeExpired) + require.ErrorIs(t, err, ErrNoRegistrationForGivenPoetFound) require.ErrorContains(t, err, "poet round has already started") require.Nil(t, nipost) }) diff --git a/cmd/merge-nodes/internal/merge_action_test.go b/cmd/merge-nodes/internal/merge_action_test.go index 84be92650b..f2fdc11135 100644 --- a/cmd/merge-nodes/internal/merge_action_test.go +++ b/cmd/merge-nodes/internal/merge_action_test.go @@ -332,7 +332,7 @@ func Test_MergeDBs_Successful_Existing_Node(t *testing.T) { require.NoError(t, err) require.Equal(t, sig1Post, *post) - poet1, err := nipost.PoetRegistrationsByNodeId(dstDB, sig1.NodeID()) + poet1, err := nipost.PoetRegistrations(dstDB, sig1.NodeID()) require.NoError(t, err) require.Equal(t, poet1[0], sig1Poet1) require.Equal(t, poet1[1], sig1Poet2) @@ -345,7 +345,7 @@ func Test_MergeDBs_Successful_Existing_Node(t *testing.T) { require.NoError(t, err) require.Equal(t, sig2Post, *post) - poet2, err := nipost.PoetRegistrationsByNodeId(dstDB, sig2.NodeID()) + poet2, err := nipost.PoetRegistrations(dstDB, sig2.NodeID()) require.NoError(t, err) require.Equal(t, poet2[0], sig2Poet1) require.Equal(t, poet2[1], sig2Poet2) @@ -432,7 +432,7 @@ func Test_MergeDBs_Successful_Empty_Dir(t *testing.T) { require.NoError(t, err) require.Equal(t, sigPost, *post) - poet, err := nipost.PoetRegistrationsByNodeId(dstDB, sig.NodeID()) + poet, err := nipost.PoetRegistrations(dstDB, sig.NodeID()) require.NoError(t, err) require.Equal(t, poet[0], sigPoet1) require.Equal(t, poet[1], sigPoet2) diff --git a/sql/localsql/nipost/poet_registration.go b/sql/localsql/nipost/poet_registration.go index 55ae978c44..743de7b4cc 100644 --- a/sql/localsql/nipost/poet_registration.go +++ b/sql/localsql/nipost/poet_registration.go @@ -1,9 +1,7 @@ package nipost import ( - "errors" "fmt" - "strings" "time" "github.com/spacemeshos/go-spacemesh/common/types" @@ -49,7 +47,7 @@ func ClearPoetRegistrations(db sql.Executor, nodeID types.NodeID) error { return nil } -func PoetRegistrationsByNodeId(db sql.Executor, nodeID types.NodeID) ([]PoETRegistration, error) { +func PoetRegistrations(db sql.Executor, nodeID types.NodeID) ([]PoETRegistration, error) { var registrations []PoETRegistration enc := func(stmt *sql.Statement) { @@ -76,53 +74,3 @@ func PoetRegistrationsByNodeId(db sql.Executor, nodeID types.NodeID) ([]PoETRegi return registrations, nil } - -func PoetRegistrationsByNodeIdAndAddresses( - db sql.Executor, - nodeID types.NodeID, - addresses []string, -) ([]PoETRegistration, error) { - var registrations []PoETRegistration - - if len(addresses) == 0 { - return nil, errors.New("addresses list is empty") - } - - enc := func(stmt *sql.Statement) { - stmt.BindBytes(1, nodeID.Bytes()) - for i, address := range addresses { - stmt.BindText(i+2, address) - } - } - - dec := func(stmt *sql.Statement) bool { - registration := PoETRegistration{ - Address: stmt.ColumnText(1), - RoundID: stmt.ColumnText(2), - RoundEnd: time.Unix(stmt.ColumnInt64(3), 0), - } - stmt.ColumnBytes(0, registration.ChallengeHash[:]) - registrations = append(registrations, registration) - return true - } - - placeholders := make([]string, len(addresses)) - for i := range addresses { - placeholders[i] = fmt.Sprintf("?%d", i+2) - } - - query := fmt.Sprintf( - `SELECT hash, address, round_id, round_end - FROM poet_registration - WHERE id = ?1 AND address IN (%s);`, - strings.Join(placeholders, ", "), - ) - - _, err := db.Exec(query, enc, dec) - if err != nil { - return nil, fmt.Errorf("get poet registrations for node id %s and addresses %v: %w", - nodeID.ShortString(), addresses, err) - } - - return registrations, nil -} diff --git a/sql/localsql/nipost/poet_registration_test.go b/sql/localsql/nipost/poet_registration_test.go index 12cf5ed7a5..a4228a6371 100644 --- a/sql/localsql/nipost/poet_registration_test.go +++ b/sql/localsql/nipost/poet_registration_test.go @@ -31,14 +31,14 @@ func Test_AddPoetRegistration(t *testing.T) { err := AddPoetRegistration(db, nodeID, reg1) require.NoError(t, err) - registrations, err := PoetRegistrationsByNodeId(db, nodeID) + registrations, err := PoetRegistrations(db, nodeID) require.NoError(t, err) require.Len(t, registrations, 1) err = AddPoetRegistration(db, nodeID, reg2) require.NoError(t, err) - registrations, err = PoetRegistrationsByNodeId(db, nodeID) + registrations, err = PoetRegistrations(db, nodeID) require.NoError(t, err) require.Len(t, registrations, 2) require.Equal(t, reg1, registrations[0]) @@ -47,59 +47,11 @@ func Test_AddPoetRegistration(t *testing.T) { err = ClearPoetRegistrations(db, nodeID) require.NoError(t, err) - registrations, err = PoetRegistrationsByNodeId(db, nodeID) + registrations, err = PoetRegistrations(db, nodeID) require.NoError(t, err) require.Empty(t, registrations) } -func Test_PoetRegistrations_and_PoetRegistrationCount(t *testing.T) { - t.Parallel() - - db := localsql.InMemory() - - nodeID := types.RandomNodeID() - reg1 := PoETRegistration{ - ChallengeHash: types.RandomHash(), - Address: "address1", - RoundID: "round1", - RoundEnd: time.Now().Round(time.Second), - } - reg2 := PoETRegistration{ - ChallengeHash: types.RandomHash(), - Address: "address2", - RoundID: "round2", - RoundEnd: time.Now().Round(time.Second), - } - - err := AddPoetRegistration(db, nodeID, reg1) - require.NoError(t, err) - - err = AddPoetRegistration(db, nodeID, reg2) - require.NoError(t, err) - - registrations, err := PoetRegistrationsByNodeId(db, nodeID) - require.NoError(t, err) - require.Len(t, registrations, 2) - require.Equal(t, reg1, registrations[0]) - require.Equal(t, reg2, registrations[1]) - - registrations, err = PoetRegistrationsByNodeIdAndAddresses(db, nodeID, []string{"address1"}) - require.NoError(t, err) - require.Len(t, registrations, 1) - require.Equal(t, reg1, registrations[0]) - - registrations, err = PoetRegistrationsByNodeIdAndAddresses(db, nodeID, []string{"address2"}) - require.NoError(t, err) - require.Len(t, registrations, 1) - require.Equal(t, reg2, registrations[0]) - - registrations, err = PoetRegistrationsByNodeIdAndAddresses(db, nodeID, []string{"address2", "address1"}) - require.NoError(t, err) - require.Len(t, registrations, 2) - require.Equal(t, reg1, registrations[0]) - require.Equal(t, reg2, registrations[1]) -} - func Test_AddPoetRegistration_NoDuplicates(t *testing.T) { db := localsql.InMemory() @@ -114,14 +66,14 @@ func Test_AddPoetRegistration_NoDuplicates(t *testing.T) { err := AddPoetRegistration(db, nodeID, reg) require.NoError(t, err) - registrations, err := PoetRegistrationsByNodeId(db, nodeID) + registrations, err := PoetRegistrations(db, nodeID) require.NoError(t, err) require.Len(t, registrations, 1) err = AddPoetRegistration(db, nodeID, reg) require.ErrorIs(t, err, sql.ErrObjectExists) - registrations, err = PoetRegistrationsByNodeId(db, nodeID) + registrations, err = PoetRegistrations(db, nodeID) require.NoError(t, err) require.Len(t, registrations, 1) } From 66ea05e4b55f1c2a3fac320df16dc32c23d85c8d Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Mon, 15 Jul 2024 12:31:21 +0200 Subject: [PATCH 07/26] make linter happy --- activation/activation_errors.go | 2 +- activation/nipost.go | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/activation/activation_errors.go b/activation/activation_errors.go index e4aa928577..1a3ce2e83a 100644 --- a/activation/activation_errors.go +++ b/activation/activation_errors.go @@ -12,7 +12,7 @@ var ( ErrPoetServiceUnstable = &PoetSvcUnstableError{} // ErrPoetProofNotReceived is returned when no poet proof was received. ErrPoetProofNotReceived = errors.New("builder: didn't receive any poet proof") - // ErrNoRegistrationForGivenPoetFound is returned when there are exising registrations for given node id + // ErrNoRegistrationForGivenPoetFound is returned when there are existing registrations for given node id // in current poet round, but for other poet services and poet round has already started. // So poet proof will not be fetched. ErrNoRegistrationForGivenPoetFound = errors.New("builder: no registration found for given poet set") diff --git a/activation/nipost.go b/activation/nipost.go index 849df5202e..48e99d9b5c 100644 --- a/activation/nipost.go +++ b/activation/nipost.go @@ -5,14 +5,16 @@ import ( "context" "errors" "fmt" - "golang.org/x/exp/maps" "math/rand/v2" "time" + "golang.org/x/exp/maps" + "github.com/spacemeshos/merkle-tree" "github.com/spacemeshos/poet/shared" postshared "github.com/spacemeshos/post/shared" "go.uber.org/zap" + "golang.org/x/exp/maps" "golang.org/x/sync/errgroup" "github.com/spacemeshos/go-spacemesh/activation/metrics" From f6f22775fc43d91b2a9c2338da93e815990ae877 Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Thu, 18 Jul 2024 17:16:26 +0200 Subject: [PATCH 08/26] fixed review issues --- activation/nipost.go | 59 ++++++++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 30 deletions(-) diff --git a/activation/nipost.go b/activation/nipost.go index 48e99d9b5c..3e91fcaf79 100644 --- a/activation/nipost.go +++ b/activation/nipost.go @@ -8,8 +8,6 @@ import ( "math/rand/v2" "time" - "golang.org/x/exp/maps" - "github.com/spacemeshos/merkle-tree" "github.com/spacemeshos/poet/shared" postshared "github.com/spacemeshos/post/shared" @@ -358,7 +356,8 @@ func (nb *NIPostBuilder) submitPoetChallenge( round, err := client.Submit(submitCtx, deadline, prefix, challenge, signature, nodeID) if err != nil { - return nipost.PoETRegistration{}, &PoetSvcUnstableError{msg: "failed to submit challenge to poet service", source: err} + return nipost.PoETRegistration{}, + &PoetSvcUnstableError{msg: "failed to submit challenge to poet service", source: err} } logger.Info("challenge submitted to poet proving service", zap.String("round", round.ID)) @@ -398,13 +397,13 @@ func (nb *NIPostBuilder) submitPoetChallenges( } existingRegistrationsMap := make(map[string]nipost.PoETRegistration) - missingRegistrationsPoETAddresses := make(map[string]struct{}) + missingRegistrationsPoETAddresses := make([]string, 0) for addr := range nb.poetProvers { if val, ok := registrationsMap[addr]; ok { existingRegistrationsMap[addr] = val } else { - missingRegistrationsPoETAddresses[addr] = struct{}{} + missingRegistrationsPoETAddresses = append(missingRegistrationsPoETAddresses, addr) } } @@ -444,43 +443,43 @@ func (nb *NIPostBuilder) submitPoetChallenges( defer cancel() g, ctx := errgroup.WithContext(submitCtx) - errChan := make(chan error, len(missingRegistrationsPoETAddresses)) - submittedRegistrations := make([]nipost.PoETRegistration, 0) + submittedRegistrationsChan := make(chan nipost.PoETRegistration, len(missingRegistrationsPoETAddresses)) - for _, client := range nb.poetProvers { - if _, ok := missingRegistrationsPoETAddresses[client.Address()]; ok { + for _, addr := range missingRegistrationsPoETAddresses { + if client, ok := nb.poetProvers[addr]; ok { g.Go(func() error { - registration, err := nb.submitPoetChallenge(ctx, nodeID, poetProofDeadline, client, prefix, challenge, signature) - if err == nil { - submittedRegistrations = append(submittedRegistrations, registration) + registration, err := nb.submitPoetChallenge( + ctx, nodeID, + poetProofDeadline, + client, prefix, challenge, signature) + if err != nil { + nb.logger.Warn("failed to submit challenge to poet", + zap.Error(err), + log.ZShortStringer("smesherID", nodeID)) + } else { + submittedRegistrationsChan <- registration } - errChan <- err return nil }) + } else { + nb.logger.Warn("poet service not found", + log.ZShortStringer("smesherID", nodeID), + zap.String("poetAddr", addr)) } } g.Wait() - close(errChan) - - allInvalid := true - for err := range errChan { - if err == nil { - allInvalid = false - continue - } + close(submittedRegistrationsChan) - nb.logger.Warn("failed to submit challenge to poet", zap.Error(err), log.ZShortStringer("smesherID", nodeID)) - if !errors.Is(err, ErrInvalidRequest) { - allInvalid = false - } - } - if allInvalid { - nb.logger.Warn("all poet submits were too late. ATX challenge expires", log.ZShortStringer("smesherID", nodeID)) - return nil, ErrATXChallengeExpired + for registration := range submittedRegistrationsChan { + existingRegistrations = append(existingRegistrations, registration) } - existingRegistrations = append(existingRegistrations, submittedRegistrations...) if len(existingRegistrations) == 0 { + if curPoetRoundStartDeadline.Before(time.Now()) { + nb.logger.Warn("all poet submits were too late. ATX challenge expires", + log.ZShortStringer("smesherID", nodeID)) + return nil, ErrATXChallengeExpired + } return nil, &PoetSvcUnstableError{msg: "failed to submit challenge to any PoET", source: ctx.Err()} } From 9a5bad883211e339bbed36437964d9a7a6fed7d4 Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Tue, 23 Jul 2024 14:17:50 +0200 Subject: [PATCH 09/26] refactor tests --- activation/nipost.go | 54 ++++++++++++++++++--------------------- activation/nipost_test.go | 28 ++++++++++++++------ 2 files changed, 45 insertions(+), 37 deletions(-) diff --git a/activation/nipost.go b/activation/nipost.go index 3e91fcaf79..06206129ad 100644 --- a/activation/nipost.go +++ b/activation/nipost.go @@ -397,18 +397,17 @@ func (nb *NIPostBuilder) submitPoetChallenges( } existingRegistrationsMap := make(map[string]nipost.PoETRegistration) - missingRegistrationsPoETAddresses := make([]string, 0) - - for addr := range nb.poetProvers { + var missingRegistrations []PoetService + for addr, poet := range nb.poetProvers { if val, ok := registrationsMap[addr]; ok { existingRegistrationsMap[addr] = val } else { - missingRegistrationsPoETAddresses = append(missingRegistrationsPoETAddresses, addr) + missingRegistrations = append(missingRegistrations, poet) } } existingRegistrations := maps.Values(existingRegistrationsMap) - if len(missingRegistrationsPoETAddresses) == 0 { + if len(missingRegistrations) == 0 { return existingRegistrations, nil } @@ -443,30 +442,27 @@ func (nb *NIPostBuilder) submitPoetChallenges( defer cancel() g, ctx := errgroup.WithContext(submitCtx) - submittedRegistrationsChan := make(chan nipost.PoETRegistration, len(missingRegistrationsPoETAddresses)) - - for _, addr := range missingRegistrationsPoETAddresses { - if client, ok := nb.poetProvers[addr]; ok { - g.Go(func() error { - registration, err := nb.submitPoetChallenge( - ctx, nodeID, - poetProofDeadline, - client, prefix, challenge, signature) - if err != nil { - nb.logger.Warn("failed to submit challenge to poet", - zap.Error(err), - log.ZShortStringer("smesherID", nodeID)) - } else { - submittedRegistrationsChan <- registration - } - return nil - }) - } else { - nb.logger.Warn("poet service not found", - log.ZShortStringer("smesherID", nodeID), - zap.String("poetAddr", addr)) - } + submittedRegistrationsChan := make(chan nipost.PoETRegistration, len(missingRegistrations)) + + for _, client := range missingRegistrations { + g.Go(func() error { + registration, err := nb.submitPoetChallenge( + ctx, nodeID, + poetProofDeadline, + client, prefix, challenge, signature, + ) + if err != nil { + nb.logger.Warn("failed to submit challenge to poet", + zap.Error(err), + log.ZShortStringer("smesherID", nodeID), + ) + } else { + submittedRegistrationsChan <- registration + } + return nil + }) } + g.Wait() close(submittedRegistrationsChan) @@ -476,7 +472,7 @@ func (nb *NIPostBuilder) submitPoetChallenges( if len(existingRegistrations) == 0 { if curPoetRoundStartDeadline.Before(time.Now()) { - nb.logger.Warn("all poet submits were too late. ATX challenge expires", + nb.logger.Warn("failed to register in poets on time. ATX challenge expires", log.ZShortStringer("smesherID", nodeID)) return nil, ErrATXChallengeExpired } diff --git a/activation/nipost_test.go b/activation/nipost_test.go index 85bbb3c804..5209b99ab8 100644 --- a/activation/nipost_test.go +++ b/activation/nipost_test.go @@ -890,16 +890,20 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { challengeHash := wire.NIPostChallengeToWireV1(&challenge).Hash() ctrl := gomock.NewController(t) - poetProver := defaultPoetServiceMock(t, ctrl, poetProverAddr) + + poetProver := NewMockPoetService(ctrl) + poetProver.EXPECT().Address().Return(poetProverAddr).AnyTimes() poetProver.EXPECT().Proof(gomock.Any(), "1").Return( &types.PoetProof{}, []types.Hash32{challengeHash}, nil, ) - - addedPoetProver := defaultPoetServiceMock(t, ctrl, poetProverAddr2) + addedPoetProver := NewMockPoetService(ctrl) + addedPoetProver.EXPECT(). + Submit(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(&types.PoetRound{}, nil) + addedPoetProver.EXPECT().Address().Return(poetProverAddr2).AnyTimes() addedPoetProver.EXPECT().Proof(gomock.Any(), "").Return( &types.PoetProof{}, []types.Hash32{challengeHash}, nil, ) - mclock := defaultLayerClockMock(ctrl) postClient := NewMockPostClient(ctrl) @@ -948,10 +952,15 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { challengeHash := wire.NIPostChallengeToWireV1(&challenge).Hash() ctrl := gomock.NewController(t) - addedPoetProver := defaultPoetServiceMock(t, ctrl, poetProverAddr2) + addedPoetProver := NewMockPoetService(ctrl) + addedPoetProver.EXPECT(). + Submit(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(&types.PoetRound{}, nil) + addedPoetProver.EXPECT().Proof(gomock.Any(), "").Return( &types.PoetProof{}, []types.Hash32{challengeHash}, nil, ) + addedPoetProver.EXPECT().Address().Return(poetProverAddr2).AnyTimes() mclock := defaultLayerClockMock(ctrl) @@ -1010,12 +1019,15 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { require.NoError(t, nipost.AddChallenge(db, sig.NodeID(), &challenge)) - poetProver := defaultPoetServiceMock(t, ctrl, poetProverAddr) + poetProver := NewMockPoetService(ctrl) + poetProver.EXPECT().Address().AnyTimes().Return(poetProverAddr).AnyTimes() poetProver.EXPECT().Proof(gomock.Any(), "1").Return( &types.PoetProof{}, []types.Hash32{challengeHash}, nil, ) - addedPoetProver := defaultPoetServiceMock(t, ctrl, poetProverAddr2) + // addedPoetProver := defaultPoetServiceMock(t, ctrl, poetProverAddr2) + addedPoetProver := NewMockPoetService(ctrl) + addedPoetProver.EXPECT().Address().AnyTimes().Return(poetProverAddr2).AnyTimes() mclock := NewMocklayerClock(ctrl) mclock.EXPECT().LayerToTime(gomock.Any()).DoAndReturn( @@ -1064,7 +1076,7 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { require.NotNil(t, nipost) }) - t.Run("1 poet removed AFTER round started -> too late to register to added poet, get proof from known", + t.Run("1 poet removed AFTER round started -> too late to register to added poet, get proof from the remaining one", func(t *testing.T) { db := localsql.InMemory() const publishEpoch = 5 From 9ffb96233451595024b91fe42c09dd42ee6d65b0 Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Tue, 23 Jul 2024 21:33:38 +0200 Subject: [PATCH 10/26] refactored tests --- activation/nipost_test.go | 281 ++++++++++++-------------------------- 1 file changed, 88 insertions(+), 193 deletions(-) diff --git a/activation/nipost_test.go b/activation/nipost_test.go index 5209b99ab8..5af359ac1c 100644 --- a/activation/nipost_test.go +++ b/activation/nipost_test.go @@ -819,37 +819,23 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { PublishEpoch: postGenesisEpoch + 2, } + challengeHash := wire.NIPostChallengeToWireV1(&challenge).Hash() + const ( poetProverAddr = "http://localhost:9999" poetProverAddr2 = "http://localhost:9988" ) - t.Run("1 poet deleted BEFORE round started -> continue get proof from known poets", func(t *testing.T) { + t.Run("1 poet deleted BEFORE round started -> continue with submitted registration", func(t *testing.T) { db := localsql.InMemory() - - require.NoError(t, nipost.AddChallenge(db, sig.NodeID(), &challenge)) - challengeHash := wire.NIPostChallengeToWireV1(&challenge).Hash() - ctrl := gomock.NewController(t) - poetProver := defaultPoetServiceMock(t, ctrl, poetProverAddr) - poetProver.EXPECT().Proof(gomock.Any(), "1").Return( - &types.PoetProof{}, []types.Hash32{challengeHash}, nil, - ) - - mclock := defaultLayerClockMock(ctrl) - - postClient := NewMockPostClient(ctrl) - nonce := types.VRFPostIndex(1) - postClient.EXPECT().Proof(gomock.Any(), gomock.Any()).Return( - &types.Post{ - Indices: []byte{1, 2, 3}, - }, &types.PostInfo{ - Nonce: &nonce, - }, nil, - ) - postService := NewMockpostService(ctrl) - postService.EXPECT().Client(sig.NodeID()).Return(postClient, nil).AnyTimes() + poet := NewMockPoetService(ctrl) + poet.EXPECT(). + Submit(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + AnyTimes(). + Return(&types.PoetRound{}, nil) + poet.EXPECT().Address().AnyTimes().Return(poetProverAddr).AnyTimes() // successfully registered to 2 poets err = nipost.AddPoetRegistration(db, sig.NodeID(), nipost.PoETRegistration{ @@ -869,55 +855,39 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { nb, err := NewNIPostBuilder( db, - postService, + nil, zaptest.NewLogger(t), PoetConfig{}, - mclock, nil, - WithPoetServices(poetProver), // add only 1 poet prover + nil, + WithPoetServices(poet), // add only 1 poet prover ) require.NoError(t, err) - nipost, err := nb.BuildNIPost(context.Background(), sig, challengeHash, &types.NIPostChallenge{PublishEpoch: 7}) + existingRegistrations, err := nb.submitPoetChallenges( + context.Background(), + sig, + time.Now().Add(10*time.Second), + time.Now().Add(5*time.Second), + challengeHash.Bytes()) + require.NoError(t, err) - require.NotNil(t, nipost) + require.Equal(t, 1, len(existingRegistrations)) + require.Equal(t, poetProverAddr, existingRegistrations[0].Address) }) - t.Run("1 poet added BEFORE round started -> register to missing poet, get proof from both", func(t *testing.T) { + t.Run("1 poet added BEFORE round started -> register to missing poet", func(t *testing.T) { db := localsql.InMemory() - - require.NoError(t, nipost.AddChallenge(db, sig.NodeID(), &challenge)) - challengeHash := wire.NIPostChallengeToWireV1(&challenge).Hash() - ctrl := gomock.NewController(t) poetProver := NewMockPoetService(ctrl) poetProver.EXPECT().Address().Return(poetProverAddr).AnyTimes() - poetProver.EXPECT().Proof(gomock.Any(), "1").Return( - &types.PoetProof{}, []types.Hash32{challengeHash}, nil, - ) + addedPoetProver := NewMockPoetService(ctrl) addedPoetProver.EXPECT(). Submit(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). Return(&types.PoetRound{}, nil) addedPoetProver.EXPECT().Address().Return(poetProverAddr2).AnyTimes() - addedPoetProver.EXPECT().Proof(gomock.Any(), "").Return( - &types.PoetProof{}, []types.Hash32{challengeHash}, nil, - ) - mclock := defaultLayerClockMock(ctrl) - - postClient := NewMockPostClient(ctrl) - nonce := types.VRFPostIndex(1) - postClient.EXPECT().Proof(gomock.Any(), gomock.Any()).Return( - &types.Post{ - Indices: []byte{1, 2, 3}, - }, &types.PostInfo{ - Nonce: &nonce, - }, nil, - ) - - postService := NewMockpostService(ctrl) - postService.EXPECT().Client(sig.NodeID()).Return(postClient, nil).AnyTimes() // successfully registered to 1 poet err = nipost.AddPoetRegistration(db, sig.NodeID(), nipost.PoETRegistration{ @@ -931,52 +901,39 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { // successful post exec nb, err := NewNIPostBuilder( db, - postService, + nil, zaptest.NewLogger(t), PoetConfig{}, - mclock, + nil, nil, WithPoetServices(poetProver, addedPoetProver), // add both poet provers ) require.NoError(t, err) - nipost, err := nb.BuildNIPost(context.Background(), sig, challengeHash, &types.NIPostChallenge{PublishEpoch: 7}) + existingRegistrations, err := nb.submitPoetChallenges( + context.Background(), + sig, + time.Now().Add(10*time.Second), + time.Now().Add(5*time.Second), + challengeHash.Bytes()) + require.NoError(t, err) - require.NotNil(t, nipost) + require.Equal(t, 2, len(existingRegistrations)) + require.Equal(t, poetProverAddr, existingRegistrations[0].Address) + require.Equal(t, poetProverAddr2, existingRegistrations[1].Address) + }) t.Run("completely changed poet service BEFORE round started -> register new poet", func(t *testing.T) { db := localsql.InMemory() - - require.NoError(t, nipost.AddChallenge(db, sig.NodeID(), &challenge)) - challengeHash := wire.NIPostChallengeToWireV1(&challenge).Hash() - ctrl := gomock.NewController(t) + addedPoetProver := NewMockPoetService(ctrl) addedPoetProver.EXPECT(). Submit(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). Return(&types.PoetRound{}, nil) - - addedPoetProver.EXPECT().Proof(gomock.Any(), "").Return( - &types.PoetProof{}, []types.Hash32{challengeHash}, nil, - ) addedPoetProver.EXPECT().Address().Return(poetProverAddr2).AnyTimes() - mclock := defaultLayerClockMock(ctrl) - - postClient := NewMockPostClient(ctrl) - nonce := types.VRFPostIndex(1) - postClient.EXPECT().Proof(gomock.Any(), gomock.Any()).Return( - &types.Post{ - Indices: []byte{1, 2, 3}, - }, &types.PostInfo{ - Nonce: &nonce, - }, nil, - ) - - postService := NewMockpostService(ctrl) - postService.EXPECT().Client(sig.NodeID()).Return(postClient, nil).AnyTimes() - // successfully registered to removed poet err = nipost.AddPoetRegistration(db, sig.NodeID(), nipost.PoETRegistration{ ChallengeHash: challengeHash, @@ -986,69 +943,40 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { }) require.NoError(t, err) - // successful post exec nb, err := NewNIPostBuilder( db, - postService, + nil, zaptest.NewLogger(t), PoetConfig{}, - mclock, + nil, nil, WithPoetServices(addedPoetProver), // add new poet ) require.NoError(t, err) - nipost, err := nb.BuildNIPost(context.Background(), sig, challengeHash, &types.NIPostChallenge{PublishEpoch: 7}) + existingRegistrations, err := nb.submitPoetChallenges( + context.Background(), + sig, + time.Now().Add(10*time.Second), + time.Now().Add(5*time.Second), + challengeHash.Bytes()) + require.NoError(t, err) - require.NotNil(t, nipost) + require.Equal(t, 1, len(existingRegistrations)) + require.Equal(t, poetProverAddr2, existingRegistrations[0].Address) }) - t.Run("1 poet added AFTER round started -> too late to register to added poet, get proof from known", + t.Run("1 poet added AFTER round started -> too late to register to added poet", func(t *testing.T) { db := localsql.InMemory() - const publishEpoch = 5 - challenge := types.NIPostChallenge{ - PublishEpoch: publishEpoch, - } - challengeHash := wire.NIPostChallengeToWireV1(&challenge).Hash() - ctrl := gomock.NewController(t) - currLayer := types.EpochID(publishEpoch - 1).FirstLayer() - genesis := time.Now().Add(-time.Duration(currLayer) * layerDuration) - - require.NoError(t, nipost.AddChallenge(db, sig.NodeID(), &challenge)) - poetProver := NewMockPoetService(ctrl) poetProver.EXPECT().Address().AnyTimes().Return(poetProverAddr).AnyTimes() - poetProver.EXPECT().Proof(gomock.Any(), "1").Return( - &types.PoetProof{}, []types.Hash32{challengeHash}, nil, - ) - // addedPoetProver := defaultPoetServiceMock(t, ctrl, poetProverAddr2) addedPoetProver := NewMockPoetService(ctrl) addedPoetProver.EXPECT().Address().AnyTimes().Return(poetProverAddr2).AnyTimes() - mclock := NewMocklayerClock(ctrl) - mclock.EXPECT().LayerToTime(gomock.Any()).DoAndReturn( - func(got types.LayerID) time.Time { - return genesis.Add(layerDuration * time.Duration(got)) - }, - ).AnyTimes() - - postClient := NewMockPostClient(ctrl) - nonce := types.VRFPostIndex(1) - postClient.EXPECT().Proof(gomock.Any(), gomock.Any()).Return( - &types.Post{ - Indices: []byte{1, 2, 3}, - }, &types.PostInfo{ - Nonce: &nonce, - }, nil, - ) - - postService := NewMockpostService(ctrl) - postService.EXPECT().Client(sig.NodeID()).Return(postClient, nil).AnyTimes() - // successfully registered to 1 poet err = nipost.AddPoetRegistration(db, sig.NodeID(), nipost.PoETRegistration{ ChallengeHash: challengeHash, @@ -1061,61 +989,37 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { // successful post exec nb, err := NewNIPostBuilder( db, - postService, + nil, zaptest.NewLogger(t), PoetConfig{}, - mclock, + nil, nil, WithPoetServices(poetProver, addedPoetProver), ) require.NoError(t, err) - nipost, err := nb.BuildNIPost(context.Background(), sig, challengeHash, - &types.NIPostChallenge{PublishEpoch: publishEpoch}) + existingRegistrations, err := nb.submitPoetChallenges( + context.Background(), + sig, + time.Now().Add(10*time.Second), + time.Now().Add(-5*time.Second), // poet round started + challengeHash.Bytes()) + require.NoError(t, err) - require.NotNil(t, nipost) + require.Equal(t, 1, len(existingRegistrations)) + require.Equal(t, poetProverAddr, existingRegistrations[0].Address) }) - t.Run("1 poet removed AFTER round started -> too late to register to added poet, get proof from the remaining one", + t.Run("1 poet removed AFTER round started -> too late to register to added poet", func(t *testing.T) { db := localsql.InMemory() - const publishEpoch = 5 - challenge := types.NIPostChallenge{ - PublishEpoch: publishEpoch, - } - challengeHash := wire.NIPostChallengeToWireV1(&challenge).Hash() - ctrl := gomock.NewController(t) - currLayer := types.EpochID(publishEpoch - 1).FirstLayer() - genesis := time.Now().Add(-time.Duration(currLayer) * layerDuration) - - require.NoError(t, nipost.AddChallenge(db, sig.NodeID(), &challenge)) - - poetProver := defaultPoetServiceMock(t, ctrl, poetProverAddr) - poetProver.EXPECT().Proof(gomock.Any(), "1").Return( - &types.PoetProof{}, []types.Hash32{challengeHash}, nil, - ) - - mclock := NewMocklayerClock(ctrl) - mclock.EXPECT().LayerToTime(gomock.Any()).DoAndReturn( - func(got types.LayerID) time.Time { - return genesis.Add(layerDuration * time.Duration(got)) - }, - ).AnyTimes() - - postClient := NewMockPostClient(ctrl) - nonce := types.VRFPostIndex(1) - postClient.EXPECT().Proof(gomock.Any(), gomock.Any()).Return( - &types.Post{ - Indices: []byte{1, 2, 3}, - }, &types.PostInfo{ - Nonce: &nonce, - }, nil, - ) + poetProver := NewMockPoetService(ctrl) + poetProver.EXPECT().Address().AnyTimes().Return(poetProverAddr).AnyTimes() - postService := NewMockpostService(ctrl) - postService.EXPECT().Client(sig.NodeID()).Return(postClient, nil).AnyTimes() + addedPoetProver := NewMockPoetService(ctrl) + addedPoetProver.EXPECT().Address().AnyTimes().Return(poetProverAddr2).AnyTimes() // successfully registered to 2 poets err = nipost.AddPoetRegistration(db, sig.NodeID(), nipost.PoETRegistration{ @@ -1135,46 +1039,34 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { nb, err := NewNIPostBuilder( db, - postService, + nil, zaptest.NewLogger(t), PoetConfig{}, - mclock, + nil, nil, WithPoetServices(poetProver), ) require.NoError(t, err) - nipost, err := nb.BuildNIPost(context.Background(), sig, challengeHash, - &types.NIPostChallenge{PublishEpoch: publishEpoch}) + existingRegistrations, err := nb.submitPoetChallenges( + context.Background(), + sig, + time.Now().Add(10*time.Second), + time.Now().Add(-5*time.Second), // poet round started + challengeHash.Bytes()) + require.NoError(t, err) - require.NotNil(t, nipost) + require.Equal(t, 1, len(existingRegistrations)) + require.Equal(t, poetProverAddr, existingRegistrations[0].Address) }) t.Run("completely changed poet service AFTER round started -> fail, too late to register again", func(t *testing.T) { - const publishEpoch = 5 - challenge := types.NIPostChallenge{ - PublishEpoch: publishEpoch, - } - + db := localsql.InMemory() ctrl := gomock.NewController(t) - currLayer := types.EpochID(publishEpoch - 1).FirstLayer() - genesis := time.Now().Add(-time.Duration(currLayer) * layerDuration) poetProver := NewMockPoetService(ctrl) - poetProver.EXPECT().Address().Return(poetProverAddr).AnyTimes() - - mclock := NewMocklayerClock(ctrl) - mclock.EXPECT().LayerToTime(gomock.Any()).DoAndReturn( - func(got types.LayerID) time.Time { - return genesis.Add(layerDuration * time.Duration(got)) - }, - ).AnyTimes() - - db := localsql.InMemory() - require.NoError(t, nipost.AddChallenge(db, sig.NodeID(), &challenge)) - - challengeHash := wire.NIPostChallengeToWireV1(&challenge).Hash() + poetProver.EXPECT().Address().AnyTimes().Return(poetProverAddr).AnyTimes() // successfully registered to removed poet err = nipost.AddPoetRegistration(db, sig.NodeID(), nipost.PoETRegistration{ @@ -1185,24 +1077,27 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { }) require.NoError(t, err) - postService := NewMockpostService(ctrl) - nb, err := NewNIPostBuilder( db, - postService, + nil, zaptest.NewLogger(t), PoetConfig{}, - mclock, + nil, nil, WithPoetServices(poetProver), ) require.NoError(t, err) - nipost, err := nb.BuildNIPost(context.Background(), sig, types.RandomHash(), - &types.NIPostChallenge{PublishEpoch: currLayer.GetEpoch()}) + existingRegistrations, err := nb.submitPoetChallenges( + context.Background(), + sig, + time.Now().Add(10*time.Second), + time.Now().Add(-5*time.Second), // poet round started + challengeHash.Bytes()) + require.ErrorIs(t, err, ErrNoRegistrationForGivenPoetFound) require.ErrorContains(t, err, "poet round has already started") - require.Nil(t, nipost) + require.Equal(t, 0, len(existingRegistrations)) }) } From c4bb081c5692a92e19a0f7bd12b17654e3b83070 Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Wed, 24 Jul 2024 22:27:18 +0200 Subject: [PATCH 11/26] fix text and make linter happy --- activation/nipost_test.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/activation/nipost_test.go b/activation/nipost_test.go index 5af359ac1c..9442ed9486 100644 --- a/activation/nipost_test.go +++ b/activation/nipost_test.go @@ -718,7 +718,7 @@ func TestNIPSTBuilder_PoetUnstable(t *testing.T) { require.ErrorIs(t, err, ErrPoetServiceUnstable) require.Nil(t, nipst) }) - t.Run("Submit hangs", func(t *testing.T) { + t.Run("Submit hangs, no registrations submitted", func(t *testing.T) { t.Parallel() ctrl := gomock.NewController(t) mclock := defaultLayerClockMock(ctrl) @@ -752,7 +752,7 @@ func TestNIPSTBuilder_PoetUnstable(t *testing.T) { nipst, err := nb.BuildNIPost(context.Background(), sig, challenge, &types.NIPostChallenge{PublishEpoch: postGenesisEpoch + 2}) - require.ErrorIs(t, err, ErrPoetServiceUnstable) + require.ErrorIs(t, err, ErrATXChallengeExpired) require.Nil(t, nipst) }) t.Run("GetProof fails", func(t *testing.T) { @@ -872,7 +872,7 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { challengeHash.Bytes()) require.NoError(t, err) - require.Equal(t, 1, len(existingRegistrations)) + require.Len(t, existingRegistrations, 1) require.Equal(t, poetProverAddr, existingRegistrations[0].Address) }) @@ -918,10 +918,9 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { challengeHash.Bytes()) require.NoError(t, err) - require.Equal(t, 2, len(existingRegistrations)) + require.Len(t, existingRegistrations, 2) require.Equal(t, poetProverAddr, existingRegistrations[0].Address) require.Equal(t, poetProverAddr2, existingRegistrations[1].Address) - }) t.Run("completely changed poet service BEFORE round started -> register new poet", func(t *testing.T) { @@ -962,7 +961,7 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { challengeHash.Bytes()) require.NoError(t, err) - require.Equal(t, 1, len(existingRegistrations)) + require.Len(t, existingRegistrations, 1) require.Equal(t, poetProverAddr2, existingRegistrations[0].Address) }) @@ -1006,7 +1005,7 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { challengeHash.Bytes()) require.NoError(t, err) - require.Equal(t, 1, len(existingRegistrations)) + require.Len(t, existingRegistrations, 1) require.Equal(t, poetProverAddr, existingRegistrations[0].Address) }) @@ -1056,7 +1055,7 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { challengeHash.Bytes()) require.NoError(t, err) - require.Equal(t, 1, len(existingRegistrations)) + require.Len(t, existingRegistrations, 1) require.Equal(t, poetProverAddr, existingRegistrations[0].Address) }) @@ -1097,7 +1096,7 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { require.ErrorIs(t, err, ErrNoRegistrationForGivenPoetFound) require.ErrorContains(t, err, "poet round has already started") - require.Equal(t, 0, len(existingRegistrations)) + require.Empty(t, existingRegistrations) }) } From 46ce1465e6f70c4c81931f9c11cfc0d417059254 Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Mon, 29 Jul 2024 14:40:45 +0200 Subject: [PATCH 12/26] fix minor issues --- activation/nipost.go | 6 +++--- activation/nipost_test.go | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/activation/nipost.go b/activation/nipost.go index 06206129ad..d8cefa8f17 100644 --- a/activation/nipost.go +++ b/activation/nipost.go @@ -441,11 +441,11 @@ func (nb *NIPostBuilder) submitPoetChallenges( submitCtx, cancel := context.WithDeadline(ctx, curPoetRoundStartDeadline) defer cancel() - g, ctx := errgroup.WithContext(submitCtx) + eg, ctx := errgroup.WithContext(submitCtx) submittedRegistrationsChan := make(chan nipost.PoETRegistration, len(missingRegistrations)) for _, client := range missingRegistrations { - g.Go(func() error { + eg.Go(func() error { registration, err := nb.submitPoetChallenge( ctx, nodeID, poetProofDeadline, @@ -463,7 +463,7 @@ func (nb *NIPostBuilder) submitPoetChallenges( }) } - g.Wait() + eg.Wait() close(submittedRegistrationsChan) for registration := range submittedRegistrationsChan { diff --git a/activation/nipost_test.go b/activation/nipost_test.go index 9442ed9486..ec2d59f91a 100644 --- a/activation/nipost_test.go +++ b/activation/nipost_test.go @@ -835,7 +835,7 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { Submit(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). AnyTimes(). Return(&types.PoetRound{}, nil) - poet.EXPECT().Address().AnyTimes().Return(poetProverAddr).AnyTimes() + poet.EXPECT().Address().Return(poetProverAddr).AnyTimes() // successfully registered to 2 poets err = nipost.AddPoetRegistration(db, sig.NodeID(), nipost.PoETRegistration{ @@ -971,10 +971,10 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { ctrl := gomock.NewController(t) poetProver := NewMockPoetService(ctrl) - poetProver.EXPECT().Address().AnyTimes().Return(poetProverAddr).AnyTimes() + poetProver.EXPECT().Address().Return(poetProverAddr).AnyTimes() addedPoetProver := NewMockPoetService(ctrl) - addedPoetProver.EXPECT().Address().AnyTimes().Return(poetProverAddr2).AnyTimes() + addedPoetProver.EXPECT().Address().Return(poetProverAddr2).AnyTimes() // successfully registered to 1 poet err = nipost.AddPoetRegistration(db, sig.NodeID(), nipost.PoETRegistration{ @@ -1015,10 +1015,10 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { ctrl := gomock.NewController(t) poetProver := NewMockPoetService(ctrl) - poetProver.EXPECT().Address().AnyTimes().Return(poetProverAddr).AnyTimes() + poetProver.EXPECT().Address().Return(poetProverAddr).AnyTimes() addedPoetProver := NewMockPoetService(ctrl) - addedPoetProver.EXPECT().Address().AnyTimes().Return(poetProverAddr2).AnyTimes() + addedPoetProver.EXPECT().Address().Return(poetProverAddr2).AnyTimes() // successfully registered to 2 poets err = nipost.AddPoetRegistration(db, sig.NodeID(), nipost.PoETRegistration{ @@ -1065,7 +1065,7 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { ctrl := gomock.NewController(t) poetProver := NewMockPoetService(ctrl) - poetProver.EXPECT().Address().AnyTimes().Return(poetProverAddr).AnyTimes() + poetProver.EXPECT().Address().Return(poetProverAddr).AnyTimes() // successfully registered to removed poet err = nipost.AddPoetRegistration(db, sig.NodeID(), nipost.PoETRegistration{ From 105e872c7a4abf054ed0948fa968539bd94a404a Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Fri, 2 Aug 2024 01:05:55 +0200 Subject: [PATCH 13/26] final review issues fix --- activation/activation_errors.go | 4 ---- activation/nipost.go | 37 +++++++++++++++++++++++---------- activation/nipost_test.go | 15 +++++++------ activation/poet.go | 16 +++++++++++++- node/node.go | 2 +- 5 files changed, 51 insertions(+), 23 deletions(-) diff --git a/activation/activation_errors.go b/activation/activation_errors.go index 1a3ce2e83a..601541e83c 100644 --- a/activation/activation_errors.go +++ b/activation/activation_errors.go @@ -12,10 +12,6 @@ var ( ErrPoetServiceUnstable = &PoetSvcUnstableError{} // ErrPoetProofNotReceived is returned when no poet proof was received. ErrPoetProofNotReceived = errors.New("builder: didn't receive any poet proof") - // ErrNoRegistrationForGivenPoetFound is returned when there are existing registrations for given node id - // in current poet round, but for other poet services and poet round has already started. - // So poet proof will not be fetched. - ErrNoRegistrationForGivenPoetFound = errors.New("builder: no registration found for given poet set") ) // PoetSvcUnstableError means there was a problem communicating diff --git a/activation/nipost.go b/activation/nipost.go index d8cefa8f17..50334f7962 100644 --- a/activation/nipost.go +++ b/activation/nipost.go @@ -406,6 +406,30 @@ func (nb *NIPostBuilder) submitPoetChallenges( } } + if len(registrations) != 0 && len(existingRegistrationsMap) == 0 { + nb.logger.Panic( + "Invalid PoET registrations found. Local state only contains PoETs not in node config", + zap.Strings("registrations_addresses", maps.Keys(registrationsMap)), + zap.Strings("configured_poets", maps.Keys(nb.poetProvers)), + log.ZShortStringer("smesherID", nodeID), + ) + } + + misconfiguredRegistrations := make(map[string]nipost.PoETRegistration) + for addr := range registrationsMap { + if reg, ok := existingRegistrationsMap[addr]; !ok { + misconfiguredRegistrations[addr] = reg + } + } + + if len(misconfiguredRegistrations) != 0 { + nb.logger.Warn( + "Invalid PoET registrations found. Local state contains PoETs not in node config", + zap.Strings("registrations_addresses", maps.Keys(misconfiguredRegistrations)), + log.ZShortStringer("smesherID", nodeID), + ) + } + existingRegistrations := maps.Values(existingRegistrationsMap) if len(missingRegistrations) == 0 { return existingRegistrations, nil @@ -414,19 +438,10 @@ func (nb *NIPostBuilder) submitPoetChallenges( now := time.Now() if curPoetRoundStartDeadline.Before(now) { - if len(existingRegistrations) == 0 { - if len(registrations) == 0 { - // no existing registration at all, drop current registration challenge - err = ErrATXChallengeExpired - } else { - // no existing registration for given poets set - err = ErrNoRegistrationForGivenPoetFound - nb.logger.Warn("revert poet configuration to previous is needed immediately", - zap.Error(err), log.ZShortStringer("smesherID", nodeID)) - } + if len(existingRegistrations) == 0 && len(registrations) == 0 { return nil, fmt.Errorf( "%w: poet round has already started at %s (now: %s)", - err, + ErrATXChallengeExpired, curPoetRoundStartDeadline, now, ) diff --git a/activation/nipost_test.go b/activation/nipost_test.go index ec2d59f91a..be8c9ae0c4 100644 --- a/activation/nipost_test.go +++ b/activation/nipost_test.go @@ -1076,10 +1076,11 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { }) require.NoError(t, err) + logger := zaptest.NewLogger(t) nb, err := NewNIPostBuilder( db, nil, - zaptest.NewLogger(t), + logger, PoetConfig{}, nil, nil, @@ -1087,16 +1088,18 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { ) require.NoError(t, err) - existingRegistrations, err := nb.submitPoetChallenges( + defer func() { + if r := recover(); r == nil { + t.Errorf("Panic is expected") + } + }() + + nb.submitPoetChallenges( context.Background(), sig, time.Now().Add(10*time.Second), time.Now().Add(-5*time.Second), // poet round started challengeHash.Bytes()) - - require.ErrorIs(t, err, ErrNoRegistrationForGivenPoetFound) - require.ErrorContains(t, err, "poet round has already started") - require.Empty(t, existingRegistrations) }) } diff --git a/activation/poet.go b/activation/poet.go index 174e3b66c1..c317244a5e 100644 --- a/activation/poet.go +++ b/activation/poet.go @@ -29,6 +29,7 @@ import ( var ( ErrInvalidRequest = errors.New("invalid request") ErrUnauthorized = errors.New("unauthorized") + ErrPhaseShiftMismatch = errors.New("phase shift mismatch") errCertificatesNotSupported = errors.New("poet doesn't support certificates") ) @@ -151,6 +152,19 @@ func NewHTTPPoetClient(server types.PoetServer, cfg PoetConfig, opts ...PoetClie opt(poetClient) } + resp, err := poetClient.info(context.Background()) + if err != nil { + return nil, err + } + + if resp.PhaseShift.AsDuration() != cfg.PhaseShift { + poetClient.logger.Warn("getting info about poet service configuration", + zap.Duration("server: phase shift", resp.PhaseShift.AsDuration()), + zap.Duration("config: cycle gap", cfg.PhaseShift), + ) + return nil, ErrPhaseShiftMismatch + } + poetClient.logger.Info( "created poet client", zap.Stringer("url", baseURL), @@ -374,7 +388,7 @@ func NewPoetService( ) (*poetService, error) { client, err := NewHTTPPoetClient(server, cfg, WithLogger(logger)) if err != nil { - return nil, fmt.Errorf("creating HTTP poet client %s: %w", server.Address, err) + return nil, err } return NewPoetServiceWithClient( db, diff --git a/node/node.go b/node/node.go index 9d7e8ee9b1..512f172925 100644 --- a/node/node.go +++ b/node/node.go @@ -996,7 +996,7 @@ func (app *App) initServices(ctx context.Context) error { activation.WithCertifier(certifier), ) if err != nil { - app.log.Panic("failed to create poet client: %v", err) + app.log.Panic("failed to create poet client with address %v: %v", server.Address, err) } poetClients = append(poetClients, client) } From 11a53234096f06582637559460554134349f0661 Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Fri, 2 Aug 2024 01:12:19 +0200 Subject: [PATCH 14/26] fix test --- activation/nipost.go | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/activation/nipost.go b/activation/nipost.go index 50334f7962..69bce8a836 100644 --- a/activation/nipost.go +++ b/activation/nipost.go @@ -406,15 +406,6 @@ func (nb *NIPostBuilder) submitPoetChallenges( } } - if len(registrations) != 0 && len(existingRegistrationsMap) == 0 { - nb.logger.Panic( - "Invalid PoET registrations found. Local state only contains PoETs not in node config", - zap.Strings("registrations_addresses", maps.Keys(registrationsMap)), - zap.Strings("configured_poets", maps.Keys(nb.poetProvers)), - log.ZShortStringer("smesherID", nodeID), - ) - } - misconfiguredRegistrations := make(map[string]nipost.PoETRegistration) for addr := range registrationsMap { if reg, ok := existingRegistrationsMap[addr]; !ok { @@ -438,15 +429,26 @@ func (nb *NIPostBuilder) submitPoetChallenges( now := time.Now() if curPoetRoundStartDeadline.Before(now) { - if len(existingRegistrations) == 0 && len(registrations) == 0 { + switch { + case len(existingRegistrations) == 0 && len(registrations) == 0: + // no existing registration at all, drop current registration challenge return nil, fmt.Errorf( "%w: poet round has already started at %s (now: %s)", ErrATXChallengeExpired, curPoetRoundStartDeadline, now, ) + case len(existingRegistrations) == 0: + // no existing registration for given poets set + nb.logger.Panic( + "Invalid PoET registrations found. Local state only contains PoETs not in node config", + zap.Strings("registrations", maps.Keys(registrationsMap)), + zap.Strings("configured_poets", maps.Keys(nb.poetProvers)), + log.ZShortStringer("smesherID", nodeID), + ) + default: + return existingRegistrations, nil } - return existingRegistrations, nil } // send registrations to missing addresses From b24522940d06d68b8b681e4e60219acc36295630 Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Fri, 2 Aug 2024 12:48:16 +0200 Subject: [PATCH 15/26] fix issues --- activation/nipost.go | 11 ++++++----- activation/nipost_test.go | 1 + activation/poet.go | 9 +++------ 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/activation/nipost.go b/activation/nipost.go index 556643a641..399e07c9e5 100644 --- a/activation/nipost.go +++ b/activation/nipost.go @@ -406,16 +406,16 @@ func (nb *NIPostBuilder) submitPoetChallenges( } } - misconfiguredRegistrations := make(map[string]nipost.PoETRegistration) + misconfiguredRegistrations := make(map[string]struct{}) for addr := range registrationsMap { - if reg, ok := existingRegistrationsMap[addr]; !ok { - misconfiguredRegistrations[addr] = reg + if _, ok := existingRegistrationsMap[addr]; !ok { + misconfiguredRegistrations[addr] = struct{}{} } } if len(misconfiguredRegistrations) != 0 { nb.logger.Warn( - "Invalid PoET registrations found. Local state contains PoETs not in node config", + "Found existing registrations for poets not listed in the config. Will not fetch proof from them.", zap.Strings("registrations_addresses", maps.Keys(misconfiguredRegistrations)), log.ZShortStringer("smesherID", nodeID), ) @@ -441,7 +441,8 @@ func (nb *NIPostBuilder) submitPoetChallenges( case len(existingRegistrations) == 0: // no existing registration for given poets set nb.logger.Panic( - "Invalid PoET registrations found. Local state only contains PoETs not in node config", + "None of the poets listed in the config matches the existing registrations. "+ + "Verify your config and local database state.", zap.Strings("registrations", maps.Keys(registrationsMap)), zap.Strings("configured_poets", maps.Keys(nb.poetProvers)), log.ZShortStringer("smesherID", nodeID), diff --git a/activation/nipost_test.go b/activation/nipost_test.go index be8c9ae0c4..cc368c9c44 100644 --- a/activation/nipost_test.go +++ b/activation/nipost_test.go @@ -1077,6 +1077,7 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { require.NoError(t, err) logger := zaptest.NewLogger(t) + nb, err := NewNIPostBuilder( db, nil, diff --git a/activation/poet.go b/activation/poet.go index ea0e3dc35f..f7b8a3cc31 100644 --- a/activation/poet.go +++ b/activation/poet.go @@ -155,10 +155,8 @@ func NewHTTPPoetClient(server types.PoetServer, cfg PoetConfig, opts ...PoetClie resp, err := poetClient.info(context.Background()) if err != nil { - return nil, err - } - - if resp.PhaseShift.AsDuration() != cfg.PhaseShift { + poetClient.logger.Error("getting info about poet service configuration", zap.Error(err)) + } else if resp.PhaseShift.AsDuration() != cfg.PhaseShift { poetClient.logger.Warn("getting info about poet service configuration", zap.Duration("server: phase shift", resp.PhaseShift.AsDuration()), zap.Duration("config: cycle gap", cfg.PhaseShift), @@ -497,7 +495,6 @@ func (c *poetService) reauthorize( ctx context.Context, id types.NodeID, challenge []byte, - logger *zap.Logger, ) (*PoetAuth, error) { if c.certifier != nil { if _, pubkey, err := c.getCertifierInfo(ctx); err == nil { @@ -538,7 +535,7 @@ func (c *poetService) Submit( return round, nil case errors.Is(err, ErrUnauthorized): logger.Warn("failed to submit challenge as unauthorized - authorizing again", zap.Error(err)) - auth, err := c.reauthorize(ctx, nodeID, challenge, logger) + auth, err := c.reauthorize(ctx, nodeID, challenge) if err != nil { return nil, fmt.Errorf("authorizing: %w", err) } From 814001281dd1020e20735a6f406232b911dc29d9 Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Fri, 2 Aug 2024 17:05:42 +0200 Subject: [PATCH 16/26] verifying poets configuration --- activation/activation_errors.go | 2 ++ activation/nipost.go | 8 +++++- activation/nipost_test.go | 9 ++---- activation/poet.go | 51 ++++++++++++++++++++++----------- 4 files changed, 46 insertions(+), 24 deletions(-) diff --git a/activation/activation_errors.go b/activation/activation_errors.go index 601541e83c..e8503fc128 100644 --- a/activation/activation_errors.go +++ b/activation/activation_errors.go @@ -12,6 +12,8 @@ var ( ErrPoetServiceUnstable = &PoetSvcUnstableError{} // ErrPoetProofNotReceived is returned when no poet proof was received. ErrPoetProofNotReceived = errors.New("builder: didn't receive any poet proof") + // ErrNoRegistrationForGivenPoetFound is returned when configured poets do not match existing registrations in db at all + ErrNoRegistrationForGivenPoetFound = errors.New("builder: none of configured poets matches the existing registrations") ) // PoetSvcUnstableError means there was a problem communicating diff --git a/activation/nipost.go b/activation/nipost.go index 399e07c9e5..2e467b3714 100644 --- a/activation/nipost.go +++ b/activation/nipost.go @@ -237,6 +237,11 @@ func (nb *NIPostBuilder) BuildNIPost( poetProofDeadline, poetRoundStart, challenge.Bytes()) if err != nil { + if errors.Is(err, ErrNoRegistrationForGivenPoetFound) { + // misconfiguration of poet services detected, user has to fix config + logger.Fatal("submitting to poets", zap.Error(err)) + } + return nil, fmt.Errorf("submitting to poets: %w", err) } @@ -440,13 +445,14 @@ func (nb *NIPostBuilder) submitPoetChallenges( ) case len(existingRegistrations) == 0: // no existing registration for given poets set - nb.logger.Panic( + nb.logger.Warn( "None of the poets listed in the config matches the existing registrations. "+ "Verify your config and local database state.", zap.Strings("registrations", maps.Keys(registrationsMap)), zap.Strings("configured_poets", maps.Keys(nb.poetProvers)), log.ZShortStringer("smesherID", nodeID), ) + return nil, ErrNoRegistrationForGivenPoetFound default: return existingRegistrations, nil } diff --git a/activation/nipost_test.go b/activation/nipost_test.go index cc368c9c44..fe59df97a0 100644 --- a/activation/nipost_test.go +++ b/activation/nipost_test.go @@ -1089,18 +1089,13 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { ) require.NoError(t, err) - defer func() { - if r := recover(); r == nil { - t.Errorf("Panic is expected") - } - }() - - nb.submitPoetChallenges( + _, err = nb.submitPoetChallenges( context.Background(), sig, time.Now().Add(10*time.Second), time.Now().Add(-5*time.Second), // poet round started challengeHash.Bytes()) + require.ErrorIs(t, err, ErrNoRegistrationForGivenPoetFound) }) } diff --git a/activation/poet.go b/activation/poet.go index f7b8a3cc31..f9914de804 100644 --- a/activation/poet.go +++ b/activation/poet.go @@ -68,10 +68,12 @@ type PoetClient interface { // HTTPPoetClient implements PoetProvingServiceClient interface. type HTTPPoetClient struct { - id []byte - baseURL *url.URL - client *retryablehttp.Client - logger *zap.Logger + id []byte + baseURL *url.URL + client *retryablehttp.Client + logger *zap.Logger + expectedPhaseShift time.Duration + fetchedPhaseShift time.Duration } func checkRetry(ctx context.Context, resp *http.Response, err error) (bool, error) { @@ -144,24 +146,18 @@ func NewHTTPPoetClient(server types.PoetServer, cfg PoetConfig, opts ...PoetClie } poetClient := &HTTPPoetClient{ - id: server.Pubkey.Bytes(), - baseURL: baseURL, - client: client, - logger: zap.NewNop(), + id: server.Pubkey.Bytes(), + baseURL: baseURL, + client: client, + logger: zap.NewNop(), + expectedPhaseShift: cfg.PhaseShift, } for _, opt := range opts { opt(poetClient) } - resp, err := poetClient.info(context.Background()) - if err != nil { + if err := poetClient.verifyPhaseShiftConfiguration(context.Background()); err != nil { poetClient.logger.Error("getting info about poet service configuration", zap.Error(err)) - } else if resp.PhaseShift.AsDuration() != cfg.PhaseShift { - poetClient.logger.Warn("getting info about poet service configuration", - zap.Duration("server: phase shift", resp.PhaseShift.AsDuration()), - zap.Duration("config: cycle gap", cfg.PhaseShift), - ) - return nil, ErrPhaseShiftMismatch } poetClient.logger.Info( @@ -212,6 +208,25 @@ func (c *HTTPPoetClient) CertifierInfo(ctx context.Context) (*url.URL, []byte, e return url, certifierInfo.Pubkey, nil } +func (c *HTTPPoetClient) verifyPhaseShiftConfiguration(ctx context.Context) error { + if c.fetchedPhaseShift != 0 { + return nil + } + + resp, err := c.info(ctx) + if err != nil { + return err + } else if resp.PhaseShift.AsDuration() != c.expectedPhaseShift { + c.logger.Panic("verifying poet service configuration", + zap.Duration("server: phase shift", resp.PhaseShift.AsDuration()), + zap.Duration("config: phase shift", c.expectedPhaseShift), + ) + } + + c.fetchedPhaseShift = resp.PhaseShift.AsDuration() + return nil +} + // Submit registers a challenge in the proving service current open round. func (c *HTTPPoetClient) Submit( ctx context.Context, @@ -221,6 +236,10 @@ func (c *HTTPPoetClient) Submit( nodeID types.NodeID, auth PoetAuth, ) (*types.PoetRound, error) { + if err := c.verifyPhaseShiftConfiguration(context.Background()); err != nil { + return nil, err + } + request := rpcapi.SubmitRequest{ Prefix: prefix, Challenge: challenge, From a02a90c8bd6a84f51723171f7d27fcfb6af1380d Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Tue, 6 Aug 2024 10:13:42 +0200 Subject: [PATCH 17/26] verifying poets configuration on poet service layer, fixing tests, fing review issues --- activation/activation_errors.go | 29 +++++- activation/e2e/poet_client.go | 9 +- activation/e2e/poet_test.go | 8 +- activation/mocks.go | 40 ++++++++ activation/nipost.go | 29 +++--- activation/poet.go | 173 ++++++++++++++++++-------------- activation/poet_client_test.go | 135 ++++++++++++++++++++++++- activation/poet_mocks.go | 57 +++++++++-- api/grpcserver/mocks.go | 22 ++-- common/types/poet.go | 13 +++ go.mod | 1 + go.sum | 2 + 12 files changed, 396 insertions(+), 122 deletions(-) diff --git a/activation/activation_errors.go b/activation/activation_errors.go index e8503fc128..d9bdcfe7c3 100644 --- a/activation/activation_errors.go +++ b/activation/activation_errors.go @@ -3,6 +3,7 @@ package activation import ( "errors" "fmt" + "strings" ) var ( @@ -12,8 +13,9 @@ var ( ErrPoetServiceUnstable = &PoetSvcUnstableError{} // ErrPoetProofNotReceived is returned when no poet proof was received. ErrPoetProofNotReceived = errors.New("builder: didn't receive any poet proof") - // ErrNoRegistrationForGivenPoetFound is returned when configured poets do not match existing registrations in db at all - ErrNoRegistrationForGivenPoetFound = errors.New("builder: none of configured poets matches the existing registrations") + // ErrNoRegistrationForGivenPoetFound is returned when configured poets + // do not match existing registrations in db at all. + ErrNoRegistrationForGivenPoetFound = &PoetRegistrationMismatchError{} ) // PoetSvcUnstableError means there was a problem communicating @@ -35,3 +37,26 @@ func (e *PoetSvcUnstableError) Is(target error) bool { _, ok := target.(*PoetSvcUnstableError) return ok } + +type PoetRegistrationMismatchError struct { + registrations []string + configuredPoets []string +} + +func (e *PoetRegistrationMismatchError) Error() string { + var sb strings.Builder + sb.WriteString("builder: none of configured poets matches the existing registrations.\n") + sb.WriteString("registrations:\n") + for _, r := range e.registrations { + sb.WriteString("\t") + sb.WriteString(r) + sb.WriteString("\n") + } + sb.WriteString("\n configured poets:\n") + for _, p := range e.configuredPoets { + sb.WriteString("\t") + sb.WriteString(p) + sb.WriteString("\n") + } + return sb.String() +} diff --git a/activation/e2e/poet_client.go b/activation/e2e/poet_client.go index c025ed5302..7311200bf9 100644 --- a/activation/e2e/poet_client.go +++ b/activation/e2e/poet_client.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "net/url" "strconv" "sync" "time" @@ -66,8 +65,12 @@ func (p *TestPoet) Submit( return &types.PoetRound{ID: strconv.Itoa(round), End: time.Now()}, nil } -func (p *TestPoet) CertifierInfo(ctx context.Context) (*url.URL, []byte, error) { - return nil, nil, errors.New("not supported") +func (p *TestPoet) CertifierInfo(ctx context.Context) (*types.CertifierInfo, error) { + return nil, errors.New("CertifierInfo: not supported") +} + +func (p *TestPoet) Info(ctx context.Context) (*types.PoetInfo, error) { + return nil, errors.New("Info: not supported") } // Build a proof. diff --git a/activation/e2e/poet_test.go b/activation/e2e/poet_test.go index 3aef153168..27d50f18e0 100644 --- a/activation/e2e/poet_test.go +++ b/activation/e2e/poet_test.go @@ -259,10 +259,10 @@ func TestCertifierInfo(t *testing.T) { ) require.NoError(t, err) - url, pubkey, err := client.CertifierInfo(context.Background()) + certInfo, err := client.CertifierInfo(context.Background()) r.NoError(err) - r.Equal("http://localhost:8080", url.String()) - r.Equal([]byte("pubkey"), pubkey) + r.Equal("http://localhost:8080", certInfo.Url.String()) + r.Equal([]byte("pubkey"), certInfo.Pubkey) } func TestNoCertifierInfo(t *testing.T) { @@ -291,6 +291,6 @@ func TestNoCertifierInfo(t *testing.T) { ) require.NoError(t, err) - _, _, err = client.CertifierInfo(context.Background()) + _, err = client.CertifierInfo(context.Background()) r.ErrorContains(err, "poet doesn't support certificates") } diff --git a/activation/mocks.go b/activation/mocks.go index dd1c6f637d..dcc3d9cc9c 100644 --- a/activation/mocks.go +++ b/activation/mocks.go @@ -20,6 +20,7 @@ import ( signing "github.com/spacemeshos/go-spacemesh/signing" certifier "github.com/spacemeshos/go-spacemesh/sql/localsql/certifier" nipost "github.com/spacemeshos/go-spacemesh/sql/localsql/nipost" + v1 "github.com/spacemeshos/poet/release/proto/go/rpc/api/v1" shared "github.com/spacemeshos/post/shared" gomock "go.uber.org/mock/gomock" ) @@ -1732,6 +1733,45 @@ func (c *MockPoetServiceCertifyCall) DoAndReturn(f func(context.Context, types.N return c } +// Info mocks base method. +func (m *MockPoetService) Info(ctx context.Context) (*v1.InfoResponse, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Info", ctx) + ret0, _ := ret[0].(*v1.InfoResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Info indicates an expected call of Info. +func (mr *MockPoetServiceMockRecorder) Info(ctx any) *MockPoetServiceInfoCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Info", reflect.TypeOf((*MockPoetService)(nil).Info), ctx) + return &MockPoetServiceInfoCall{Call: call} +} + +// MockPoetServiceInfoCall wrap *gomock.Call +type MockPoetServiceInfoCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockPoetServiceInfoCall) Return(arg0 *v1.InfoResponse, arg1 error) *MockPoetServiceInfoCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockPoetServiceInfoCall) Do(f func(context.Context) (*v1.InfoResponse, error)) *MockPoetServiceInfoCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockPoetServiceInfoCall) DoAndReturn(f func(context.Context) (*v1.InfoResponse, error)) *MockPoetServiceInfoCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + // Proof mocks base method. func (m *MockPoetService) Proof(ctx context.Context, roundID string) (*types.PoetProof, []types.Hash32, error) { m.ctrl.T.Helper() diff --git a/activation/nipost.go b/activation/nipost.go index 2e467b3714..9ba89dfc01 100644 --- a/activation/nipost.go +++ b/activation/nipost.go @@ -236,12 +236,17 @@ func (nb *NIPostBuilder) BuildNIPost( signer, poetProofDeadline, poetRoundStart, challenge.Bytes()) + + regErr := &PoetRegistrationMismatchError{} if err != nil { - if errors.Is(err, ErrNoRegistrationForGivenPoetFound) { - // misconfiguration of poet services detected, user has to fix config - logger.Fatal("submitting to poets", zap.Error(err)) + if errors.As(err, ®Err) { + logger.Fatal( + "None of the poets listed in the config matches the existing registrations. "+ + "Verify your config and local database state.", + zap.Strings("registrations", regErr.registrations), + zap.Strings("configured_poets", regErr.configuredPoets), + ) } - return nil, fmt.Errorf("submitting to poets: %w", err) } @@ -299,13 +304,17 @@ func (nb *NIPostBuilder) BuildNIPost( defer cancel() nb.logger.Info("starting post execution", zap.Binary("challenge", poetProofRef[:])) + startTime := time.Now() proof, postInfo, err := nb.Proof(postCtx, signer.NodeID(), poetProofRef[:], postChallenge) if err != nil { return nil, fmt.Errorf("failed to generate Post: %w", err) } + postGenDuration := time.Since(startTime) + nb.logger.Info("finished post execution", zap.Duration("duration", postGenDuration)) + metrics.PostDuration.Set(float64(postGenDuration.Nanoseconds())) public.PostSeconds.Set(postGenDuration.Seconds()) @@ -445,14 +454,10 @@ func (nb *NIPostBuilder) submitPoetChallenges( ) case len(existingRegistrations) == 0: // no existing registration for given poets set - nb.logger.Warn( - "None of the poets listed in the config matches the existing registrations. "+ - "Verify your config and local database state.", - zap.Strings("registrations", maps.Keys(registrationsMap)), - zap.Strings("configured_poets", maps.Keys(nb.poetProvers)), - log.ZShortStringer("smesherID", nodeID), - ) - return nil, ErrNoRegistrationForGivenPoetFound + return nil, &PoetRegistrationMismatchError{ + registrations: maps.Keys(registrationsMap), + configuredPoets: maps.Keys(nb.poetProvers), + } default: return existingRegistrations, nil } diff --git a/activation/poet.go b/activation/poet.go index f9914de804..402761a3f0 100644 --- a/activation/poet.go +++ b/activation/poet.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "go.uber.org/atomic" "io" "net/http" "net/url" @@ -31,7 +32,7 @@ var ( ErrInvalidRequest = errors.New("invalid request") ErrUnauthorized = errors.New("unauthorized") ErrCertificatesNotSupported = errors.New("poet doesn't support certificates") - ErrPhaseShiftMismatch = errors.New("phase shift mismatch") + ErrIncompatiblePhaseShift = errors.New("fetched poet phase_shift is incompatible with configured phase_shift") ) type PoetPowParams struct { @@ -54,7 +55,7 @@ type PoetClient interface { Address() string PowParams(ctx context.Context) (*PoetPowParams, error) - CertifierInfo(ctx context.Context) (*url.URL, []byte, error) + CertifierInfo(ctx context.Context) (*types.CertifierInfo, error) Submit( ctx context.Context, deadline time.Time, @@ -64,16 +65,15 @@ type PoetClient interface { auth PoetAuth, ) (*types.PoetRound, error) Proof(ctx context.Context, roundID string) (*types.PoetProofMessage, []types.Hash32, error) + Info(ctx context.Context) (*types.PoetInfo, error) } // HTTPPoetClient implements PoetProvingServiceClient interface. type HTTPPoetClient struct { - id []byte - baseURL *url.URL - client *retryablehttp.Client - logger *zap.Logger - expectedPhaseShift time.Duration - fetchedPhaseShift time.Duration + id []byte + baseURL *url.URL + client *retryablehttp.Client + logger *zap.Logger } func checkRetry(ctx context.Context, resp *http.Response, err error) (bool, error) { @@ -146,20 +146,15 @@ func NewHTTPPoetClient(server types.PoetServer, cfg PoetConfig, opts ...PoetClie } poetClient := &HTTPPoetClient{ - id: server.Pubkey.Bytes(), - baseURL: baseURL, - client: client, - logger: zap.NewNop(), - expectedPhaseShift: cfg.PhaseShift, + id: server.Pubkey.Bytes(), + baseURL: baseURL, + client: client, + logger: zap.NewNop(), } for _, opt := range opts { opt(poetClient) } - if err := poetClient.verifyPhaseShiftConfiguration(context.Background()); err != nil { - poetClient.logger.Error("getting info about poet service configuration", zap.Error(err)) - } - poetClient.logger.Info( "created poet client", zap.Stringer("url", baseURL), @@ -192,39 +187,15 @@ func (c *HTTPPoetClient) PowParams(ctx context.Context) (*PoetPowParams, error) }, nil } -func (c *HTTPPoetClient) CertifierInfo(ctx context.Context) (*url.URL, []byte, error) { - info, err := c.info(ctx) +func (c *HTTPPoetClient) CertifierInfo(ctx context.Context) (*types.CertifierInfo, error) { + info, err := c.Info(ctx) if err != nil { - return nil, nil, err - } - certifierInfo := info.GetCertifier() - if certifierInfo == nil { - return nil, nil, ErrCertificatesNotSupported + return nil, err } - url, err := url.Parse(certifierInfo.Url) - if err != nil { - return nil, nil, fmt.Errorf("parsing certifier address: %w", err) + if info.Certifier == nil { + return nil, ErrCertificatesNotSupported } - return url, certifierInfo.Pubkey, nil -} - -func (c *HTTPPoetClient) verifyPhaseShiftConfiguration(ctx context.Context) error { - if c.fetchedPhaseShift != 0 { - return nil - } - - resp, err := c.info(ctx) - if err != nil { - return err - } else if resp.PhaseShift.AsDuration() != c.expectedPhaseShift { - c.logger.Panic("verifying poet service configuration", - zap.Duration("server: phase shift", resp.PhaseShift.AsDuration()), - zap.Duration("config: phase shift", c.expectedPhaseShift), - ) - } - - c.fetchedPhaseShift = resp.PhaseShift.AsDuration() - return nil + return info.Certifier, nil } // Submit registers a challenge in the proving service current open round. @@ -236,10 +207,6 @@ func (c *HTTPPoetClient) Submit( nodeID types.NodeID, auth PoetAuth, ) (*types.PoetRound, error) { - if err := c.verifyPhaseShiftConfiguration(context.Background()); err != nil { - return nil, err - } - request := rpcapi.SubmitRequest{ Prefix: prefix, Challenge: challenge, @@ -273,12 +240,30 @@ func (c *HTTPPoetClient) Submit( return &types.PoetRound{ID: resBody.RoundId, End: roundEnd}, nil } -func (c *HTTPPoetClient) info(ctx context.Context) (*rpcapi.InfoResponse, error) { +func (c *HTTPPoetClient) Info(ctx context.Context) (*types.PoetInfo, error) { resBody := rpcapi.InfoResponse{} if err := c.req(ctx, http.MethodGet, "/v1/info", nil, &resBody); err != nil { return nil, fmt.Errorf("getting poet info: %w", err) } - return &resBody, nil + + var certifierInfo *types.CertifierInfo + if resBody.GetCertifier() != nil { + url, err := url.Parse(resBody.GetCertifier().Url) + if err != nil { + return nil, fmt.Errorf("parsing certifier address: %w", err) + } + certifierInfo = &types.CertifierInfo{ + Url: url, + Pubkey: resBody.GetCertifier().Pubkey, + } + } + + return &types.PoetInfo{ + ServicePubkey: resBody.ServicePubkey, + PhaseShift: resBody.PhaseShift.AsDuration(), + CycleGap: resBody.CycleGap.AsDuration(), + Certifier: certifierInfo, + }, nil } // Proof implements PoetProvingServiceClient. @@ -363,11 +348,6 @@ func (c *HTTPPoetClient) req(ctx context.Context, method, path string, reqBody, return nil } -type certifierInfo struct { - url *url.URL - pubkey []byte -} - type cachedData[T any] struct { mu sync.Mutex data T @@ -404,7 +384,9 @@ type poetService struct { certifier certifierService - certifierInfoCache cachedData[*certifierInfo] + certifierInfoCache cachedData[*types.CertifierInfo] + expectedPhaseShift time.Duration + fetchedPhaseShift *atomic.Duration powParamsCache cachedData[*PoetPowParams] } @@ -443,21 +425,48 @@ func NewPoetServiceWithClient( logger *zap.Logger, opts ...PoetServiceOpt, ) *poetService { - poetClient := &poetService{ + service := &poetService{ db: db, logger: logger, client: client, requestTimeout: cfg.RequestTimeout, - certifierInfoCache: cachedData[*certifierInfo]{ttl: cfg.CertifierInfoCacheTTL}, + certifierInfoCache: cachedData[*types.CertifierInfo]{ttl: cfg.CertifierInfoCacheTTL}, powParamsCache: cachedData[*PoetPowParams]{ttl: cfg.PowParamsCacheTTL}, proofMembers: make(map[string][]types.Hash32, 1), + expectedPhaseShift: cfg.PhaseShift, + fetchedPhaseShift: &atomic.Duration{}, } - for _, opt := range opts { - opt(poetClient) + opt(service) + } + + err := service.verifyPhaseShiftConfiguration(context.Background()) + if err != nil { + if errors.Is(err, ErrIncompatiblePhaseShift) { + logger.Panic("failed to create poet service", + zap.Error(err), + zap.String("poet", client.Address())) + } + logger.Warn("failed to fetch poet phase shift", + zap.Error(err), + zap.String("poet", client.Address())) + } + return service +} + +func (c *poetService) verifyPhaseShiftConfiguration(ctx context.Context) error { + if c.fetchedPhaseShift.Load() != 0 { + return nil + } + resp, err := c.client.Info(ctx) + if err != nil { + return err + } else if resp.PhaseShift != c.expectedPhaseShift { + return ErrIncompatiblePhaseShift } - return poetClient + c.fetchedPhaseShift.Store(resp.PhaseShift) + return nil } func (c *poetService) Address() string { @@ -483,6 +492,7 @@ func (c *poetService) authorize( // Fallback to PoW // TODO: remove this fallback once we migrate to certificates fully. logger.Info("falling back to PoW authorization") + powCtx, cancel := withConditionalTimeout(ctx, c.requestTimeout) defer cancel() powParams, err := c.powParams(powCtx) @@ -516,8 +526,8 @@ func (c *poetService) reauthorize( challenge []byte, ) (*PoetAuth, error) { if c.certifier != nil { - if _, pubkey, err := c.getCertifierInfo(ctx); err == nil { - if err := c.certifier.DeleteCertificate(id, pubkey); err != nil { + if info, err := c.getCertifierInfo(ctx); err == nil { + if err := c.certifier.DeleteCertificate(id, info.Pubkey); err != nil { return nil, fmt.Errorf("deleting cert: %w", err) } } @@ -538,7 +548,19 @@ func (c *poetService) Submit( log.ZShortStringer("smesherID", nodeID), ) - // Try obtain a certificate + if err := c.verifyPhaseShiftConfiguration(ctx); err != nil { + if errors.Is(err, ErrIncompatiblePhaseShift) { + logger.Panic("failed to submit challenge", + zap.Error(err), + zap.String("poet", c.client.Address())) + } + logger.Warn("failed to submit challenge: couldn't fetch poet phase shift", + zap.Error(err), + zap.String("poet", c.client.Address())) + return nil, err + } + + // Try to obtain a certificate auth, err := c.authorize(ctx, nodeID, challenge, logger) if err != nil { return nil, fmt.Errorf("authorizing: %w", err) @@ -598,26 +620,25 @@ func (c *poetService) Certify(ctx context.Context, id types.NodeID) (*certifier. if c.certifier == nil { return nil, errors.New("certifier not configured") } - url, pubkey, err := c.getCertifierInfo(ctx) + info, err := c.getCertifierInfo(ctx) if err != nil { return nil, err } - return c.certifier.Certificate(ctx, id, url, pubkey) + return c.certifier.Certificate(ctx, id, info.Url, info.Pubkey) } -func (c *poetService) getCertifierInfo(ctx context.Context) (*url.URL, []byte, error) { - info, err := c.certifierInfoCache.get(func() (*certifierInfo, error) { - url, pubkey, err := c.client.CertifierInfo(ctx) +func (c *poetService) getCertifierInfo(ctx context.Context) (*types.CertifierInfo, error) { + info, err := c.certifierInfoCache.get(func() (*types.CertifierInfo, error) { + certifierInfo, err := c.client.CertifierInfo(ctx) if err != nil { return nil, fmt.Errorf("getting certifier info: %w", err) } - return &certifierInfo{url: url, pubkey: pubkey}, nil + return certifierInfo, nil }) if err != nil { - return nil, nil, err + return nil, err } - - return info.url, info.pubkey, nil + return info, nil } func (c *poetService) powParams(ctx context.Context) (*PoetPowParams, error) { diff --git a/activation/poet_client_test.go b/activation/poet_client_test.go index 782872d347..ca7606aeb9 100644 --- a/activation/poet_client_test.go +++ b/activation/poet_client_test.go @@ -452,6 +452,7 @@ func TestPoetClient_FallbacksToPowWhenCannotRecertify(t *testing.T) { client, err := NewHTTPPoetClient(server, cfg, withCustomHttpClient(ts.Client())) require.NoError(t, err) + poet := NewPoetServiceWithClient(nil, client, cfg, zaptest.NewLogger(t), WithCertifier(mCertifier)) _, err = poet.Submit(context.Background(), time.Time{}, nil, nil, types.RandomEdSignature(), sig.NodeID()) @@ -475,18 +476,24 @@ func TestPoetService_CachesCertifierInfo(t *testing.T) { cfg.CertifierInfoCacheTTL = tc.ttl client := NewMockPoetClient(gomock.NewController(t)) db := NewPoetDb(sql.InMemory(), zaptest.NewLogger(t)) + + client.EXPECT().Address().Return("some_addr").AnyTimes() + client.EXPECT().Info(gomock.Any()).Return(&types.PoetInfo{}, nil) + poet := NewPoetServiceWithClient(db, client, cfg, zaptest.NewLogger(t)) + url := &url.URL{Host: "certifier.hello"} pubkey := []byte("pubkey") - exp := client.EXPECT().CertifierInfo(gomock.Any()).Return(url, pubkey, nil) + exp := client.EXPECT().CertifierInfo(gomock.Any()). + Return(&types.CertifierInfo{Url: url, Pubkey: pubkey}, nil) if tc.ttl == 0 { exp.Times(5) } for range 5 { - gotUrl, gotPubkey, err := poet.getCertifierInfo(context.Background()) + info, err := poet.getCertifierInfo(context.Background()) require.NoError(t, err) - require.Equal(t, url, gotUrl) - require.Equal(t, pubkey, gotPubkey) + require.Equal(t, url, info.Url) + require.Equal(t, pubkey, info.Pubkey) } }) } @@ -525,3 +532,123 @@ func TestPoetService_CachesPowParams(t *testing.T) { }) } } + +func TestPoetService_FetchPoetPhaseShift(t *testing.T) { + t.Parallel() + const phaseShift = time.Second + + t.Run("poet service created: expected and fetched phase shift are matching", + func(t *testing.T) { + cfg := DefaultPoetConfig() + cfg.PhaseShift = phaseShift + + client := NewMockPoetClient(gomock.NewController(t)) + client.EXPECT().Address().Return("some_addr").AnyTimes() + client.EXPECT().Info(gomock.Any()).Return(&types.PoetInfo{ + PhaseShift: phaseShift, + }, nil) + + NewPoetServiceWithClient(nil, client, cfg, zaptest.NewLogger(t)) + }) + + t.Run("poet service created: phase shift is not fetched", + func(t *testing.T) { + cfg := DefaultPoetConfig() + cfg.PhaseShift = phaseShift + + client := NewMockPoetClient(gomock.NewController(t)) + client.EXPECT().Address().Return("some_addr").AnyTimes() + client.EXPECT().Info(gomock.Any()).Return(nil, errors.New("some error")) + + NewPoetServiceWithClient(nil, client, cfg, zaptest.NewLogger(t)) + }) + + t.Run("poet service creation failed: expected and fetched phase shift are not matching", + func(t *testing.T) { + cfg := DefaultPoetConfig() + cfg.PhaseShift = phaseShift + + client := NewMockPoetClient(gomock.NewController(t)) + client.EXPECT().Address().Return("some_addr").AnyTimes() + client.EXPECT().Info(gomock.Any()).Return(&types.PoetInfo{ + PhaseShift: phaseShift * 2, + }, nil) + + defer func() { + if r := recover(); r == nil { + t.Errorf("Panic is expected") + } + }() + NewPoetServiceWithClient(nil, client, cfg, zaptest.NewLogger(t)) + }) + + t.Run("fetch phase shift before submitting challenge: success", + func(t *testing.T) { + cfg := DefaultPoetConfig() + cfg.PhaseShift = phaseShift + + client := NewMockPoetClient(gomock.NewController(t)) + client.EXPECT().Address().Return("some_addr").AnyTimes() + client.EXPECT().Info(gomock.Any()).Return(nil, errors.New("some error")) + + poet := NewPoetServiceWithClient(nil, client, cfg, zaptest.NewLogger(t)) + sig, err := signing.NewEdSigner() + require.NoError(t, err) + + client.EXPECT().Info(gomock.Any()).Return(&types.PoetInfo{PhaseShift: phaseShift}, nil) + client.EXPECT().PowParams(gomock.Any()).Return(&PoetPowParams{}, nil) + client.EXPECT(). + Submit( + gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), + gomock.Any(), gomock.Any(), gomock.Any()). + Return(&types.PoetRound{}, nil) + + _, err = poet.Submit(context.Background(), time.Time{}, nil, nil, types.RandomEdSignature(), sig.NodeID()) + require.NoError(t, err) + }) + + t.Run("fetch phase shift before submitting challenge: failed to fetch poet info", + func(t *testing.T) { + cfg := DefaultPoetConfig() + cfg.PhaseShift = phaseShift + + client := NewMockPoetClient(gomock.NewController(t)) + client.EXPECT().Address().Return("some_addr").AnyTimes() + client.EXPECT().Info(gomock.Any()).Return(nil, errors.New("some error")) + + poet := NewPoetServiceWithClient(nil, client, cfg, zaptest.NewLogger(t)) + sig, err := signing.NewEdSigner() + require.NoError(t, err) + + expectedErr := errors.New("some error") + client.EXPECT().Info(gomock.Any()).Return(nil, expectedErr) + + _, err = poet.Submit(context.Background(), time.Time{}, nil, nil, types.RandomEdSignature(), sig.NodeID()) + require.ErrorIs(t, err, expectedErr) + }) + + t.Run("fetch phase shift before submitting challenge: fetched and expected phase shift do not match", + func(t *testing.T) { + cfg := DefaultPoetConfig() + cfg.PhaseShift = phaseShift + + client := NewMockPoetClient(gomock.NewController(t)) + client.EXPECT().Address().Return("some_addr").AnyTimes() + client.EXPECT().Info(gomock.Any()).Return(nil, errors.New("some error")) + + poet := NewPoetServiceWithClient(nil, client, cfg, zaptest.NewLogger(t)) + sig, err := signing.NewEdSigner() + require.NoError(t, err) + + client.EXPECT().Info(gomock.Any()).Return(&types.PoetInfo{ + PhaseShift: phaseShift * 2, + }, nil) + + defer func() { + if r := recover(); r == nil { + t.Errorf("Panic is expected") + } + }() + poet.Submit(context.Background(), time.Time{}, nil, nil, types.RandomEdSignature(), sig.NodeID()) + }) +} diff --git a/activation/poet_mocks.go b/activation/poet_mocks.go index ff746b4de6..885105fef3 100644 --- a/activation/poet_mocks.go +++ b/activation/poet_mocks.go @@ -11,7 +11,6 @@ package activation import ( context "context" - url "net/url" reflect "reflect" time "time" @@ -81,13 +80,12 @@ func (c *MockPoetClientAddressCall) DoAndReturn(f func() string) *MockPoetClient } // CertifierInfo mocks base method. -func (m *MockPoetClient) CertifierInfo(ctx context.Context) (*url.URL, []byte, error) { +func (m *MockPoetClient) CertifierInfo(ctx context.Context) (*types.CertifierInfo, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "CertifierInfo", ctx) - ret0, _ := ret[0].(*url.URL) - ret1, _ := ret[1].([]byte) - ret2, _ := ret[2].(error) - return ret0, ret1, ret2 + ret0, _ := ret[0].(*types.CertifierInfo) + ret1, _ := ret[1].(error) + return ret0, ret1 } // CertifierInfo indicates an expected call of CertifierInfo. @@ -103,19 +101,19 @@ type MockPoetClientCertifierInfoCall struct { } // Return rewrite *gomock.Call.Return -func (c *MockPoetClientCertifierInfoCall) Return(arg0 *url.URL, arg1 []byte, arg2 error) *MockPoetClientCertifierInfoCall { - c.Call = c.Call.Return(arg0, arg1, arg2) +func (c *MockPoetClientCertifierInfoCall) Return(arg0 *types.CertifierInfo, arg1 error) *MockPoetClientCertifierInfoCall { + c.Call = c.Call.Return(arg0, arg1) return c } // Do rewrite *gomock.Call.Do -func (c *MockPoetClientCertifierInfoCall) Do(f func(context.Context) (*url.URL, []byte, error)) *MockPoetClientCertifierInfoCall { +func (c *MockPoetClientCertifierInfoCall) Do(f func(context.Context) (*types.CertifierInfo, error)) *MockPoetClientCertifierInfoCall { c.Call = c.Call.Do(f) return c } // DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockPoetClientCertifierInfoCall) DoAndReturn(f func(context.Context) (*url.URL, []byte, error)) *MockPoetClientCertifierInfoCall { +func (c *MockPoetClientCertifierInfoCall) DoAndReturn(f func(context.Context) (*types.CertifierInfo, error)) *MockPoetClientCertifierInfoCall { c.Call = c.Call.DoAndReturn(f) return c } @@ -158,6 +156,45 @@ func (c *MockPoetClientIdCall) DoAndReturn(f func() []byte) *MockPoetClientIdCal return c } +// Info mocks base method. +func (m *MockPoetClient) Info(ctx context.Context) (*types.PoetInfo, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Info", ctx) + ret0, _ := ret[0].(*types.PoetInfo) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Info indicates an expected call of Info. +func (mr *MockPoetClientMockRecorder) Info(ctx any) *MockPoetClientInfoCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Info", reflect.TypeOf((*MockPoetClient)(nil).Info), ctx) + return &MockPoetClientInfoCall{Call: call} +} + +// MockPoetClientInfoCall wrap *gomock.Call +type MockPoetClientInfoCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockPoetClientInfoCall) Return(arg0 *types.PoetInfo, arg1 error) *MockPoetClientInfoCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockPoetClientInfoCall) Do(f func(context.Context) (*types.PoetInfo, error)) *MockPoetClientInfoCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockPoetClientInfoCall) DoAndReturn(f func(context.Context) (*types.PoetInfo, error)) *MockPoetClientInfoCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + // PowParams mocks base method. func (m *MockPoetClient) PowParams(ctx context.Context) (*PoetPowParams, error) { m.ctrl.T.Helper() diff --git a/api/grpcserver/mocks.go b/api/grpcserver/mocks.go index ab849fa95c..7812313f72 100644 --- a/api/grpcserver/mocks.go +++ b/api/grpcserver/mocks.go @@ -15,7 +15,7 @@ import ( time "time" network "github.com/libp2p/go-libp2p/core/network" - multiaddr "github.com/multiformats/go-multiaddr" + go_multiaddr "github.com/multiformats/go-multiaddr" activation "github.com/spacemeshos/go-spacemesh/activation" types "github.com/spacemeshos/go-spacemesh/common/types" wire "github.com/spacemeshos/go-spacemesh/malfeasance/wire" @@ -126,10 +126,10 @@ func (c *MocknetworkInfoIDCall) DoAndReturn(f func() p2p.Peer) *MocknetworkInfoI } // KnownAddresses mocks base method. -func (m *MocknetworkInfo) KnownAddresses() []multiaddr.Multiaddr { +func (m *MocknetworkInfo) KnownAddresses() []go_multiaddr.Multiaddr { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "KnownAddresses") - ret0, _ := ret[0].([]multiaddr.Multiaddr) + ret0, _ := ret[0].([]go_multiaddr.Multiaddr) return ret0 } @@ -146,28 +146,28 @@ type MocknetworkInfoKnownAddressesCall struct { } // Return rewrite *gomock.Call.Return -func (c *MocknetworkInfoKnownAddressesCall) Return(arg0 []multiaddr.Multiaddr) *MocknetworkInfoKnownAddressesCall { +func (c *MocknetworkInfoKnownAddressesCall) Return(arg0 []go_multiaddr.Multiaddr) *MocknetworkInfoKnownAddressesCall { c.Call = c.Call.Return(arg0) return c } // Do rewrite *gomock.Call.Do -func (c *MocknetworkInfoKnownAddressesCall) Do(f func() []multiaddr.Multiaddr) *MocknetworkInfoKnownAddressesCall { +func (c *MocknetworkInfoKnownAddressesCall) Do(f func() []go_multiaddr.Multiaddr) *MocknetworkInfoKnownAddressesCall { c.Call = c.Call.Do(f) return c } // DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MocknetworkInfoKnownAddressesCall) DoAndReturn(f func() []multiaddr.Multiaddr) *MocknetworkInfoKnownAddressesCall { +func (c *MocknetworkInfoKnownAddressesCall) DoAndReturn(f func() []go_multiaddr.Multiaddr) *MocknetworkInfoKnownAddressesCall { c.Call = c.Call.DoAndReturn(f) return c } // ListenAddresses mocks base method. -func (m *MocknetworkInfo) ListenAddresses() []multiaddr.Multiaddr { +func (m *MocknetworkInfo) ListenAddresses() []go_multiaddr.Multiaddr { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ListenAddresses") - ret0, _ := ret[0].([]multiaddr.Multiaddr) + ret0, _ := ret[0].([]go_multiaddr.Multiaddr) return ret0 } @@ -184,19 +184,19 @@ type MocknetworkInfoListenAddressesCall struct { } // Return rewrite *gomock.Call.Return -func (c *MocknetworkInfoListenAddressesCall) Return(arg0 []multiaddr.Multiaddr) *MocknetworkInfoListenAddressesCall { +func (c *MocknetworkInfoListenAddressesCall) Return(arg0 []go_multiaddr.Multiaddr) *MocknetworkInfoListenAddressesCall { c.Call = c.Call.Return(arg0) return c } // Do rewrite *gomock.Call.Do -func (c *MocknetworkInfoListenAddressesCall) Do(f func() []multiaddr.Multiaddr) *MocknetworkInfoListenAddressesCall { +func (c *MocknetworkInfoListenAddressesCall) Do(f func() []go_multiaddr.Multiaddr) *MocknetworkInfoListenAddressesCall { c.Call = c.Call.Do(f) return c } // DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MocknetworkInfoListenAddressesCall) DoAndReturn(f func() []multiaddr.Multiaddr) *MocknetworkInfoListenAddressesCall { +func (c *MocknetworkInfoListenAddressesCall) DoAndReturn(f func() []go_multiaddr.Multiaddr) *MocknetworkInfoListenAddressesCall { c.Call = c.Call.DoAndReturn(f) return c } diff --git a/common/types/poet.go b/common/types/poet.go index 0d1d1177ea..f04b613517 100644 --- a/common/types/poet.go +++ b/common/types/poet.go @@ -3,6 +3,7 @@ package types import ( "encoding/hex" "fmt" + "net/url" "time" poetShared "github.com/spacemeshos/poet/shared" @@ -97,3 +98,15 @@ type PoetRound struct { ID string `scale:"max=32"` End time.Time } + +type PoetInfo struct { + ServicePubkey []byte + PhaseShift time.Duration + CycleGap time.Duration + Certifier *CertifierInfo +} + +type CertifierInfo struct { + Url *url.URL + Pubkey []byte +} diff --git a/go.mod b/go.mod index e92dee84d5..02824c09c6 100644 --- a/go.mod +++ b/go.mod @@ -53,6 +53,7 @@ require ( github.com/stretchr/testify v1.9.0 github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7 github.com/zeebo/blake3 v0.2.3 + go.uber.org/atomic v1.11.0 go.uber.org/mock v0.4.0 go.uber.org/zap v1.27.0 golang.org/x/exp v0.0.0-20240707233637-46b078467d37 diff --git a/go.sum b/go.sum index f666518207..da209b72e3 100644 --- a/go.sum +++ b/go.sum @@ -692,6 +692,8 @@ go.opentelemetry.io/otel/trace v1.24.0 h1:CsKnnL4dUAr/0llH9FKuc698G04IrpWV0MQA/Y go.opentelemetry.io/otel/trace v1.24.0/go.mod h1:HPc3Xr/cOApsBI154IU0OI0HJexz+aw5uPdbs3UCjNU= go.uber.org/atomic v1.6.0/go.mod h1:sABNBOSYdrvTF6hTgEIbc7YasKWGhgEQZyfxyTvoXHQ= go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= +go.uber.org/atomic v1.11.0 h1:ZvwS0R+56ePWxUNi+Atn9dWONBPp/AUETXlHW0DxSjE= +go.uber.org/atomic v1.11.0/go.mod h1:LUxbIzbOniOlMKjJjyPfpl4v+PKK2cNJn91OQbhoJI0= go.uber.org/dig v1.17.1 h1:Tga8Lz8PcYNsWsyHMZ1Vm0OQOUaJNDyvPImgbAu9YSc= go.uber.org/dig v1.17.1/go.mod h1:Us0rSJiThwCv2GteUN0Q7OKvU7n5J4dxZ9JKUXozFdE= go.uber.org/fx v1.22.1 h1:nvvln7mwyT5s1q201YE29V/BFrGor6vMiDNpU/78Mys= From e910e34051eb2701d16197412cbeb01224563be4 Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Tue, 6 Aug 2024 11:03:42 +0200 Subject: [PATCH 18/26] test fix --- activation/activation_errors.go | 6 ++++++ activation/poet_client_test.go | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/activation/activation_errors.go b/activation/activation_errors.go index d9bdcfe7c3..9e5fc948db 100644 --- a/activation/activation_errors.go +++ b/activation/activation_errors.go @@ -60,3 +60,9 @@ func (e *PoetRegistrationMismatchError) Error() string { } return sb.String() } + +func (e *PoetRegistrationMismatchError) Is(target error) bool { + var poetRegistrationMismatchError *PoetRegistrationMismatchError + ok := errors.As(target, &poetRegistrationMismatchError) + return ok +} diff --git a/activation/poet_client_test.go b/activation/poet_client_test.go index ca7606aeb9..73ac9a0283 100644 --- a/activation/poet_client_test.go +++ b/activation/poet_client_test.go @@ -514,6 +514,10 @@ func TestPoetService_CachesPowParams(t *testing.T) { cfg := DefaultPoetConfig() cfg.PowParamsCacheTTL = tc.ttl client := NewMockPoetClient(gomock.NewController(t)) + + client.EXPECT().Info(gomock.Any()).Return(&types.PoetInfo{}, nil) + client.EXPECT().Address().Return("some_address").AnyTimes() + poet := NewPoetServiceWithClient(nil, client, cfg, zaptest.NewLogger(t)) params := PoetPowParams{ From 6d12ab64b18cd9d1dcf40d7b74b5f22592a5d8e0 Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Tue, 6 Aug 2024 17:43:26 +0200 Subject: [PATCH 19/26] review fixes --- activation/activation_errors.go | 3 +-- activation/nipost_test.go | 5 ++++- activation/poet.go | 18 ++++++++---------- go.mod | 1 - go.sum | 2 -- 5 files changed, 13 insertions(+), 16 deletions(-) diff --git a/activation/activation_errors.go b/activation/activation_errors.go index ec72a26afa..8d6960f076 100644 --- a/activation/activation_errors.go +++ b/activation/activation_errors.go @@ -53,6 +53,5 @@ func (e *PoetRegistrationMismatchError) Error() string { func (e *PoetRegistrationMismatchError) As(target error) bool { var poetRegistrationMismatchError *PoetRegistrationMismatchError - ok := errors.As(target, &poetRegistrationMismatchError) - return ok + return errors.As(target, &poetRegistrationMismatchError) } diff --git a/activation/nipost_test.go b/activation/nipost_test.go index 1532ef83dd..898de1fa19 100644 --- a/activation/nipost_test.go +++ b/activation/nipost_test.go @@ -1099,13 +1099,16 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { ) require.NoError(t, err) + poetErr := &PoetRegistrationMismatchError{} _, err = nb.submitPoetChallenges( context.Background(), sig, time.Now().Add(10*time.Second), time.Now().Add(-5*time.Second), // poet round started challengeHash.Bytes()) - require.ErrorIs(t, err, ErrNoRegistrationForGivenPoetFound) + require.ErrorAs(t, err, &poetErr) + require.ElementsMatch(t, []string{poetProverAddr2}, poetErr.registrations) + require.ElementsMatch(t, []string{poetProverAddr}, poetErr.configuredPoets) }) } diff --git a/activation/poet.go b/activation/poet.go index 402761a3f0..4a250ee9f9 100644 --- a/activation/poet.go +++ b/activation/poet.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "go.uber.org/atomic" "io" "net/http" "net/url" @@ -385,8 +384,9 @@ type poetService struct { certifier certifierService certifierInfoCache cachedData[*types.CertifierInfo] + mtx *sync.Mutex expectedPhaseShift time.Duration - fetchedPhaseShift *atomic.Duration + fetchedPhaseShift time.Duration powParamsCache cachedData[*PoetPowParams] } @@ -434,7 +434,7 @@ func NewPoetServiceWithClient( powParamsCache: cachedData[*PoetPowParams]{ttl: cfg.PowParamsCacheTTL}, proofMembers: make(map[string][]types.Hash32, 1), expectedPhaseShift: cfg.PhaseShift, - fetchedPhaseShift: &atomic.Duration{}, + mtx: &sync.Mutex{}, } for _, opt := range opts { opt(service) @@ -444,7 +444,6 @@ func NewPoetServiceWithClient( if err != nil { if errors.Is(err, ErrIncompatiblePhaseShift) { logger.Panic("failed to create poet service", - zap.Error(err), zap.String("poet", client.Address())) } logger.Warn("failed to fetch poet phase shift", @@ -455,7 +454,10 @@ func NewPoetServiceWithClient( } func (c *poetService) verifyPhaseShiftConfiguration(ctx context.Context) error { - if c.fetchedPhaseShift.Load() != 0 { + c.mtx.Lock() + defer c.mtx.Unlock() + + if c.fetchedPhaseShift != 0 { return nil } resp, err := c.client.Info(ctx) @@ -465,7 +467,7 @@ func (c *poetService) verifyPhaseShiftConfiguration(ctx context.Context) error { return ErrIncompatiblePhaseShift } - c.fetchedPhaseShift.Store(resp.PhaseShift) + c.fetchedPhaseShift = resp.PhaseShift return nil } @@ -551,12 +553,8 @@ func (c *poetService) Submit( if err := c.verifyPhaseShiftConfiguration(ctx); err != nil { if errors.Is(err, ErrIncompatiblePhaseShift) { logger.Panic("failed to submit challenge", - zap.Error(err), zap.String("poet", c.client.Address())) } - logger.Warn("failed to submit challenge: couldn't fetch poet phase shift", - zap.Error(err), - zap.String("poet", c.client.Address())) return nil, err } diff --git a/go.mod b/go.mod index 646d143cc7..931e19278b 100644 --- a/go.mod +++ b/go.mod @@ -53,7 +53,6 @@ require ( github.com/stretchr/testify v1.9.0 github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7 github.com/zeebo/blake3 v0.2.3 - go.uber.org/atomic v1.11.0 go.uber.org/mock v0.4.0 go.uber.org/zap v1.27.0 golang.org/x/exp v0.0.0-20240707233637-46b078467d37 diff --git a/go.sum b/go.sum index bf9861fc2f..be27867b00 100644 --- a/go.sum +++ b/go.sum @@ -692,8 +692,6 @@ go.opentelemetry.io/otel/trace v1.24.0 h1:CsKnnL4dUAr/0llH9FKuc698G04IrpWV0MQA/Y go.opentelemetry.io/otel/trace v1.24.0/go.mod h1:HPc3Xr/cOApsBI154IU0OI0HJexz+aw5uPdbs3UCjNU= go.uber.org/atomic v1.6.0/go.mod h1:sABNBOSYdrvTF6hTgEIbc7YasKWGhgEQZyfxyTvoXHQ= go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= -go.uber.org/atomic v1.11.0 h1:ZvwS0R+56ePWxUNi+Atn9dWONBPp/AUETXlHW0DxSjE= -go.uber.org/atomic v1.11.0/go.mod h1:LUxbIzbOniOlMKjJjyPfpl4v+PKK2cNJn91OQbhoJI0= go.uber.org/dig v1.17.1 h1:Tga8Lz8PcYNsWsyHMZ1Vm0OQOUaJNDyvPImgbAu9YSc= go.uber.org/dig v1.17.1/go.mod h1:Us0rSJiThwCv2GteUN0Q7OKvU7n5J4dxZ9JKUXozFdE= go.uber.org/fx v1.22.1 h1:nvvln7mwyT5s1q201YE29V/BFrGor6vMiDNpU/78Mys= From 71bd472c48fb6043b1ebd9de1207fd696289b7e6 Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Tue, 6 Aug 2024 17:52:28 +0200 Subject: [PATCH 20/26] fix lint --- activation/activation_errors.go | 4 ++-- api/grpcserver/v2alpha1/network.go | 1 - api/grpcserver/v2alpha1/network_test.go | 1 - 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/activation/activation_errors.go b/activation/activation_errors.go index 8d6960f076..9d4c062b79 100644 --- a/activation/activation_errors.go +++ b/activation/activation_errors.go @@ -51,7 +51,7 @@ func (e *PoetRegistrationMismatchError) Error() string { return sb.String() } -func (e *PoetRegistrationMismatchError) As(target error) bool { +func (e *PoetRegistrationMismatchError) As(target any) bool { var poetRegistrationMismatchError *PoetRegistrationMismatchError - return errors.As(target, &poetRegistrationMismatchError) + return errors.As(poetRegistrationMismatchError, target) } diff --git a/api/grpcserver/v2alpha1/network.go b/api/grpcserver/v2alpha1/network.go index f9454394cc..813defe4ec 100644 --- a/api/grpcserver/v2alpha1/network.go +++ b/api/grpcserver/v2alpha1/network.go @@ -57,6 +57,5 @@ func (s *NetworkService) Info(context.Context, Hrp: types.NetworkHRP(), EffectiveGenesisLayer: types.GetEffectiveGenesis().Uint32(), LayersPerEpoch: types.GetLayersPerEpoch(), - LabelsPerUnit: s.labelsPerUnit, }, nil } diff --git a/api/grpcserver/v2alpha1/network_test.go b/api/grpcserver/v2alpha1/network_test.go index 0e87e3a7d1..eddeed02c4 100644 --- a/api/grpcserver/v2alpha1/network_test.go +++ b/api/grpcserver/v2alpha1/network_test.go @@ -34,6 +34,5 @@ func TestNetworkService_Info(t *testing.T) { require.Equal(t, types.NetworkHRP(), info.Hrp) require.Equal(t, types.GetEffectiveGenesis().Uint32(), info.EffectiveGenesisLayer) require.Equal(t, types.GetLayersPerEpoch(), info.LayersPerEpoch) - require.Equal(t, c.POST.LabelsPerUnit, info.LabelsPerUnit) }) } From 0ea3dc1edda5e509c14d8cca24f02cdba7c9bcff Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Tue, 6 Aug 2024 18:02:20 +0200 Subject: [PATCH 21/26] fix merge --- api/grpcserver/v2alpha1/network.go | 1 + api/grpcserver/v2alpha1/network_test.go | 1 + 2 files changed, 2 insertions(+) diff --git a/api/grpcserver/v2alpha1/network.go b/api/grpcserver/v2alpha1/network.go index 813defe4ec..f9454394cc 100644 --- a/api/grpcserver/v2alpha1/network.go +++ b/api/grpcserver/v2alpha1/network.go @@ -57,5 +57,6 @@ func (s *NetworkService) Info(context.Context, Hrp: types.NetworkHRP(), EffectiveGenesisLayer: types.GetEffectiveGenesis().Uint32(), LayersPerEpoch: types.GetLayersPerEpoch(), + LabelsPerUnit: s.labelsPerUnit, }, nil } diff --git a/api/grpcserver/v2alpha1/network_test.go b/api/grpcserver/v2alpha1/network_test.go index eddeed02c4..0e87e3a7d1 100644 --- a/api/grpcserver/v2alpha1/network_test.go +++ b/api/grpcserver/v2alpha1/network_test.go @@ -34,5 +34,6 @@ func TestNetworkService_Info(t *testing.T) { require.Equal(t, types.NetworkHRP(), info.Hrp) require.Equal(t, types.GetEffectiveGenesis().Uint32(), info.EffectiveGenesisLayer) require.Equal(t, types.GetLayersPerEpoch(), info.LayersPerEpoch) + require.Equal(t, c.POST.LabelsPerUnit, info.LabelsPerUnit) }) } From 9dc3874462823c4abb755ba199974ed9769fa5d6 Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Tue, 6 Aug 2024 18:05:23 +0200 Subject: [PATCH 22/26] fix mocks --- activation/mocks.go | 40 ---------------------------------------- api/grpcserver/mocks.go | 22 +++++++++++----------- 2 files changed, 11 insertions(+), 51 deletions(-) diff --git a/activation/mocks.go b/activation/mocks.go index dcc3d9cc9c..dd1c6f637d 100644 --- a/activation/mocks.go +++ b/activation/mocks.go @@ -20,7 +20,6 @@ import ( signing "github.com/spacemeshos/go-spacemesh/signing" certifier "github.com/spacemeshos/go-spacemesh/sql/localsql/certifier" nipost "github.com/spacemeshos/go-spacemesh/sql/localsql/nipost" - v1 "github.com/spacemeshos/poet/release/proto/go/rpc/api/v1" shared "github.com/spacemeshos/post/shared" gomock "go.uber.org/mock/gomock" ) @@ -1733,45 +1732,6 @@ func (c *MockPoetServiceCertifyCall) DoAndReturn(f func(context.Context, types.N return c } -// Info mocks base method. -func (m *MockPoetService) Info(ctx context.Context) (*v1.InfoResponse, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Info", ctx) - ret0, _ := ret[0].(*v1.InfoResponse) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// Info indicates an expected call of Info. -func (mr *MockPoetServiceMockRecorder) Info(ctx any) *MockPoetServiceInfoCall { - mr.mock.ctrl.T.Helper() - call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Info", reflect.TypeOf((*MockPoetService)(nil).Info), ctx) - return &MockPoetServiceInfoCall{Call: call} -} - -// MockPoetServiceInfoCall wrap *gomock.Call -type MockPoetServiceInfoCall struct { - *gomock.Call -} - -// Return rewrite *gomock.Call.Return -func (c *MockPoetServiceInfoCall) Return(arg0 *v1.InfoResponse, arg1 error) *MockPoetServiceInfoCall { - c.Call = c.Call.Return(arg0, arg1) - return c -} - -// Do rewrite *gomock.Call.Do -func (c *MockPoetServiceInfoCall) Do(f func(context.Context) (*v1.InfoResponse, error)) *MockPoetServiceInfoCall { - c.Call = c.Call.Do(f) - return c -} - -// DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockPoetServiceInfoCall) DoAndReturn(f func(context.Context) (*v1.InfoResponse, error)) *MockPoetServiceInfoCall { - c.Call = c.Call.DoAndReturn(f) - return c -} - // Proof mocks base method. func (m *MockPoetService) Proof(ctx context.Context, roundID string) (*types.PoetProof, []types.Hash32, error) { m.ctrl.T.Helper() diff --git a/api/grpcserver/mocks.go b/api/grpcserver/mocks.go index 7812313f72..ab849fa95c 100644 --- a/api/grpcserver/mocks.go +++ b/api/grpcserver/mocks.go @@ -15,7 +15,7 @@ import ( time "time" network "github.com/libp2p/go-libp2p/core/network" - go_multiaddr "github.com/multiformats/go-multiaddr" + multiaddr "github.com/multiformats/go-multiaddr" activation "github.com/spacemeshos/go-spacemesh/activation" types "github.com/spacemeshos/go-spacemesh/common/types" wire "github.com/spacemeshos/go-spacemesh/malfeasance/wire" @@ -126,10 +126,10 @@ func (c *MocknetworkInfoIDCall) DoAndReturn(f func() p2p.Peer) *MocknetworkInfoI } // KnownAddresses mocks base method. -func (m *MocknetworkInfo) KnownAddresses() []go_multiaddr.Multiaddr { +func (m *MocknetworkInfo) KnownAddresses() []multiaddr.Multiaddr { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "KnownAddresses") - ret0, _ := ret[0].([]go_multiaddr.Multiaddr) + ret0, _ := ret[0].([]multiaddr.Multiaddr) return ret0 } @@ -146,28 +146,28 @@ type MocknetworkInfoKnownAddressesCall struct { } // Return rewrite *gomock.Call.Return -func (c *MocknetworkInfoKnownAddressesCall) Return(arg0 []go_multiaddr.Multiaddr) *MocknetworkInfoKnownAddressesCall { +func (c *MocknetworkInfoKnownAddressesCall) Return(arg0 []multiaddr.Multiaddr) *MocknetworkInfoKnownAddressesCall { c.Call = c.Call.Return(arg0) return c } // Do rewrite *gomock.Call.Do -func (c *MocknetworkInfoKnownAddressesCall) Do(f func() []go_multiaddr.Multiaddr) *MocknetworkInfoKnownAddressesCall { +func (c *MocknetworkInfoKnownAddressesCall) Do(f func() []multiaddr.Multiaddr) *MocknetworkInfoKnownAddressesCall { c.Call = c.Call.Do(f) return c } // DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MocknetworkInfoKnownAddressesCall) DoAndReturn(f func() []go_multiaddr.Multiaddr) *MocknetworkInfoKnownAddressesCall { +func (c *MocknetworkInfoKnownAddressesCall) DoAndReturn(f func() []multiaddr.Multiaddr) *MocknetworkInfoKnownAddressesCall { c.Call = c.Call.DoAndReturn(f) return c } // ListenAddresses mocks base method. -func (m *MocknetworkInfo) ListenAddresses() []go_multiaddr.Multiaddr { +func (m *MocknetworkInfo) ListenAddresses() []multiaddr.Multiaddr { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ListenAddresses") - ret0, _ := ret[0].([]go_multiaddr.Multiaddr) + ret0, _ := ret[0].([]multiaddr.Multiaddr) return ret0 } @@ -184,19 +184,19 @@ type MocknetworkInfoListenAddressesCall struct { } // Return rewrite *gomock.Call.Return -func (c *MocknetworkInfoListenAddressesCall) Return(arg0 []go_multiaddr.Multiaddr) *MocknetworkInfoListenAddressesCall { +func (c *MocknetworkInfoListenAddressesCall) Return(arg0 []multiaddr.Multiaddr) *MocknetworkInfoListenAddressesCall { c.Call = c.Call.Return(arg0) return c } // Do rewrite *gomock.Call.Do -func (c *MocknetworkInfoListenAddressesCall) Do(f func() []go_multiaddr.Multiaddr) *MocknetworkInfoListenAddressesCall { +func (c *MocknetworkInfoListenAddressesCall) Do(f func() []multiaddr.Multiaddr) *MocknetworkInfoListenAddressesCall { c.Call = c.Call.Do(f) return c } // DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MocknetworkInfoListenAddressesCall) DoAndReturn(f func() []go_multiaddr.Multiaddr) *MocknetworkInfoListenAddressesCall { +func (c *MocknetworkInfoListenAddressesCall) DoAndReturn(f func() []multiaddr.Multiaddr) *MocknetworkInfoListenAddressesCall { c.Call = c.Call.DoAndReturn(f) return c } From 4c44a4929562d0c3e714272359a68f05e958275b Mon Sep 17 00:00:00 2001 From: Matthias <5011972+fasmat@users.noreply.github.com> Date: Tue, 6 Aug 2024 17:07:24 +0000 Subject: [PATCH 23/26] Fix tests --- activation/activation_errors.go | 11 +++----- activation/nipost.go | 29 ++++++++++----------- activation/nipost_test.go | 12 ++++----- activation/poet.go | 27 +++++++++---------- activation/poet_client_test.go | 46 ++++++++++++++------------------- 5 files changed, 57 insertions(+), 68 deletions(-) diff --git a/activation/activation_errors.go b/activation/activation_errors.go index 9d4c062b79..e2e2215e5f 100644 --- a/activation/activation_errors.go +++ b/activation/activation_errors.go @@ -22,7 +22,7 @@ type PoetSvcUnstableError struct { source error } -func (e *PoetSvcUnstableError) Error() string { +func (e PoetSvcUnstableError) Error() string { return fmt.Sprintf("poet service is unstable: %s (%v)", e.msg, e.source) } @@ -33,7 +33,7 @@ type PoetRegistrationMismatchError struct { configuredPoets []string } -func (e *PoetRegistrationMismatchError) Error() string { +func (e PoetRegistrationMismatchError) Error() string { var sb strings.Builder sb.WriteString("builder: none of configured poets matches the existing registrations.\n") sb.WriteString("registrations:\n") @@ -42,7 +42,7 @@ func (e *PoetRegistrationMismatchError) Error() string { sb.WriteString(r) sb.WriteString("\n") } - sb.WriteString("\n configured poets:\n") + sb.WriteString("\nconfigured poets:\n") for _, p := range e.configuredPoets { sb.WriteString("\t") sb.WriteString(p) @@ -50,8 +50,3 @@ func (e *PoetRegistrationMismatchError) Error() string { } return sb.String() } - -func (e *PoetRegistrationMismatchError) As(target any) bool { - var poetRegistrationMismatchError *PoetRegistrationMismatchError - return errors.As(poetRegistrationMismatchError, target) -} diff --git a/activation/nipost.go b/activation/nipost.go index 9ba89dfc01..ec9f8ac9c5 100644 --- a/activation/nipost.go +++ b/activation/nipost.go @@ -231,22 +231,23 @@ func (nb *NIPostBuilder) BuildNIPost( // Phase 0: Submit challenge to PoET services. // Deadline: start of PoET round: we will not accept registrations after that - submitedRegistrations, err := nb.submitPoetChallenges( + submittedRegistrations, err := nb.submitPoetChallenges( ctx, signer, poetProofDeadline, - poetRoundStart, challenge.Bytes()) - + poetRoundStart, challenge.Bytes(), + ) regErr := &PoetRegistrationMismatchError{} - if err != nil { - if errors.As(err, ®Err) { - logger.Fatal( - "None of the poets listed in the config matches the existing registrations. "+ - "Verify your config and local database state.", - zap.Strings("registrations", regErr.registrations), - zap.Strings("configured_poets", regErr.configuredPoets), - ) - } + switch { + case errors.As(err, ®Err): + logger.Fatal( + "None of the poets listed in the config matches the existing registrations. "+ + "Verify your config and local database state.", + zap.Strings("registrations", regErr.registrations), + zap.Strings("configured_poets", regErr.configuredPoets), + ) + return nil, err + case err != nil: return nil, fmt.Errorf("submitting to poets: %w", err) } @@ -270,7 +271,7 @@ func (nb *NIPostBuilder) BuildNIPost( } events.EmitPoetWaitProof(signer.NodeID(), postChallenge.PublishEpoch, curPoetRoundEnd) - poetProofRef, membership, err = nb.getBestProof(ctx, signer.NodeID(), challenge, submitedRegistrations) + poetProofRef, membership, err = nb.getBestProof(ctx, signer.NodeID(), challenge, submittedRegistrations) if err != nil { return nil, &PoetSvcUnstableError{msg: "getBestProof failed", source: err} } @@ -501,8 +502,6 @@ func (nb *NIPostBuilder) submitPoetChallenges( if len(existingRegistrations) == 0 { if curPoetRoundStartDeadline.Before(time.Now()) { - nb.logger.Warn("failed to register in poets on time. ATX challenge expires", - log.ZShortStringer("smesherID", nodeID)) return nil, ErrATXChallengeExpired } return nil, &PoetSvcUnstableError{msg: "failed to submit challenge to any PoET", source: ctx.Err()} diff --git a/activation/nipost_test.go b/activation/nipost_test.go index 898de1fa19..b1f2845db2 100644 --- a/activation/nipost_test.go +++ b/activation/nipost_test.go @@ -761,8 +761,7 @@ func TestNIPSTBuilder_PoetUnstable(t *testing.T) { challenge, &types.NIPostChallenge{PublishEpoch: postGenesisEpoch + 2}, ) - poetErr := &PoetSvcUnstableError{} - require.ErrorAs(t, err, &poetErr) + require.ErrorIs(t, err, ErrATXChallengeExpired) require.Nil(t, nipst) }) t.Run("GetProof fails", func(t *testing.T) { @@ -1099,16 +1098,17 @@ func TestNIPoSTBuilder_PoETConfigChange(t *testing.T) { ) require.NoError(t, err) - poetErr := &PoetRegistrationMismatchError{} _, err = nb.submitPoetChallenges( context.Background(), sig, time.Now().Add(10*time.Second), time.Now().Add(-5*time.Second), // poet round started - challengeHash.Bytes()) + challengeHash.Bytes(), + ) + poetErr := &PoetRegistrationMismatchError{} require.ErrorAs(t, err, &poetErr) - require.ElementsMatch(t, []string{poetProverAddr2}, poetErr.registrations) - require.ElementsMatch(t, []string{poetProverAddr}, poetErr.configuredPoets) + require.ElementsMatch(t, poetErr.configuredPoets, []string{poetProverAddr}) + require.ElementsMatch(t, poetErr.registrations, []string{poetProverAddr2}) }) } diff --git a/activation/poet.go b/activation/poet.go index 4a250ee9f9..af9f1f09a1 100644 --- a/activation/poet.go +++ b/activation/poet.go @@ -384,7 +384,7 @@ type poetService struct { certifier certifierService certifierInfoCache cachedData[*types.CertifierInfo] - mtx *sync.Mutex + mtx sync.Mutex expectedPhaseShift time.Duration fetchedPhaseShift time.Duration powParamsCache cachedData[*PoetPowParams] @@ -434,21 +434,21 @@ func NewPoetServiceWithClient( powParamsCache: cachedData[*PoetPowParams]{ttl: cfg.PowParamsCacheTTL}, proofMembers: make(map[string][]types.Hash32, 1), expectedPhaseShift: cfg.PhaseShift, - mtx: &sync.Mutex{}, } for _, opt := range opts { opt(service) } err := service.verifyPhaseShiftConfiguration(context.Background()) - if err != nil { - if errors.Is(err, ErrIncompatiblePhaseShift) { - logger.Panic("failed to create poet service", - zap.String("poet", client.Address())) - } + switch { + case errors.Is(err, ErrIncompatiblePhaseShift): + logger.Fatal("failed to create poet service", zap.String("poet", client.Address())) + return nil + case err != nil: logger.Warn("failed to fetch poet phase shift", + zap.String("poet", client.Address()), zap.Error(err), - zap.String("poet", client.Address())) + ) } return service } @@ -550,11 +550,12 @@ func (c *poetService) Submit( log.ZShortStringer("smesherID", nodeID), ) - if err := c.verifyPhaseShiftConfiguration(ctx); err != nil { - if errors.Is(err, ErrIncompatiblePhaseShift) { - logger.Panic("failed to submit challenge", - zap.String("poet", c.client.Address())) - } + err := c.verifyPhaseShiftConfiguration(ctx) + switch { + case errors.Is(err, ErrIncompatiblePhaseShift): + logger.Fatal("failed to submit challenge", zap.String("poet", c.client.Address())) + return nil, err + case err != nil: return nil, err } diff --git a/activation/poet_client_test.go b/activation/poet_client_test.go index 73ac9a0283..2ed127a069 100644 --- a/activation/poet_client_test.go +++ b/activation/poet_client_test.go @@ -16,6 +16,7 @@ import ( "github.com/spacemeshos/poet/server" "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" + "go.uber.org/zap" "go.uber.org/zap/zaptest" "golang.org/x/sync/errgroup" "google.golang.org/protobuf/encoding/protojson" @@ -199,26 +200,27 @@ func TestPoetClient_CachesProof(t *testing.T) { func TestPoetClient_QueryProofTimeout(t *testing.T) { t.Parallel() - block := make(chan struct{}) - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - <-block - })) - defer ts.Close() - defer close(block) - - server := types.PoetServer{ - Address: ts.URL, - Pubkey: types.NewBase64Enc([]byte("pubkey")), - } cfg := PoetConfig{ RequestTimeout: time.Millisecond * 100, + PhaseShift: 10 * time.Second, } - client, err := NewHTTPPoetClient(server, cfg, withCustomHttpClient(ts.Client())) - require.NoError(t, err) + client := NewMockPoetClient(gomock.NewController(t)) + // first call on info returns the expected value + client.EXPECT().Info(gomock.Any()).Return(&types.PoetInfo{ + PhaseShift: cfg.PhaseShift, + }, nil) poet := NewPoetServiceWithClient(nil, client, cfg, zaptest.NewLogger(t)) + // any additional call on Info will block + client.EXPECT().Proof(gomock.Any(), "1").DoAndReturn( + func(ctx context.Context, _ string) (*types.PoetProofMessage, []types.Hash32, error) { + <-ctx.Done() + return nil, nil, ctx.Err() + }, + ).AnyTimes() + start := time.Now() - eg := errgroup.Group{} + var eg errgroup.Group for range 50 { eg.Go(func() error { _, _, err := poet.Proof(context.Background(), "1") @@ -578,12 +580,8 @@ func TestPoetService_FetchPoetPhaseShift(t *testing.T) { PhaseShift: phaseShift * 2, }, nil) - defer func() { - if r := recover(); r == nil { - t.Errorf("Panic is expected") - } - }() - NewPoetServiceWithClient(nil, client, cfg, zaptest.NewLogger(t)) + log := zaptest.NewLogger(t).WithOptions(zap.WithFatalHook(calledFatal(t))) + NewPoetServiceWithClient(nil, client, cfg, log) }) t.Run("fetch phase shift before submitting challenge: success", @@ -640,7 +638,8 @@ func TestPoetService_FetchPoetPhaseShift(t *testing.T) { client.EXPECT().Address().Return("some_addr").AnyTimes() client.EXPECT().Info(gomock.Any()).Return(nil, errors.New("some error")) - poet := NewPoetServiceWithClient(nil, client, cfg, zaptest.NewLogger(t)) + log := zaptest.NewLogger(t).WithOptions(zap.WithFatalHook(calledFatal(t))) + poet := NewPoetServiceWithClient(nil, client, cfg, log) sig, err := signing.NewEdSigner() require.NoError(t, err) @@ -648,11 +647,6 @@ func TestPoetService_FetchPoetPhaseShift(t *testing.T) { PhaseShift: phaseShift * 2, }, nil) - defer func() { - if r := recover(); r == nil { - t.Errorf("Panic is expected") - } - }() poet.Submit(context.Background(), time.Time{}, nil, nil, types.RandomEdSignature(), sig.NodeID()) }) } From 15ebc7b1881eaba3c6ac5f6c8a03126a2df10fc0 Mon Sep 17 00:00:00 2001 From: Matthias <5011972+fasmat@users.noreply.github.com> Date: Tue, 6 Aug 2024 17:33:58 +0000 Subject: [PATCH 24/26] Fix e2e tests --- activation/e2e/atx_merge_test.go | 2 +- activation/e2e/builds_atx_v2_test.go | 2 +- activation/e2e/checkpoint_merged_test.go | 2 +- activation/e2e/checkpoint_test.go | 2 +- activation/e2e/nipost_test.go | 4 ++-- activation/e2e/poet_client.go | 13 +++++++++---- activation/e2e/validation_test.go | 2 +- 7 files changed, 16 insertions(+), 11 deletions(-) diff --git a/activation/e2e/atx_merge_test.go b/activation/e2e/atx_merge_test.go index fda7593427..49f3f6d0a0 100644 --- a/activation/e2e/atx_merge_test.go +++ b/activation/e2e/atx_merge_test.go @@ -246,7 +246,7 @@ func Test_MarryAndMerge(t *testing.T) { GracePeriod: epoch / 4, } - client := ae2e.NewTestPoetClient(2) + client := ae2e.NewTestPoetClient(2, poetCfg) poetSvc := activation.NewPoetServiceWithClient(poetDb, client, poetCfg, logger) clock, err := timesync.NewClock( diff --git a/activation/e2e/builds_atx_v2_test.go b/activation/e2e/builds_atx_v2_test.go index 9c56b565be..2baf326060 100644 --- a/activation/e2e/builds_atx_v2_test.go +++ b/activation/e2e/builds_atx_v2_test.go @@ -92,7 +92,7 @@ func TestBuilder_SwitchesToBuildV2(t *testing.T) { require.NoError(t, err) t.Cleanup(clock.Close) - client := ae2e.NewTestPoetClient(1) + client := ae2e.NewTestPoetClient(1, poetCfg) poetClient := activation.NewPoetServiceWithClient(poetDb, client, poetCfg, logger) localDB := localsql.InMemory() diff --git a/activation/e2e/checkpoint_merged_test.go b/activation/e2e/checkpoint_merged_test.go index 3984d926f8..f06c319f3d 100644 --- a/activation/e2e/checkpoint_merged_test.go +++ b/activation/e2e/checkpoint_merged_test.go @@ -81,7 +81,7 @@ func Test_CheckpointAfterMerge(t *testing.T) { GracePeriod: epoch / 4, } - client := ae2e.NewTestPoetClient(2) + client := ae2e.NewTestPoetClient(2, poetCfg) poetSvc := activation.NewPoetServiceWithClient(poetDb, client, poetCfg, logger) clock, err := timesync.NewClock( diff --git a/activation/e2e/checkpoint_test.go b/activation/e2e/checkpoint_test.go index 048469b2ff..4e825d944c 100644 --- a/activation/e2e/checkpoint_test.go +++ b/activation/e2e/checkpoint_test.go @@ -71,7 +71,7 @@ func TestCheckpoint_PublishingSoloATXs(t *testing.T) { CycleGap: 3 * epoch / 4, GracePeriod: epoch / 4, } - client := ae2e.NewTestPoetClient(1) + client := ae2e.NewTestPoetClient(1, poetCfg) poetService := activation.NewPoetServiceWithClient(poetDb, client, poetCfg, logger) // ensure that genesis aligns with layer timings diff --git a/activation/e2e/nipost_test.go b/activation/e2e/nipost_test.go index 2a135e3256..74a9e29b57 100644 --- a/activation/e2e/nipost_test.go +++ b/activation/e2e/nipost_test.go @@ -198,7 +198,7 @@ func TestNIPostBuilderWithClients(t *testing.T) { err = nipost.AddPost(localDb, sig.NodeID(), *fullPost(post, info, shared.ZeroChallenge)) require.NoError(t, err) - client := ae2e.NewTestPoetClient(1) + client := ae2e.NewTestPoetClient(1, poetCfg) poetService := activation.NewPoetServiceWithClient(poetDb, client, poetCfg, logger) localDB := localsql.InMemory() @@ -272,7 +272,7 @@ func Test_NIPostBuilderWithMultipleClients(t *testing.T) { } poetDb := activation.NewPoetDb(db, logger.Named("poetDb")) - client := ae2e.NewTestPoetClient(len(signers)) + client := ae2e.NewTestPoetClient(len(signers), poetCfg) poetService := activation.NewPoetServiceWithClient(poetDb, client, poetCfg, logger) mclock := activation.NewMocklayerClock(ctrl) diff --git a/activation/e2e/poet_client.go b/activation/e2e/poet_client.go index 7311200bf9..01fee564ec 100644 --- a/activation/e2e/poet_client.go +++ b/activation/e2e/poet_client.go @@ -19,15 +19,17 @@ import ( ) type TestPoet struct { - mu sync.Mutex - round int + mu sync.Mutex + round int + poetCfg activation.PoetConfig expectedMembers int registrations chan []byte } -func NewTestPoetClient(expectedMembers int) *TestPoet { +func NewTestPoetClient(expectedMembers int, poetCfg activation.PoetConfig) *TestPoet { return &TestPoet{ + poetCfg: poetCfg, expectedMembers: expectedMembers, registrations: make(chan []byte, expectedMembers), } @@ -70,7 +72,10 @@ func (p *TestPoet) CertifierInfo(ctx context.Context) (*types.CertifierInfo, err } func (p *TestPoet) Info(ctx context.Context) (*types.PoetInfo, error) { - return nil, errors.New("Info: not supported") + return &types.PoetInfo{ + PhaseShift: p.poetCfg.PhaseShift, + CycleGap: p.poetCfg.CycleGap, + }, nil } // Build a proof. diff --git a/activation/e2e/validation_test.go b/activation/e2e/validation_test.go index 124a5762f3..dde1ff6162 100644 --- a/activation/e2e/validation_test.go +++ b/activation/e2e/validation_test.go @@ -51,7 +51,7 @@ func TestValidator_Validate(t *testing.T) { } poetDb := activation.NewPoetDb(sql.InMemory(), logger.Named("poetDb")) - client := ae2e.NewTestPoetClient(1) + client := ae2e.NewTestPoetClient(1, poetCfg) poetService := activation.NewPoetServiceWithClient(poetDb, client, poetCfg, logger) mclock := activation.NewMocklayerClock(ctrl) From 2206f3725de3029675eae64beaf236360f732542 Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Wed, 7 Aug 2024 16:55:54 +0200 Subject: [PATCH 25/26] fix test --- node/node_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/node_test.go b/node/node_test.go index 3a8f5a2407..ddca024552 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -997,7 +997,7 @@ func TestAdminEvents(t *testing.T) { select { case <-app.Started(): - case <-time.After(10 * time.Second): + case <-time.After(15 * time.Second): require.Fail(t, "app did not start in time") } From f9af85a5f42959905a8e6af8a2111fe07d5d79b8 Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Wed, 7 Aug 2024 16:57:41 +0200 Subject: [PATCH 26/26] fix test2 --- node/node_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/node_test.go b/node/node_test.go index ddca024552..274ed57ca7 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -1090,7 +1090,7 @@ func TestAdminEvents_MultiSmesher(t *testing.T) { select { case <-app.Started(): - case <-time.After(10 * time.Second): + case <-time.After(15 * time.Second): require.Fail(t, "app did not start in time") }