From 0e7920ce30257aa1aced00e34ac637423f50965e Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Tue, 29 Nov 2022 18:28:16 -0600 Subject: [PATCH] server: Always respond to getheaders. The existing logic for responding to getheaders is based on the old sync model before headers-first syncing was introduced and thus avoids responding whenever the current local chain is not considered to be current. This existing behavior was important under the the old sync model since it would otherwise lead to unnecessarily downloading a bunch of blocks among other undesirable corner cases, however, it does come with some downsides. For example, one less than ideal consequence of not responding is that it can lead to peers appearing to be unresponsive and/or stalled rather than merely not having anything interesting yet. Also, there are a variety of corner cases where a peer might temporarily no longer consider itself current such as after being unable to communicate with the network for a long time, or in testing scenarios where there are necessarily long periods of time without any new blocks. However, since peers now use the headers to determine what blocks to download, serving up headers before the peer is known to be current no longer leads to undesirable behavior. Taking the aforementioned factors into consideration, this modifies the semantics to always respond to getheaders requests, even before the local chain is fully synced. Further, in order to avoid needlessly sending extra data early during the initial sync process and to provide some additional protection against low-effort DoS attempts, a new check is introduced to respond with empty headers when the local chain does not yet have the minimum cumulative work value already known from the chain parameters to have been achieved on the network. Responding with empty headers signals to the remote peer that there are not any interesting headers available without the local peer appearing unresponsive. --- server.go | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/server.go b/server.go index 0bf1e4b548..773ad4af77 100644 --- a/server.go +++ b/server.go @@ -44,6 +44,7 @@ import ( "github.com/decred/dcrd/internal/netsync" "github.com/decred/dcrd/internal/rpcserver" "github.com/decred/dcrd/internal/version" + "github.com/decred/dcrd/math/uint256" "github.com/decred/dcrd/peer/v3" "github.com/decred/dcrd/txscript/v4" "github.com/decred/dcrd/wire" @@ -471,6 +472,13 @@ type server struct { bytesSent uint64 // Total bytes sent by all peers since start. shutdown int32 + // minKnownWork houses the minimum known work from the associated network + // params converted to a uint256 so the conversion only needs to be + // performed once when the server is initialized. Ideally, the chain params + // should be updated to use the new type, but that will be a major version + // bump, so a one-time conversion is a good tradeoff in the mean time. + minKnownWork uint256.Uint256 + chainParams *chaincfg.Params addrManager *addrmgr.AddrManager connManager *connmgr.ConnManager @@ -1258,13 +1266,18 @@ func (sp *serverPeer) OnGetBlocks(_ *peer.Peer, msg *wire.MsgGetBlocks) { // OnGetHeaders is invoked when a peer receives a getheaders wire message. func (sp *serverPeer) OnGetHeaders(_ *peer.Peer, msg *wire.MsgGetHeaders) { - // Ignore getheaders requests if not in sync unless the local best chain - // is exactly at the same tip as the requesting peer. - locatorHashes := msg.BlockLocatorHashes + // Send an empty headers message in the case the local best known chain does + // not have the minimum cumulative work value already known to have been + // achieved on the network. This signals to the remote peer that there are + // no interesting headers available without appearing unresponsive. chain := sp.server.chain - if !sp.server.syncManager.IsCurrent() && (len(locatorHashes) == 0 || - *locatorHashes[0] != chain.BestSnapshot().PrevHash) { - + tipHash := chain.BestSnapshot().Hash + workSum, err := chain.ChainWork(&tipHash) + if err == nil && workSum.Lt(&sp.server.minKnownWork) { + srvrLog.Debugf("Sending empty headers to peer %s in response to "+ + "getheaders due to local best known tip having too little work", + sp.Peer) + sp.QueueMessage(&wire.MsgHeaders{}, nil) return } @@ -1276,6 +1289,7 @@ func (sp *serverPeer) OnGetHeaders(_ *peer.Peer, msg *wire.MsgGetHeaders) { // Use the block after the genesis block if no other blocks in the // provided locator are known. This does mean the client will start // over with the genesis block if unknown block locators are provided. + locatorHashes := msg.BlockLocatorHashes headers := chain.LocateHeaders(locatorHashes, &msg.HashStop) // Send found headers to the requesting peer. @@ -3406,6 +3420,15 @@ func newServer(ctx context.Context, listenAddrs []string, db database.DB, quit: make(chan struct{}), } + // Convert the minimum known work to a uint256 when it exists. Ideally, the + // chain params should be updated to use the new type, but that will be a + // major version bump, so a one-time conversion is a good tradeoff in the + // mean time. + minKnownWorkBig := chainParams.MinKnownChainWork + if minKnownWorkBig != nil { + s.minKnownWork.SetBig(minKnownWorkBig) + } + feC := fees.EstimatorConfig{ MinBucketFee: cfg.minRelayTxFee, MaxBucketFee: dcrutil.Amount(fees.DefaultMaxBucketFeeMultiplier) * cfg.minRelayTxFee,