From 7b267008ff8e77f8d6ec7e905db6fd66f30573e0 Mon Sep 17 00:00:00 2001 From: rene <41963722+renaynay@users.noreply.github.com> Date: Thu, 24 Aug 2023 16:03:18 +0200 Subject: [PATCH 1/5] refactor(sync): Add `recencyThreshold` param and set the default to 20 seconds --- sync/options.go | 14 +++++++++++++- sync/sync_head.go | 4 ++-- sync/sync_head_test.go | 2 +- sync/sync_test.go | 6 +++++- 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/sync/options.go b/sync/options.go index a7a532fe..d2016d03 100644 --- a/sync/options.go +++ b/sync/options.go @@ -26,12 +26,16 @@ type Parameters struct { // Keeping it private to disable serialization for it. // default value is set to 0 so syncer will constantly request networking head. blockTime time.Duration + // recencyThreshold describes the time period for which a header is + // considered "recent". The default is blockTime + 5 seconds. + recencyThreshold time.Duration } // DefaultParameters returns the default params to configure the syncer. func DefaultParameters() Parameters { return Parameters{ - TrustingPeriod: 168 * time.Hour, + TrustingPeriod: 168 * time.Hour, + recencyThreshold: time.Second * 20, // this default is based on a block time of 15 seconds } } @@ -50,6 +54,14 @@ func WithBlockTime(duration time.Duration) Options { } } +// WithRecencyThreshold is a functional option that configures the +// `recencyThreshold` parameter. +func WithRecencyThreshold(threshold time.Duration) Options { + return func(p *Parameters) { + p.recencyThreshold = threshold + } +} + // WithTrustingPeriod is a functional option that configures the // `TrustingPeriod` parameter. func WithTrustingPeriod(duration time.Duration) Options { diff --git a/sync/sync_head.go b/sync/sync_head.go index 7fa78d0d..bd4f7e73 100644 --- a/sync/sync_head.go +++ b/sync/sync_head.go @@ -19,7 +19,7 @@ func (s *Syncer[H]) Head(ctx context.Context, _ ...header.HeadOption[H]) (H, err return sbjHead, err } // if subjective header is recent enough (relative to the network's block time) - just use it - if isRecent(sbjHead, s.Params.blockTime) { + if isRecent(sbjHead, s.Params.recencyThreshold) { return sbjHead, nil } // otherwise, request head from the network @@ -98,7 +98,7 @@ func (s *Syncer[H]) subjectiveHead(ctx context.Context) (H, error) { return trustHead, nil case isExpired(trustHead, s.Params.TrustingPeriod): log.Warnw("subjective initialization with an expired header", "height", trustHead.Height()) - case !isRecent(trustHead, s.Params.blockTime): + case !isRecent(trustHead, s.Params.recencyThreshold): log.Warnw("subjective initialization with an old header", "height", trustHead.Height()) } log.Warn("trusted peer is out of sync") diff --git a/sync/sync_head_test.go b/sync/sync_head_test.go index 4af2ed25..9ef6c266 100644 --- a/sync/sync_head_test.go +++ b/sync/sync_head_test.go @@ -75,8 +75,8 @@ func TestSyncer_HeadWithTrustedHead(t *testing.T) { wrappedGetter, localStore, headertest.NewDummySubscriber(), - // forces a request for a new sync target WithBlockTime(time.Nanosecond), + WithRecencyThreshold(time.Nanosecond), // forces a request for a new sync target // ensures that syncer's store contains a subjective head that is within // the unbonding period so that the syncer can use a header from the network // as a sync target diff --git a/sync/sync_test.go b/sync/sync_test.go index 02936462..20a087bb 100644 --- a/sync/sync_test.go +++ b/sync/sync_test.go @@ -34,6 +34,7 @@ func TestSyncSimpleRequestingHead(t *testing.T) { localStore, headertest.NewDummySubscriber(), WithBlockTime(time.Second*30), + WithRecencyThreshold(time.Second*35), // add 5 second buffer WithTrustingPeriod(time.Microsecond), ) require.NoError(t, err) @@ -72,6 +73,8 @@ func TestDoSyncFullRangeFromExternalPeer(t *testing.T) { local.NewExchange(remoteStore), localStore, headertest.NewDummySubscriber(), + WithBlockTime(time.Nanosecond), + WithRecencyThreshold(time.Nanosecond), ) require.NoError(t, err) require.NoError(t, syncer.Start(ctx)) @@ -306,7 +309,8 @@ func TestSync_InvalidSyncTarget(t *testing.T) { local.NewExchange[*headertest.DummyHeader](remoteStore), localStore, headertest.NewDummySubscriber(), - WithBlockTime(time.Nanosecond), // force syncer to request more recent sync target + WithBlockTime(time.Nanosecond), + WithRecencyThreshold(time.Nanosecond), // force syncer to request more recent sync target ) require.NoError(t, err) From 9b67d13ed61fdec3a0234418d167071dc15ec442 Mon Sep 17 00:00:00 2001 From: rene <41963722+renaynay@users.noreply.github.com> Date: Fri, 25 Aug 2023 11:55:37 +0200 Subject: [PATCH 2/5] fix: remove default, set default in `isRecent` if 0 value --- sync/options.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sync/options.go b/sync/options.go index d2016d03..7264a3b5 100644 --- a/sync/options.go +++ b/sync/options.go @@ -34,8 +34,7 @@ type Parameters struct { // DefaultParameters returns the default params to configure the syncer. func DefaultParameters() Parameters { return Parameters{ - TrustingPeriod: 168 * time.Hour, - recencyThreshold: time.Second * 20, // this default is based on a block time of 15 seconds + TrustingPeriod: 168 * time.Hour, } } From 36d220c6a6b24956350d7a00a18ba592a51beb42 Mon Sep 17 00:00:00 2001 From: rene <41963722+renaynay@users.noreply.github.com> Date: Fri, 25 Aug 2023 12:40:39 +0200 Subject: [PATCH 3/5] fix rebase --- sync/sync_head.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/sync/sync_head.go b/sync/sync_head.go index bd4f7e73..6fbaccb1 100644 --- a/sync/sync_head.go +++ b/sync/sync_head.go @@ -190,6 +190,12 @@ func isExpired[H header.Header[H]](header H, period time.Duration) bool { } // isRecent checks if header is recent against the given blockTime. -func isRecent[H header.Header[H]](header H, blockTime time.Duration) bool { - return time.Since(header.Time()) <= blockTime+blockTime/2 // add half block time drift +func isRecent[H header.Header[H]](header H, recencyThreshold time.Duration) bool { + if recencyThreshold == 0 { + recencyThreshold = time.Second * 20 // this default is based on a block time of 15 seconds + } + // double the block time and add half block time drift to allow headersub + // some time to deliver a more recent head before eagerly requesting a new + // head + return time.Since(header.Time()) <= recencyThreshold } From 68dd6da99ac069a2c24695dc8db2381c0e6c59b9 Mon Sep 17 00:00:00 2001 From: rene <41963722+renaynay@users.noreply.github.com> Date: Fri, 25 Aug 2023 12:41:06 +0200 Subject: [PATCH 4/5] fix doc --- sync/sync_head.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/sync/sync_head.go b/sync/sync_head.go index 6fbaccb1..0ce9695b 100644 --- a/sync/sync_head.go +++ b/sync/sync_head.go @@ -189,13 +189,10 @@ func isExpired[H header.Header[H]](header H, period time.Duration) bool { return !expirationTime.After(time.Now()) } -// isRecent checks if header is recent against the given blockTime. +// isRecent checks if header is recent against the given recency threshold. func isRecent[H header.Header[H]](header H, recencyThreshold time.Duration) bool { if recencyThreshold == 0 { recencyThreshold = time.Second * 20 // this default is based on a block time of 15 seconds } - // double the block time and add half block time drift to allow headersub - // some time to deliver a more recent head before eagerly requesting a new - // head return time.Since(header.Time()) <= recencyThreshold } From ef65ee7a5b3eb66918467b60f42eda851954d043 Mon Sep 17 00:00:00 2001 From: rene <41963722+renaynay@users.noreply.github.com> Date: Fri, 25 Aug 2023 12:48:45 +0200 Subject: [PATCH 5/5] estimate default recency threshold via blocktime --- sync/sync_head.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sync/sync_head.go b/sync/sync_head.go index 0ce9695b..762205df 100644 --- a/sync/sync_head.go +++ b/sync/sync_head.go @@ -19,7 +19,7 @@ func (s *Syncer[H]) Head(ctx context.Context, _ ...header.HeadOption[H]) (H, err return sbjHead, err } // if subjective header is recent enough (relative to the network's block time) - just use it - if isRecent(sbjHead, s.Params.recencyThreshold) { + if isRecent(sbjHead, s.Params.blockTime, s.Params.recencyThreshold) { return sbjHead, nil } // otherwise, request head from the network @@ -98,7 +98,7 @@ func (s *Syncer[H]) subjectiveHead(ctx context.Context) (H, error) { return trustHead, nil case isExpired(trustHead, s.Params.TrustingPeriod): log.Warnw("subjective initialization with an expired header", "height", trustHead.Height()) - case !isRecent(trustHead, s.Params.recencyThreshold): + case !isRecent(trustHead, s.Params.blockTime, s.Params.recencyThreshold): log.Warnw("subjective initialization with an old header", "height", trustHead.Height()) } log.Warn("trusted peer is out of sync") @@ -190,9 +190,9 @@ func isExpired[H header.Header[H]](header H, period time.Duration) bool { } // isRecent checks if header is recent against the given recency threshold. -func isRecent[H header.Header[H]](header H, recencyThreshold time.Duration) bool { +func isRecent[H header.Header[H]](header H, blockTime, recencyThreshold time.Duration) bool { if recencyThreshold == 0 { - recencyThreshold = time.Second * 20 // this default is based on a block time of 15 seconds + recencyThreshold = blockTime + blockTime/2 // allow some drift by adding additional buffer of 1/2 of block time } return time.Since(header.Time()) <= recencyThreshold }