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 5 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
33 changes: 17 additions & 16 deletions activation/activation.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,6 @@

const (
defaultPoetRetryInterval = 5 * time.Second

// Jitter added to the wait time before building a nipost challenge.
// It is expressed as % of poet grace period which translates to:
// mainnet (grace period 1h) -> 36s
// systest (grace period 10s) -> 0.1s
maxNipostChallengeBuildJitter = 1.0
)

// Config defines configuration for Builder.
Expand Down Expand Up @@ -549,8 +543,11 @@
until = time.Until(b.poetRoundStart(current))
}
publish := current + 1

poetStartsAt := b.poetRoundStart(current)

metrics.PublishOntimeWindowLatency.Observe(until.Seconds())
wait := buildNipostChallengeStartDeadline(b.poetRoundStart(current), b.poetCfg.GracePeriod)
wait := buildNipostChallengeStartDeadline(poetStartsAt, 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 @@ -565,13 +562,12 @@
}
}

poetStartsAt := b.poetRoundStart(current)
if b.poetCfg.PositioningATXSelectionTimeout > 0 {
var cancel context.CancelFunc

Check warning on line 566 in activation/activation.go

View check run for this annotation

Codecov / codecov/patch

activation/activation.go#L566

Added line #L566 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 570 in activation/activation.go

View check run for this annotation

Codecov / codecov/patch

activation/activation.go#L568-L570

Added lines #L568 - L570 were not covered by tests
}

prevAtx, err = b.GetPrevAtx(nodeID)
Expand Down Expand Up @@ -925,11 +921,18 @@
return types.EmptyATXID, err
}

