[NOD-448] Fix initial sync in API Server crashing due to misaligned getBlocks calls (#482)

* [NOD-448] Change GetBlocksCmd to be able to include both raw and verbose block data.

* [NOD-448] Update sync logic to only make one getBlocks call per page.

* [NOD-448] Make GetBlocks get each block only once.

apiserver/sync.go

@@ -107,10 +107,10 @@ func syncBlocks(client *jsonrpc.Client) error {
 		return err
 	}
 
-	var blocks []string
+	var rawBlocks []string
-	var rawBlocks []btcjson.GetBlockVerboseResult
+	var verboseBlocks []btcjson.GetBlockVerboseResult
 	for {
-		blocksResult, err := client.GetBlocks(true, false, startHash)
+		blocksResult, err := client.GetBlocks(true, true, startHash)
 		if err != nil {
 			return err
 		}
@@ -118,17 +118,12 @@ func syncBlocks(client *jsonrpc.Client) error {
 			break
 		}
 
-		rawBlocksResult, err := client.GetBlocks(true, true, startHash)
-		if err != nil {
-			return err
-		}
-
 		startHash = &blocksResult.Hashes[len(blocksResult.Hashes)-1]
-		blocks = append(blocks, blocksResult.Blocks...)
+		rawBlocks = append(rawBlocks, blocksResult.RawBlocks...)
-		rawBlocks = append(rawBlocks, rawBlocksResult.RawBlocks...)
+		verboseBlocks = append(verboseBlocks, blocksResult.VerboseBlocks...)
 	}
 
-	return addBlocks(client, blocks, rawBlocks)
+	return addBlocks(client, rawBlocks, verboseBlocks)
 }
 
 // syncSelectedParentChain attempts to download the selected parent
