make ValidateTransactionScripts accept PrevOutputFetcher

[laizy]

I want to use ValidateTransactionScripts in our lib to verify transaction, UtxoViewpoint is much harder to construct.

[coveralls]

Pull Request Test Coverage Report for Build 5143581345

• 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
• 3 unchanged lines in 1 file lost coverage.
• Overall coverage increased (+0.02%) to 55.29%

---

Files with Coverage Reduction	New Missed Lines	%
peer/peer.go	3	73.49%
<!--	Total:	3		-->

Totals
Change from base Build 5075515644:	0.02%
Covered Lines:	26675
Relevant Lines:	48246

---

💛 - Coveralls

[kcalvinalvin]

I'm not a fan of using txscript.PrevOutputFetcher as there's an allocation made for the txOut on return.https://github.com/btcsuite/btcd/blob/253b688c68b89eca7eb75d4d5443dbdbc928db3c/blockchain/utxoviewpoint.go#L173-L176

LookupEntry on the other hand doesn't do an allocation.

This would slow down block validation because of that allocation. I wrote up some benchmarks with the change and the results are that it takes ~3 times longer than current master.

[I] calvin@bitcoin ~/b/g/u/g/s/g/b/b/blockchain ((dee4cbd9…))> go test -run=XXX -bench=BenchmarkUtxoViewpoint -benchmem
goos: linux
goarch: amd64
pkg: github.com/btcsuite/btcd/blockchain
cpu: AMD Ryzen 5 3600 6-Core Processor              
BenchmarkUtxoViewpointLookupEntry-10             8953658               133.1 ns/op             0 B/op          0 allocs/op
BenchmarkUtxoViewpointFetchPrevOutput-10         3364036               361.5 ns/op           192 B/op          6 allocs/op
PASS
ok      github.com/btcsuite/btcd/blockchain     2.910s

The code I've used is:

package blockchain

import (
        "testing"

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

var out *UtxoEntry

func BenchmarkUtxoViewpointLookupEntry(b *testing.B) {
        // Boilerplate to create outpoints/utxoviewpoint.
        block := Block100000
        transactions := block.Transactions

        utxoView := NewUtxoViewpoint()
        fetchOutpoints := make([]wire.OutPoint, 0, len(transactions))
        for i := range transactions {
                tx := transactions[i]
                utxoView.AddTxOuts(btcutil.NewTx(tx), 100_000)

                txHash := tx.TxHash()
                for i := range tx.TxOut {
                        op := wire.OutPoint{Hash: txHash, Index: uint32(i)}
                        fetchOutpoints = append(fetchOutpoints, op)
                }
        }

        // Actual benchmark.
        b.ResetTimer()
        for i := 0; i < b.N; i++ {
                for i := range fetchOutpoints {
                        out = utxoView.LookupEntry(fetchOutpoints[i])
                }
        }
}

var txOut *wire.TxOut

func BenchmarkUtxoViewpointFetchPrevOutput(b *testing.B) {
        // Boilerplate to create outpoints/utxoviewpoint.
        block := Block100000
        transactions := block.Transactions

        utxoView := NewUtxoViewpoint()
        fetchOutpoints := make([]wire.OutPoint, 0, len(transactions))
        for i := range transactions {
                tx := transactions[i]
                utxoView.AddTxOuts(btcutil.NewTx(tx), 100_000)

                txHash := tx.TxHash()
                for i := range tx.TxOut {
                        op := wire.OutPoint{Hash: txHash, Index: uint32(i)}
                        fetchOutpoints = append(fetchOutpoints, op)
                }
        }

        // Actual benchmark.
        b.ResetTimer()
        for i := 0; i < b.N; i++ {
                for i := range fetchOutpoints {
                        txOut = utxoView.FetchPrevOutput(fetchOutpoints[i])
                }
        }
}

[laizy]

@kcalvinalvin If you care this allocation, why not define a *wire.TxOut field in UtxoEntry directly. Since the UtxoEntry is constructed from an existing Txout most of the time, there are a lot of assignment code like:

    return &UtxoEntry{
        amount:      txOut.Value,
        pkScript:    txOut.PkScript,

And I think a complete block verification benchmark is much better to show the performance difference.

[kcalvinalvin]

Before I address your comments, I think you guys should use something like the below code to create a UtxoViewpoint. Here's a playground link as well https://go.dev/play/p/jrvY_iy-R0l

package main

import (
    "fmt"

    "github.com/btcsuite/btcd/blockchain"
    "github.com/btcsuite/btcd/chaincfg/chainhash"
    "github.com/btcsuite/btcd/wire"
)

func main() {
    utxo := blockchain.NewUtxoEntry(&wire.TxOut{Value: 0, PkScript: []byte{}}, 0, false)

    viewpoint := blockchain.NewUtxoViewpoint()
    viewpoint.Entries()[wire.OutPoint{Hash: chainhash.Hash{}, Index: 0}] = utxo

    for k, v := range viewpoint.Entries() {
        fmt.Println(k, v)
    }
}

@kcalvinalvin If you care this allocation, why not define a *wire.TxOut field in UtxoEntry directly. Since the UtxoEntry is constructed from an existing Txout most of the time, there are a lot of assignment code like:

  return &UtxoEntry{
      amount:      txOut.Value,
      pkScript:    txOut.PkScript,

You could do that. But it'd also have to be with changes in the wire package side to avoid memory leak as the slice of txOut in https://github.com/btcsuite/btcd/blob/7fd5c1e92cfaffee41a0646a435d9ea23926ddf0/wire/msgtx.go#L573-L575

wouldn't get gc-ed because the UTXO is still pointing to it.

And I think a complete block verification benchmark is much better to show the performance difference.

It would be. I think that burden would normally be on the person suggesting the change.

[laizy]

@kcalvinalvin Thanks for your code, I didn't notice the Entries method before, but it does simplify the construction.