Remove BlockHeader.TxnCount field.
This commit removes the TxnCount field from the BlockHeader type and updates the tests accordingly. Note that this change does not affect the actual wire protocol encoding in any way. The reason the field has been removed is it really doesn't belong there even though the wire protocol wiki entry on the official bitcoin wiki implies it does. The implication is an artifact from the way the reference implementation serializes headers (MsgHeaders) messages. It includes the transaction count, which is naturally always 0 for headers, along with every header. However, in reality, a block header does not include the transaction count. This can be evidenced by looking at how a block hash is calculated. It is only up to and including the Nonce field (a total of 80 bytes). From an API standpoint, having the field as part of the BlockHeader type results in several odd cases. For example, the transaction count for MsgBlocks (the only place that actually has a real transaction count since MsgHeaders does not) is available by taking the len of the Transactions slice. As such, having the extra field in the BlockHeader is really a useless field that could potentially get out of sync and cause the encode to fail. Another example is related to deserializing a block header from the database in order to serve it in response to a getheaders (MsgGetheaders) request. If a block header is assumed to have the transaction count as a part of it, then derserializing a block header not only consumes more than the 80 bytes that actually comprise the header as stated above, but you then need to change the transaction count to 0 before sending the headers (MsgHeaders) message. So, not only are you reading and deserializing more bytes than needed, but worse, you generally have to make a copy of it so you can change the transaction count without busting cached headers. This is part 1 of #13.
This commit is contained in:
parent
6c7f45fdb7
commit
144822d4bf
7 changed files with 49 additions and 52 deletions
|
@ -14,8 +14,8 @@ import (
|
|||
const BlockVersion uint32 = 2
|
||||
|
||||
// Version 4 bytes + Timestamp 4 bytes + Bits 4 bytes + Nonce 4 bytes +
|
||||
// TxnCount (varInt) + PrevBlock and MerkleRoot hashes.
|
||||
const maxBlockHeaderPayload = 16 + maxVarIntPayload + (HashSize * 2)
|
||||
// PrevBlock and MerkleRoot hashes.
|
||||
const maxBlockHeaderPayload = 16 + (HashSize * 2)
|
||||
|
||||
// BlockHeader defines information about a block and is used in the bitcoin
|
||||
// block (MsgBlock) and headers (MsgHeaders) messages.
|
||||
|
@ -38,11 +38,6 @@ type BlockHeader struct {
|
|||
|
||||
// Nonce used to generate the block.
|
||||
Nonce uint32
|
||||
|
||||
// Number of transactions in the block. For the bitcoin headers
|
||||
// (MsgHeaders) message, this must be 0. This is encoded as a variable
|
||||
// length integer on the wire.
|
||||
TxnCount uint64
|
||||
}
|
||||
|
||||
// blockHashLen is a constant that represents how much of the block header is
|
||||
|
@ -81,7 +76,6 @@ func NewBlockHeader(prevHash *ShaHash, merkleRootHash *ShaHash, bits uint32,
|
|||
Timestamp: time.Now(),
|
||||
Bits: bits,
|
||||
Nonce: nonce,
|
||||
TxnCount: 0,
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -95,12 +89,6 @@ func readBlockHeader(r io.Reader, pver uint32, bh *BlockHeader) error {
|
|||
}
|
||||
bh.Timestamp = time.Unix(int64(sec), 0)
|
||||
|
||||
count, err := readVarInt(r, pver)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
bh.TxnCount = count
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
|
@ -113,10 +101,5 @@ func writeBlockHeader(w io.Writer, pver uint32, bh *BlockHeader) error {
|
|||
return err
|
||||
}
|
||||
|
||||
err = writeVarInt(w, pver, bh.TxnCount)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
|
|
@ -61,7 +61,6 @@ func TestBlockHeaderWire(t *testing.T) {
|
|||
Timestamp: time.Unix(0x495fab29, 0), // 2009-01-03 12:15:05 -0600 CST
|
||||
Bits: bits,
|
||||
Nonce: nonce,
|
||||
TxnCount: 0,
|
||||
}
|
||||
|
||||
// baseBlockHdrEncoded is the wire encoded bytes of baseBlockHdr.
|
||||
|
@ -78,7 +77,6 @@ func TestBlockHeaderWire(t *testing.T) {
|
|||
0x29, 0xab, 0x5f, 0x49, // Timestamp
|
||||
0xff, 0xff, 0x00, 0x1d, // Bits
|
||||
0xf3, 0xe0, 0x01, 0x00, // Nonce
|
||||
0x00, // TxnCount Varint
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
|
|
|
@ -80,7 +80,6 @@ var GenesisBlock = MsgBlock{
|
|||
Timestamp: time.Unix(0x495fab29, 0), // 2009-01-03 18:15:05 +0000 UTC
|
||||
Bits: 0x1d00ffff, // 486604799 [00000000ffff0000000000000000000000000000000000000000000000000000]
|
||||
Nonce: 0x7c2bac1d, // 2083236893
|
||||
TxnCount: 1,
|
||||
},
|
||||
Transactions: []*MsgTx{&genesisCoinbaseTx},
|
||||
}
|
||||
|
@ -109,7 +108,6 @@ var TestNetGenesisBlock = MsgBlock{
|
|||
Timestamp: time.Unix(1296688602, 0), // 2011-02-02 23:16:42 +0000 UTC
|
||||
Bits: 0x207fffff, // 545259519 [7fffff0000000000000000000000000000000000000000000000000000000000]
|
||||
Nonce: 2,
|
||||
TxnCount: 1,
|
||||
},
|
||||
Transactions: []*MsgTx{&genesisCoinbaseTx},
|
||||
}
|
||||
|
@ -138,7 +136,6 @@ var TestNet3GenesisBlock = MsgBlock{
|
|||
Timestamp: time.Unix(1296688602, 0), // 2011-02-02 23:16:42 +0000 UTC
|
||||
Bits: 0x1d00ffff, // 486604799 [00000000ffff0000000000000000000000000000000000000000000000000000]
|
||||
Nonce: 0x18aea41a, // 414098458
|
||||
TxnCount: 1,
|
||||
},
|
||||
Transactions: []*MsgTx{&genesisCoinbaseTx},
|
||||
}
|
||||
|
|
32
msgblock.go
32
msgblock.go
|
@ -49,22 +49,18 @@ type MsgBlock struct {
|
|||
Transactions []*MsgTx
|
||||
}
|
||||
|
||||
// AddTransaction adds a transaction to the message and updates Header.TxnCount
|
||||
// accordingly.
|
||||
// AddTransaction adds a transaction to the message.
|
||||
func (msg *MsgBlock) AddTransaction(tx *MsgTx) error {
|
||||
// TODO: Return error if adding the transaction would make the message
|
||||
// too large.
|
||||
msg.Transactions = append(msg.Transactions, tx)
|
||||
msg.Header.TxnCount = uint64(len(msg.Transactions))
|
||||
return nil
|
||||
|
||||
}
|
||||
|
||||
// ClearTransactions removes all transactions from the message and updates
|
||||
// Header.TxnCount accordingly.
|
||||
// ClearTransactions removes all transactions from the message.
|
||||
func (msg *MsgBlock) ClearTransactions() {
|
||||
msg.Transactions = make([]*MsgTx, 0, defaultTransactionAlloc)
|
||||
msg.Header.TxnCount = 0
|
||||
}
|
||||
|
||||
// BtcDecode decodes r using the bitcoin protocol encoding into the receiver.
|
||||
|
@ -77,10 +73,14 @@ func (msg *MsgBlock) BtcDecode(r io.Reader, pver uint32) error {
|
|||
return err
|
||||
}
|
||||
|
||||
txCount, err := readVarInt(r, pver)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// Prevent more transactions than could possibly fit into a block.
|
||||
// It would be possible to cause memory exhaustion and panics without
|
||||
// a sane upper bound on this count.
|
||||
txCount := msg.Header.TxnCount
|
||||
if txCount > maxTxPerBlock {
|
||||
str := fmt.Sprintf("too many transactions to fit into a block "+
|
||||
"[count %d, max %d]", txCount, maxTxPerBlock)
|
||||
|
@ -130,10 +130,14 @@ func (msg *MsgBlock) DeserializeTxLoc(r *bytes.Buffer) ([]TxLoc, error) {
|
|||
return nil, err
|
||||
}
|
||||
|
||||
txCount, err := readVarInt(r, 0)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Prevent more transactions than could possibly fit into a block.
|
||||
// It would be possible to cause memory exhaustion and panics without
|
||||
// a sane upper bound on this count.
|
||||
txCount := msg.Header.TxnCount
|
||||
if txCount > maxTxPerBlock {
|
||||
str := fmt.Sprintf("too many transactions to fit into a block "+
|
||||
"[count %d, max %d]", txCount, maxTxPerBlock)
|
||||
|
@ -163,13 +167,16 @@ func (msg *MsgBlock) DeserializeTxLoc(r *bytes.Buffer) ([]TxLoc, error) {
|
|||
// See Serialize for encoding blocks to be stored to disk, such as in a
|
||||
// database, as opposed to encoding blocks for the wire.
|
||||
func (msg *MsgBlock) BtcEncode(w io.Writer, pver uint32) error {
|
||||
msg.Header.TxnCount = uint64(len(msg.Transactions))
|
||||
|
||||
err := writeBlockHeader(w, pver, &msg.Header)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
err = writeVarInt(w, pver, uint64(len(msg.Transactions)))
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
for _, tx := range msg.Transactions {
|
||||
err = tx.BtcEncode(w, pver)
|
||||
if err != nil {
|
||||
|
@ -205,8 +212,9 @@ func (msg *MsgBlock) Command() string {
|
|||
// MaxPayloadLength returns the maximum length the payload can be for the
|
||||
// receiver. This is part of the Message interface implementation.
|
||||
func (msg *MsgBlock) MaxPayloadLength(pver uint32) uint32 {
|
||||
// Block header at 81 bytes + max transactions which can vary up to the
|
||||
// maxBlockPayload (including the block header).
|
||||
// Block header at 80 bytes + transaction count + max transactions
|
||||
// which can vary up to the MaxBlockPayload (including the block header
|
||||
// and transaction count).
|
||||
return MaxBlockPayload
|
||||
}
|
||||
|
||||
|
|
|
@ -468,7 +468,6 @@ var blockOne = btcwire.MsgBlock{
|
|||
Timestamp: time.Unix(0x4966bc61, 0), // 2009-01-08 20:54:25 -0600 CST
|
||||
Bits: 0x1d00ffff, // 486604799
|
||||
Nonce: 0x9962e301, // 2573394689
|
||||
TxnCount: 1,
|
||||
},
|
||||
Transactions: []*btcwire.MsgTx{
|
||||
&btcwire.MsgTx{
|
||||
|
|
|
@ -57,10 +57,15 @@ func (msg *MsgHeaders) BtcDecode(r io.Reader, pver uint32) error {
|
|||
return err
|
||||
}
|
||||
|
||||
txCount, err := readVarInt(r, pver)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// Ensure the transaction count is zero for headers.
|
||||
if bh.TxnCount > 0 {
|
||||
if txCount > 0 {
|
||||
str := fmt.Sprintf("block headers may not contain "+
|
||||
"transactions [count %v]", bh.TxnCount)
|
||||
"transactions [count %v]", txCount)
|
||||
return messageError("MsgHeaders.BtcDecode", str)
|
||||
}
|
||||
msg.AddBlockHeader(&bh)
|
||||
|
@ -86,17 +91,20 @@ func (msg *MsgHeaders) BtcEncode(w io.Writer, pver uint32) error {
|
|||
}
|
||||
|
||||
for _, bh := range msg.Headers {
|
||||
// Ensure block headers do not contain a transaction count.
|
||||
if bh.TxnCount > 0 {
|
||||
str := fmt.Sprintf("block headers may not contain "+
|
||||
"transactions [count %v]", bh.TxnCount)
|
||||
return messageError("MsgHeaders.BtcEncode", str)
|
||||
}
|
||||
|
||||
err := writeBlockHeader(w, pver, bh)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// The wire protocol encoding always includes a 0 for the number
|
||||
// of transactions on header messages. This is really just an
|
||||
// artifact of the way the original implementation serializes
|
||||
// block headers, but it is required.
|
||||
err = writeVarInt(w, pver, 0)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
return nil
|
||||
|
@ -111,8 +119,10 @@ func (msg *MsgHeaders) Command() string {
|
|||
// MaxPayloadLength returns the maximum length the payload can be for the
|
||||
// receiver. This is part of the Message interface implementation.
|
||||
func (msg *MsgHeaders) MaxPayloadLength(pver uint32) uint32 {
|
||||
// Num headers (varInt) + max allowed headers.
|
||||
return maxVarIntPayload + (maxBlockHeaderPayload * MaxBlockHeadersPerMsg)
|
||||
// Num headers (varInt) + max allowed headers (header length + 1 byte
|
||||
// for the number of transactions which is always 0).
|
||||
return maxVarIntPayload + ((maxBlockHeaderPayload + 1) *
|
||||
MaxBlockHeadersPerMsg)
|
||||
}
|
||||
|
||||
// NewMsgHeaders returns a new bitcoin headers message that conforms to the
|
||||
|
|
|
@ -26,8 +26,9 @@ func TestHeaders(t *testing.T) {
|
|||
}
|
||||
|
||||
// Ensure max payload is expected value for latest protocol version.
|
||||
// Num headers (varInt) + max allowed headers.
|
||||
wantPayload := uint32(178009)
|
||||
// Num headers (varInt) + max allowed headers (header length + 1 byte
|
||||
// for the number of transactions which is always 0).
|
||||
wantPayload := uint32(162009)
|
||||
maxPayload := msg.MaxPayloadLength(pver)
|
||||
if maxPayload != wantPayload {
|
||||
t.Errorf("MaxPayloadLength: wrong max payload length for "+
|
||||
|
@ -262,7 +263,6 @@ func TestHeadersWireErrors(t *testing.T) {
|
|||
bhTrans := btcwire.NewBlockHeader(&hash, &merkleHash, bits, nonce)
|
||||
bhTrans.Version = blockOne.Header.Version
|
||||
bhTrans.Timestamp = blockOne.Header.Timestamp
|
||||
bhTrans.TxnCount = 1
|
||||
|
||||
transHeader := btcwire.NewMsgHeaders()
|
||||
transHeader.AddBlockHeader(bhTrans)
|
||||
|
@ -298,8 +298,10 @@ func TestHeadersWireErrors(t *testing.T) {
|
|||
{oneHeader, oneHeaderEncoded, pver, 5, io.ErrShortWrite, io.EOF},
|
||||
// Force error with greater than max headers.
|
||||
{maxHeaders, maxHeadersEncoded, pver, 3, btcwireErr, btcwireErr},
|
||||
// Force error with number of transactions.
|
||||
{transHeader, transHeaderEncoded, pver, 81, io.ErrShortWrite, io.EOF},
|
||||
// Force error with included transactions.
|
||||
{transHeader, transHeaderEncoded, pver, len(transHeaderEncoded), btcwireErr, btcwireErr},
|
||||
{transHeader, transHeaderEncoded, pver, len(transHeaderEncoded), nil, btcwireErr},
|
||||
}
|
||||
|
||||
t.Logf("Running %d tests", len(tests))
|
||||
|
|
Loading…
Reference in a new issue