[NOD-1064] Don't send GetBlockInvsMsg with lowHash = nil (#769)

* [NOD-1048] Make leveldb compaction much less frequent. Also, allocate an entire gigabyte for leveldb's blockCache and writeBuffer.

* [NOD-1048] Implement changing the options for testing purposes.

* [NOD-1048] Rename originalOptions to originalLDBOptions.

* [NOD-1048] Add a comment.

blockdag/test_utils.go

@@ -5,9 +5,11 @@ package blockdag
 import (
 	"compress/bzip2"
 	"encoding/binary"
+	"github.com/kaspanet/kaspad/database/ffldb/ldb"
 	"github.com/kaspanet/kaspad/dbaccess"
 	"github.com/kaspanet/kaspad/util"
 	"github.com/pkg/errors"
+	"github.com/syndtr/goleveldb/leveldb/opt"
 	"io"
 	"io/ioutil"
 	"os"
@@ -62,6 +64,15 @@ func DAGSetup(dbName string, openDb bool, config Config) (*BlockDAG, func(), err
 			return nil, nil, errors.Errorf("error creating temp dir: %s", err)
 		}
 
+		// We set ldb.Options here to return nil because normally
+		// the database is initialized with very large caches that
+		// can make opening/closing the database for every test
+		// quite heavy.
+		originalLDBOptions := ldb.Options
+		ldb.Options = func() *opt.Options {
+			return nil
+		}
+
 		dbPath := filepath.Join(tmpDir, dbName)
 		_ = os.RemoveAll(dbPath)
 		err = dbaccess.Open(dbPath)
@@ -75,6 +86,7 @@ func DAGSetup(dbName string, openDb bool, config Config) (*BlockDAG, func(), err
 			spawnWaitGroup.Wait()
 			spawn = realSpawn
 			dbaccess.Close()
+			ldb.Options = originalLDBOptions
 			os.RemoveAll(dbPath)
 		}
 	} else {

blockdag/utxodiffstore.go

@@ -153,20 +153,27 @@ func (diffStore *utxoDiffStore) clearDirtyEntries() {
 // maxBlueScoreDifferenceToKeepLoaded is the maximum difference
 // between the virtual's blueScore and a blockNode's blueScore
 // under which to keep diff data loaded in memory.
-var maxBlueScoreDifferenceToKeepLoaded uint64 = 1500
+var maxBlueScoreDifferenceToKeepLoaded uint64 = 100
 
 // clearOldEntries removes entries whose blue score is lower than
-// virtual.blueScore - maxBlueScoreDifferenceToKeepLoaded.
+// virtual.blueScore - maxBlueScoreDifferenceToKeepLoaded. Note
+// that tips are not removed either even if their blue score is
+// lower than the above.
 func (diffStore *utxoDiffStore) clearOldEntries() {
+	diffStore.mtx.HighPriorityWriteLock()
+	defer diffStore.mtx.HighPriorityWriteUnlock()
+
 	virtualBlueScore := diffStore.dag.VirtualBlueScore()
 	minBlueScore := virtualBlueScore - maxBlueScoreDifferenceToKeepLoaded
 	if maxBlueScoreDifferenceToKeepLoaded > virtualBlueScore {
 		minBlueScore = 0
 	}
 
+	tips := diffStore.dag.virtual.tips()
+
 	toRemove := make(map[*blockNode]struct{})
 	for node := range diffStore.loaded {
-		if node.blueScore < minBlueScore {
+		if node.blueScore < minBlueScore && !tips.contains(node) {
 			toRemove[node] = struct{}{}
 		}
 	}

blockdag/utxodiffstore_test.go

@@ -149,10 +149,11 @@ func TestClearOldEntries(t *testing.T) {
 		t.Fatalf("TestClearOldEntries: missing blockNode for hash %s", processedBlock.BlockHash())
 	}
 
-	// Make sure that the child-of-genesis node isn't in the loaded set
+	// Make sure that the child-of-genesis node is in the loaded set, since it
+	// is a tip.
 	_, ok := dag.utxoDiffStore.loaded[node]
-	if ok {
-		t.Fatalf("TestClearOldEntries: diffData for node %s is in the loaded set", node.hash)
+	if !ok {
+		t.Fatalf("TestClearOldEntries: diffData for node %s is not in the loaded set", node.hash)
 	}
 
 	// Make sure that all the old nodes still do not exist in the loaded set

database/ffldb/ldb/leveldb.go

@@ -15,7 +15,7 @@ type LevelDB struct {
 // NewLevelDB opens a leveldb instance defined by the given path.
 func NewLevelDB(path string) (*LevelDB, error) {
 	// Open leveldb. If it doesn't exist, create it.
-	ldb, err := leveldb.OpenFile(path, nil)
+	ldb, err := leveldb.OpenFile(path, Options())
 
 	// If the database is corrupted, attempt to recover.
 	if _, corrupted := err.(*ldbErrors.ErrCorrupted); corrupted {

database/ffldb/ldb/options.go

@@ -0,0 +1,19 @@
+package ldb
+
+import "github.com/syndtr/goleveldb/leveldb/opt"
+
+var (
+	defaultOptions = opt.Options{
+		Compression:            opt.NoCompression,
+		BlockCacheCapacity:     256 * opt.MiB,
+		WriteBuffer:            128 * opt.MiB,
+		DisableSeeksCompaction: true,
+	}
+
+	// Options is a function that returns a leveldb
+	// opt.Options struct for opening a database.
+	// It's defined as a variable for the sake of testing.
+	Options = func() *opt.Options {
+		return &defaultOptions
+	}
+)

go.mod

@@ -14,7 +14,7 @@ require (
 	github.com/kaspanet/go-secp256k1 v0.0.2
 	github.com/kr/pretty v0.1.0 // indirect
 	github.com/pkg/errors v0.9.1
-	github.com/syndtr/goleveldb v1.0.0
+	github.com/syndtr/goleveldb v1.0.1-0.20190923125748-758128399b1d
 	golang.org/x/crypto v0.0.0-20200323165209-0ec3e9974c59
 	golang.org/x/sys v0.0.0-20190426135247-a129542de9ae // indirect
 	golang.org/x/text v0.3.2 // indirect

go.sum

@@ -38,6 +38,8 @@ github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
 github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/syndtr/goleveldb v1.0.0 h1:fBdIW9lB4Iz0n9khmH8w27SJ3QEJ7+IgjPEwGSZiFdE=
 github.com/syndtr/goleveldb v1.0.0/go.mod h1:ZVVdQEZoIme9iO1Ch2Jdy24qqXrMMOU6lpPAyBWyWuQ=
+github.com/syndtr/goleveldb v1.0.1-0.20190923125748-758128399b1d h1:gZZadD8H+fF+n9CmNhYL1Y0dJB+kLOmKd7FbPJLeGHs=
+github.com/syndtr/goleveldb v1.0.1-0.20190923125748-758128399b1d/go.mod h1:9OrXJhf154huy1nPWmuSrkgjPUtUNhA+Zmy+6AESzuA=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550 h1:ObdrDkeb4kJdCP557AjRjq69pTHfNouLtWZG7j9rPN8=
 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=

netsync/manager.go

@@ -533,15 +533,6 @@ func (sm *SyncManager) handleBlockMsg(bmsg *blockMsg) {
 	}
 
 	if isOrphan {
-		// If we received an orphan block from the sync peer, it is
-		// misbehaving and must be disconnected from.
-		if peer == sm.syncPeer {
-			log.Errorf("Received an orphan block %s from sync peer %s. Disconnecting...",
-				blockHash, peer)
-			peer.Disconnect()
-			return
-		}
-
 		// Request the parents for the orphan block from the peer that sent it.
 		missingAncestors, err := sm.dag.GetOrphanMissingAncestorHashes(blockHash)
 		if err != nil {
@@ -852,7 +843,6 @@ func (sm *SyncManager) sendInvsFromRequestQueue(peer *peerpkg.Peer, state *peerS
 		return err
 	}
 	if !sm.isSyncing || sm.isSynced() {
-		log.Criticalf("wtf? sm.isSyncing: %t sm.isSynced: %t", sm.isSyncing, sm.isSynced())
 		err := sm.addInvsToGetDataMessageFromQueue(gdmsg, state, wire.InvTypeBlock, wire.MaxInvPerGetDataMsg)
 		if err != nil {
 			return err

peer/peer_test.go

@@ -585,7 +585,7 @@ func TestOutboundPeer(t *testing.T) {
 	if _, err := p2.PushAddrMsg(addrs, nil); err != nil {
 		t.Fatalf("PushAddrMsg: unexpected err %v\n", err)
 	}
-	if err := p2.PushGetBlockInvsMsg(nil, &daghash.Hash{}); err != nil {
+	if err := p2.PushGetBlockInvsMsg(&daghash.Hash{}, &daghash.Hash{}); err != nil {
 		t.Fatalf("PushGetBlockInvsMsg: unexpected err %v\n", err)
 	}
 

server/p2p/on_addr.go

@@ -18,14 +18,6 @@ func (sp *Peer) OnAddr(_ *peer.Peer, msg *wire.MsgAddr) {
 		return
 	}
 
-	// A message that has no addresses is invalid.
-	if len(msg.AddrList) == 0 {
-		peerLog.Errorf("Command [%s] from %s does not contain any addresses",
-			msg.Command(), sp.Peer)
-		sp.Disconnect()
-		return
-	}
-
 	if msg.IncludeAllSubnetworks {
 		peerLog.Errorf("Got unexpected IncludeAllSubnetworks=true in [%s] command from %s",
 			msg.Command(), sp.Peer)