diff --git a/blockchain/chainio.go b/blockchain/chainio.go index 9d183a72..be5c9a20 100644 --- a/blockchain/chainio.go +++ b/blockchain/chainio.go @@ -1168,6 +1168,35 @@ func (b *BlockChain) createChainState() error { return err } +// DBBlockFromBytes deserializes a block fetched from the local database, +// tolerating trailing bytes rather than rejecting them outright. Databases +// written by older btcd versions may have persisted blocks with trailing +// bytes, and failing here would make such blocks permanently unreadable. +// Instead, any trailing bytes are logged, ignored, and excluded from the +// serialization cached in the returned block. +// +// This lenient parsing is only appropriate for blocks read back from the +// node's own database. Blocks from external sources (p2p, RPC) should be +// parsed with the strict btcutil.NewBlockFromBytes instead. +func DBBlockFromBytes(blockBytes []byte, hash chainhash.Hash) (*btcutil.Block, + error) { + + blockReader := bytes.NewReader(blockBytes) + var msgBlock wire.MsgBlock + if err := msgBlock.Deserialize(blockReader); err != nil { + return nil, err + } + if trailing := blockReader.Len(); trailing > 0 { + log.Debugf("Block %v has %d trailing bytes in the database; "+ + "ignoring them", hash, trailing) + blockBytes = blockBytes[:len(blockBytes)-trailing] + } + + // Cache the exact serialization on the block so downstream consumers + // of the raw bytes never observe the trailing bytes. + return btcutil.NewBlockFromBlockAndBytes(&msgBlock, blockBytes), nil +} + // initChainState attempts to load and initialize the chain state from the // database. When the db does not yet contain any chain state, both it and the // chain state are initialized to the genesis block. @@ -1374,35 +1403,6 @@ func dbFetchHeaderByHeight(dbTx database.Tx, height int32) (*wire.BlockHeader, e return dbFetchHeaderByHash(dbTx, hash) } -// DBBlockFromBytes deserializes a block fetched from the local database, -// tolerating trailing bytes rather than rejecting them outright. Databases -// written by older btcd versions may have persisted blocks with trailing -// bytes, and failing here would make such blocks permanently unreadable. -// Instead, any trailing bytes are logged, ignored, and excluded from the -// serialization cached in the returned block. -// -// This lenient parsing is only appropriate for blocks read back from the -// node's own database. Blocks from external sources (p2p, RPC) should be -// parsed with the strict btcutil.NewBlockFromBytes instead. -func DBBlockFromBytes(blockBytes []byte, hash chainhash.Hash) (*btcutil.Block, - error) { - - blockReader := bytes.NewReader(blockBytes) - var msgBlock wire.MsgBlock - if err := msgBlock.Deserialize(blockReader); err != nil { - return nil, err - } - if trailing := blockReader.Len(); trailing > 0 { - log.Warnf("Block %v has %d trailing bytes in the database; "+ - "ignoring them", hash, trailing) - blockBytes = blockBytes[:len(blockBytes)-trailing] - } - - // Cache the exact serialization on the block so downstream consumers - // of the raw bytes never observe the trailing bytes. - return btcutil.NewBlockFromBlockAndBytes(&msgBlock, blockBytes), nil -} - // dbFetchBlockByNode uses an existing database transaction to retrieve the // raw block for the provided node, deserialize it, and return a btcutil.Block // with the height set. diff --git a/blockchain/indexers/manager.go b/blockchain/indexers/manager.go index 28f608fb..533f0073 100644 --- a/blockchain/indexers/manager.go +++ b/blockchain/indexers/manager.go @@ -309,7 +309,9 @@ func (m *Manager) Init(chain *blockchain.BlockChain, interrupt <-chan struct{}) if err != nil { return err } - block, err = btcutil.NewBlockFromBytes(blockBytes) + block, err = blockchain.DBBlockFromBytes( + blockBytes, *hash, + ) if err != nil { return err } diff --git a/rpcserver.go b/rpcserver.go index fb5f0665..dcc97fc9 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -1082,9 +1082,17 @@ func handleGetBlock(s *rpcServer, cmd interface{}, closeChan <-chan struct{}) (i } var blkBytes []byte err = s.cfg.DB.View(func(dbTx database.Tx) error { - var err error - blkBytes, err = dbTx.FetchBlock(hash) - return err + dbBlockBytes, err := dbTx.FetchBlock(hash) + if err != nil { + return err + } + + // FetchBlock bytes are only valid for the lifetime of the + // transaction. Copy them before returning from the view so the + // lenient parser below owns its backing memory. + blkBytes = append([]byte(nil), dbBlockBytes...) + + return nil }) if err != nil { return nil, &btcjson.RPCError{ @@ -1092,6 +1100,21 @@ func handleGetBlock(s *rpcServer, cmd interface{}, closeChan <-chan struct{}) (i Message: "Block not found", } } + // Deserialize the block. Blocks read back from the node's own + // database are parsed leniently: trailing bytes persisted by older + // btcd versions are stripped rather than treated as an error, so the + // bytes served below are always the exact block serialization. + blk, err := blockchain.DBBlockFromBytes(blkBytes, *hash) + if err != nil { + context := "Failed to deserialize block" + return nil, internalRPCError(err.Error(), context) + } + blkBytes, err = blk.Bytes() + if err != nil { + context := "Failed to serialize block" + return nil, internalRPCError(err.Error(), context) + } + // If verbosity is 0, return the serialized block as a hex encoded string. if c.Verbosity != nil && *c.Verbosity == 0 { return hex.EncodeToString(blkBytes), nil @@ -1099,13 +1122,6 @@ func handleGetBlock(s *rpcServer, cmd interface{}, closeChan <-chan struct{}) (i // Otherwise, generate the JSON object and return it. - // Deserialize the block. - blk, err := btcutil.NewBlockFromBytes(blkBytes) - if err != nil { - context := "Failed to deserialize block" - return nil, internalRPCError(err.Error(), context) - } - // Get the block height from chain. blockHeight, err := s.cfg.Chain.BlockHeightByHash(hash) if err != nil { diff --git a/rpcserver_test.go b/rpcserver_test.go index e4625642..d3e9da06 100644 --- a/rpcserver_test.go +++ b/rpcserver_test.go @@ -10,6 +10,7 @@ import ( "github.com/btcsuite/btcd/btcutil/v2" "github.com/btcsuite/btcd/chaincfg/v2" "github.com/btcsuite/btcd/chainhash/v2" + "github.com/btcsuite/btcd/database" "github.com/btcsuite/btcd/mempool" "github.com/btcsuite/btcd/wire/v2" "github.com/stretchr/testify/mock" @@ -111,6 +112,60 @@ func blockHexWithTrailingByte(t *testing.T) string { return hex.EncodeToString(append(block.Bytes(), 0x00)) } +// invalidatingBlockDB clears fetched block bytes as soon as its managed view +// ends. This models database backends whose zero-copy buffers are only valid +// for the lifetime of a transaction. +type invalidatingBlockDB struct { + database.DB + blockBytes []byte +} + +// View runs the callback with a transaction backed by the configured block +// bytes, then invalidates those bytes before returning. +func (d *invalidatingBlockDB) View(fn func(database.Tx) error) error { + err := fn(&invalidatingBlockTx{blockBytes: d.blockBytes}) + clear(d.blockBytes) + + return err +} + +// invalidatingBlockTx returns the parent database's transaction-scoped block +// bytes. +type invalidatingBlockTx struct { + database.Tx + blockBytes []byte +} + +// FetchBlock returns bytes that are invalidated when the enclosing view ends. +func (t *invalidatingBlockTx) FetchBlock(*chainhash.Hash) ([]byte, error) { + return t.blockBytes, nil +} + +// TestHandleGetBlockCopiesTransactionBytes verifies getblock does not retain +// transaction-scoped database memory after its managed view ends. +func TestHandleGetBlockCopiesTransactionBytes(t *testing.T) { + t.Parallel() + + var serializedBlock bytes.Buffer + err := chaincfg.MainNetParams.GenesisBlock.Serialize(&serializedBlock) + require.NoError(t, err) + + wantBytes := serializedBlock.Bytes() + dbBytes := append([]byte(nil), wantBytes...) + dbBytes = append(dbBytes, 0x00) + db := &invalidatingBlockDB{blockBytes: dbBytes} + + verbosity := 0 + cmd := btcjson.NewGetBlockCmd( + chaincfg.MainNetParams.GenesisHash.String(), &verbosity, + ) + result, err := handleGetBlock( + &rpcServer{cfg: rpcserverConfig{DB: db}}, cmd, make(chan struct{}), + ) + require.NoError(t, err) + require.Equal(t, hex.EncodeToString(wantBytes), result) +} + // TestHandleSendRawTransactionRejectsTrailingBytes ensures sendrawtransaction // rejects byte strings that contain a valid transaction plus trailing data. func TestHandleSendRawTransactionRejectsTrailingBytes(t *testing.T) {