From fc8e9e3f20dc9f025041503c94a5773d082bff8e Mon Sep 17 00:00:00 2001 From: rene <41963722+renaynay@users.noreply.github.com> Date: Tue, 4 Jul 2023 17:12:29 +0200 Subject: [PATCH] refactor: WithSubjectiveInit --> DisableSubjectiveInit(header) --- headertest/dummy_header.go | 4 ++-- headertest/dummy_suite.go | 1 + opts.go | 22 +++++++++++++--------- p2p/exchange.go | 22 ++++++++++++++++++---- p2p/exchange_test.go | 5 ++++- sync/sync_head.go | 2 +- 6 files changed, 39 insertions(+), 17 deletions(-) diff --git a/headertest/dummy_header.go b/headertest/dummy_header.go index f4ec3507..223b11d5 100644 --- a/headertest/dummy_header.go +++ b/headertest/dummy_header.go @@ -106,11 +106,11 @@ func (d *DummyHeader) Verify(header header.Header) error { } if header.Height() <= d.Height() { - return fmt.Errorf("expected new header Height to be larger than old header Time") + return fmt.Errorf("expected new header Height %d to be larger than old header Height %d", header.Height(), d.Height()) } if header.Time().Before(d.Time()) { - return fmt.Errorf("expected new header Time to be after old header Time") + return fmt.Errorf("expected new header Time %v to be after old header Time %v", header.Time(), d.Time()) } return nil diff --git a/headertest/dummy_suite.go b/headertest/dummy_suite.go index 740c0d6c..f5a81bee 100644 --- a/headertest/dummy_suite.go +++ b/headertest/dummy_suite.go @@ -42,6 +42,7 @@ func (s *DummySuite) NextHeader() *DummyHeader { } dh := RandDummyHeader(s.t) + dh.Raw.Time = s.head.Time().Add(time.Nanosecond) dh.Raw.Height = s.head.Height() + 1 dh.Raw.PreviousHash = s.head.Hash() _ = dh.rehash() diff --git a/opts.go b/opts.go index 7057f876..9220d7c8 100644 --- a/opts.go +++ b/opts.go @@ -5,20 +5,24 @@ type HeadOption func(opts *HeadRequestParams) // HeadRequestParams contains options to be used for header Exchange // requests. type HeadRequestParams struct { - // SubjectiveInit determines whether the Exchange should use - // trusted peers for its Head request (true = yes). - SubjectiveInit bool + // DisableSubjectiveInit indicates whether the Exchange should use + // trusted peers for its Head request. If nil, trusted peers will + // be used. If non-nil, Exchange will ask several peers from its + // network for their Head and verify against the given header. + DisableSubjectiveInit Header } func DefaultHeadRequestParams() HeadRequestParams { return HeadRequestParams{ - SubjectiveInit: true, + DisableSubjectiveInit: nil, } } -// WithDisabledSubjectiveInit sets the SubjectiveInit parameter to false, -// indicating to the Head method to use the tracked peerset rather than -// the trusted peerset (if enough tracked peers are available) -func WithDisabledSubjectiveInit(opts *HeadRequestParams) { - opts.SubjectiveInit = false +// WithDisabledSubjectiveInit sets the DisableSubjectiveInit parameter to the +// given header, indicating to the Head method to use the tracked peerset rather +// than the trusted peerset (if enough tracked peers are available). +func WithDisabledSubjectiveInit(verified Header) func(opts *HeadRequestParams) { + return func(opts *HeadRequestParams) { + opts.DisableSubjectiveInit = verified + } } diff --git a/p2p/exchange.go b/p2p/exchange.go index 1e164da7..30338071 100644 --- a/p2p/exchange.go +++ b/p2p/exchange.go @@ -132,11 +132,13 @@ func (ex *Exchange[H]) Head(ctx context.Context, opts ...header.HeadOption) (H, } peers := ex.peerTracker.getPeers() - if reqParams.SubjectiveInit || len(peers) < numUntrustedHeadRequests { - peers = ex.trustedPeers() - } else { - // only query a subset + if reqParams.DisableSubjectiveInit != nil && len(peers) >= numUntrustedHeadRequests { + // subjective initialisation was disabled, request head from a subset of + // tracked peers peers = peers[:numUntrustedHeadRequests] + } else { + // default to using trusted peers + peers = ex.trustedPeers() } var ( @@ -155,6 +157,18 @@ func (ex *Exchange[H]) Head(ctx context.Context, opts ...header.HeadOption) (H, headerRespCh <- zero return } + // if tracked (untrusted) peers were requested, verify head + if reqParams.DisableSubjectiveInit != nil { + err = reqParams.DisableSubjectiveInit.Verify(headers[0]) + if err != nil { + log.Errorw("head request to untrusted peer failed", "untrusted peer", from, + "err", err) + // bad head was given, block peer + ex.peerTracker.blockPeer(from, fmt.Errorf("returned bad head: %w", err)) + headerRespCh <- zero + return + } + } // request ensures that the result slice will have at least one Header headerRespCh <- headers[0] }(from) diff --git a/p2p/exchange_test.go b/p2p/exchange_test.go index a81faef7..70a16bb1 100644 --- a/p2p/exchange_test.go +++ b/p2p/exchange_test.go @@ -56,18 +56,21 @@ func TestExchange_RequestHead(t *testing.T) { tests := []struct { withSubjInit bool + lastHeader header.Header expectedHeight int64 expectedHash header.Hash }{ // routes to trusted peer only { withSubjInit: true, + lastHeader: trustedStore.Headers[trustedStore.HeadHeight-1], expectedHeight: trustedStore.HeadHeight, expectedHash: trustedStore.Headers[trustedStore.HeadHeight].Hash(), }, // routes to tracked peers and takes highest chain head { withSubjInit: false, + lastHeader: trackedStore.Headers[trackedStore.HeadHeight-1], expectedHeight: trackedStore.HeadHeight, expectedHash: trackedStore.Headers[trackedStore.HeadHeight].Hash(), }, @@ -77,7 +80,7 @@ func TestExchange_RequestHead(t *testing.T) { t.Run(strconv.Itoa(i), func(t *testing.T) { var opts []header.HeadOption if !tt.withSubjInit { - opts = append(opts, header.WithDisabledSubjectiveInit) + opts = append(opts, header.WithDisabledSubjectiveInit(tt.lastHeader)) } header, err := exchg.Head(ctx, opts...) diff --git a/sync/sync_head.go b/sync/sync_head.go index 74b0d6ae..46abf2f4 100644 --- a/sync/sync_head.go +++ b/sync/sync_head.go @@ -31,7 +31,7 @@ func (s *Syncer[H]) Head(ctx context.Context, _ ...header.HeadOption) (H, error) // * If now >= TNH && now <= TNH + (THP) header propagation time // * Wait for header to arrive instead of requesting it // * This way we don't request as we know the new network header arrives exactly - netHead, err := s.getter.Head(ctx, header.WithDisabledSubjectiveInit) + netHead, err := s.getter.Head(ctx, header.WithDisabledSubjectiveInit(sbjHead)) if err != nil { return netHead, err }