if previous != nil && id != previous.ID() {
if candidate, err := atxs.Get(b.db, id); err == nil {
if previous.TickHeight() >= candidate.TickHeight() {
id = previous.ID()
}
if previous != nil && id == previous.ID() {
b.logger.Info("selected previous as positioning atx",
log.ZShortStringer("id", id),
log.ZShortStringer("smesherID", nodeID),
)
return id, nil
}

candidate, err := atxs.Get(b.db, id)
if err == nil {
if previous != nil && previous.TickHeight() >= candidate.TickHeight() {
fasmat marked this conversation as resolved.
Show resolved Hide resolved
id = previous.ID()
}
}

Expand Down Expand Up @@ -987,11 +990,9 @@
// iterate trough epochs, to get first valid, not malicious ATX with the biggest height
atxdata.IterateHighTicksInEpoch(publish+1, func(id types.ATXID) (contSearch bool) {
logger.Info("found candidate for high-tick atx", log.ZShortStringer("id", id))

if ctx.Err() != nil {
return false
}

// verify ATX-candidate by getting their dependencies (previous Atx, positioning ATX etc.)
// and verifying PoST for every dependency
if err := validator.VerifyChain(ctx, id, goldenATXID, opts...); err != nil {
Expand All @@ -1003,7 +1004,7 @@
})

if ctx.Err() != nil {
return types.ATXID{}, ctx.Err()
return types.ATXID{}, ErrNotFound
}
fasmat marked this conversation as resolved.
Show resolved Hide resolved

if found == nil {
Expand Down
26 changes: 12 additions & 14 deletions activation/activation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1330,25 +1330,23 @@ func TestWaitPositioningAtx(t *testing.T) {

func TestWaitingToBuildNipostChallengeWithJitter(t *testing.T) {
poszu marked this conversation as resolved.
Show resolved Hide resolved
t.Run("before grace period", func(t *testing.T) {
// ┌──grace period─
// │
// ───▲─────|──────|─────────|----> time
// │ └jitter| └round start
// ┌──grace period─┐
// │ │
// ───▲─────|───────────────|----> time
// │ └ └round start
// now
deadline := buildNipostChallengeStartDeadline(time.Now().Add(2*time.Hour), time.Hour)
require.Greater(t, deadline, time.Now().Add(time.Hour))
require.LessOrEqual(t, deadline, time.Now().Add(time.Hour+time.Second*36))
require.LessOrEqual(t, deadline, time.Now().Add(time.Hour))
})
t.Run("after grace period, within max jitter value", func(t *testing.T) {
// ┌──grace period─
// │
// ─────────|──▲────|────────|----> time
// └ji│tter| └round start
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.GreaterOrEqual(t, deadline, time.Now().Add(-time.Second*10))
// jitter is 1% = 36s for 1h grace period
require.LessOrEqual(t, deadline, time.Now().Add(time.Second*(36-10)))
require.LessOrEqual(t, deadline, time.Now().Add(-time.Second*10))
})
t.Run("after jitter max value", func(t *testing.T) {
// ┌──grace period──┐
Expand Down
5 changes: 4 additions & 1 deletion activation/e2e/certifier_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,10 @@ func (c *testCertifier) certify(w http.ResponseWriter, r *http.Request) {
NumUnits: req.Metadata.NumUnits,
LabelsPerUnit: c.cfg.LabelsPerUnit,
}
if err := c.postVerifier.Verify(context.Background(), proof, metadata, c.opts...); err != nil {
if err := c.postVerifier.Verify(
context.Background(),
proof, metadata,
activation.WithVerifierOptions(c.opts...)); err != nil {
http.Error(w, fmt.Sprintf("verifying POST: %v", err), http.StatusBadRequest)
return
}
Expand Down
6 changes: 3 additions & 3 deletions activation/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type scaler interface {
}

type postVerifierCallOption struct {
prioritised bool
prioritized bool
verifierOptions []verifying.OptionFunc
}

Expand All @@ -46,9 +46,9 @@ func applyOptions(options ...postVerifierOptionFunc) postVerifierCallOption {
return opts
}

func PrioritisedCall() postVerifierOptionFunc {
func PrioritizedCall() postVerifierOptionFunc {
return func(o *postVerifierCallOption) {
o.prioritised = true
o.prioritized = true
}
}

Expand Down
7 changes: 3 additions & 4 deletions activation/mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion activation/post_verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ func (v *offloadingPostVerifier) Verify(
defer metrics.PostVerificationQueue.Dec()

jobChannel := v.jobs
if opt.prioritised {
if opt.prioritized {
v.log.Debug("prioritizing post verification call")
jobChannel = v.prioritized
} else {
Expand All @@ -333,6 +333,7 @@ func (v *offloadingPostVerifier) Verify(

select {
case jobChannel <- job:
fmt.Printf("job is written")
fasmat marked this conversation as resolved.
Show resolved Hide resolved
case <-v.stop:
return errors.New("verifier is closed")
case <-ctx.Done():
Expand Down
13 changes: 9 additions & 4 deletions activation/post_verifier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,17 @@ func TestPostVerifierPrioritization(t *testing.T) {
require.NoError(t, err)

verifier.EXPECT().
Verify(context.WithValue(context.Background(), prioritizedVerifyCall, true),
gomock.Any(), &shared.ProofMetadata{NodeId: nodeID.Bytes()}, gomock.Any()).
Verify(
context.Background(),
gomock.Any(),
&shared.ProofMetadata{}, gomock.Any()).
Return(nil)

err = v.Verify(context.WithValue(context.Background(), prioritizedVerifyCall, true),
&shared.Proof{}, &shared.ProofMetadata{NodeId: nodeID.Bytes()})
err = v.Verify(
context.Background(),
&shared.Proof{},
&shared.ProofMetadata{},
PrioritizedCall())
fasmat marked this conversation as resolved.
Show resolved Hide resolved
require.NoError(t, err)

verifier.EXPECT().Close().Return(nil)
Expand Down
4 changes: 2 additions & 2 deletions activation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@
"context"
"errors"
"fmt"
"github.com/spacemeshos/post/verifying"
"time"

"github.com/spacemeshos/merkle-tree"
poetShared "github.com/spacemeshos/poet/shared"
"github.com/spacemeshos/post/config"
"github.com/spacemeshos/post/shared"
"github.com/spacemeshos/post/verifying"
"go.uber.org/zap"

"github.com/spacemeshos/go-spacemesh/activation/metrics"
Expand Down Expand Up @@ -53,9 +53,9 @@
}
}

func PrioritizeCall() validatorOption {
return func(o *validatorOptions) {
o.prioritized = true

Check warning on line 58 in activation/validation.go

View check run for this annotation

Codecov / codecov/patch

activation/validation.go#L56-L58

Added lines #L56 - L58 were not covered by tests
}
}

Expand Down Expand Up @@ -210,7 +210,7 @@

callOpts := []postVerifierOptionFunc{WithVerifierOptions(verifyOpts...)}
if options.prioritized {
callOpts = append(callOpts, PrioritisedCall())
callOpts = append(callOpts, PrioritizedCall())

Check warning on line 213 in activation/validation.go

View check run for this annotation

Codecov / codecov/patch

activation/validation.go#L213

Added line #L213 was not covered by tests
}

start := time.Now()
Expand Down Expand Up @@ -552,7 +552,7 @@

var validatorOpts []validatorOption
if opts.prioritizedCall {
validatorOpts = append(validatorOpts, PrioritizeCall())

Check warning on line 555 in activation/validation.go

View check run for this annotation

Codecov / codecov/patch

activation/validation.go#L555

Added line #L555 was not covered by tests
}

if err := v.Post(
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,8 @@ github.com/spacemeshos/merkle-tree v0.2.3 h1:zGEgOR9nxAzJr0EWjD39QFngwFEOxfxMloE
github.com/spacemeshos/merkle-tree v0.2.3/go.mod h1:VomOcQ5pCBXz7goiWMP5hReyqOfDXGSKbrH2GB9Htww=
github.com/spacemeshos/poet v0.10.3 h1:ZDPqihukDphdM+Jr/3xgn7vXadheaRMa6wF70Zsv4fg=
github.com/spacemeshos/poet v0.10.3/go.mod h1:TPZ/aX+YIgIqs/bvYTcJIwUWEUzvZw6jueFPxdhCGpY=
github.com/spacemeshos/post v0.12.6 h1:BtKK4n8qa7d0APtQZ2KBx9fQpx1B5LSt2OD7XIgE8g4=
github.com/spacemeshos/post v0.12.6/go.mod h1:NEstvZ4BKHuiGTcb+H+cQsZiNSh0G7GOLjZv6jjnHxM=
github.com/spacemeshos/sha256-simd v0.1.0 h1:G7Mfu5RYdQiuE+wu4ZyJ7I0TI74uqLhFnKblEnSpjYI=
github.com/spacemeshos/sha256-simd v0.1.0/go.mod h1:O8CClVIilId7RtuCMV2+YzMj6qjVn75JsxOxaE8vcfM=
github.com/spaolacci/murmur3 v1.1.0 h1:7c1g84S4BPRrfL5Xrdp6fOJ206sU9y293DDHaoy0bLI=
Expand Down
Loading