Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - Fix/6041 bug #6053

Closed
wants to merge 25 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 28 additions & 19 deletions activation/activation.go
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,8 @@
poetStartsAt := b.poetRoundStart(current)

metrics.PublishOntimeWindowLatency.Observe(until.Seconds())
wait := buildNipostChallengeStartDeadline(poetStartsAt, b.poetCfg.GracePeriod)

wait := poetStartsAt.Add(-b.poetCfg.GracePeriod)
if time.Until(wait) > 0 {
logger.Info("paused building NiPoST challenge. Waiting until closer to poet start to get a better posATX",
zap.Duration("till poet round", until),
Expand All @@ -563,11 +564,11 @@
}

if b.poetCfg.PositioningATXSelectionTimeout > 0 {
var cancel context.CancelFunc

Check warning on line 567 in activation/activation.go

View check run for this annotation

Codecov / codecov/patch

activation/activation.go#L567

Added line #L567 was not covered by tests

deadline := poetStartsAt.Add(-b.poetCfg.GracePeriod).Add(b.poetCfg.PositioningATXSelectionTimeout)
ctx, cancel = context.WithDeadline(ctx, deadline)
defer cancel()

Check warning on line 571 in activation/activation.go

View check run for this annotation

Codecov / codecov/patch

activation/activation.go#L569-L571

Added lines #L569 - L571 were not covered by tests
}

prevAtx, err = b.GetPrevAtx(nodeID)
Expand Down Expand Up @@ -856,7 +857,6 @@
ctx context.Context,
nodeID types.NodeID,
publish types.EpochID,
previous *types.ActivationTx,
) (types.ATXID, error) {
logger := b.logger.With(log.ZShortStringer("smesherID", nodeID), zap.Uint32("publish epoch", publish.Uint32()))

Expand Down Expand Up @@ -890,13 +890,8 @@
VerifyChainOpts.PrioritizeCall(),
)
if err != nil {
if previous != nil {
id = previous.ID()
logger.Info("search failed - using previous atx as positioning atx", zap.Error(err))
} else {
id = b.conf.GoldenATXID
logger.Info("search failed - using golden atx as positioning atx", zap.Error(err))
}
logger.Info("search failed - using golden atx as positioning atx", zap.Error(err))
id = b.conf.GoldenATXID
}

b.posAtxFinder.found = &struct {
Expand All @@ -916,12 +911,23 @@
publish types.EpochID,
previous *types.ActivationTx,
) (types.ATXID, error) {
id, err := b.searchPositioningAtx(ctx, nodeID, publish, previous)
id, err := b.searchPositioningAtx(ctx, nodeID, publish)
if err != nil {
return types.EmptyATXID, err
}

if previous != nil && id == previous.ID() {
b.logger.Info("found candidate positioning atx",
log.ZShortStringer("id", id),
log.ZShortStringer("smesherID", nodeID),
)

if previous == nil && id == b.conf.GoldenATXID {
b.logger.Info("selected golden atx as positioning atx", log.ZShortStringer("smesherID", nodeID))
return id, nil
}

if previous != nil && id == b.conf.GoldenATXID {
id = previous.ID()
b.logger.Info("selected previous as positioning atx",
log.ZShortStringer("id", id),
log.ZShortStringer("smesherID", nodeID),
Expand All @@ -930,10 +936,17 @@
}

candidate, err := atxs.Get(b.db, id)
if err == nil {
if previous != nil && previous.TickHeight() >= candidate.TickHeight() {
id = previous.ID()
}
if err != nil {
return types.EmptyATXID, fmt.Errorf("get candidate pos ATX %s: %w", id.ShortString(), err)

Check warning on line 940 in activation/activation.go

View check run for this annotation

Codecov / codecov/patch

activation/activation.go#L940

Added line #L940 was not covered by tests
}

if previous != nil && previous.TickHeight() >= candidate.TickHeight() {
id = previous.ID()
b.logger.Info("selected previous as positioning atx",
log.ZShortStringer("id", id),
log.ZShortStringer("smesherID", nodeID),
)
return id, nil
}
fasmat marked this conversation as resolved.
Show resolved Hide resolved

b.logger.Info("selected positioning atx", log.ZShortStringer("id", id), log.ZShortStringer("smesherID", nodeID))
Expand Down Expand Up @@ -962,10 +975,6 @@
return nil
}

func buildNipostChallengeStartDeadline(roundStart time.Time, gracePeriod time.Duration) time.Time {
return roundStart.Add(-gracePeriod)
}

func (b *Builder) version(publish types.EpochID) types.AtxVersion {
version := types.AtxV1
for _, v := range b.versions {
Expand Down
36 changes: 4 additions & 32 deletions activation/activation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1292,10 +1292,13 @@ func TestWaitPositioningAtx(t *testing.T) {
tab.mnipost.EXPECT().
BuildNIPost(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
Return(&nipost.NIPostState{}, nil)

closed := make(chan struct{})
close(closed)

tab.mclock.EXPECT().AwaitLayer(types.EpochID(1).FirstLayer()).Return(closed).AnyTimes()
tab.mclock.EXPECT().AwaitLayer(types.EpochID(2).FirstLayer()).Return(closed).AnyTimes()

tab.mpub.EXPECT().Publish(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(
func(_ context.Context, _ string, got []byte) error {
var atx wire.ActivationTxV1
Expand Down Expand Up @@ -1328,37 +1331,6 @@ func TestWaitPositioningAtx(t *testing.T) {
}
}

func TestWaitingToBuildNipostChallengeWithJitter(t *testing.T) {
t.Run("before grace period", func(t *testing.T) {
// ┌──grace period─┐
// │ │
// ───▲─────|───────────────|----> time
// │ └ └round start
// now
deadline := buildNipostChallengeStartDeadline(time.Now().Add(2*time.Hour), time.Hour)
require.LessOrEqual(t, deadline, time.Now().Add(time.Hour))
})
t.Run("after grace period", func(t *testing.T) {
// ┌──grace period─┐
// │ │
// ─────────|──▲────────────|----> time
// │ └round start
// now

deadline := buildNipostChallengeStartDeadline(time.Now().Add(time.Hour-time.Second*10), time.Hour)
require.LessOrEqual(t, deadline, time.Now().Add(-time.Second*10))
})
t.Run("after jitter max value", func(t *testing.T) {
// ┌──grace period──┐
// │ │
// ─────────|──────|──▲──────|----> time
// └jitter| │ └round start
// now
deadline := buildNipostChallengeStartDeadline(time.Now().Add(time.Hour-time.Second*37), time.Hour)
require.Less(t, deadline, time.Now())
})
}

// Test if GetPositioningAtx disregards ATXs with invalid POST in their chain.
// It should pick an ATX with valid POST even though it's a lower height.
func TestGetPositioningAtxPicksAtxWithValidChain(t *testing.T) {
Expand Down Expand Up @@ -1454,7 +1426,7 @@ func TestGetPositioningAtx(t *testing.T) {
prev.SetID(types.RandomATXID())

tab.mValidator.EXPECT().VerifyChain(gomock.Any(), atxInDb.ID(), tab.goldenATXID, gomock.Any())
found, err := tab.searchPositioningAtx(context.Background(), types.EmptyNodeID, 99, prev)
found, err := tab.searchPositioningAtx(context.Background(), types.EmptyNodeID, 99)
require.NoError(t, err)
require.Equal(t, atxInDb.ID(), found)

Expand Down
Loading