Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

blockchain: Use slices when fetching utxos #1972

Merged
merged 2 commits into from
May 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions blockchain/bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"testing"

"github.com/btcsuite/btcd/btcutil"
"github.com/btcsuite/btcd/wire"
)

// BenchmarkIsCoinBase performs a simple benchmark against the IsCoinBase
Expand All @@ -29,3 +30,33 @@ func BenchmarkIsCoinBaseTx(b *testing.B) {
IsCoinBaseTx(tx)
}
}

func BenchmarkUtxoFetchMap(b *testing.B) {
block := Block100000
transactions := block.Transactions
b.ResetTimer()

for i := 0; i < b.N; i++ {
needed := make(map[wire.OutPoint]struct{}, len(transactions))
for _, tx := range transactions[1:] {
for _, txIn := range tx.TxIn {
needed[txIn.PreviousOutPoint] = struct{}{}
}
}
}
}

func BenchmarkUtxoFetchSlices(b *testing.B) {
block := Block100000
transactions := block.Transactions
b.ResetTimer()

for i := 0; i < b.N; i++ {
needed := make([]wire.OutPoint, 0, len(transactions))
for _, tx := range transactions[1:] {
for _, txIn := range tx.TxIn {
needed = append(needed, txIn.PreviousOutPoint)
}
}
}
}
38 changes: 21 additions & 17 deletions blockchain/utxoviewpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ func (view *UtxoViewpoint) commit() {
// Upon completion of this function, the view will contain an entry for each
// requested outpoint. Spent outputs, or those which otherwise don't exist,
// will result in a nil entry in the view.
func (view *UtxoViewpoint) fetchUtxosMain(db database.DB, outpoints map[wire.OutPoint]struct{}) error {
func (view *UtxoViewpoint) fetchUtxosMain(db database.DB, outpoints []wire.OutPoint) error {
// Nothing to do if there are no requested outputs.
if len(outpoints) == 0 {
return nil
Expand All @@ -517,13 +517,13 @@ func (view *UtxoViewpoint) fetchUtxosMain(db database.DB, outpoints map[wire.Out
// so other code can use the presence of an entry in the store as a way
// to unnecessarily avoid attempting to reload it from the database.
return db.View(func(dbTx database.Tx) error {
for outpoint := range outpoints {
entry, err := dbFetchUtxoEntry(dbTx, outpoint)
for i := range outpoints {
entry, err := dbFetchUtxoEntry(dbTx, outpoints[i])
if err != nil {
return err
}

view.entries[outpoint] = entry
view.entries[outpoints[i]] = entry
}

return nil
Expand All @@ -533,25 +533,25 @@ func (view *UtxoViewpoint) fetchUtxosMain(db database.DB, outpoints map[wire.Out
// fetchUtxos loads the unspent transaction outputs for the provided set of
// outputs into the view from the database as needed unless they already exist
// in the view in which case they are ignored.
func (view *UtxoViewpoint) fetchUtxos(db database.DB, outpoints map[wire.OutPoint]struct{}) error {
func (view *UtxoViewpoint) fetchUtxos(db database.DB, outpoints []wire.OutPoint) error {
// Nothing to do if there are no requested outputs.
if len(outpoints) == 0 {
return nil
}

// Filter entries that are already in the view.
neededSet := make(map[wire.OutPoint]struct{})
for outpoint := range outpoints {
needed := make([]wire.OutPoint, 0, len(outpoints))
for i := range outpoints {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: could do for _, outpoint := range outpoints to minimize diff

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I explicitly asked to use the index based access, because the append() would otherwise not work as expected because of the Golang loop variable issue (see my resolved comments above).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that is an issue in this case, since it's a non-ref struct being used?

That being said, better safe than sorry 😀

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, you're right, in this case it's not problematic. But we've been burned by this a couple of times in the past, so I basically only use the key/index based access. Also makes it easier to review if you know the loop bug can't bite you.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed!

// Already loaded into the current view.
if _, ok := view.entries[outpoint]; ok {
if _, ok := view.entries[outpoints[i]]; ok {
continue
}

neededSet[outpoint] = struct{}{}
needed = append(needed, outpoints[i])
}

// Request the input utxos from the database.
return view.fetchUtxosMain(db, neededSet)
return view.fetchUtxosMain(db, needed)
}

// fetchInputUtxos loads the unspent transaction outputs for the inputs
Expand All @@ -572,7 +572,7 @@ func (view *UtxoViewpoint) fetchInputUtxos(db database.DB, block *btcutil.Block)
// Loop through all of the transaction inputs (except for the coinbase
// which has no inputs) collecting them into sets of what is needed and
// what is already known (in-flight).
neededSet := make(map[wire.OutPoint]struct{})
needed := make([]wire.OutPoint, 0, len(transactions))
for i, tx := range transactions[1:] {
for _, txIn := range tx.MsgTx().TxIn {
// It is acceptable for a transaction input to reference
Expand Down Expand Up @@ -601,12 +601,12 @@ func (view *UtxoViewpoint) fetchInputUtxos(db database.DB, block *btcutil.Block)
continue
}

neededSet[txIn.PreviousOutPoint] = struct{}{}
needed = append(needed, txIn.PreviousOutPoint)
}
}

// Request the input utxos from the database.
return view.fetchUtxosMain(db, neededSet)
return view.fetchUtxosMain(db, needed)
}

// NewUtxoViewpoint returns a new empty unspent transaction output view.
Expand All @@ -626,23 +626,27 @@ func (b *BlockChain) FetchUtxoView(tx *btcutil.Tx) (*UtxoViewpoint, error) {
// Create a set of needed outputs based on those referenced by the
// inputs of the passed transaction and the outputs of the transaction
// itself.
neededSet := make(map[wire.OutPoint]struct{})
neededLen := len(tx.MsgTx().TxOut)
if !IsCoinBase(tx) {
neededLen += len(tx.MsgTx().TxIn)
}
needed := make([]wire.OutPoint, 0, neededLen)
prevOut := wire.OutPoint{Hash: *tx.Hash()}
for txOutIdx := range tx.MsgTx().TxOut {
prevOut.Index = uint32(txOutIdx)
neededSet[prevOut] = struct{}{}
needed = append(needed, prevOut)
}
if !IsCoinBase(tx) {
for _, txIn := range tx.MsgTx().TxIn {
neededSet[txIn.PreviousOutPoint] = struct{}{}
needed = append(needed, txIn.PreviousOutPoint)
}
}

// Request the utxos from the point of view of the end of the main
// chain.
view := NewUtxoViewpoint()
b.chainLock.RLock()
err := view.fetchUtxosMain(b.db, neededSet)
err := view.fetchUtxosMain(b.db, needed)
b.chainLock.RUnlock()
return view, err
}
Expand Down
20 changes: 10 additions & 10 deletions blockchain/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,8 @@ func CheckTransactionSanity(tx *btcutil.Tx) error {
// target difficulty as claimed.
//
// The flags modify the behavior of this function as follows:
// - BFNoPoWCheck: The check to ensure the block hash is less than the target
// difficulty is not performed.
// - BFNoPoWCheck: The check to ensure the block hash is less than the target
// difficulty is not performed.
func checkProofOfWork(header *wire.BlockHeader, powLimit *big.Int, flags BehaviorFlags) error {
// The target difficulty must be larger than zero.
target := CompactToBig(header.Bits)
Expand Down Expand Up @@ -637,8 +637,8 @@ func checkSerializedHeight(coinbaseTx *btcutil.Tx, wantHeight int32) error {
// which depend on its position within the block chain.
//
// The flags modify the behavior of this function as follows:
// - BFFastAdd: All checks except those involving comparing the header against
// the checkpoints are not performed.
// - BFFastAdd: All checks except those involving comparing the header against
// the checkpoints are not performed.
//
// This function MUST be called with the chain state lock held (for writes).
func (b *BlockChain) checkBlockHeaderContext(header *wire.BlockHeader, prevNode *blockNode, flags BehaviorFlags) error {
Expand Down Expand Up @@ -716,8 +716,8 @@ func (b *BlockChain) checkBlockHeaderContext(header *wire.BlockHeader, prevNode
// on its position within the block chain.
//
// The flags modify the behavior of this function as follows:
// - BFFastAdd: The transaction are not checked to see if they are finalized
// and the somewhat expensive BIP0034 validation is not performed.
// - BFFastAdd: The transaction are not checked to see if they are finalized
// and the somewhat expensive BIP0034 validation is not performed.
//
// The flags are also passed to checkBlockHeaderContext. See its documentation
// for how the flags modify its behavior.
Expand Down Expand Up @@ -832,22 +832,22 @@ func (b *BlockChain) checkBlockContext(block *btcutil.Block, prevNode *blockNode
func (b *BlockChain) checkBIP0030(node *blockNode, block *btcutil.Block, view *UtxoViewpoint) error {
// Fetch utxos for all of the transaction ouputs in this block.
// Typically, there will not be any utxos for any of the outputs.
fetchSet := make(map[wire.OutPoint]struct{})
fetch := make([]wire.OutPoint, 0, len(block.Transactions()))
for _, tx := range block.Transactions() {
prevOut := wire.OutPoint{Hash: *tx.Hash()}
for txOutIdx := range tx.MsgTx().TxOut {
prevOut.Index = uint32(txOutIdx)
fetchSet[prevOut] = struct{}{}
fetch = append(fetch, prevOut)
}
}
err := view.fetchUtxos(b.db, fetchSet)
err := view.fetchUtxos(b.db, fetch)
if err != nil {
return err
}

// Duplicate transactions are only allowed if the previous transaction
// is fully spent.
for outpoint := range fetchSet {
for _, outpoint := range fetch {
utxo := view.LookupEntry(outpoint)
if utxo != nil && !utxo.IsSpent() {
str := fmt.Sprintf("tried to overwrite transaction %v "+
Expand Down