From 70c14042d8376bc461a0c1acf424a6b5486aa18b Mon Sep 17 00:00:00 2001 From: David Hill Date: Mon, 29 Apr 2019 15:41:06 -0400 Subject: [PATCH] multi: fix recent govet findings add govet to linter tests. --- blockchain/chainio.go | 2 +- blockmanager.go | 54 ++++++------ mining.go | 199 +++++++++++++++++++++--------------------- rpcserver.go | 10 +-- run_tests.sh | 8 +- wire/msgtx.go | 2 +- 6 files changed, 134 insertions(+), 141 deletions(-) diff --git a/blockchain/chainio.go b/blockchain/chainio.go index ceb30f37..f25c925b 100644 --- a/blockchain/chainio.go +++ b/blockchain/chainio.go @@ -1576,7 +1576,7 @@ func (b *BlockChain) initChainState() error { } // The database bucket for the versioning information is missing. - if dbInfo == nil && err == nil { + if dbInfo == nil { return nil } diff --git a/blockmanager.go b/blockmanager.go index 0d1bf6d0..4df24f13 100644 --- a/blockmanager.go +++ b/blockmanager.go @@ -733,37 +733,35 @@ func (b *blockManager) checkBlockForHiddenVotes(block *dcrutil.Block) { var oldRevocations []*dcrutil.Tx oldVoteMap := make(map[chainhash.Hash]struct{}, int(b.server.chainParams.TicketsPerBlock)) - if template != nil { - templateBlock := dcrutil.NewBlock(template.Block) + templateBlock := dcrutil.NewBlock(template.Block) - // Add all the votes found in our template. Keep their - // hashes in a map for easy lookup in the next loop. - for _, stx := range templateBlock.STransactions() { - mstx := stx.MsgTx() - txType := stake.DetermineTxType(mstx) - if txType == stake.TxTypeSSGen { - ticketH := mstx.TxIn[1].PreviousOutPoint.Hash - oldVoteMap[ticketH] = struct{}{} - newVotes = append(newVotes, stx) - } - - // Create a list of old tickets and revocations - // while we're in this loop. - if txType == stake.TxTypeSStx { - oldTickets = append(oldTickets, stx) - } - if txType == stake.TxTypeSSRtx { - oldRevocations = append(oldRevocations, stx) - } + // Add all the votes found in our template. Keep their + // hashes in a map for easy lookup in the next loop. + for _, stx := range templateBlock.STransactions() { + mstx := stx.MsgTx() + txType := stake.DetermineTxType(mstx) + if txType == stake.TxTypeSSGen { + ticketH := mstx.TxIn[1].PreviousOutPoint.Hash + oldVoteMap[ticketH] = struct{}{} + newVotes = append(newVotes, stx) } - // Check the votes seen in the block. If the votes - // are new, append them. - for _, vote := range votesFromBlock { - ticketH := vote.MsgTx().TxIn[1].PreviousOutPoint.Hash - if _, exists := oldVoteMap[ticketH]; !exists { - newVotes = append(newVotes, vote) - } + // Create a list of old tickets and revocations + // while we're in this loop. + if txType == stake.TxTypeSStx { + oldTickets = append(oldTickets, stx) + } + if txType == stake.TxTypeSSRtx { + oldRevocations = append(oldRevocations, stx) + } + } + + // Check the votes seen in the block. If the votes + // are new, append them. + for _, vote := range votesFromBlock { + ticketH := vote.MsgTx().TxIn[1].PreviousOutPoint.Hash + if _, exists := oldVoteMap[ticketH]; !exists { + newVotes = append(newVotes, vote) } } diff --git a/mining.go b/mining.go index 20fa97ca..a3477e8d 100644 --- a/mining.go +++ b/mining.go @@ -849,107 +849,106 @@ func handleTooFewVoters(subsidyCache *blockchain.SubsidyCache, nextHeight int64, return cptCopy, nil } - // We may have just started mining and stored the current block - // template, so we don't have a parent. - if curTemplate == nil { - // Fetch the latest block and head and begin working - // off of it with an empty transaction tree regular - // and the contents of that stake tree. In the future - // we should have the option of readding some - // transactions from this block, too. - topBlock, err := bm.chain.BlockByHash(&best.Hash) - if err != nil { - str := fmt.Sprintf("unable to get tip block %s", - best.PrevHash) - return nil, miningRuleError(ErrGetTopBlock, str) - } - btMsgBlock := new(wire.MsgBlock) - rand, err := wire.RandomUint64() - if err != nil { - return nil, err - } - coinbaseScript := make([]byte, len(coinbaseFlags)+2) - copy(coinbaseScript[2:], coinbaseFlags) - opReturnPkScript, err := - standardCoinbaseOpReturn(topBlock.MsgBlock().Header.Height, - rand) - if err != nil { - return nil, err - } - coinbaseTx, err := createCoinbaseTx(subsidyCache, - coinbaseScript, - opReturnPkScript, - topBlock.Height(), - miningAddress, - topBlock.MsgBlock().Header.Voters, - bm.server.chainParams) - if err != nil { - return nil, err - } - btMsgBlock.AddTransaction(coinbaseTx.MsgTx()) - - for _, stx := range topBlock.STransactions() { - btMsgBlock.AddSTransaction(stx.MsgTx()) - } - - // Copy the rest of the header. - btMsgBlock.Header = topBlock.MsgBlock().Header - - // Set a fresh timestamp. - ts := medianAdjustedTime(best, timeSource) - btMsgBlock.Header.Timestamp = ts - - // If we're on testnet, the time since this last block - // listed as the parent must be taken into consideration. - if bm.server.chainParams.ReduceMinDifficulty { - parentHash := topBlock.MsgBlock().Header.PrevBlock - - requiredDifficulty, err := - bm.CalcNextRequiredDiffNode(&parentHash, ts) - if err != nil { - return nil, miningRuleError(ErrGettingDifficulty, - err.Error()) - } - - btMsgBlock.Header.Bits = requiredDifficulty - } - - // Recalculate the size. - btMsgBlock.Header.Size = uint32(btMsgBlock.SerializeSize()) - - bt := &BlockTemplate{ - Block: btMsgBlock, - Fees: []int64{0}, - SigOpCounts: []int64{0}, - Height: int64(topBlock.MsgBlock().Header.Height), - ValidPayAddress: miningAddress != nil, - } - - // Recalculate the merkle roots. Use a temporary 'immutable' - // block object as we're changing the header contents. - btBlockTemp := dcrutil.NewBlockDeepCopyCoinbase(btMsgBlock) - merkles := - blockchain.BuildMerkleTreeStore(btBlockTemp.Transactions()) - merklesStake := - blockchain.BuildMerkleTreeStore(btBlockTemp.STransactions()) - btMsgBlock.Header.MerkleRoot = *merkles[len(merkles)-1] - btMsgBlock.Header.StakeRoot = *merklesStake[len(merklesStake)-1] - - // Make sure the block validates. - btBlock := dcrutil.NewBlockDeepCopyCoinbase(btMsgBlock) - err = bm.chain.CheckConnectBlockTemplate(btBlock) - if err != nil { - str := fmt.Sprintf("failed to check template: %v while "+ - "constructing a new parent", err.Error()) - return nil, miningRuleError(ErrCheckConnectBlock, - str) - } - - // Make a copy to return. - cptCopy := deepCopyBlockTemplate(bt) - - return cptCopy, nil + // We may have just started mining and stored the + // current block template, so we don't have a parent. + // + // Fetch the latest block and head and begin working + // off of it with an empty transaction tree regular + // and the contents of that stake tree. In the future + // we should have the option of readding some + // transactions from this block, too. + topBlock, err := bm.chain.BlockByHash(&best.Hash) + if err != nil { + str := fmt.Sprintf("unable to get tip block %s", + best.PrevHash) + return nil, miningRuleError(ErrGetTopBlock, str) } + btMsgBlock := new(wire.MsgBlock) + rand, err := wire.RandomUint64() + if err != nil { + return nil, err + } + coinbaseScript := make([]byte, len(coinbaseFlags)+2) + copy(coinbaseScript[2:], coinbaseFlags) + opReturnPkScript, err := + standardCoinbaseOpReturn(topBlock.MsgBlock().Header.Height, + rand) + if err != nil { + return nil, err + } + coinbaseTx, err := createCoinbaseTx(subsidyCache, + coinbaseScript, + opReturnPkScript, + topBlock.Height(), + miningAddress, + topBlock.MsgBlock().Header.Voters, + bm.server.chainParams) + if err != nil { + return nil, err + } + btMsgBlock.AddTransaction(coinbaseTx.MsgTx()) + + for _, stx := range topBlock.STransactions() { + btMsgBlock.AddSTransaction(stx.MsgTx()) + } + + // Copy the rest of the header. + btMsgBlock.Header = topBlock.MsgBlock().Header + + // Set a fresh timestamp. + ts := medianAdjustedTime(best, timeSource) + btMsgBlock.Header.Timestamp = ts + + // If we're on testnet, the time since this last block + // listed as the parent must be taken into consideration. + if bm.server.chainParams.ReduceMinDifficulty { + parentHash := topBlock.MsgBlock().Header.PrevBlock + + requiredDifficulty, err := + bm.CalcNextRequiredDiffNode(&parentHash, ts) + if err != nil { + return nil, miningRuleError(ErrGettingDifficulty, + err.Error()) + } + + btMsgBlock.Header.Bits = requiredDifficulty + } + + // Recalculate the size. + btMsgBlock.Header.Size = uint32(btMsgBlock.SerializeSize()) + + bt := &BlockTemplate{ + Block: btMsgBlock, + Fees: []int64{0}, + SigOpCounts: []int64{0}, + Height: int64(topBlock.MsgBlock().Header.Height), + ValidPayAddress: miningAddress != nil, + } + + // Recalculate the merkle roots. Use a temporary 'immutable' + // block object as we're changing the header contents. + btBlockTemp := dcrutil.NewBlockDeepCopyCoinbase(btMsgBlock) + merkles := + blockchain.BuildMerkleTreeStore(btBlockTemp.Transactions()) + merklesStake := + blockchain.BuildMerkleTreeStore(btBlockTemp.STransactions()) + btMsgBlock.Header.MerkleRoot = *merkles[len(merkles)-1] + btMsgBlock.Header.StakeRoot = *merklesStake[len(merklesStake)-1] + + // Make sure the block validates. + btBlock := dcrutil.NewBlockDeepCopyCoinbase(btMsgBlock) + err = bm.chain.CheckConnectBlockTemplate(btBlock) + if err != nil { + str := fmt.Sprintf("failed to check template: %v while "+ + "constructing a new parent", err.Error()) + return nil, miningRuleError(ErrCheckConnectBlock, + str) + } + + // Make a copy to return. + cptCopy := deepCopyBlockTemplate(bt) + + return cptCopy, nil } } diff --git a/rpcserver.go b/rpcserver.go index 43806e6e..f1a77608 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -2946,7 +2946,7 @@ func handleGetBlockTemplateProposal(s *rpcServer, request *dcrjson.TemplateReque // Ensure the block is building from the expected previous block. expectedPrevHash := &s.server.blockManager.chain.BestSnapshot().Hash prevHash := &block.MsgBlock().Header.PrevBlock - if expectedPrevHash == nil || !expectedPrevHash.IsEqual(prevHash) { + if *expectedPrevHash != *prevHash { return "bad-prevblk", nil } @@ -3233,7 +3233,7 @@ func handleGetMiningInfo(s *rpcServer, cmd interface{}, closeChan <-chan struct{ } networkHashesPerSec, ok := networkHashesPerSecIface.(int64) if !ok { - return nil, rpcInternalError(err.Error(), + return nil, rpcInternalError("invalid network hashes per sec", fmt.Sprintf("Invalid type: %q", networkHashesPerSecIface)) } @@ -4098,12 +4098,6 @@ func handleGetWorkRequest(s *rpcServer) (interface{}, error) { blockchain.CompactToBig(msgBlock.Header.Bits), msgBlock.Header.MerkleRoot) } else { - if msgBlock == nil { - context := "Failed to create new block template, " + - "no previous state" - return nil, rpcInternalError("internal error", context) - } - // At this point, there is a saved block template and a new // request for work was made, but either the available // transactions haven't change or it hasn't been long enough to diff --git a/run_tests.sh b/run_tests.sh index 5120d455..33bf5f44 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -12,12 +12,13 @@ set -ex # The script does automatic checking on a Go package and its sub-packages, # including: -# 1. gofmt (http://golang.org/cmd/gofmt/) +# 1. gofmt (https://golang.org/cmd/gofmt/) # 2. gosimple (https://github.com/dominikh/go-simple) # 3. unconvert (https://github.com/mdempsky/unconvert) # 4. ineffassign (https://github.com/gordonklaus/ineffassign) -# 5. misspell (https://github.com/client9/misspell) -# 6. race detector (http://blog.golang.org/race-detector) +# 5. go vet (https://golang.org/cmd/vet) +# 6. misspell (https://github.com/client9/misspell) +# 7. race detector (https://blog.golang.org/race-detector) # golangci-lint (github.com/golangci/golangci-lint) is used to run each each # static checker. @@ -57,6 +58,7 @@ testrepo () { --enable=gosimple \ --enable=unconvert \ --enable=ineffassign \ + --enable=govet \ --enable=misspell ./${module}/... done diff --git a/wire/msgtx.go b/wire/msgtx.go index 80a3855f..a136b74d 100644 --- a/wire/msgtx.go +++ b/wire/msgtx.go @@ -1224,7 +1224,7 @@ func writeTxInWitness(w io.Writer, pver uint32, version uint16, ti *TxIn) error } // BlockIndex. - binarySerializer.PutUint32(w, littleEndian, ti.BlockIndex) + err = binarySerializer.PutUint32(w, littleEndian, ti.BlockIndex) if err != nil { return err }