From a4a52ae24f2244a23f13f829aa3632dd0de52d6c Mon Sep 17 00:00:00 2001 From: Dave Collins <davec@conformal.com> Date: Fri, 17 Apr 2015 01:09:21 -0500 Subject: [PATCH] wire: Remove errs from BlockHeader/MsgBlock/MsgTx. This commit removes the error returns from the BlockHeader.BlockSha, MsgBlock.BlockSha, and MsgTx.TxSha functions since they can never fail and end up causing a lot of unneeded error checking throughout the code base. It also updates all call sites for the change. --- blockchain/common_test.go | 5 +---- blockmanager.go | 9 +-------- chaincfg/genesis_test.go | 20 ++++---------------- cpuminer.go | 2 +- database/ldb/insertremove_test.go | 2 +- database/ldb/operational_test.go | 2 +- database/ldb/tx.go | 2 +- database/memdb/memdb.go | 30 ++++++------------------------ database/memdb/memdb_test.go | 5 +---- log.go | 6 ++---- rpcserver.go | 8 +++----- txscript/example_test.go | 7 +------ txscript/internal_test.go | 21 +++++---------------- wire/blockheader.go | 7 ++----- wire/msgblock.go | 7 ++----- wire/msgblock_test.go | 5 +---- wire/msgtx.go | 8 ++------ wire/msgtx_test.go | 5 +---- 18 files changed, 36 insertions(+), 115 deletions(-) diff --git a/blockchain/common_test.go b/blockchain/common_test.go index 6494995b2..9044fdb94 100644 --- a/blockchain/common_test.go +++ b/blockchain/common_test.go @@ -176,10 +176,7 @@ func loadTxStore(filename string) (blockchain.TxStore, error) { txD.Tx = btcutil.NewTx(&msgTx) // Transaction hash. - txHash, err := msgTx.TxSha() - if err != nil { - return nil, err - } + txHash := msgTx.TxSha() txD.Hash = &txHash // Block height the transaction came from. diff --git a/blockmanager.go b/blockmanager.go index b8952145c..4aed37085 100644 --- a/blockmanager.go +++ b/blockmanager.go @@ -799,14 +799,7 @@ func (b *blockManager) handleHeadersMsg(hmsg *headersMsg) { receivedCheckpoint := false var finalHash *wire.ShaHash for _, blockHeader := range msg.Headers { - blockHash, err := blockHeader.BlockSha() - if err != nil { - bmgrLog.Warnf("Failed to compute hash of header "+ - "received from peer %s -- disconnecting", - hmsg.peer.addr) - hmsg.peer.Disconnect() - return - } + blockHash := blockHeader.BlockSha() finalHash = &blockHash // Ensure there is a previous header to compare against. diff --git a/chaincfg/genesis_test.go b/chaincfg/genesis_test.go index 393041084..f949a8ab0 100644 --- a/chaincfg/genesis_test.go +++ b/chaincfg/genesis_test.go @@ -30,10 +30,7 @@ func TestGenesisBlock(t *testing.T) { } // Check hash of the block against expected hash. - hash, err := chaincfg.MainNetParams.GenesisBlock.BlockSha() - if err != nil { - t.Fatalf("BlockSha: %v", err) - } + hash := chaincfg.MainNetParams.GenesisBlock.BlockSha() if !chaincfg.MainNetParams.GenesisHash.IsEqual(&hash) { t.Fatalf("TestGenesisBlock: Genesis block hash does not "+ "appear valid - got %v, want %v", spew.Sdump(hash), @@ -60,10 +57,7 @@ func TestRegTestGenesisBlock(t *testing.T) { } // Check hash of the block against expected hash. - hash, err := chaincfg.RegressionNetParams.GenesisBlock.BlockSha() - if err != nil { - t.Errorf("BlockSha: %v", err) - } + hash := chaincfg.RegressionNetParams.GenesisBlock.BlockSha() if !chaincfg.RegressionNetParams.GenesisHash.IsEqual(&hash) { t.Fatalf("TestRegTestGenesisBlock: Genesis block hash does "+ "not appear valid - got %v, want %v", spew.Sdump(hash), @@ -90,10 +84,7 @@ func TestTestNet3GenesisBlock(t *testing.T) { } // Check hash of the block against expected hash. - hash, err := chaincfg.TestNet3Params.GenesisBlock.BlockSha() - if err != nil { - t.Fatalf("BlockSha: %v", err) - } + hash := chaincfg.TestNet3Params.GenesisBlock.BlockSha() if !chaincfg.TestNet3Params.GenesisHash.IsEqual(&hash) { t.Fatalf("TestTestNet3GenesisBlock: Genesis block hash does "+ "not appear valid - got %v, want %v", spew.Sdump(hash), @@ -120,10 +111,7 @@ func TestSimNetGenesisBlock(t *testing.T) { } // Check hash of the block against expected hash. - hash, err := chaincfg.SimNetParams.GenesisBlock.BlockSha() - if err != nil { - t.Fatalf("BlockSha: %v", err) - } + hash := chaincfg.SimNetParams.GenesisBlock.BlockSha() if !chaincfg.SimNetParams.GenesisHash.IsEqual(&hash) { t.Fatalf("TestSimNetGenesisBlock: Genesis block hash does "+ "not appear valid - got %v, want %v", spew.Sdump(hash), diff --git a/cpuminer.go b/cpuminer.go index 45ce7cfa2..5923e05a3 100644 --- a/cpuminer.go +++ b/cpuminer.go @@ -233,7 +233,7 @@ func (m *CPUMiner) solveBlock(msgBlock *wire.MsgBlock, blockHeight int64, // increment the number of hashes completed for each // attempt accordingly. header.Nonce = i - hash, _ := header.BlockSha() + hash := header.BlockSha() hashesCompleted += 2 // The block is solved when the new block hash is less diff --git a/database/ldb/insertremove_test.go b/database/ldb/insertremove_test.go index 9640d45ae..06b702dff 100644 --- a/database/ldb/insertremove_test.go +++ b/database/ldb/insertremove_test.go @@ -91,7 +91,7 @@ endtest: t.Errorf("referenced tx not found %v ", origintxsha) } } - txshaname, _ := tx.TxSha() + txshaname := tx.TxSha() txlookupList = append(txlookupList, &txshaname) txOutList = append(txOutList, &txshaname) } diff --git a/database/ldb/operational_test.go b/database/ldb/operational_test.go index 1219b32e5..5fa3f159a 100644 --- a/database/ldb/operational_test.go +++ b/database/ldb/operational_test.go @@ -367,7 +367,7 @@ func testBackout(t *testing.T) { block := testDb.blocks[119] mblock := block.MsgBlock() - txsha, err := mblock.Transactions[0].TxSha() + txsha := mblock.Transactions[0].TxSha() exists, err := testDb.db.ExistsTxSha(&txsha) if err != nil { t.Errorf("ExistsTxSha: unexpected error %v ", err) diff --git a/database/ldb/tx.go b/database/ldb/tx.go index 7fff041ed..72041b320 100644 --- a/database/ldb/tx.go +++ b/database/ldb/tx.go @@ -482,7 +482,7 @@ func (db *LevelDb) FetchTxsForAddr(addr btcutil.Address, skip int, continue } - txSha, _ := tx.TxSha() + txSha := tx.TxSha() txReply := &database.TxListReply{Sha: &txSha, Tx: tx, BlkSha: blkSha, Height: blkHeight, TxSpent: []bool{}, Err: err} diff --git a/database/memdb/memdb.go b/database/memdb/memdb.go index 9c2073ece..56c59dac7 100644 --- a/database/memdb/memdb.go +++ b/database/memdb/memdb.go @@ -184,7 +184,7 @@ func (db *MemDb) DropAfterBlockBySha(sha *wire.ShaHash) error { transactions := db.blocks[i].Transactions for j := len(transactions) - 1; j >= 0; j-- { tx := transactions[j] - txHash, _ := tx.TxSha() + txHash := tx.TxSha() db.removeTx(tx, &txHash) } @@ -291,11 +291,7 @@ func (db *MemDb) FetchBlockShaByHeight(height int64) (*wire.ShaHash, error) { } msgBlock := db.blocks[height] - blockHash, err := msgBlock.BlockSha() - if err != nil { - return nil, err - } - + blockHash := msgBlock.BlockSha() return &blockHash, nil } @@ -337,10 +333,7 @@ func (db *MemDb) FetchHeightRange(startHeight, endHeight int64) ([]wire.ShaHash, } msgBlock := db.blocks[i] - blockHash, err := msgBlock.BlockSha() - if err != nil { - return nil, err - } + blockHash := msgBlock.BlockSha() hashList = append(hashList, blockHash) } @@ -390,10 +383,7 @@ func (db *MemDb) FetchTxBySha(txHash *wire.ShaHash) ([]*database.TxListReply, er replyList := make([]*database.TxListReply, len(txns)) for i, txD := range txns { msgBlock := db.blocks[txD.blockHeight] - blockSha, err := msgBlock.BlockSha() - if err != nil { - return nil, err - } + blockSha := msgBlock.BlockSha() spentBuf := make([]bool, len(txD.spentBuf)) copy(spentBuf, txD.spentBuf) @@ -455,11 +445,7 @@ func (db *MemDb) fetchTxByShaList(txShaList []*wire.ShaHash, includeSpent bool) // the reply error appropriately and go to the next // requested transaction if anything goes wrong. msgBlock := db.blocks[txD.blockHeight] - blockSha, err := msgBlock.BlockSha() - if err != nil { - reply.Err = err - continue - } + blockSha := msgBlock.BlockSha() // Make a copy of the spent buf to return so the caller // can't accidentally modify it. @@ -685,11 +671,7 @@ func (db *MemDb) NewestSha() (*wire.ShaHash, int64, error) { return &zeroHash, -1, nil } - blockSha, err := db.blocks[numBlocks-1].BlockSha() - if err != nil { - return nil, -1, err - } - + blockSha := db.blocks[numBlocks-1].BlockSha() return &blockSha, int64(numBlocks - 1), nil } diff --git a/database/memdb/memdb_test.go b/database/memdb/memdb_test.go index 02dc026ff..f1d72bb35 100644 --- a/database/memdb/memdb_test.go +++ b/database/memdb/memdb_test.go @@ -55,10 +55,7 @@ func TestClosed(t *testing.T) { } genesisCoinbaseTx := chaincfg.MainNetParams.GenesisBlock.Transactions[0] - coinbaseHash, err := genesisCoinbaseTx.TxSha() - if err != nil { - t.Errorf("TxSha: unexpected error %v", err) - } + coinbaseHash := genesisCoinbaseTx.TxSha() if _, err := db.ExistsTxSha(&coinbaseHash); err != memdb.ErrDbClosed { t.Errorf("ExistsTxSha: unexpected error %v", err) } diff --git a/log.go b/log.go index 1f010761a..6ab447c02 100644 --- a/log.go +++ b/log.go @@ -314,15 +314,13 @@ func messageSummary(msg wire.Message) string { // No summary. case *wire.MsgTx: - hash, _ := msg.TxSha() return fmt.Sprintf("hash %s, %d inputs, %d outputs, lock %s", - hash, len(msg.TxIn), len(msg.TxOut), + msg.TxSha(), len(msg.TxIn), len(msg.TxOut), formatLockTime(msg.LockTime)) case *wire.MsgBlock: header := &msg.Header - hash, _ := msg.BlockSha() - return fmt.Sprintf("hash %s, ver %d, %d tx, %s", hash, + return fmt.Sprintf("hash %s, ver %d, %d tx, %s", msg.BlockSha(), header.Version, len(msg.Transactions), header.Timestamp) case *wire.MsgInv: diff --git a/rpcserver.go b/rpcserver.go index 2987b5f83..402f242f9 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -726,11 +726,10 @@ func handleDecodeRawTransaction(s *rpcServer, cmd interface{}, closeChan <-chan Message: "TX decode failed: " + err.Error(), } } - txHash, _ := mtx.TxSha() // Create and return the result. txReply := btcjson.TxRawDecodeResult{ - Txid: txHash.String(), + Txid: mtx.TxSha().String(), Version: mtx.Version, Locktime: mtx.LockTime, Vin: createVinList(&mtx), @@ -1357,7 +1356,7 @@ func (state *gbtWorkState) blockTemplateResult(useCoinbaseValue bool, submitOld transactions := make([]btcjson.GetBlockTemplateResultTx, 0, numTx-1) txIndex := make(map[wire.ShaHash]int64, numTx) for i, tx := range msgBlock.Transactions { - txHash, _ := tx.TxSha() + txHash := tx.TxSha() txIndex[txHash] = int64(i) // Skip the coinbase transaction. @@ -1441,7 +1440,6 @@ func (state *gbtWorkState) blockTemplateResult(useCoinbaseValue bool, submitOld // Serialize the transaction for conversion to hex. tx := msgBlock.Transactions[0] - txHash, _ := tx.TxSha() txBuf := bytes.NewBuffer(make([]byte, 0, tx.SerializeSize())) if err := tx.Serialize(txBuf); err != nil { context := "Failed to serialize transaction" @@ -1450,7 +1448,7 @@ func (state *gbtWorkState) blockTemplateResult(useCoinbaseValue bool, submitOld resultTx := btcjson.GetBlockTemplateResultTx{ Data: hex.EncodeToString(txBuf.Bytes()), - Hash: txHash.String(), + Hash: tx.TxSha().String(), Depends: []int64{}, Fee: template.fees[0], SigOps: template.sigOpCounts[0], diff --git a/txscript/example_test.go b/txscript/example_test.go index 1c39a1bd2..64d0a987e 100644 --- a/txscript/example_test.go +++ b/txscript/example_test.go @@ -111,12 +111,7 @@ func ExampleSignTxOutput() { } txOut := wire.NewTxOut(100000000, pkScript) originTx.AddTxOut(txOut) - - originTxHash, err := originTx.TxSha() - if err != nil { - fmt.Println(err) - return - } + originTxHash := originTx.TxSha() // Create the transaction to redeem the fake transaction. redeemTx := wire.NewMsgTx() diff --git a/txscript/internal_test.go b/txscript/internal_test.go index 14bf5caa8..c1c3bd80b 100644 --- a/txscript/internal_test.go +++ b/txscript/internal_test.go @@ -4140,7 +4140,7 @@ func ParseShortForm(script string) ([]byte, error) { // createSpendTx generates a basic spending transaction given the passed // signature and public key scripts. -func createSpendingTx(sigScript, pkScript []byte) (*wire.MsgTx, error) { +func createSpendingTx(sigScript, pkScript []byte) *wire.MsgTx { coinbaseTx := wire.NewMsgTx() outPoint := wire.NewOutPoint(&wire.ShaHash{}, ^uint32(0)) @@ -4150,10 +4150,7 @@ func createSpendingTx(sigScript, pkScript []byte) (*wire.MsgTx, error) { coinbaseTx.AddTxOut(txOut) spendingTx := wire.NewMsgTx() - coinbaseTxSha, err := coinbaseTx.TxSha() - if err != nil { - return nil, err - } + coinbaseTxSha := coinbaseTx.TxSha() outPoint = wire.NewOutPoint(&coinbaseTxSha, 0) txIn = wire.NewTxIn(outPoint, sigScript) txOut = wire.NewTxOut(0, nil) @@ -4161,7 +4158,7 @@ func createSpendingTx(sigScript, pkScript []byte) (*wire.MsgTx, error) { spendingTx.AddTxIn(txIn) spendingTx.AddTxOut(txOut) - return spendingTx, nil + return spendingTx } func TestBitcoindInvalidTests(t *testing.T) { @@ -4204,11 +4201,7 @@ func TestBitcoindInvalidTests(t *testing.T) { t.Errorf("%s: %v", name, err) continue } - tx, err := createSpendingTx(scriptSig, scriptPubKey) - if err != nil { - t.Errorf("createSpendingTx failed on test %s: %v", name, err) - continue - } + tx := createSpendingTx(scriptSig, scriptPubKey) s, err := NewScript(scriptSig, scriptPubKey, 0, tx, flags) if err == nil { if err := s.Execute(); err == nil { @@ -4260,11 +4253,7 @@ func TestBitcoindValidTests(t *testing.T) { t.Errorf("%s: %v", name, err) continue } - tx, err := createSpendingTx(scriptSig, scriptPubKey) - if err != nil { - t.Errorf("createSpendingTx failed on test %s: %v", name, err) - continue - } + tx := createSpendingTx(scriptSig, scriptPubKey) s, err := NewScript(scriptSig, scriptPubKey, 0, tx, flags) if err != nil { t.Errorf("%s failed to create script: %v", name, err) diff --git a/wire/blockheader.go b/wire/blockheader.go index 99e1eeb96..240f4868d 100644 --- a/wire/blockheader.go +++ b/wire/blockheader.go @@ -45,7 +45,7 @@ type BlockHeader struct { const blockHeaderLen = 80 // BlockSha computes the block identifier hash for the given block header. -func (h *BlockHeader) BlockSha() (ShaHash, error) { +func (h *BlockHeader) BlockSha() ShaHash { // Encode the header and double sha256 everything prior to the number of // transactions. Ignore the error returns since there is no way the // encode could fail except being out of memory which would cause a @@ -53,10 +53,7 @@ func (h *BlockHeader) BlockSha() (ShaHash, error) { var buf bytes.Buffer _ = writeBlockHeader(&buf, 0, h) - // Even though this function can't currently fail, it still returns - // a potential error to help future proof the API should a failure - // become possible. - return DoubleSha256SH(buf.Bytes()), nil + return DoubleSha256SH(buf.Bytes()) } // Deserialize decodes a block header from r into the receiver using a format diff --git a/wire/msgblock.go b/wire/msgblock.go index b97ae19c0..4f2c71c18 100644 --- a/wire/msgblock.go +++ b/wire/msgblock.go @@ -225,7 +225,7 @@ func (msg *MsgBlock) MaxPayloadLength(pver uint32) uint32 { } // BlockSha computes the block identifier hash for this block. -func (msg *MsgBlock) BlockSha() (ShaHash, error) { +func (msg *MsgBlock) BlockSha() ShaHash { return msg.Header.BlockSha() } @@ -233,10 +233,7 @@ func (msg *MsgBlock) BlockSha() (ShaHash, error) { func (msg *MsgBlock) TxShas() ([]ShaHash, error) { shaList := make([]ShaHash, 0, len(msg.Transactions)) for _, tx := range msg.Transactions { - // Ignore error here since TxSha can't fail in the current - // implementation except due to run-time panics. - sha, _ := tx.TxSha() - shaList = append(shaList, sha) + shaList = append(shaList, tx.TxSha()) } return shaList, nil } diff --git a/wire/msgblock_test.go b/wire/msgblock_test.go index 1db706262..11774684c 100644 --- a/wire/msgblock_test.go +++ b/wire/msgblock_test.go @@ -101,10 +101,7 @@ func TestBlockSha(t *testing.T) { } // Ensure the hash produced is expected. - blockHash, err := blockOne.BlockSha() - if err != nil { - t.Errorf("BlockSha: %v", err) - } + blockHash := blockOne.BlockSha() if !blockHash.IsEqual(wantHash) { t.Errorf("BlockSha: wrong hash - got %v, want %v", spew.Sprint(blockHash), spew.Sprint(wantHash)) diff --git a/wire/msgtx.go b/wire/msgtx.go index f24ae0988..6ec51056a 100644 --- a/wire/msgtx.go +++ b/wire/msgtx.go @@ -166,18 +166,14 @@ func (msg *MsgTx) AddTxOut(to *TxOut) { } // TxSha generates the ShaHash name for the transaction. -func (msg *MsgTx) TxSha() (ShaHash, error) { +func (msg *MsgTx) TxSha() ShaHash { // Encode the transaction and calculate double sha256 on the result. // Ignore the error returns since the only way the encode could fail // is being out of memory or due to nil pointers, both of which would // cause a run-time panic. buf := bytes.NewBuffer(make([]byte, 0, msg.SerializeSize())) _ = msg.Serialize(buf) - - // Even though this function can't currently fail, it still returns - // a potential error to help future proof the API should a failure - // become possible. - return DoubleSha256SH(buf.Bytes()), nil + return DoubleSha256SH(buf.Bytes()) } // Copy creates a deep copy of a transaction so that the original does not get diff --git a/wire/msgtx_test.go b/wire/msgtx_test.go index e1b64d33f..78e7fed07 100644 --- a/wire/msgtx_test.go +++ b/wire/msgtx_test.go @@ -169,10 +169,7 @@ func TestTxSha(t *testing.T) { msgTx.LockTime = 0 // Ensure the hash produced is expected. - txHash, err := msgTx.TxSha() - if err != nil { - t.Errorf("TxSha: %v", err) - } + txHash := msgTx.TxSha() if !txHash.IsEqual(wantHash) { t.Errorf("TxSha: wrong hash - got %v, want %v", spew.Sprint(txHash), spew.Sprint(wantHash))