Skip to content

Commit

Permalink
refactor: WithSubjectiveInit --> DisableSubjectiveInit(header)
Browse files Browse the repository at this point in the history
  • Loading branch information
renaynay committed Jul 4, 2023
1 parent 3301aca commit fc8e9e3
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 17 deletions.
4 changes: 2 additions & 2 deletions headertest/dummy_header.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions headertest/dummy_suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
22 changes: 13 additions & 9 deletions opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
22 changes: 18 additions & 4 deletions p2p/exchange.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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)
Expand Down
5 changes: 4 additions & 1 deletion p2p/exchange_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
},
Expand All @@ -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...)
Expand Down
2 changes: 1 addition & 1 deletion sync/sync_head.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down

0 comments on commit fc8e9e3

Please sign in to comment.