@@ -191,7 +186,7 @@ func findHashOfBluestBlock(mustBeChainBlock bool) (*string, error) {
 // fetchBlock downloads the serialized block and raw block data of
 // the block with hash blockHash.
 func fetchBlock(client *jsonrpc.Client, blockHash *daghash.Hash) (
-	block string, rawBlock *btcjson.GetBlockVerboseResult, err error) {
+	rawBlock string, verboseBlock *btcjson.GetBlockVerboseResult, err error) {
 	msgBlock, err := client.GetBlock(blockHash, nil)
 	if err != nil {
 		return "", nil, err
@@ -201,21 +196,21 @@ func fetchBlock(client *jsonrpc.Client, blockHash *daghash.Hash) (
 	if err != nil {
 		return "", nil, err
 	}
-	block = hex.EncodeToString(writer.Bytes())
+	rawBlock = hex.EncodeToString(writer.Bytes())
 
-	rawBlock, err = client.GetBlockVerboseTx(blockHash, nil)
+	verboseBlock, err = client.GetBlockVerboseTx(blockHash, nil)
 	if err != nil {
 		return "", nil, err
 	}
-	return block, rawBlock, nil
+	return rawBlock, verboseBlock, nil
 }
 
-// addBlocks inserts data in the given blocks and rawBlocks pairwise
+// addBlocks inserts data in the given rawBlocks and verboseBlocks pairwise
 // into the database. See addBlock for further details.
-func addBlocks(client *jsonrpc.Client, blocks []string, rawBlocks []btcjson.GetBlockVerboseResult) error {
+func addBlocks(client *jsonrpc.Client, rawBlocks []string, verboseBlocks []btcjson.GetBlockVerboseResult) error {
 	for i, rawBlock := range rawBlocks {
-		block := blocks[i]
+		verboseBlock := verboseBlocks[i]
-		err := addBlock(client, block, rawBlock)
+		err := addBlock(client, rawBlock, verboseBlock)
 		if err != nil {
 			return err
 		}
@@ -235,12 +230,12 @@ func doesBlockExist(dbTx *gorm.DB, blockHash string) (bool, error) {
 	return !httpserverutils.IsDBRecordNotFoundError(dbErrors), nil
 }
 
-// addBlocks inserts all the data that could be gleaned out of the serialized
+// addBlocks inserts all the data that could be gleaned out of the verbose
 // block and raw block data into the database. This includes transactions,
 // subnetworks, and addresses.
 // Note that if this function may take a nil dbTx, in which case it would start
 // a database transaction by itself and commit it before returning.
-func addBlock(client *jsonrpc.Client, block string, rawBlock btcjson.GetBlockVerboseResult) error {
+func addBlock(client *jsonrpc.Client, rawBlock string, verboseBlock btcjson.GetBlockVerboseResult) error {
 	db, err := database.DB()
 	if err != nil {
 		return err
@@ -249,7 +244,7 @@ func addBlock(client *jsonrpc.Client, block string, rawBlock btcjson.GetBlockVer
 	defer dbTx.RollbackUnlessCommitted()
 
 	// Skip this block if it already exists.
-	blockExists, err := doesBlockExist(dbTx, rawBlock.Hash)
+	blockExists, err := doesBlockExist(dbTx, verboseBlock.Hash)
 	if err != nil {
 		return err
 	}
@@ -258,21 +253,21 @@ func addBlock(client *jsonrpc.Client, block string, rawBlock btcjson.GetBlockVer
 		return nil
 	}
 
-	dbBlock, err := insertBlock(dbTx, rawBlock)
+	dbBlock, err := insertBlock(dbTx, verboseBlock)
 	if err != nil {
 		return err
 	}
-	err = insertBlockParents(dbTx, rawBlock, dbBlock)
+	err = insertBlockParents(dbTx, verboseBlock, dbBlock)
 	if err != nil {
 		return err
 	}
-	err = insertBlockData(dbTx, block, dbBlock)
+	err = insertRawBlockData(dbTx, rawBlock, dbBlock)
 	if err != nil {
 		return err
 	}
 
 	blockMass := uint64(0)
-	for i, transaction := range rawBlock.RawTx {
+	for i, transaction := range verboseBlock.RawTx {
 		dbSubnetwork, err := insertSubnetwork(dbTx, &transaction, client)
 		if err != nil {
 			return err
@@ -307,32 +302,32 @@ func addBlock(client *jsonrpc.Client, block string, rawBlock btcjson.GetBlockVer
 
 	// If the block was previously missing, remove it from
 	// the missing blocks collection.
-	if _, ok := missingBlocks[rawBlock.Hash]; ok {
+	if _, ok := missingBlocks[verboseBlock.Hash]; ok {
-		delete(missingBlocks, rawBlock.Hash)
+		delete(missingBlocks, verboseBlock.Hash)
 	}
 	return nil
 }
 
-func insertBlock(dbTx *gorm.DB, rawBlock btcjson.GetBlockVerboseResult) (*dbmodels.Block, error) {
+func insertBlock(dbTx *gorm.DB, verboseBlock btcjson.GetBlockVerboseResult) (*dbmodels.Block, error) {
-	bits, err := strconv.ParseUint(rawBlock.Bits, 16, 32)
+	bits, err := strconv.ParseUint(verboseBlock.Bits, 16, 32)
 	if err != nil {
 		return nil, err
 	}
 	dbBlock := dbmodels.Block{
-		BlockHash:            rawBlock.Hash,
+		BlockHash:            verboseBlock.Hash,
-		Version:              rawBlock.Version,
+		Version:              verboseBlock.Version,
-		HashMerkleRoot:       rawBlock.HashMerkleRoot,
+		HashMerkleRoot:       verboseBlock.HashMerkleRoot,
-		AcceptedIDMerkleRoot: rawBlock.AcceptedIDMerkleRoot,
+		AcceptedIDMerkleRoot: verboseBlock.AcceptedIDMerkleRoot,
-		UTXOCommitment:       rawBlock.UTXOCommitment,
+		UTXOCommitment:       verboseBlock.UTXOCommitment,
-		Timestamp:            time.Unix(rawBlock.Time, 0),
+		Timestamp:            time.Unix(verboseBlock.Time, 0),
 		Bits:                 uint32(bits),
-		Nonce:                rawBlock.Nonce,
+		Nonce:                verboseBlock.Nonce,
-		BlueScore:            rawBlock.BlueScore,
+		BlueScore:            verboseBlock.BlueScore,
 		IsChainBlock:         false, // This must be false for updateSelectedParentChain to work properly
 	}
 
 	// Set genesis block as the initial chain block
-	if len(rawBlock.ParentHashes) == 0 {
+	if len(verboseBlock.ParentHashes) == 0 {
 		dbBlock.IsChainBlock = true
 	}
 	dbResult := dbTx.Create(&dbBlock)
@@ -343,14 +338,14 @@ func insertBlock(dbTx *gorm.DB, rawBlock btcjson.GetBlockVerboseResult) (*dbmode
 	return &dbBlock, nil
 }
 
-func insertBlockParents(dbTx *gorm.DB, rawBlock btcjson.GetBlockVerboseResult, dbBlock *dbmodels.Block) error {
+func insertBlockParents(dbTx *gorm.DB, verboseBlock btcjson.GetBlockVerboseResult, dbBlock *dbmodels.Block) error {
 	// Exit early if this is the genesis block
-	if len(rawBlock.ParentHashes) == 0 {
+	if len(verboseBlock.ParentHashes) == 0 {
 		return nil
 	}
 
-	hashesIn := make([]string, len(rawBlock.ParentHashes))
+	hashesIn := make([]string, len(verboseBlock.ParentHashes))
-	for i, parentHash := range rawBlock.ParentHashes {
+	for i, parentHash := range verboseBlock.ParentHashes {
 		hashesIn[i] = parentHash
 	}
 	var dbParents []dbmodels.Block
@@ -361,10 +356,10 @@ func insertBlockParents(dbTx *gorm.DB, rawBlock btcjson.GetBlockVerboseResult, d
 	if httpserverutils.HasDBError(dbErrors) {
 		return httpserverutils.NewErrorFromDBErrors("failed to find blocks: ", dbErrors)
 	}
-	if len(dbParents) != len(rawBlock.ParentHashes) {
+	if len(dbParents) != len(verboseBlock.ParentHashes) {
-		missingParents := make([]string, 0, len(rawBlock.ParentHashes)-len(dbParents))
+		missingParents := make([]string, 0, len(verboseBlock.ParentHashes)-len(dbParents))
 	outerLoop:
-		for _, parentHash := range rawBlock.ParentHashes {
+		for _, parentHash := range verboseBlock.ParentHashes {
 			for _, dbParent := range dbParents {
 				if dbParent.BlockHash == parentHash {
 					continue outerLoop
@@ -372,7 +367,7 @@ func insertBlockParents(dbTx *gorm.DB, rawBlock btcjson.GetBlockVerboseResult, d
 			}
 			missingParents = append(missingParents, parentHash)
 		}
-		return errors.Errorf("some parents are missing for block %s: %s", rawBlock.Hash, strings.Join(missingParents, ", "))
+		return errors.Errorf("some parents are missing for block %s: %s", verboseBlock.Hash, strings.Join(missingParents, ", "))
 	}
 
 	for _, dbParent := range dbParents {
@@ -389,8 +384,8 @@ func insertBlockParents(dbTx *gorm.DB, rawBlock btcjson.GetBlockVerboseResult, d
 	return nil
 }
 
-func insertBlockData(dbTx *gorm.DB, block string, dbBlock *dbmodels.Block) error {
+func insertRawBlockData(dbTx *gorm.DB, rawBlock string, dbBlock *dbmodels.Block) error {
-	blockData, err := hex.DecodeString(block)
+	blockData, err := hex.DecodeString(rawBlock)
 	if err != nil {
 		return err
 	}
@@ -950,12 +945,12 @@ func processBlockAddedMsgs(client *jsonrpc.Client) {
 
 		hash := blockAdded.Header.BlockHash()
 		log.Debugf("Getting block %s from the RPC server", hash)
-		block, rawBlock, err := fetchBlock(client, hash)
+		rawBlock, verboseBlock, err := fetchBlock(client, hash)
 		if err != nil {
 			log.Warnf("Could not fetch block %s: %s", hash, err)
 			return
 		}
-		err = addBlock(client, block, *rawBlock)
+		err = addBlock(client, rawBlock, *verboseBlock)
 		if err != nil {
 			log.Errorf("Could not insert block %s: %s", hash, err)
 			return
@@ -1023,11 +1018,11 @@ func handleMissingParent(client *jsonrpc.Client, missingParentHash string) error
 		if err != nil {
 			return errors.Errorf("Could not create hash: %s", err)
 		}
-		block, rawBlock, err := fetchBlock(client, hash)
+		rawBlock, verboseBlock, err := fetchBlock(client, hash)
 		if err != nil {
 			return errors.Errorf("Could not fetch block %s: %s", hash, err)
 		}
-		err = addBlock(client, block, *rawBlock)
+		err = addBlock(client, rawBlock, *verboseBlock)
 		if err != nil {
 			return errors.Errorf("Could not insert block %s: %s", hash, err)
 		}

btcjson/dagsvrcmds.go

@@ -157,18 +157,18 @@ func NewGetBlockCmd(hash string, verbose, verboseTx *bool, subnetworkID *string)
 
 // GetBlocksCmd defines the getBlocks JSON-RPC command.
 type GetBlocksCmd struct {
-	IncludeBlocks bool    `json:"includeBlocks"`
+	IncludeRawBlockData     bool    `json:"includeRawBlockData"`
-	VerboseBlocks bool    `json:"verboseBlocks"`
+	IncludeVerboseBlockData bool    `json:"includeVerboseBlockData"`
-	StartHash     *string `json:"startHash"`
+	StartHash               *string `json:"startHash"`
 }
 
 // NewGetBlocksCmd returns a new instance which can be used to issue a
 // GetGetBlocks JSON-RPC command.
-func NewGetBlocksCmd(includeBlocks bool, verboseBlocks bool, startHash *string) *GetBlocksCmd {
+func NewGetBlocksCmd(includeRawBlockData bool, includeVerboseBlockData bool, startHash *string) *GetBlocksCmd {
 	return &GetBlocksCmd{
-		IncludeBlocks: includeBlocks,
+		IncludeRawBlockData:     includeRawBlockData,
-		VerboseBlocks: verboseBlocks,
+		IncludeVerboseBlockData: includeVerboseBlockData,
-		StartHash:     startHash,
+		StartHash:               startHash,
 	}
 }
 

btcjson/dagsvrcmds_test.go

@@ -200,9 +200,9 @@ func TestDAGSvrCmds(t *testing.T) {
 			},
 			marshalled: `{"jsonrpc":"1.0","method":"getBlocks","params":[true,true,"123"],"id":1}`,
 			unmarshalled: &btcjson.GetBlocksCmd{
-				IncludeBlocks: true,
+				IncludeRawBlockData:     true,
-				VerboseBlocks: true,
+				IncludeVerboseBlockData: true,
-				StartHash:     btcjson.String("123"),
+				StartHash:               btcjson.String("123"),
 			},
 		},
 		{

btcjson/dagsvrresults.go

@@ -507,7 +507,7 @@ type GetChainFromBlockResult struct {
 
 // GetBlocksResult models the data from the getBlocks command.
 type GetBlocksResult struct {
-	Hashes    []string                `json:"hashes"`
+	Hashes        []string                `json:"hashes"`
-	Blocks    []string                `json:"blocks"`
+	RawBlocks     []string                `json:"rawBlocks"`
-	RawBlocks []GetBlockVerboseResult `json:"rawBlocks"`
+	VerboseBlocks []GetBlockVerboseResult `json:"verboseBlocks"`
 }

rpcclient/dag.go

@@ -133,15 +133,15 @@ func (r FutureGetBlocksResult) Receive() (*btcjson.GetBlocksResult, error) {
 // returned instance.
 //
 // See GetBlocks for the blocking version and more details.
-func (c *Client) GetBlocksAsync(includeBlocks bool, verboseBlocks bool, startHash *string) FutureGetBlocksResult {
+func (c *Client) GetBlocksAsync(includeRawBlockData bool, IncludeVerboseBlockData bool, startHash *string) FutureGetBlocksResult {
-	cmd := btcjson.NewGetBlocksCmd(includeBlocks, verboseBlocks, startHash)
+	cmd := btcjson.NewGetBlocksCmd(includeRawBlockData, IncludeVerboseBlockData, startHash)
 	return c.sendCmd(cmd)
 }
 
 // GetBlocks returns the blocks starting from startHash up to the virtual ordered
 // by blue score.
-func (c *Client) GetBlocks(includeBlocks bool, verboseBlocks bool, startHash *string) (*btcjson.GetBlocksResult, error) {
+func (c *Client) GetBlocks(includeRawBlockData bool, includeVerboseBlockData bool, startHash *string) (*btcjson.GetBlocksResult, error) {
-	return c.GetBlocksAsync(includeBlocks, verboseBlocks, startHash).Receive()
+	return c.GetBlocksAsync(includeRawBlockData, includeVerboseBlockData, startHash).Receive()
 }
 
 // FutureGetBlockVerboseResult is a future promise to deliver the result of a

server/rpc/handle_get_blocks.go

@@ -4,6 +4,7 @@ import (
 	"encoding/hex"
 	"github.com/daglabs/btcd/btcjson"
 	"github.com/daglabs/btcd/database"
+	"github.com/daglabs/btcd/util"
 	"github.com/daglabs/btcd/util/daghash"
 )
 
@@ -48,39 +49,41 @@ func handleGetBlocks(s *Server, cmd interface{}, closeChan <-chan struct{}) (int
 	}
 
 	result := &btcjson.GetBlocksResult{
-		Hashes: hashes,
+		Hashes:        hashes,
-		Blocks: nil,
+		RawBlocks:     nil,
+		VerboseBlocks: nil,
 	}
 
-	// If the user specified to include the blocks, collect them as well.
+	// Include more data if requested
-	if c.IncludeBlocks {
+	if c.IncludeRawBlockData || c.IncludeVerboseBlockData {
-		if c.VerboseBlocks {
+		blockBytesSlice, err := hashesToBlockBytes(s, blockHashes)
-			getBlockVerboseResults, err := hashesToGetBlockVerboseResults(s, blockHashes)
+		if err != nil {
+			return nil, err
+		}
+		if c.IncludeRawBlockData {
+			result.RawBlocks = blockBytesToStrings(blockBytesSlice)
+		}
+		if c.IncludeVerboseBlockData {
+			verboseBlocks, err := blockBytesToBlockVerboseResults(s, blockBytesSlice)
 			if err != nil {
 				return nil, err
 			}
-			result.RawBlocks = getBlockVerboseResults
+			result.VerboseBlocks = verboseBlocks
-		} else {
-			blocks, err := hashesToBlockStrings(s, blockHashes)
-			if err != nil {
-				return nil, err
-			}
-			result.Blocks = blocks
 		}
 	}
 
 	return result, nil
 }
 
-func hashesToBlockStrings(s *Server, hashes []*daghash.Hash) ([]string, error) {
+func hashesToBlockBytes(s *Server, hashes []*daghash.Hash) ([][]byte, error) {
-	blocks := make([]string, len(hashes))
+	blocks := make([][]byte, len(hashes))
 	err := s.cfg.DB.View(func(dbTx database.Tx) error {
 		for i, hash := range hashes {
 			blockBytes, err := dbTx.FetchBlock(hash)
 			if err != nil {
 				return err
 			}
-			blocks[i] = hex.EncodeToString(blockBytes)
+			blocks[i] = blockBytes
 		}
 		return nil
 	})
@@ -89,3 +92,27 @@ func hashesToBlockStrings(s *Server, hashes []*daghash.Hash) ([]string, error) {
 	}
 	return blocks, nil
 }
+
+func blockBytesToStrings(blockBytesSlice [][]byte) []string {
+	rawBlocks := make([]string, len(blockBytesSlice))
+	for i, blockBytes := range blockBytesSlice {
+		rawBlocks[i] = hex.EncodeToString(blockBytes)
+	}
+	return rawBlocks
+}
+
+func blockBytesToBlockVerboseResults(s *Server, blockBytesSlice [][]byte) ([]btcjson.GetBlockVerboseResult, error) {
+	verboseBlocks := make([]btcjson.GetBlockVerboseResult, len(blockBytesSlice))
+	for i, blockBytes := range blockBytesSlice {
+		block, err := util.NewBlockFromBytes(blockBytes)
+		if err != nil {
+			return nil, err
+		}
+		getBlockVerboseResult, err := buildGetBlockVerboseResult(s, block, false)
+		if err != nil {
+			return nil, err
+		}
+		verboseBlocks[i] = *getBlockVerboseResult
+	}
+	return verboseBlocks, nil
+}

server/rpc/rpcserverhelp.go

@@ -178,16 +178,16 @@ var helpDescsEnUS = map[string]string{
 	"getBlock--result0":    "Hex-encoded bytes of the serialized block",
 
 	// GetBlocksCmd help.
-	"getBlocks--synopsis":     "Return the blocks starting from startHash up to the virtual ordered by blue score.",
+	"getBlocks--synopsis":               "Return the blocks starting from startHash up to the virtual ordered by blue score.",
-	"getBlocks-includeBlocks": "If set to true - the block contents would be also included.",
+	"getBlocks-includeRawBlockData":     "If set to true - the raw block data would be also included.",
-	"getBlocks-verboseBlocks": "If set to true - each block is returned as a JSON object",
+	"getBlocks-includeVerboseBlockData": "If set to true - the verbose block data would also be included.",
-	"getBlocks-startHash":     "Hash of the block with the bottom blue score. If this hash is unknown - returns an error.",
+	"getBlocks-startHash":               "Hash of the block with the bottom blue score. If this hash is unknown - returns an error.",
-	"getBlocks--result0":      "Blocks starting from startHash. The result may contains up to 1000 blocks. For the remainder, call the command again with the bluest block's hash.",
+	"getBlocks--result0":                "Blocks starting from startHash. The result may contains up to 1000 blocks. For the remainder, call the command again with the bluest block's hash.",
 
 	// GetChainFromBlockResult help.
-	"getBlocksResult-hashes":    "List of hashes from StartHash (excluding StartHash) ordered by smallest blue score to greatest.",
+	"getBlocksResult-hashes":        "List of hashes from StartHash (excluding StartHash) ordered by smallest blue score to greatest.",
-	"getBlocksResult-blocks":    "If includeBlocks=true - contains the block contents. Otherwise - omitted.",
+	"getBlocksResult-rawBlocks":     "If includeBlocks=true - contains the block contents. Otherwise - omitted.",
-	"getBlocksResult-rawBlocks": "If includeBlocks=true and verboseBlocks=true - each block is returned as a JSON object. Otherwise - hex encoded string.",
+	"getBlocksResult-verboseBlocks": "If includeBlocks=true and verboseBlocks=true - each block is returned as a JSON object. Otherwise - hex encoded string.",
 
 	// GetBlockChainInfoCmd help.
 	"getBlockDagInfo--synopsis": "Returns information about the current blockDAG state and the status of any active soft-fork deployments.",