From a69432bc18ef40f1b14706d3842a03a47c406242 Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Wed, 22 Apr 2026 17:48:06 -0500 Subject: [PATCH 1/9] blockchain/stake: Remove commented test code. --- blockchain/stake/treasury_test.go | 30 +----------------------------- 1 file changed, 1 insertion(+), 29 deletions(-) diff --git a/blockchain/stake/treasury_test.go b/blockchain/stake/treasury_test.go index 2b6827472a..00f6d29fe3 100644 --- a/blockchain/stake/treasury_test.go +++ b/blockchain/stake/treasury_test.go @@ -1,4 +1,4 @@ -// Copyright (c) 2020-2024 The Decred developers +// Copyright (c) 2020-2026 The Decred developers // Use of this source code is governed by an ISC // license that can be found in the LICENSE file. @@ -136,34 +136,6 @@ var ( } ) -// generateKeys generates all the keys that are hard coded in this file. -//func generateKeys() { -// key := secp256k1.PrivKeyFromBytes(privateKey) -// pubKey := key.PubKey() -// message := "test message" -// messageHash := chainhash.HashB([]byte(message)) -// signature, err := schnorr.Sign(key, messageHash) -// if err != nil { -// panic(err) -// } -// fmt.Printf("Sig 0x%x: %x\n", len(signature.Serialize()), -// signature.Serialize()) -// fmt.Printf("Public key 0x%x: %x\n", len(pubKey.SerializeCompressed()), -// pubKey.SerializeCompressed()) -// for k, v := range signature.Serialize() { -// if k%8 == 0 { -// fmt.Printf("\n") -// } -// fmt.Printf("0x%02x,", v) -// } -// fmt.Printf("\n") -//} -// -//func init() { -// generateKeys() -// panic("x") -//} - // newTxOut returns a new transaction output with the given parameters. func newTxOut(amount int64, pkScriptVer uint16, pkScript []byte) *wire.TxOut { return &wire.TxOut{ From 9c391a71ddd8cb5d8e5607ea53e8d8415cb7406f Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Wed, 22 Apr 2026 17:48:06 -0500 Subject: [PATCH 2/9] blockchain/stake: Rework treasury tx type tests. This reworks the tests in TestTreasuryIsFunctions for the treasury add, treasurybase, and treasury spend identification funcs to make them more comprehensive, correct some that weren't actually testing what they claimed, and make them much more consistent with the other tests throughout the code base. Not only does it perform more comprehensive testing, it reduces the test code by about 42%. In particular: - Use hex to bytes for hard-coded byte slices for some of the globals instead of the much more verbose raw byte slices - Introduce helper functions to create the various components of the transactions - Start with well-formed transactions and modify them for each test instead of building them from scratch every time - Run all identification funcs against all of the transaction types to help ensure none of them are incorrectly detected as any other - Significantly improves readability and adds descriptions to make it clear for people not familiar with the code - Modernize the test formatting - Effectively add more tests overall due to cross testing - Correct test intending to pass stakebase but not treasury add and assert it actually passes the stakebase checks This is part of a larger overall effort to bring the treasury code up to the standards used throughout the rest of the blockchain consensus code. --- blockchain/stake/treasury_test.go | 633 +++++++++++------------------- 1 file changed, 237 insertions(+), 396 deletions(-) diff --git a/blockchain/stake/treasury_test.go b/blockchain/stake/treasury_test.go index 00f6d29fe3..31c23990a7 100644 --- a/blockchain/stake/treasury_test.go +++ b/blockchain/stake/treasury_test.go @@ -6,6 +6,7 @@ package stake import ( "bytes" + "encoding/binary" "encoding/hex" "errors" "math" @@ -14,7 +15,6 @@ import ( "github.com/decred/dcrd/chaincfg/chainhash" "github.com/decred/dcrd/chaincfg/v3" - "github.com/decred/dcrd/dcrec/secp256k1/v4" "github.com/decred/dcrd/dcrutil/v4" "github.com/decred/dcrd/txscript/v4" "github.com/decred/dcrd/txscript/v4/stdaddr" @@ -24,33 +24,17 @@ import ( // Private and public keys for tests. var ( // Serialized private key. - //privateKey = []byte{ - // 0x76, 0x87, 0x56, 0x13, 0x94, 0xcc, 0xc6, 0x11, - // 0x01, 0x51, 0xbd, 0x9f, 0x26, 0xd4, 0x22, 0x8e, - // 0xb2, 0xd5, 0x7b, 0xe1, 0x28, 0xc0, 0x36, 0x12, - // 0xe3, 0x9a, 0x84, 0x4a, 0x3e, 0xcd, 0x3c, 0xcf, - //} + // privateKey = hexToBytes("7687561394ccc6110151bd9f26d4228eb2d57be128c036" + + // "12e39a844a3ecd3ccf") // Serialized compressed public key. - publicKey = []byte{ - 0x02, 0xa4, 0xf6, 0x45, 0x86, 0xe1, 0x72, 0xc3, - 0xd9, 0xa2, 0x0c, 0xfa, 0x6c, 0x7a, 0xc8, 0xfb, - 0x12, 0xf0, 0x11, 0x5b, 0x3f, 0x69, 0xc3, 0xc3, - 0x5a, 0xec, 0x93, 0x3a, 0x4c, 0x47, 0xc7, 0xd9, - 0x2c, - } + publicKey = hexToBytes("02a4f64586e172c3d9a20cfa6c7ac8fb12f0115b3f69c3c3" + + "5aec933a4c47c7d92c") // Valid signature of chainhash.HashB([]byte("test message")) - validSignature = []byte{ - 0x77, 0x69, 0x84, 0xf6, 0x83, 0x13, 0xb1, 0xac, - 0x62, 0x9e, 0x62, 0x4a, 0xf0, 0x59, 0x5b, 0xdc, - 0x09, 0xd8, 0xde, 0xd0, 0x2b, 0xc2, 0xb2, 0x9f, - 0xbd, 0xb3, 0x95, 0x95, 0xe0, 0x3a, 0xc8, 0xb0, - 0xcf, 0x81, 0x8c, 0xa5, 0x36, 0x72, 0x3e, 0x63, - 0x90, 0xd3, 0x08, 0x4e, 0x0e, 0x31, 0xc7, 0x94, - 0x22, 0x29, 0x15, 0x3c, 0xe3, 0x4d, 0x87, 0x39, - 0x29, 0xb1, 0x60, 0x88, 0xd9, 0xe1, 0xaf, 0x43, - } + validSignature = hexToBytes("776984f68313b1ac629e624af0595bdc09d8ded02bc2" + + "b29fbdb39595e03ac8b0cf818ca536723e6390d3084e0e31c7942229153ce34d8739" + + "29b16088d9e1af43") // OP_DATA_64 OP_TSPEND tspendValidKey = []byte{ @@ -136,6 +120,61 @@ var ( } ) +// opReturnScript returns a provably-pruneable OP_RETURN script with the +// provided data. +func opReturnScript(data []byte) []byte { + builder := txscript.NewScriptBuilder() + script, err := builder.AddOp(txscript.OP_RETURN).AddData(data).Script() + if err != nil { + panic(err) + } + return script +} + +// treasurybaseOpReturnScript returns a script suitable for use as the second +// output of the treasurybase transaction of a new block. In particular, the +// serialized data used with the OP_RETURN starts with the block height and is +// followed by 8 bytes of cryptographically random data. +func treasurybaseOpReturnScript(blockHeight uint32) []byte { + data := make([]byte, 12) + binary.LittleEndian.PutUint32(data[0:4], blockHeight) + binary.LittleEndian.PutUint64(data[4:12], rand.Uint64()) + return opReturnScript(data) +} + +// treasurySpendOpReturnScript returns a script suitable for use as the first +// output of a treasury spend transaction. In particular, the serialized data +// used with the OP_RETURN starts with the total spend amount and is followed by +// 24 bytes of cryptographically random data. +func treasurySpendOpReturnScript(amount int64) []byte { + data := make([]byte, 32) + binary.LittleEndian.PutUint64(data[0:8], uint64(amount)) + rand.Read(data[8:]) + return opReturnScript(data) +} + +// treasurySpendSignature returns a treasury spend signature script with the +// provided signature and public key. +func treasurySpendSignature(sig, pubKey []byte) []byte { + builder := txscript.NewScriptBuilder() + builder.AddData(sig) + builder.AddData(pubKey) + builder.AddOp(txscript.OP_TSPEND) + script, err := builder.Script() + if err != nil { + panic(err) + } + return script +} + +// fakeTreasurySpendSignature returns a signature script that is valid enough to +// pass all checks, but would fail if actually checked. This identification +// funcs in this package do not verify signatures, so valid signatures are not +// required for the tests. +func fakeTreasurySpendSignature() []byte { + return treasurySpendSignature(validSignature, publicKey) +} + // newTxOut returns a new transaction output with the given parameters. func newTxOut(amount int64, pkScriptVer uint16, pkScript []byte) *wire.TxOut { return &wire.TxOut{ @@ -145,398 +184,200 @@ func newTxOut(amount int64, pkScriptVer uint16, pkScript []byte) *wire.TxOut { } } -// TestTreasuryIsFunctions goes through all valid treasury opcode combinations. +var ( + // opTrueScript is a simple public key script that contains the OP_TRUE + // opcode. + opTrueScript = []byte{txscript.OP_TRUE} + + // p2shOpTrueAddr is a pay-to-script-hash address that can be redeemed with + // [opTrueScript]. + p2shOpTrueAddr = func() *stdaddr.AddressScriptHashV0 { + params := chaincfg.RegNetParams() + addr, err := stdaddr.NewAddressScriptHashV0(opTrueScript, params) + if err != nil { + panic(err) + } + return addr + }() + + // baseTreasuryAddTx is a valid treasury add transaction that includes a + // change output. It is used as a base to be further manipulated in the + // tests. + baseTreasuryAddTx = func() *wire.MsgTx { + changeScriptVer, changeScript := p2shOpTrueAddr.StakeChangeScript() + + tx := wire.NewMsgTx() + tx.Version = wire.TxVersionTreasury + tx.AddTxIn(&wire.TxIn{}) // One input required + tx.AddTxOut(newTxOut(0, 0, []byte{txscript.OP_TADD})) + tx.AddTxOut(newTxOut(1, changeScriptVer, changeScript)) + return tx + }() + + // baseTreasuryBaseTx is a valid treasury base transaction that commits to a + // random height. It is used as a base to be further manipulated in the + // tests. + baseTreasuryBaseTx = func() *wire.MsgTx { + tx := wire.NewMsgTx() + tx.Version = wire.TxVersionTreasury + tx.AddTxIn(&wire.TxIn{ + // Treasurybase transactions have no inputs, so previous outpoint is + // zero hash and max index. + PreviousOutPoint: *wire.NewOutPoint(zeroHash, wire.MaxPrevOutIndex, + wire.TxTreeRegular), + Sequence: wire.MaxTxInSequenceNum, + ValueIn: 0, + BlockHeight: wire.NullBlockHeight, + BlockIndex: wire.NullBlockIndex, + SignatureScript: nil, // Must be nil by consensus. + }) + tx.AddTxOut(newTxOut(0, 0, []byte{txscript.OP_TADD})) + tx.AddTxOut(newTxOut(0, 0, treasurybaseOpReturnScript(rand.Uint32()))) + return tx + }() + + // baseTreasurySpendTx is a valid treasury spend transaction that pays to a + // p2sh script. It is used as a base to be further manipulated in the + // tests. + baseTreasurySpendTx = func() *wire.MsgTx { + const payout = 1e8 + const fee = 5000 + payoutScriptVer, payoutScript := p2shOpTrueAddr.PayFromTreasuryScript() + + tx := wire.NewMsgTx() + tx.Version = wire.TxVersionTreasury + tx.AddTxIn(&wire.TxIn{ + // Treasury spend transactions have no inputs, so previous outpoint + // is zero hash and max index. + PreviousOutPoint: *wire.NewOutPoint(zeroHash, wire.MaxPrevOutIndex, + wire.TxTreeRegular), + Sequence: wire.MaxTxInSequenceNum, + ValueIn: fee + payout, + BlockHeight: wire.NullBlockHeight, + BlockIndex: wire.NullBlockIndex, + SignatureScript: fakeTreasurySpendSignature(), + }) + tx.AddTxOut(newTxOut(0, 0, treasurySpendOpReturnScript(payout))) + tx.AddTxOut(newTxOut(0, payoutScriptVer, payoutScript)) + return tx + }() +) + +// TestTreasuryIsFunctions confirms the various treasury transaction type +// identification functions return the expected results. Each transaction is +// tested against all funcs to help ensure none of them are incorrectly detected +// as any other. func TestTreasuryIsFunctions(t *testing.T) { tests := []struct { - name string - createTx func() *wire.MsgTx - is func(*wire.MsgTx) bool - expected bool - check func(*wire.MsgTx) error + name string // test description + tx *wire.MsgTx // transaction to test + treasuryAdd bool // expected check is treasury add + treasuryBase bool // expected is treasury base + treasurySpend bool // expected is treasury spend }{{ - name: "tadd from user, no change", - createTx: func() *wire.MsgTx { - builder := txscript.NewScriptBuilder() - builder.AddOp(txscript.OP_TADD) - script, err := builder.Script() - if err != nil { - panic(err) - } - msgTx := wire.NewMsgTx() - msgTx.Version = wire.TxVersionTreasury - msgTx.AddTxOut(wire.NewTxOut(0, script)) - msgTx.AddTxIn(&wire.TxIn{}) // One input required - return msgTx - }, - is: IsTAdd, - expected: true, - check: checkTAdd, + name: "treasury add from user with change", + tx: baseTreasuryAddTx, + treasuryAdd: true, }, { - name: "check tadd from user, no change with istreasurybase", - createTx: func() *wire.MsgTx { - builder := txscript.NewScriptBuilder() - builder.AddOp(txscript.OP_TADD) - script, err := builder.Script() - if err != nil { - panic(err) - } - msgTx := wire.NewMsgTx() - msgTx.Version = wire.TxVersionTreasury - msgTx.AddTxOut(wire.NewTxOut(0, script)) - msgTx.AddTxIn(&wire.TxIn{}) // One input required - return msgTx - }, - is: IsTreasuryBase, - expected: false, - check: checkTreasuryBase, + name: "treasury add from user with no change", + tx: func() *wire.MsgTx { + tx := baseTreasuryAddTx.Copy() + tx.TxOut = tx.TxOut[:1] + return tx + }(), + treasuryAdd: true, }, { - // This is a valid stakebase but NOT a valid TADD. - name: "tadd from user, with OP_RETURN", - createTx: func() *wire.MsgTx { - builder := txscript.NewScriptBuilder() - builder.AddOp(txscript.OP_TADD) - script, err := builder.Script() - if err != nil { - panic(err) - } - msgTx := wire.NewMsgTx() - msgTx.Version = wire.TxVersionTreasury - msgTx.AddTxOut(wire.NewTxOut(0, script)) - - // OP_RETURN - payload := make([]byte, chainhash.HashSize) - _, err = rand.Read(payload) - if err != nil { - panic(err) - } - builder = txscript.NewScriptBuilder() - builder.AddOp(txscript.OP_RETURN) - builder.AddData(payload) - script, err = builder.Script() - if err != nil { - panic(err) - } - msgTx.AddTxOut(wire.NewTxOut(0, script)) - - msgTx.AddTxIn(&wire.TxIn{ - // Stakebase transactions have no - // inputs, so previous outpoint is zero - // hash and max index. - PreviousOutPoint: *wire.NewOutPoint(&chainhash.Hash{}, - wire.MaxPrevOutIndex, wire.TxTreeRegular), - Sequence: wire.MaxTxInSequenceNum, - BlockHeight: wire.NullBlockHeight, - BlockIndex: wire.NullBlockIndex, - SignatureScript: []byte{txscript.OP_TRUE}, + // This passes stakebase checks but is NOT a valid TADD. + name: "treasury add from user with OP_RETURN", + tx: func() *wire.MsgTx { + params := chaincfg.RegNetParams() + + const voteSubsidy = 1e8 + const ticketPrice = 2e8 + tx := baseTreasuryBaseTx.Copy() + tx.TxIn[0].ValueIn = voteSubsidy + tx.TxIn[0].SignatureScript = params.StakeBaseSigScript + tx.AddTxIn(&wire.TxIn{ + PreviousOutPoint: *wire.NewOutPoint(zeroHash, 0, wire.TxTreeStake), + Sequence: wire.MaxTxInSequenceNum, + ValueIn: ticketPrice, + BlockHeight: wire.NullBlockHeight, + BlockIndex: wire.NullBlockIndex, + SignatureScript: opTrueScript, }) - return msgTx - }, - is: IsTAdd, - expected: false, - check: checkTAdd, - }, { - name: "tadd from user, with change", - createTx: func() *wire.MsgTx { - builder := txscript.NewScriptBuilder() - builder.AddOp(txscript.OP_TADD) - script, err := builder.Script() - if err != nil { - panic(err) - } - msgTx := wire.NewMsgTx() - msgTx.Version = wire.TxVersionTreasury - msgTx.AddTxOut(wire.NewTxOut(0, script)) - - opTrueScript := []byte{txscript.OP_TRUE} - p2shOpTrueAddr, err := stdaddr.NewAddressScriptHashV0(opTrueScript, - chaincfg.MainNetParams()) - if err != nil { - panic(err) + if !IsStakeBase(tx) { + panic("transaction does not pass stakebase checks") } - changeScriptVer, changeScript := p2shOpTrueAddr.StakeChangeScript() - msgTx.AddTxOut(newTxOut(1, changeScriptVer, changeScript)) - msgTx.AddTxIn(&wire.TxIn{}) // One input required - return msgTx - }, - is: IsTAdd, - expected: true, - check: checkTAdd, + return tx + }(), }, { - name: "tadd from treasurybase", - createTx: func() *wire.MsgTx { - builder := txscript.NewScriptBuilder() - builder.AddOp(txscript.OP_TADD) - script, err := builder.Script() - if err != nil { - panic(err) - } - msgTx := wire.NewMsgTx() - msgTx.Version = wire.TxVersionTreasury - msgTx.AddTxOut(wire.NewTxOut(0, script)) - - // OP_RETURN - payload := make([]byte, 12) - _, err = rand.Read(payload) - if err != nil { - panic(err) - } - builder = txscript.NewScriptBuilder() - builder.AddOp(txscript.OP_RETURN) - builder.AddData(payload) - script, err = builder.Script() - if err != nil { - panic(err) - } - msgTx.AddTxOut(wire.NewTxOut(0, script)) - - // treasurybase - msgTx.AddTxIn(&wire.TxIn{ - // Stakebase transactions have no - // inputs, so previous outpoint is zero - // hash and max index. - PreviousOutPoint: *wire.NewOutPoint(&chainhash.Hash{}, - wire.MaxPrevOutIndex, wire.TxTreeRegular), - Sequence: wire.MaxTxInSequenceNum, - BlockHeight: wire.NullBlockHeight, - BlockIndex: wire.NullBlockIndex, - SignatureScript: nil, - }) - - return msgTx - }, - is: IsTreasuryBase, - expected: true, - check: checkTreasuryBase, + name: "treasury add from treasurybase", + tx: baseTreasuryBaseTx, + treasuryBase: true, }, { - name: "check treasury base with tadd", - createTx: func() *wire.MsgTx { - builder := txscript.NewScriptBuilder() - builder.AddOp(txscript.OP_TADD) - script, err := builder.Script() - if err != nil { - panic(err) - } - msgTx := wire.NewMsgTx() - msgTx.Version = wire.TxVersionTreasury - msgTx.AddTxOut(wire.NewTxOut(0, script)) - - // OP_RETURN - payload := make([]byte, 12) - _, err = rand.Read(payload) - if err != nil { - panic(err) - } - builder = txscript.NewScriptBuilder() - builder.AddOp(txscript.OP_RETURN) - builder.AddData(payload) - script, err = builder.Script() - if err != nil { - panic(err) - } - msgTx.AddTxOut(wire.NewTxOut(0, script)) - - msgTx.AddTxIn(&wire.TxIn{ - // Stakebase transactions have no - // inputs, so previous outpoint is zero - // hash and max index. - PreviousOutPoint: *wire.NewOutPoint(&chainhash.Hash{}, - wire.MaxPrevOutIndex, wire.TxTreeRegular), - Sequence: wire.MaxTxInSequenceNum, - BlockHeight: wire.NullBlockHeight, - BlockIndex: wire.NullBlockIndex, - SignatureScript: nil, - }) - - return msgTx - }, - is: IsTAdd, - expected: false, - check: checkTAdd, + name: "treasury spend p2sh", + tx: baseTreasurySpendTx, + treasurySpend: true, }, { - name: "tspend P2SH", - createTx: func() *wire.MsgTx { - // OP_RETURN <32 byte random> - payload := make([]byte, chainhash.HashSize) - _, err := rand.Read(payload) - if err != nil { - panic(err) - } - builder := txscript.NewScriptBuilder() - builder.AddOp(txscript.OP_RETURN) - builder.AddData(payload) - opretScript, err := builder.Script() - if err != nil { - panic(err) - } - msgTx := wire.NewMsgTx() - msgTx.Version = wire.TxVersionTreasury - msgTx.AddTxOut(wire.NewTxOut(0, opretScript)) - - // OP_TGEN - opTrueScript := []byte{txscript.OP_TRUE} - p2shOpTrueAddr, err := stdaddr.NewAddressScriptHashV0(opTrueScript, - chaincfg.MainNetParams()) - if err != nil { - panic(err) - } - genScriptVer, genScript := p2shOpTrueAddr.PayFromTreasuryScript() - msgTx.AddTxOut(newTxOut(0, genScriptVer, genScript)) - - // tspend - builder = txscript.NewScriptBuilder() - builder.AddData(validSignature) - builder.AddData(publicKey) - builder.AddOp(txscript.OP_TSPEND) - tspendScript, err := builder.Script() - if err != nil { - panic(err) - } - - msgTx.AddTxIn(&wire.TxIn{ - // Stakebase transactions have no - // inputs, so previous outpoint is zero - // hash and max index. - PreviousOutPoint: *wire.NewOutPoint(&chainhash.Hash{}, - wire.MaxPrevOutIndex, wire.TxTreeRegular), - Sequence: wire.MaxTxInSequenceNum, - BlockHeight: wire.NullBlockHeight, - BlockIndex: wire.NullBlockIndex, - SignatureScript: tspendScript, - }) - - return msgTx - }, - is: IsTSpend, - expected: true, - check: checkTSpend, - }, { - name: "tspend invalid output 1 (not P2SH/P2PKH)", - createTx: func() *wire.MsgTx { - // OP_RETURN <32 byte random> - payload := make([]byte, chainhash.HashSize) - _, err := rand.Read(payload) - if err != nil { - panic(err) - } - builder := txscript.NewScriptBuilder() - builder.AddOp(txscript.OP_RETURN) - builder.AddData(payload) - opretScript, err := builder.Script() - if err != nil { - panic(err) - } - msgTx := wire.NewMsgTx() - msgTx.Version = wire.TxVersionTreasury - msgTx.AddTxOut(wire.NewTxOut(0, opretScript)) - - // OP_TGEN - privKey := secp256k1.NewPrivateKey(new(secp256k1.ModNScalar).SetInt(1)) - pubKey := privKey.PubKey().SerializeCompressed() - p2pkAddr, err := stdaddr.NewAddressPubKeyEcdsaSecp256k1V0Raw(pubKey, - chaincfg.MainNetParams()) - if err != nil { - panic(err) - } - p2pkScriptVer, p2pkScript := p2pkAddr.PaymentScript() - script := make([]byte, len(p2pkScript)+1) - script[0] = txscript.OP_TGEN - copy(script[1:], p2pkScript) - msgTx.AddTxOut(newTxOut(0, p2pkScriptVer, script)) - - // tspend - builder = txscript.NewScriptBuilder() - builder.AddData(validSignature) - builder.AddData(publicKey) - builder.AddOp(txscript.OP_TSPEND) - tspendScript, err := builder.Script() - if err != nil { - panic(err) - } - - msgTx.AddTxIn(&wire.TxIn{ - // Stakebase transactions have no - // inputs, so previous outpoint is zero - // hash and max index. - PreviousOutPoint: *wire.NewOutPoint(&chainhash.Hash{}, - wire.MaxPrevOutIndex, wire.TxTreeRegular), - Sequence: wire.MaxTxInSequenceNum, - BlockHeight: wire.NullBlockHeight, - BlockIndex: wire.NullBlockIndex, - SignatureScript: tspendScript, - }) - - return msgTx - }, - is: IsTSpend, - expected: false, - check: checkTSpend, - }, { - name: "tspend P2PKH", - createTx: func() *wire.MsgTx { - // OP_RETURN <32 byte random> - payload := make([]byte, chainhash.HashSize) - _, err := rand.Read(payload) - if err != nil { - panic(err) - } - builder := txscript.NewScriptBuilder() - builder.AddOp(txscript.OP_RETURN) - builder.AddData(payload) - opretScript, err := builder.Script() - if err != nil { - panic(err) - } - msgTx := wire.NewMsgTx() - msgTx.Version = wire.TxVersionTreasury - msgTx.AddTxOut(wire.NewTxOut(0, opretScript)) - - // OP_TGEN - privKey := secp256k1.NewPrivateKey(new(secp256k1.ModNScalar).SetInt(1)) - pubKey := privKey.PubKey() - pkHash := stdaddr.Hash160(pubKey.SerializeCompressed()) + name: "treasury spend p2pkh", + tx: func() *wire.MsgTx { + params := chaincfg.RegNetParams() + pkHash := stdaddr.Hash160(publicKey) p2pkhAddr, err := stdaddr.NewAddressPubKeyHashEcdsaSecp256k1V0( - pkHash, chaincfg.MainNetParams()) + pkHash, params) if err != nil { panic(err) } - genScriptVer, genScript := p2pkhAddr.PayFromTreasuryScript() - msgTx.AddTxOut(newTxOut(0, genScriptVer, genScript)) - - // tspend - builder = txscript.NewScriptBuilder() - builder.AddData(validSignature) - builder.AddData(publicKey) - builder.AddOp(txscript.OP_TSPEND) - tspendScript, err := builder.Script() + payoutScriptVer, payoutScript := p2pkhAddr.PayFromTreasuryScript() + + tx := baseTreasurySpendTx.Copy() + tx.TxOut[1].Version = payoutScriptVer + tx.TxOut[1].PkScript = payoutScript + return tx + }(), + treasurySpend: true, + }, { + name: "treasury spend invalid output 1 p2pk (not p2sh/p2pkh)", + tx: func() *wire.MsgTx { + // Start with a normal payment script for the p2pk and manually add + // the OP_TGEN prefix since there is no standard method to create + // the pay from treasury script on a p2pk address given it is + // invalid. + params := chaincfg.RegNetParams() + p2pkAddr, err := stdaddr.NewAddressPubKeyEcdsaSecp256k1V0Raw( + publicKey, params) if err != nil { panic(err) } + payoutScriptVer, payScript := p2pkAddr.PaymentScript() + payoutScript := make([]byte, len(payScript)+1) + payoutScript[0] = txscript.OP_TGEN + copy(payoutScript[1:], payScript) + + tx := baseTreasurySpendTx.Copy() + tx.TxOut[1].Version = payoutScriptVer + tx.TxOut[1].PkScript = payoutScript + return tx + }(), + }} - msgTx.AddTxIn(&wire.TxIn{ - // Stakebase transactions have no - // inputs, so previous outpoint is zero - // hash and max index. - PreviousOutPoint: *wire.NewOutPoint(&chainhash.Hash{}, - wire.MaxPrevOutIndex, wire.TxTreeRegular), - Sequence: wire.MaxTxInSequenceNum, - BlockHeight: wire.NullBlockHeight, - BlockIndex: wire.NullBlockIndex, - SignatureScript: tspendScript, - }) + for _, test := range tests { + gotTreasuryAdd := IsTAdd(test.tx) + if gotTreasuryAdd != test.treasuryAdd { + t.Errorf("%s: unexpected treasury add result - got %v, want %v", + test.name, gotTreasuryAdd, test.treasuryAdd) + } - return msgTx - }, - is: IsTSpend, - expected: true, - check: checkTSpend, - }} + gotTreasuryBase := IsTreasuryBase(test.tx) + if gotTreasuryBase != test.treasuryBase { + t.Errorf("%s: unexpected treasurybase result - got %v, want %v", + test.name, gotTreasuryBase, test.treasuryBase) + } - for i, test := range tests { - if got := test.is(test.createTx()); got != test.expected { - // Obtain error - err := test.check(test.createTx()) - t.Fatalf("%v %v: failed got %v want %v error %v", - i, test.name, got, test.expected, err) + gotTreasurySpend := IsTSpend(test.tx) + if gotTreasurySpend != test.treasurySpend { + t.Errorf("%s: unexpected treasury spend result - got %v, want %v", + test.name, gotTreasurySpend, test.treasurySpend) } } } From 605214b513450807d87576a81783193f3e581dea Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Wed, 22 Apr 2026 17:48:07 -0500 Subject: [PATCH 3/9] blockchain/stake: Remove duplicate test. Now that the updated treasury spend tests cover the fully valid case, there is no benefit to repeating it in another test. This is part of a larger overall effort to bring the treasury code up to the standards used throughout the rest of the blockchain consensus code. --- blockchain/stake/treasury_test.go | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/blockchain/stake/treasury_test.go b/blockchain/stake/treasury_test.go index 31c23990a7..300472d020 100644 --- a/blockchain/stake/treasury_test.go +++ b/blockchain/stake/treasury_test.go @@ -5,9 +5,7 @@ package stake import ( - "bytes" "encoding/binary" - "encoding/hex" "errors" "math" "math/rand" @@ -839,25 +837,6 @@ var tspendInvalidTxVersion = &wire.MsgTx{ Expiry: 0, } -func TestTSpendGenerated(t *testing.T) { - rawScript := "03000000010000000000000000000000000000000000000000000000000000000000000000ffffffff00ffffffff0200000000000000000000226a20562ce42e7531d1710ea1ee02628191190ef5152bbbcd23acca864433c4e4e7849cf1052a01000000000018c3a914f5a8302ee8695bf836258b8f2b57b38a0be14e478700000000520000000100f2052a0100000000000000ffffffff64408ea1c04f5e5dd59350847fad8b800887200ae7268da3b70488a605dd5f4ad28e6e240dbd483a8ba46324a047cf0d6c506e6ebb61d93cae6e868b86f31d9bda892103b459ccf3ce4935a676414fd9ec93ecf7c9dad081a52ed6993bf073c627499388c2" - s, err := hex.DecodeString(rawScript) - if err != nil { - t.Fatal(err) - } - var tx wire.MsgTx - err = tx.Deserialize(bytes.NewReader(s)) - if err != nil { - t.Fatalf("Deserialize: %v", err) - } - tx.Version = wire.TxVersionTreasury - - err = checkTSpend(&tx) - if err != nil { - t.Fatalf("checkTSpend: %v", err) - } -} - func TestTSpendErrors(t *testing.T) { tests := []struct { name string From 0175551281572fbc05949bb354b72d8533f6d8da Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Wed, 22 Apr 2026 17:48:07 -0500 Subject: [PATCH 4/9] blockchain/stake: Rework treasury spend err tests. This reworks the treasury spend error tests to use the newly introduced functions that start with a valid treasury spend and then mutates a copy to induce the specific error to test. In the process, it also corrects some tests that weren't actually tsting what they claimed. The result is significantly more readable, provides more comprehensive test coverage, is more consistent with the other tests throughout the code base, and reduces the test code for the relevant tests by about 69%. This is part of a larger overall effort to bring the treasury code up to the standards used throughout the rest of the blockchain consensus code. --- blockchain/stake/treasury_test.go | 824 +++++++----------------------- 1 file changed, 194 insertions(+), 630 deletions(-) diff --git a/blockchain/stake/treasury_test.go b/blockchain/stake/treasury_test.go index 300472d020..9d1f9f3b7b 100644 --- a/blockchain/stake/treasury_test.go +++ b/blockchain/stake/treasury_test.go @@ -33,89 +33,6 @@ var ( validSignature = hexToBytes("776984f68313b1ac629e624af0595bdc09d8ded02bc2" + "b29fbdb39595e03ac8b0cf818ca536723e6390d3084e0e31c7942229153ce34d8739" + "29b16088d9e1af43") - - // OP_DATA_64 OP_TSPEND - tspendValidKey = []byte{ - 0x40, // OP_DATA_64 valid signature - 0x77, 0x69, 0x84, 0xf6, 0x83, 0x13, 0xb1, 0xac, - 0x62, 0x9e, 0x62, 0x4a, 0xf0, 0x59, 0x5b, 0xdc, - 0x09, 0xd8, 0xde, 0xd0, 0x2b, 0xc2, 0xb2, 0x9f, - 0xbd, 0xb3, 0x95, 0x95, 0xe0, 0x3a, 0xc8, 0xb0, - 0xcf, 0x81, 0x8c, 0xa5, 0x36, 0x72, 0x3e, 0x63, - 0x90, 0xd3, 0x08, 0x4e, 0x0e, 0x31, 0xc7, 0x94, - 0x22, 0x29, 0x15, 0x3c, 0xe3, 0x4d, 0x87, 0x39, - 0x29, 0xb1, 0x60, 0x88, 0xd9, 0xe1, 0xaf, 0x43, - 0x21, // OP_DATA_33 valid public key - 0x02, 0xa4, 0xf6, 0x45, 0x86, 0xe1, 0x72, 0xc3, - 0xd9, 0xa2, 0x0c, 0xfa, 0x6c, 0x7a, 0xc8, 0xfb, - 0x12, 0xf0, 0x11, 0x5b, 0x3f, 0x69, 0xc3, 0xc3, - 0x5a, 0xec, 0x93, 0x3a, 0x4c, 0x47, 0xc7, 0xd9, - 0x2c, - 0xc2, // OP_TSPEND - } - - // OP_DATA_64 - tspendNoTSpend = []byte{ - 0x40, // OP_DATA_64 valid signature - 0x77, 0x69, 0x84, 0xf6, 0x83, 0x13, 0xb1, 0xac, - 0x62, 0x9e, 0x62, 0x4a, 0xf0, 0x59, 0x5b, 0xdc, - 0x09, 0xd8, 0xde, 0xd0, 0x2b, 0xc2, 0xb2, 0x9f, - 0xbd, 0xb3, 0x95, 0x95, 0xe0, 0x3a, 0xc8, 0xb0, - 0xcf, 0x81, 0x8c, 0xa5, 0x36, 0x72, 0x3e, 0x63, - 0x90, 0xd3, 0x08, 0x4e, 0x0e, 0x31, 0xc7, 0x94, - 0x22, 0x29, 0x15, 0x3c, 0xe3, 0x4d, 0x87, 0x39, - 0x29, 0xb1, 0x60, 0x88, 0xd9, 0xe1, 0xaf, 0x43, - 0x21, // OP_DATA_33 valid public key - 0x02, 0xa4, 0xf6, 0x45, 0x86, 0xe1, 0x72, 0xc3, - 0xd9, 0xa2, 0x0c, 0xfa, 0x6c, 0x7a, 0xc8, 0xfb, - 0x12, 0xf0, 0x11, 0x5b, 0x3f, 0x69, 0xc3, 0xc3, - 0x5a, 0xec, 0x93, 0x3a, 0x4c, 0x47, 0xc7, 0xd9, - 0x2c, // No OP_TSPEND - } - - // nolint: dupword - // - // OP_DATA_64 OP_TSPEND OP_TSPEND - tspendTwoTSpend = []byte{ - 0x40, // OP_DATA_64 valid signature - 0x77, 0x69, 0x84, 0xf6, 0x83, 0x13, 0xb1, 0xac, - 0x62, 0x9e, 0x62, 0x4a, 0xf0, 0x59, 0x5b, 0xdc, - 0x09, 0xd8, 0xde, 0xd0, 0x2b, 0xc2, 0xb2, 0x9f, - 0xbd, 0xb3, 0x95, 0x95, 0xe0, 0x3a, 0xc8, 0xb0, - 0xcf, 0x81, 0x8c, 0xa5, 0x36, 0x72, 0x3e, 0x63, - 0x90, 0xd3, 0x08, 0x4e, 0x0e, 0x31, 0xc7, 0x94, - 0x22, 0x29, 0x15, 0x3c, 0xe3, 0x4d, 0x87, 0x39, - 0x29, 0xb1, 0x60, 0x88, 0xd9, 0xe1, 0xaf, 0x43, - 0x21, // OP_DATA_33 valid public key - 0x02, 0xa4, 0xf6, 0x45, 0x86, 0xe1, 0x72, 0xc3, - 0xd9, 0xa2, 0x0c, 0xfa, 0x6c, 0x7a, 0xc8, 0xfb, - 0x12, 0xf0, 0x11, 0x5b, 0x3f, 0x69, 0xc3, 0xc3, - 0x5a, 0xec, 0x93, 0x3a, 0x4c, 0x47, 0xc7, 0xd9, - 0x2c, // No OP_TSPEND - 0xc2, // OP_TSPEND - 0xc2, // Extra OP_TSPEND - } - - // OP_DATA_64 OP_TSPEND OP_DATA_1 - tspendTrailingData = []byte{ - 0x40, // OP_DATA_64 valid signature - 0x77, 0x69, 0x84, 0xf6, 0x83, 0x13, 0xb1, 0xac, - 0x62, 0x9e, 0x62, 0x4a, 0xf0, 0x59, 0x5b, 0xdc, - 0x09, 0xd8, 0xde, 0xd0, 0x2b, 0xc2, 0xb2, 0x9f, - 0xbd, 0xb3, 0x95, 0x95, 0xe0, 0x3a, 0xc8, 0xb0, - 0xcf, 0x81, 0x8c, 0xa5, 0x36, 0x72, 0x3e, 0x63, - 0x90, 0xd3, 0x08, 0x4e, 0x0e, 0x31, 0xc7, 0x94, - 0x22, 0x29, 0x15, 0x3c, 0xe3, 0x4d, 0x87, 0x39, - 0x29, 0xb1, 0x60, 0x88, 0xd9, 0xe1, 0xaf, 0x43, - 0x21, // OP_DATA_33 valid public key - 0x02, 0xa4, 0xf6, 0x45, 0x86, 0xe1, 0x72, 0xc3, - 0xd9, 0xa2, 0x0c, 0xfa, 0x6c, 0x7a, 0xc8, 0xfb, - 0x12, 0xf0, 0x11, 0x5b, 0x3f, 0x69, 0xc3, 0xc3, - 0x5a, 0xec, 0x93, 0x3a, 0x4c, 0x47, 0xc7, 0xd9, - 0x2c, // No OP_TSPEND - 0xc2, // OP_TSPEND - 0x01, // OP_DATA_1, ByteIndex test in CheckTSpend - } ) // opReturnScript returns a provably-pruneable OP_RETURN script with the @@ -380,557 +297,204 @@ func TestTreasuryIsFunctions(t *testing.T) { } } -var tspendTxInNoPubkey = wire.TxIn{ - PreviousOutPoint: wire.OutPoint{ - Hash: chainhash.Hash{}, - Index: 0xffffffff, - Tree: wire.TxTreeRegular, - }, - SignatureScript: []byte{ - 0xc2, // OP_TSPEND - }, - BlockHeight: wire.NullBlockHeight, - BlockIndex: wire.NullBlockIndex, - Sequence: 0xffffffff, -} - -// tspendTxInInvalidPubkey is a TxIn with an invalid key on the OP_TSPEND. -var tspendTxInInvalidPubkey = wire.TxIn{ - PreviousOutPoint: wire.OutPoint{ - Hash: chainhash.Hash{}, - Index: 0xffffffff, - Tree: wire.TxTreeRegular, - }, - SignatureScript: []byte{ - 0xc2, // OP_TSPEND - 0x23, // OP_DATA_35 - 0x03, // Valid pubkey version - 0x00, // invalid compressed key - }, - BlockHeight: wire.NullBlockHeight, - BlockIndex: wire.NullBlockIndex, - Sequence: 0xffffffff, -} - -// tspendTxInInvalidOpcode is a TxIn with an invalid opcode where OP_TSPEND was -// supposed to be. -var tspendTxInInvalidOpcode = wire.TxIn{ - PreviousOutPoint: wire.OutPoint{ - Hash: chainhash.Hash{}, - Index: 0xffffffff, - Tree: wire.TxTreeRegular, - }, - SignatureScript: []byte{ - 0x40, // OP_DATA_64 valid signature - 0x77, 0x69, 0x84, 0xf6, 0x83, 0x13, 0xb1, 0xac, - 0x62, 0x9e, 0x62, 0x4a, 0xf0, 0x59, 0x5b, 0xdc, - 0x09, 0xd8, 0xde, 0xd0, 0x2b, 0xc2, 0xb2, 0x9f, - 0xbd, 0xb3, 0x95, 0x95, 0xe0, 0x3a, 0xc8, 0xb0, - 0xcf, 0x81, 0x8c, 0xa5, 0x36, 0x72, 0x3e, 0x63, - 0x90, 0xd3, 0x08, 0x4e, 0x0e, 0x31, 0xc7, 0x94, - 0x22, 0x29, 0x15, 0x3c, 0xe3, 0x4d, 0x87, 0x39, - 0x29, 0xb1, 0x60, 0x88, 0xd9, 0xe1, 0xaf, 0x43, - 0x21, // OP_DATA_33 valid public key - 0x02, 0xa4, 0xf6, 0x45, 0x86, 0xe1, 0x72, 0xc3, - 0xd9, 0xa2, 0x0c, 0xfa, 0x6c, 0x7a, 0xc8, 0xfb, - 0x12, 0xf0, 0x11, 0x5b, 0x3f, 0x69, 0xc3, 0xc3, - 0x5a, 0xec, 0x93, 0x3a, 0x4c, 0x47, 0xc7, 0xd9, - 0x2c, - 0x6a, // OP_RETURN instead of OP_TSPEND - }, - BlockHeight: wire.NullBlockHeight, - BlockIndex: wire.NullBlockIndex, - Sequence: 0xffffffff, -} - -// tspendTxInInvalidPubkey2 is a TxIn with an invalid public key on the -// OP_TSPEND. -var tspendTxInInvalidPubkey2 = wire.TxIn{ - PreviousOutPoint: wire.OutPoint{ - Hash: chainhash.Hash{}, - Index: 0xffffffff, - Tree: wire.TxTreeRegular, - }, - SignatureScript: []byte{ - 0x40, // OP_DATA_64 valid signature - 0x77, 0x69, 0x84, 0xf6, 0x83, 0x13, 0xb1, 0xac, - 0x62, 0x9e, 0x62, 0x4a, 0xf0, 0x59, 0x5b, 0xdc, - 0x09, 0xd8, 0xde, 0xd0, 0x2b, 0xc2, 0xb2, 0x9f, - 0xbd, 0xb3, 0x95, 0x95, 0xe0, 0x3a, 0xc8, 0xb0, - 0xcf, 0x81, 0x8c, 0xa5, 0x36, 0x72, 0x3e, 0x63, - 0x90, 0xd3, 0x08, 0x4e, 0x0e, 0x31, 0xc7, 0x94, - 0x22, 0x29, 0x15, 0x3c, 0xe3, 0x4d, 0x87, 0x39, - 0x29, 0xb1, 0x60, 0x88, 0xd9, 0xe1, 0xaf, 0x43, - 0x21, // OP_DATA_33 INVALID public key - 0x00, 0xa4, 0xf6, 0x45, 0x86, 0xe1, 0x72, 0xc3, - 0xd9, 0xa2, 0x0c, 0xfa, 0x6c, 0x7a, 0xc8, 0xfb, - 0x12, 0xf0, 0x11, 0x5b, 0x3f, 0x69, 0xc3, 0xc3, - 0x5a, 0xec, 0x93, 0x3a, 0x4c, 0x47, 0xc7, 0xd9, - 0x2c, - 0xc2, // OP_TSPEND - }, - BlockHeight: wire.NullBlockHeight, - BlockIndex: wire.NullBlockIndex, - Sequence: 0xffffffff, -} - -var tspendTxOutValidReturn = wire.TxOut{ - Value: 500000000, - Version: 0, - PkScript: []byte{ - 0x6a, // OP_RETURN - 0x20, // OP_DATA_32 - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - }, -} - -var tspendTxOutInvalidReturn = wire.TxOut{ - Value: 500000000, - Version: 0, - PkScript: []byte{ - 0x6a, // OP_RETURN - 0x20, // OP_DATA_32 - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // 1 byte short - }, -} - -// tspendTxInValidPubkey is a TxIn with a public key on the OP_TSPEND. -var tspendTxInValidPubkey = wire.TxIn{ - PreviousOutPoint: wire.OutPoint{ - Hash: chainhash.Hash{}, - Index: 0xffffffff, - Tree: wire.TxTreeRegular, - }, - SignatureScript: tspendValidKey, - BlockHeight: wire.NullBlockHeight, - BlockIndex: wire.NullBlockIndex, - Sequence: 0xffffffff, -} - -// tspendTxInNoTSpend is a TxIn with a public key but not TSpend opcode. -var tspendTxInNoTSpend = wire.TxIn{ - PreviousOutPoint: wire.OutPoint{ - Hash: chainhash.Hash{}, - Index: 0xffffffff, - Tree: wire.TxTreeRegular, - }, - SignatureScript: tspendNoTSpend, - BlockHeight: wire.NullBlockHeight, - BlockIndex: wire.NullBlockIndex, - Sequence: 0xffffffff, -} - -// tspendTxInTwoTSpend is a TxIn with a public key but two TSpend opcodes. -var tspendTxInTwoTSpend = wire.TxIn{ - PreviousOutPoint: wire.OutPoint{ - Hash: chainhash.Hash{}, - Index: 0xffffffff, - Tree: wire.TxTreeRegular, - }, - SignatureScript: tspendTwoTSpend, - BlockHeight: wire.NullBlockHeight, - BlockIndex: wire.NullBlockIndex, - Sequence: 0xffffffff, -} - -// tspendTxTrailingData is a TxIn with a public key, one TSpend and an -// OP_DATA_1. -var tspendTxTrailingData = wire.TxIn{ - PreviousOutPoint: wire.OutPoint{ - Hash: chainhash.Hash{}, - Index: 0xffffffff, - Tree: wire.TxTreeRegular, - }, - SignatureScript: tspendTrailingData, - BlockHeight: wire.NullBlockHeight, - BlockIndex: wire.NullBlockIndex, - Sequence: 0xffffffff, -} - -// tspendInvalidInCount has an invalid TxIn count but a valid TxOut count. -var tspendInvalidInCount = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{}, - TxOut: []*wire.TxOut{ - {}, // 2 TxOuts is valid - {}, - }, - LockTime: 0, - Expiry: 0, -} - -// tspendInvalidOutCount has a valid TxIn count but an invalid TxOut count. -var tspendInvalidOutCount = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - &tspendTxInNoPubkey, - }, - TxOut: []*wire.TxOut{}, - LockTime: 0, - Expiry: 0, -} - -// tspendInvalidVersion has an invalid version in an out script. -var tspendInvalidVersion = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - &tspendTxInNoPubkey, - }, - TxOut: []*wire.TxOut{ - { - Version: 0, - PkScript: []byte{ - 0x6a, // OP_RETURN - }, - }, - { - Version: 1, // Fail - PkScript: []byte{ - 0xc3, // OP_TGEN - }, - }, - }, - LockTime: 0, - Expiry: 0, -} - -// tspendInvalidSignature has no publick key in the input script. -var tspendInvalidSignature = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - &tspendTxInNoPubkey, - }, - TxOut: []*wire.TxOut{ - { - PkScript: []byte{ - 0x6a, // OP_RETURN - }, - }, - { - PkScript: []byte{ - 0xc3, // OP_TGEN - }, - }, - }, - LockTime: 0, - Expiry: 0, -} - -// tspendInvalidSignature2 has an invalid public key in the input script. -var tspendInvalidSignature2 = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - &tspendTxInInvalidPubkey, - }, - TxOut: []*wire.TxOut{ - { - PkScript: []byte{ - 0x6a, // OP_RETURN - }, - }, - { - PkScript: []byte{ - 0xc3, // OP_TGEN - }, - }, - }, - LockTime: 0, - Expiry: 0, -} - -// tspendInvalidOpcode has an invalid opcode in the first TxIn. -var tspendInvalidOpcode = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - &tspendTxInInvalidOpcode, - }, - TxOut: []*wire.TxOut{ - { - PkScript: []byte{ - 0x6a, // OP_RETURN - }, - }, - { - PkScript: []byte{ - 0xc3, // OP_TGEN - }, - }, - }, - LockTime: 0, - Expiry: 0, -} - -// tspendInvalidPubkey has an invalid public key on the TSPEND. -var tspendInvalidPubkey = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - &tspendTxInInvalidPubkey2, - }, - TxOut: []*wire.TxOut{ - { - PkScript: []byte{ - 0x6a, // OP_RETURN - }, - }, - { - PkScript: []byte{ - 0xc3, // OP_TGEN - }, - }, - }, - LockTime: 0, - Expiry: 0, -} - -// tspendInvalidScriptLength has an invalid TxOut that has a zero length. -var tspendInvalidScriptLength = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - &tspendTxInValidPubkey, - }, - TxOut: []*wire.TxOut{ - &tspendTxOutValidReturn, - {}, - }, - LockTime: 0, - Expiry: 0, -} - -// tspendInvalidTokenCount does not have enough tokens in input script. -var tspendInvalidTokenCount = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - &tspendTxInNoTSpend, - }, - TxOut: []*wire.TxOut{ - &tspendTxOutValidReturn, - { - PkScript: []byte{ - 0xc3, // OP_TGEN - }, - }, - }, - LockTime: 0, - Expiry: 0, -} - -// tspendInvalidTokenCount2 has too many tokens on input script. -var tspendInvalidTokenCount2 = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - &tspendTxInTwoTSpend, - }, - TxOut: []*wire.TxOut{ - &tspendTxOutValidReturn, - { - PkScript: []byte{ - 0xc3, // OP_TGEN - }, - }, - }, - LockTime: 0, - Expiry: 0, -} - -// tspendInvalidTokenCount3 has trailing data after TSpend. -var tspendInvalidTokenCount3 = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - &tspendTxTrailingData, - }, - TxOut: []*wire.TxOut{ - &tspendTxOutValidReturn, - { - PkScript: []byte{ - 0xc3, // OP_TGEN - }, - }, - }, - LockTime: 0, - Expiry: 0, -} - -// tspendInvalidTransaction has an invalid hash on the OP_RETURN. -var tspendInvalidTransaction = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - &tspendTxInValidPubkey, - }, - TxOut: []*wire.TxOut{ - &tspendTxOutInvalidReturn, - { - PkScript: []byte{ - 0xc3, // OP_TGEN - }, - }, - }, - LockTime: 0, - Expiry: 0, -} - -// tspendInvalidTGen has an invalid TxOut that isn't tagged with an OP_TGEN. -var tspendInvalidTGen = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - &tspendTxInValidPubkey, - }, - TxOut: []*wire.TxOut{ - &tspendTxOutValidReturn, - { - PkScript: []byte{ - 0x6a, // OP_RETURN instead of OP_TGEN - }}, - }, - LockTime: 0, - Expiry: 0, -} +// TestTreasurySpendErrors verifies that all check treasury spend errors can be +// hit and return the proper error. +func TestTreasurySpendErrors(t *testing.T) { + tests := []struct { + name string // test description + tx *wire.MsgTx // transaction to test + expected error // expected error + }{{ + name: "treasury spend invalid tx version", + tx: func() *wire.MsgTx { + tx := baseTreasurySpendTx.Copy() + tx.Version = 1 + return tx + }(), + expected: ErrTSpendInvalidTxVersion, + }, { + name: "treasury spend with invalid num inputs", + tx: func() *wire.MsgTx { + tx := baseTreasurySpendTx.Copy() + tx.TxIn = nil + return tx + }(), + expected: ErrTSpendInvalidLength, + }, { + name: "treasury spend with invalid num outputs", + tx: func() *wire.MsgTx { + tx := baseTreasurySpendTx.Copy() + tx.TxOut = nil + return tx + }(), + expected: ErrTSpendInvalidLength, + }, { + name: "treasury spend with an invalid script version", + tx: func() *wire.MsgTx { + tx := baseTreasurySpendTx.Copy() + tx.TxOut[1].Version = 1 + return tx + }(), + expected: ErrTSpendInvalidVersion, + }, { + name: "treasury spend with invalid output - no pubkey script", + tx: func() *wire.MsgTx { + tx := baseTreasurySpendTx.Copy() + tx.TxOut[1].PkScript = nil + return tx + }(), + expected: ErrTSpendInvalidScriptLength, + }, { + name: "treasury spend invalid input sig script - wrong script length", + tx: func() *wire.MsgTx { + sig := treasurySpendSignature(validSignature, nil) + tx := baseTreasurySpendTx.Copy() + tx.TxIn[0].SignatureScript = sig + return tx + }(), + expected: ErrTSpendInvalidScript, + }, { + name: "treasury spend input sig script invalid - wrong sig len", + tx: func() *wire.MsgTx { + tx := baseTreasurySpendTx.Copy() + sig := tx.TxIn[0].SignatureScript + if sig[0] != txscript.OP_DATA_64 { + panic("signature script format changed") + } + sig[0] = txscript.OP_DATA_65 // Wrong length. + return tx + }(), + expected: ErrTSpendInvalidScript, + }, { + name: "treasury spend input sig script invalid - wrong pubkey len", + tx: func() *wire.MsgTx { + tx := baseTreasurySpendTx.Copy() + sig := tx.TxIn[0].SignatureScript + if sig[65] != txscript.OP_DATA_33 { + panic("signature script format changed") + } + sig[65] = txscript.OP_DATA_34 // Wrong length. + return tx + }(), + expected: ErrTSpendInvalidScript, + }, { + name: "treasury spend input sig invalid - wrong opcode for OP_TSPEND", + tx: func() *wire.MsgTx { + tx := baseTreasurySpendTx.Copy() + sig := tx.TxIn[0].SignatureScript + if sig[len(sig)-1] != txscript.OP_TSPEND { + panic("signature script format changed") + } + sig[len(sig)-1] = txscript.OP_RETURN // Wrong opcode. + return tx + }(), + expected: ErrTSpendInvalidScript, + }, { + name: "treasury spend input sig invalid - no tspend opcode", + tx: func() *wire.MsgTx { + tx := baseTreasurySpendTx.Copy() + sig := tx.TxIn[0].SignatureScript + tx.TxIn[0].SignatureScript = sig[:len(sig)-1] + return tx + }(), + expected: ErrTSpendInvalidScript, + }, { + name: "treasury spend input sig invalid - two tspend opcodes", + tx: func() *wire.MsgTx { + tx := baseTreasurySpendTx.Copy() + sig := tx.TxIn[0].SignatureScript + sig = append(sig, txscript.OP_TSPEND) + tx.TxIn[0].SignatureScript = sig + return tx + }(), + expected: ErrTSpendInvalidScript, + }, { + name: "treasury spend input sig invalid - trailing data", + tx: func() *wire.MsgTx { + tx := baseTreasurySpendTx.Copy() + sig := tx.TxIn[0].SignatureScript + sig = append(sig, 0x01) + tx.TxIn[0].SignatureScript = sig + return tx + }(), + expected: ErrTSpendInvalidScript, + }, { + name: "treasury spend input sig script invalid - bad pubkey type", + tx: func() *wire.MsgTx { + pubKey := make([]byte, len(publicKey)) + copy(pubKey, publicKey) + pubKey[0] |= 0x04 + sig := treasurySpendSignature(validSignature, pubKey) -// tspendInvalidP2SH has an invalid TxOut that doesn't have a valid P2SH -// script. -var tspendInvalidP2SH = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - &tspendTxInValidPubkey, - }, - TxOut: []*wire.TxOut{ - &tspendTxOutValidReturn, - { - PkScript: []byte{ - 0xc3, // OP_TGEN - 0x00, // Invalid P2SH - }}, - }, - LockTime: 0, - Expiry: 0, -} + tx := baseTreasurySpendTx.Copy() + tx.TxIn[0].SignatureScript = sig + return tx + }(), + expected: ErrTSpendInvalidPubkey, + }, { + name: "treasury spend invalid - extra empty output", + tx: func() *wire.MsgTx { + tx := baseTreasurySpendTx.Copy() + tx.AddTxOut(&wire.TxOut{}) + return tx + }(), + expected: ErrTSpendInvalidScriptLength, + }, { + name: "treasury spend invalid OP_RETURN output - short one byte", + tx: func() *wire.MsgTx { + tx := baseTreasurySpendTx.Copy() + script := tx.TxOut[0].PkScript + script = script[:len(script)-1] + tx.TxOut[0].PkScript = script + return tx + }(), + expected: ErrTSpendInvalidTransaction, + }, { + name: "treasury spend payment output - wrong opcode for OP_TGEN", + tx: func() *wire.MsgTx { + tx := baseTreasurySpendTx.Copy() + if tx.TxOut[1].PkScript[0] != txscript.OP_TGEN { + panic("payment output format changed") + } + tx.TxOut[1].PkScript[0] = txscript.OP_RETURN + return tx + }(), + expected: ErrTSpendInvalidTGen, + }, { + name: "treasury spend payment output - unsupported p2pk", + tx: func() *wire.MsgTx { + // Start with a normal payment script for the p2pk and manually add + // the OP_TGEN prefix since there is no standard method to create + // the pay from treasury script on a p2pk address given it is + // invalid. + params := chaincfg.RegNetParams() + p2pkAddr, err := stdaddr.NewAddressPubKeyEcdsaSecp256k1V0Raw( + publicKey, params) + if err != nil { + panic(err) + } + payoutScriptVer, payScript := p2pkAddr.PaymentScript() + payoutScript := make([]byte, len(payScript)+1) + payoutScript[0] = txscript.OP_TGEN + copy(payoutScript[1:], payScript) -var tspendInvalidTxVersion = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 1, // Invalid version - TxIn: []*wire.TxIn{ - &tspendTxInValidPubkey, - }, - TxOut: []*wire.TxOut{ - &tspendTxOutValidReturn, - }, - LockTime: 0, - Expiry: 0, -} + tx := baseTreasurySpendTx.Copy() + tx.TxOut[1].Version = payoutScriptVer + tx.TxOut[1].PkScript = payoutScript + return tx + }(), + expected: ErrTSpendInvalidSpendScript, + }} -func TestTSpendErrors(t *testing.T) { - tests := []struct { - name string - tx *wire.MsgTx - expected error - }{ - { - name: "tspendInvalidOutCount", - tx: tspendInvalidOutCount, - expected: ErrTSpendInvalidLength, - }, - { - name: "tspendInvalidInCount", - tx: tspendInvalidInCount, - expected: ErrTSpendInvalidLength, - }, - { - name: "tspendInvalidVersion", - tx: tspendInvalidVersion, - expected: ErrTSpendInvalidVersion, - }, - { - name: "tspendInvalidSignature", - tx: tspendInvalidSignature, - expected: ErrTSpendInvalidScript, - }, - { - name: "tspendInvalidSignature2", - tx: tspendInvalidSignature2, - expected: ErrTSpendInvalidScript, - }, - { - name: "tspendInvalidOpcode", - tx: tspendInvalidOpcode, - expected: ErrTSpendInvalidScript, - }, - { - name: "tspendInvalidPubkey", - tx: tspendInvalidPubkey, - expected: ErrTSpendInvalidPubkey, - }, - { - name: "tspendInvalidTokenCount", - tx: tspendInvalidTokenCount, - expected: ErrTSpendInvalidScript, - }, - { - name: "tspendInvalidTokenCount2", - tx: tspendInvalidTokenCount2, - expected: ErrTSpendInvalidScript, - }, - { - name: "tspendInvalidTokenCount3", - tx: tspendInvalidTokenCount3, - expected: ErrTSpendInvalidScript, - }, - { - name: "tspendInvalidScriptLength", - tx: tspendInvalidScriptLength, - expected: ErrTSpendInvalidScriptLength, - }, - { - name: "tspendInvalidTransaction", - tx: tspendInvalidTransaction, - expected: ErrTSpendInvalidTransaction, - }, - { - name: "tspendInvalidTGen", - tx: tspendInvalidTGen, - expected: ErrTSpendInvalidTGen, - }, - { - name: "tspendInvalidP2SH", - tx: tspendInvalidP2SH, - expected: ErrTSpendInvalidSpendScript, - }, - { - name: "tspendInvalidTxVersion", - tx: tspendInvalidTxVersion, - expected: ErrTSpendInvalidTxVersion, - }, - } - for i, tt := range tests { - test := dcrutil.NewTx(tt.tx) - test.SetTree(wire.TxTreeStake) - test.SetIndex(0) - err := checkTSpend(test.MsgTx()) - if !errors.Is(err, tt.expected) { - t.Errorf("%v: checkTSpend should have returned %v but "+ - "instead returned %v", tt.name, tt.expected, err) + for _, test := range tests { + err := checkTSpend(test.tx) + if !errors.Is(err, test.expected) { + t.Errorf("%q: unexpected error -- got %v, want %v", test.name, err, + test.expected) } - if IsTSpend(test.MsgTx()) { - t.Errorf("IsTSpend claimed an invalid tspend is valid"+ - " %v %v", i, tt.name) + if IsTSpend(test.tx) { + t.Errorf("%q: IsTSpend claimed an invalid treasury spend is valid", + test.name) } } } From 79c0502a729b396ab75d67c2c352271e1a8cf801 Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Wed, 22 Apr 2026 17:48:08 -0500 Subject: [PATCH 5/9] blockchain/stake: Rework treasury add err tests. This reworks the treasury add error tests to use the newly introduced functions that start with a valid treasury add transaction and then mutates a copy to induce the specific error to test. In the process, it also corrects some tests that weren't actually tsting what they claimed. The result is significantly more readable, provides more comprehensive test coverage, is more consistent with the other tests throughout the code base, and reduces the test code for the relevant tests by about 56%. This is part of a larger overall effort to bring the treasury code up to the standards used throughout the rest of the blockchain consensus code. --- blockchain/stake/treasury_test.go | 295 +++++++++--------------------- 1 file changed, 90 insertions(+), 205 deletions(-) diff --git a/blockchain/stake/treasury_test.go b/blockchain/stake/treasury_test.go index 9d1f9f3b7b..609c07022b 100644 --- a/blockchain/stake/treasury_test.go +++ b/blockchain/stake/treasury_test.go @@ -13,7 +13,6 @@ import ( "github.com/decred/dcrd/chaincfg/chainhash" "github.com/decred/dcrd/chaincfg/v3" - "github.com/decred/dcrd/dcrutil/v4" "github.com/decred/dcrd/txscript/v4" "github.com/decred/dcrd/txscript/v4/stdaddr" "github.com/decred/dcrd/wire" @@ -499,215 +498,101 @@ func TestTreasurySpendErrors(t *testing.T) { } } -// taddInvalidOutCount has a valid TxIn count but an invalid TxOut count. -var taddInvalidOutCount = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{}, - TxOut: []*wire.TxOut{}, - LockTime: 0, - Expiry: 0, -} - -// taddInvalidOutCount2 has a valid TxIn count but an invalid TxOut count. -var taddInvalidOutCount2 = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - {}, // Valid TxIn count - }, - TxOut: []*wire.TxOut{ - {}, - {}, - {}, - }, - LockTime: 0, - Expiry: 0, -} - -// taddInvalidOutCount3 has a valid TxIn count but an invalid TxIn count. -var taddInvalidOutCount3 = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{}, - TxOut: []*wire.TxOut{ - {}, - {}, - }, - LockTime: 0, - Expiry: 0, -} - -// taddInvalidVersion has an invalid out script version. -var taddInvalidVersion = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - {}, // Empty TxIn - }, - TxOut: []*wire.TxOut{ - {Version: 1}, - {Version: 0}, - }, - LockTime: 0, - Expiry: 0, -} - -// taddInvalidScriptLength has a zero script length. -var taddInvalidScriptLength = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - {}, // Empty TxIn - }, - TxOut: []*wire.TxOut{ - {Version: 0}, - {Version: 0}, - }, - LockTime: 0, - Expiry: 0, -} - -// taddInvalidLength has an invalid out script. -var taddInvalidLength = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - {}, // Empty TxIn - }, - TxOut: []*wire.TxOut{ - {PkScript: []byte{ - 0xc2, // OP_TSPEND instead of OP_TADD - 0x00, // Fail length test - }}, - }, - LockTime: 0, - Expiry: 0, -} - -// taddInvalidLength has an invalid out script opcode. -var taddInvalidOpcode = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - {}, // Empty TxIn - }, - TxOut: []*wire.TxOut{ - { - PkScript: []byte{ - 0xc2, // OP_TSPEND instead of OP_TADD - }, - }, - }, - LockTime: 0, - Expiry: 0, -} - -// taddInvalidChange has an invalid out chnage script. -var taddInvalidChange = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - {}, // Empty TxIn - }, - TxOut: []*wire.TxOut{ - { - PkScript: []byte{ - 0xc1, // OP_TADD - }, - }, - { - PkScript: []byte{ - 0x00, // Not OP_SSTXCHANGE - }, - }, - }, - LockTime: 0, - Expiry: 0, -} - -// taddInvalidTxVersion has an invalid transaction version. -var taddInvalidTxVersion = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 1, // Invalid - TxIn: []*wire.TxIn{}, - TxOut: []*wire.TxOut{ - { - PkScript: []byte{ - 0xc1, // OP_TADD - }, - }, - }, - LockTime: 0, - Expiry: 0, -} - -// TestTAddErrors verifies that all TADD errors can be hit and return the -// proper error. -func TestTAddErrors(t *testing.T) { +// TestTreasuryAddErrors verifies that all check treasury add errors can be hit +// and return the proper error. +func TestTreasuryAddErrors(t *testing.T) { tests := []struct { name string tx *wire.MsgTx expected error - }{ - { - name: "taddInvalidOutCount", - tx: taddInvalidOutCount, - expected: ErrTAddInvalidCount, - }, - { - name: "taddInvalidOutCount2", - tx: taddInvalidOutCount2, - expected: ErrTAddInvalidCount, - }, - { - name: "taddInvalidOutCount3", - tx: taddInvalidOutCount3, - expected: ErrTAddInvalidCount, - }, - { - name: "taddInvalidVersion", - tx: taddInvalidVersion, - expected: ErrTAddInvalidVersion, - }, - { - name: "taddInvalidScriptLength", - tx: taddInvalidScriptLength, - expected: ErrTAddInvalidScriptLength, - }, - { - name: "taddInvalidLength", - tx: taddInvalidLength, - expected: ErrTAddInvalidLength, - }, - { - name: "taddInvalidOpcode", - tx: taddInvalidOpcode, - expected: ErrTAddInvalidOpcode, - }, - { - name: "taddInvalidChange", - tx: taddInvalidChange, - expected: ErrTAddInvalidChange, - }, - { - name: "taddInvalidTxVersion", - tx: taddInvalidTxVersion, - expected: ErrTAddInvalidTxVersion, - }, - } - for i, tt := range tests { - test := dcrutil.NewTx(tt.tx) - test.SetTree(wire.TxTreeStake) - test.SetIndex(0) - err := checkTAdd(test.MsgTx()) - if !errors.Is(err, tt.expected) { - t.Errorf("%v: checkTAdd should have returned %v but "+ - "instead returned %v", tt.name, tt.expected, err) + }{{ + name: "treasury add invalid tx version", + tx: func() *wire.MsgTx { + tx := baseTreasuryAddTx.Copy() + tx.Version = 1 + return tx + }(), + expected: ErrTAddInvalidTxVersion, + }, { + name: "treasury add invalid num outputs - none", + tx: func() *wire.MsgTx { + tx := baseTreasuryAddTx.Copy() + tx.TxOut = nil + return tx + }(), + expected: ErrTAddInvalidCount, + }, { + name: "treasury add invalid num outputs - two change outputs", + tx: func() *wire.MsgTx { + tx := baseTreasuryAddTx.Copy() + tx.AddTxOut(tx.TxOut[1]) + return tx + }(), + expected: ErrTAddInvalidCount, + }, { + name: "treasury add invalid num inputs - none", + tx: func() *wire.MsgTx { + tx := baseTreasuryAddTx.Copy() + tx.TxIn = nil + return tx + }(), + expected: ErrTAddInvalidCount, + }, { + name: "treasury add with invalid output - bad script version", + tx: func() *wire.MsgTx { + tx := baseTreasuryAddTx.Copy() + tx.TxOut[0].Version = 1 + return tx + }(), + expected: ErrTAddInvalidVersion, + }, { + name: "treasury add with invalid output - missing script", + tx: func() *wire.MsgTx { + tx := baseTreasuryAddTx.Copy() + tx.TxOut[0].PkScript = nil + return tx + }(), + expected: ErrTAddInvalidScriptLength, + }, { + name: "treasury add with invalid output - extra trailing byte", + tx: func() *wire.MsgTx { + tx := baseTreasuryAddTx.Copy() + tx.TxOut[0].PkScript = append(tx.TxOut[0].PkScript, txscript.OP_TRUE) + return tx + }(), + expected: ErrTAddInvalidLength, + }, { + name: "treasury add with invalid output - wrong opcode for OP_TADD", + tx: func() *wire.MsgTx { + tx := baseTreasuryAddTx.Copy() + if tx.TxOut[0].PkScript[0] != txscript.OP_TADD { + panic("public key script format changed") + } + tx.TxOut[0].PkScript[0] = txscript.OP_TSPEND + return tx + }(), + expected: ErrTAddInvalidOpcode, + }, { + name: "treasury add with invalid output - wrong opcode for change", + tx: func() *wire.MsgTx { + tx := baseTreasuryAddTx.Copy() + if tx.TxOut[1].PkScript[0] != txscript.OP_SSTXCHANGE { + panic("public key script format changed") + } + tx.TxOut[1].PkScript = tx.TxOut[1].PkScript[1:] + return tx + }(), + expected: ErrTAddInvalidChange, + }} + + for _, test := range tests { + err := checkTAdd(test.tx) + if !errors.Is(err, test.expected) { + t.Errorf("%q: unexpected error -- got %v, want %v", test.name, err, + test.expected) } - if IsTAdd(test.MsgTx()) { - t.Errorf("IsTAdd claimed an invalid tadd is valid"+ - " %v %v", i, tt.name) + if IsTAdd(test.tx) { + t.Errorf("%q: IsTAdd claimed an invalid tadd is valid", test.name) } } } From 37fab816c16921927adc290f172f3137c208beab Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Wed, 22 Apr 2026 17:48:09 -0500 Subject: [PATCH 6/9] blockchain/stake: Rework treasurybase err tests. This reworks the treasurybase error tests to use the newly introduced functions that start with a valid treasurybase and then mutates a copy to induce the specific error to test. In the process, it also corrects some tests that weren't actually tsting what they claimed. The result is significantly more readable, provides more comprehensive test coverage, is more consistent with the other tests throughout the code base, and reduces the test code for the relevant tests by about 63%. This is part of a larger overall effort to bring the treasury code up to the standards used throughout the rest of the blockchain consensus code. --- blockchain/stake/treasury_test.go | 495 ++++++++---------------------- 1 file changed, 132 insertions(+), 363 deletions(-) diff --git a/blockchain/stake/treasury_test.go b/blockchain/stake/treasury_test.go index 609c07022b..525f354994 100644 --- a/blockchain/stake/treasury_test.go +++ b/blockchain/stake/treasury_test.go @@ -7,11 +7,9 @@ package stake import ( "encoding/binary" "errors" - "math" "math/rand" "testing" - "github.com/decred/dcrd/chaincfg/chainhash" "github.com/decred/dcrd/chaincfg/v3" "github.com/decred/dcrd/txscript/v4" "github.com/decred/dcrd/txscript/v4/stdaddr" @@ -597,373 +595,144 @@ func TestTreasuryAddErrors(t *testing.T) { } } -// treasurybaseInvalidInCount has an invalid TxIn count. -var treasurybaseInvalidInCount = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{}, - TxOut: []*wire.TxOut{ - {}, - {}, - }, - LockTime: 0, - Expiry: 0, -} - -// treasurybaseInvalidOutCount has an invalid TxOut count. -var treasurybaseInvalidOutCount = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - {}, - }, - TxOut: []*wire.TxOut{}, - LockTime: 0, - Expiry: 0, -} - -// treasurybaseInvalidVersion has an invalid out script version. -var treasurybaseInvalidVersion = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - {}, - }, - TxOut: []*wire.TxOut{ - {Version: 0}, - {Version: 2}, - }, - LockTime: 0, - Expiry: 0, -} - -// treasurybaseInvalidOpcode0 has an invalid out script opcode. -var treasurybaseInvalidOpcode0 = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - {}, - }, - TxOut: []*wire.TxOut{ - { - PkScript: []byte{ - 0xc2, // OP_TSPEND instead of OP_TADD - }, - }, - { - PkScript: []byte{ - 0x6a, // OP_RETURN - 0x0c, // OP_DATA_12 - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - }, - }, - }, - LockTime: 0, - Expiry: 0, -} - -// treasurybaseInvalidOpcode0Len has an invalid out script opcode length. -var treasurybaseInvalidOpcode0Len = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - {}, - }, - TxOut: []*wire.TxOut{ - { - PkScript: nil, // Invalid - }, - { - PkScript: []byte{ - 0x6a, // OP_RETURN - 0x0c, // OP_DATA_12 - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - }, - }, - }, - LockTime: 0, - Expiry: 0, -} - -// treasurybaseInvalidOpcode1 has an invalid out script opcode. -var treasurybaseInvalidOpcode1 = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - {}, - }, - TxOut: []*wire.TxOut{ - { - PkScript: []byte{ - 0xc1, // OP_TADD - }, - }, - { - PkScript: []byte{ - 0xc1, // OP_TADD instead of OP_RETURN - 0x0c, // OP_DATA_32 - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - }, - }, - }, - LockTime: 0, - Expiry: 0, -} - -// treasurybaseInvalidOpcode1Len has an invalid out script opcode length. -var treasurybaseInvalidOpcode1Len = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - {}, - }, - TxOut: []*wire.TxOut{ - { - PkScript: []byte{ - 0xc1, // OP_TADD - }, - }, - { - PkScript: nil, - }, - }, - LockTime: 0, - Expiry: 0, -} - -// treasurybaseInvalidOpcodeDataPush has an invalid out script data push in -// script 1 opcode 1. -var treasurybaseInvalidOpcodeDataPush = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - {}, - }, - TxOut: []*wire.TxOut{ - { - PkScript: []byte{ - 0xc1, // OP_TADD - }, - }, - { - PkScript: []byte{ - 0x6a, // OP_RETURN - 0x05, // OP_DATA_5 instead of OP_DATA_4 - 0x00, 0x00, 0x00, 0x00, 0x00, - }, - }, - }, - LockTime: 0, - Expiry: 0, -} - -// treasurybaseInvalid has invalid in script constants. -var treasurybaseInvalid = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - { - PreviousOutPoint: wire.OutPoint{ - Index: math.MaxUint32 - 1, - }, - }, - }, - TxOut: []*wire.TxOut{ - { - PkScript: []byte{ - 0xc1, // OP_TADD - }, - }, - { - PkScript: []byte{ - 0x6a, // OP_RETURN - 0x0c, // OP_DATA_12 - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - }, - }, - }, - LockTime: 0, - Expiry: 0, -} - -// treasurybaseInvalid2 has invalid in script constants. -var treasurybaseInvalid2 = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - { - PreviousOutPoint: wire.OutPoint{ - Index: math.MaxUint32, - Hash: chainhash.Hash{'m', 'o', 'o'}, - }, - }, - }, - TxOut: []*wire.TxOut{ - { - PkScript: []byte{ - 0xc1, // OP_TADD - }, - }, - { - PkScript: []byte{ - 0x6a, // OP_RETURN - 0x0c, // OP_DATA_12 - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - }, - }, - }, - LockTime: 0, - Expiry: 0, -} - -// treasurybaseInvalidTxVersion has an invalid transaction version. -var treasurybaseInvalidTxVersion = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 1, // Invalid - TxIn: []*wire.TxIn{ - { - PreviousOutPoint: wire.OutPoint{ - Index: math.MaxUint32, - Hash: chainhash.Hash{'m', 'o', 'o'}, - }, - }, - }, - TxOut: []*wire.TxOut{ - { - PkScript: []byte{ - 0xc1, // OP_TADD - }, - }, - { - PkScript: []byte{ - 0x6a, // OP_RETURN - 0x0c, // OP_DATA_12 - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - }, - }, - }, - LockTime: 0, - Expiry: 0, -} - -// treasurybaseInvalidLength has an invalid transaction length. -var treasurybaseInvalidLength = &wire.MsgTx{ - SerType: wire.TxSerializeFull, - Version: 3, - TxIn: []*wire.TxIn{ - { - PreviousOutPoint: wire.OutPoint{ - Index: math.MaxUint32, - Hash: chainhash.Hash{'m', 'o', 'o'}, - }, - SignatureScript: []byte{0x00}, - }, - }, - TxOut: []*wire.TxOut{ - { - PkScript: []byte{ - 0xc1, // OP_TADD - }, - }, - { - PkScript: []byte{ - 0x6a, // OP_RETURN - 0x04, // OP_DATA_4 - 0x00, 0x00, 0x00, 0x00, - }, - }, - }, - LockTime: 0, - Expiry: 0, -} - -// TestTreasuryBaseErrors verifies that all treasurybase errors can be hit and -// return the proper error. +// TestTreasuryBaseErrors verifies that all check treasurybase errors can be hit +// and return the proper error. func TestTreasuryBaseErrors(t *testing.T) { tests := []struct { name string tx *wire.MsgTx expected error - }{ - { - name: "treasurybaseInvalidInCount", - tx: treasurybaseInvalidInCount, - expected: ErrTreasuryBaseInvalidCount, - }, - { - name: "treasurybaseInvalidOutCount", - tx: treasurybaseInvalidOutCount, - expected: ErrTreasuryBaseInvalidCount, - }, - { - name: "treasurybaseInvalidVersion", - tx: treasurybaseInvalidVersion, - expected: ErrTreasuryBaseInvalidVersion, - }, - { - name: "treasurybaseInvalidOpcode0", - tx: treasurybaseInvalidOpcode0, - expected: ErrTreasuryBaseInvalidOpcode0, - }, - { - name: "treasurybaseInvalidOpcode0Len", - tx: treasurybaseInvalidOpcode0Len, - expected: ErrTreasuryBaseInvalidOpcode0, - }, - { - name: "treasurybaseInvalidOpcode1", - tx: treasurybaseInvalidOpcode1, - expected: ErrTreasuryBaseInvalidOpcode1, - }, - { - name: "treasurybaseInvalidOpcode1Len", - tx: treasurybaseInvalidOpcode1Len, - expected: ErrTreasuryBaseInvalidOpcode1, - }, - { - name: "treasurybaseInvalidDataPush", - tx: treasurybaseInvalidOpcodeDataPush, - expected: ErrTreasuryBaseInvalidOpcode1, - }, - { - name: "treasurybaseInvalid", - tx: treasurybaseInvalid, - expected: ErrTreasuryBaseInvalid, - }, - { - name: "treasurybaseInvalid2", - tx: treasurybaseInvalid2, - expected: ErrTreasuryBaseInvalid, - }, - { - name: "treasurybaseInvalidTxVersion", - tx: treasurybaseInvalidTxVersion, - expected: ErrTreasuryBaseInvalidTxVersion, - }, - { - name: "treasurybaseInvalidLength", - tx: treasurybaseInvalidLength, - expected: ErrTreasuryBaseInvalidLength, - }, - } - for i, tt := range tests { - test := dcrutil.NewTx(tt.tx) - test.SetTree(wire.TxTreeStake) - test.SetIndex(0) - err := checkTreasuryBase(test.MsgTx()) - if !errors.Is(err, tt.expected) { - t.Errorf("%v: checkTreasuryBase should have returned "+ - "%v but instead returned %v", tt.name, tt.expected, err) + }{{ + name: "treasurybase invalid tx version", + tx: func() *wire.MsgTx { + tx := baseTreasuryBaseTx.Copy() + tx.Version = 1 + return tx + }(), + expected: ErrTreasuryBaseInvalidTxVersion, + }, { + name: "treasurybase invalid num inputs - none", + tx: func() *wire.MsgTx { + tx := baseTreasuryBaseTx.Copy() + tx.TxIn = nil + return tx + }(), + expected: ErrTreasuryBaseInvalidCount, + }, { + name: "treasurybase invalid num outputs - none", + tx: func() *wire.MsgTx { + tx := baseTreasuryBaseTx.Copy() + tx.TxOut = nil + return tx + }(), + expected: ErrTreasuryBaseInvalidCount, + }, { + name: "treasurybase invalid num outputs - extra outupt", + tx: func() *wire.MsgTx { + tx := baseTreasuryBaseTx.Copy() + tx.TxOut = append(tx.TxOut, newTxOut(1, 0, opTrueScript)) + return tx + }(), + expected: ErrTreasuryBaseInvalidCount, + }, { + name: "treasurybase invalid input 0 - non-empty signature script", + tx: func() *wire.MsgTx { + tx := baseTreasuryBaseTx.Copy() + tx.TxIn[0].SignatureScript = []byte{txscript.OP_TRUE} + return tx + }(), + expected: ErrTreasuryBaseInvalidLength, + }, { + name: "treasurybase invalid output - bad script version", + tx: func() *wire.MsgTx { + tx := baseTreasuryBaseTx.Copy() + tx.TxOut[1].Version = 2 + return tx + }(), + expected: ErrTreasuryBaseInvalidVersion, + }, { + name: "treasurybase invalid output 0 - wrong opcode for OP_TADD", + tx: func() *wire.MsgTx { + tx := baseTreasuryBaseTx.Copy() + if tx.TxOut[0].PkScript[0] != txscript.OP_TADD { + panic("public key script format changed") + } + tx.TxOut[0].PkScript[0] = txscript.OP_TSPEND + return tx + }(), + expected: ErrTreasuryBaseInvalidOpcode0, + }, { + name: "treasurybase invalid output 0 - extra trailing byte", + tx: func() *wire.MsgTx { + tx := baseTreasuryBaseTx.Copy() + tx.TxOut[0].PkScript = append(tx.TxOut[0].PkScript, txscript.OP_TRUE) + return tx + }(), + expected: ErrTreasuryBaseInvalidOpcode0, + }, { + name: "treasurybase invalid output 1 - wrong opcode for OP_RETURN", + tx: func() *wire.MsgTx { + tx := baseTreasuryBaseTx.Copy() + if tx.TxOut[1].PkScript[0] != txscript.OP_RETURN { + panic("public key script format changed") + } + tx.TxOut[1].PkScript[0] = txscript.OP_TADD + return tx + }(), + expected: ErrTreasuryBaseInvalidOpcode1, + }, { + name: "treasurybase invalid output 1 - extra trailing byte", + tx: func() *wire.MsgTx { + tx := baseTreasuryBaseTx.Copy() + tx.TxOut[1].PkScript = append(tx.TxOut[1].PkScript, txscript.OP_TRUE) + return tx + }(), + expected: ErrTreasuryBaseInvalidOpcode1, + }, { + name: "treasurybase invalid output 1 - wrong data push size", + tx: func() *wire.MsgTx { + tx := baseTreasuryBaseTx.Copy() + if tx.TxOut[1].PkScript[1] != txscript.OP_DATA_12 { + panic("public key script format changed") + } + tx.TxOut[1].PkScript[1] = txscript.OP_DATA_11 + return tx + }(), + expected: ErrTreasuryBaseInvalidOpcode1, + }, { + name: "treasurybase invalid input 0 - non-null hash", + tx: func() *wire.MsgTx { + tx := baseTreasuryBaseTx.Copy() + tx.TxIn[0].PreviousOutPoint.Hash[0] = 0x01 + return tx + }(), + expected: ErrTreasuryBaseInvalid, + }, { + name: "treasurybase invalid input 0 - wrong prev index", + tx: func() *wire.MsgTx { + tx := baseTreasuryBaseTx.Copy() + tx.TxIn[0].PreviousOutPoint.Index = 1 + return tx + }(), + expected: ErrTreasuryBaseInvalid, + }, { + name: "treasurybase invalid input 0 - wrong prev tree", + tx: func() *wire.MsgTx { + tx := baseTreasuryBaseTx.Copy() + tx.TxIn[0].PreviousOutPoint.Tree = wire.TxTreeStake + return tx + }(), + expected: ErrTreasuryBaseInvalid, + }} + for _, test := range tests { + err := checkTreasuryBase(test.tx) + if !errors.Is(err, test.expected) { + t.Errorf("%q: unexpected error -- got %v, want %v", test.name, err, + test.expected) } - if IsTreasuryBase(test.MsgTx()) { - t.Errorf("IsTreasuryBase claimed an invalid treasury "+ - "base is valid %v %v", i, tt.name) + if IsTreasuryBase(test.tx) { + t.Errorf("%q: IsTreasuryBase claimed an invalid treasury base is "+ + "valid", test.name) } } } From 0b5c45fb09df17372780aba526999d3599400a03 Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Wed, 22 Apr 2026 17:48:09 -0500 Subject: [PATCH 7/9] blockchain/stake: Cleanup treasury add code. This cleans up the CheckTAdd method to make it much more consistent with the other code used in consensus throughout the rest of the code base. While there are no known exploitable issues with the func and it has worked well for a while now, it is highly inconsistent with the rest of the consensus code in style and polish and has various other issues. For example: - several of the reported error message are incorrect - most of the error message don't provide very helpful messages and reference internal names that are not visible to users - inconsistent variable names - uses less efficient inverted logic tests - various misleading and inaccurate comments - exported func comment refers to internal func that is not visible in generated documention This is part of a larger overall effort to bring the treasury code up to the standards used throughout the rest of the blockchain consensus code. --- blockchain/stake/treasury.go | 114 +++++++++++++++++------------- blockchain/stake/treasury_test.go | 2 +- 2 files changed, 66 insertions(+), 50 deletions(-) diff --git a/blockchain/stake/treasury.go b/blockchain/stake/treasury.go index e78b06ee46..18d3ff4760 100644 --- a/blockchain/stake/treasury.go +++ b/blockchain/stake/treasury.go @@ -20,11 +20,12 @@ const ( TSpendScriptLen = 100 ) -// This file contains the functions that verify that treasury transactions -// strictly adhere to the specified format. +// ----------------------------------------------------------------------------- +// This file contains functions that verify that treasury transactions strictly +// adhere to the specified format. // // == User sends to treasury == -// TxIn: Normal TxIn signature scripts +// TxIn: Normal TxIn signature scripts // TxOut[0] OP_TADD // TxOut[1] optional OP_SSTXCHANGE // @@ -37,73 +38,88 @@ const ( // TxIn[0] OP_TSPEND // TxOut[0] OP_RETURN // TxOut[1..N] OP_TGEN +// ----------------------------------------------------------------------------- -// checkTAdd verifies that the provided MsgTx is a valid TADD. -// Note: this function does not recognize treasurybase TADDs. -func checkTAdd(mtx *wire.MsgTx) error { - // Require version TxVersionTreasury. - if mtx.Version != wire.TxVersionTreasury { - return stakeRuleError(ErrTAddInvalidTxVersion, - fmt.Sprintf("invalid TADD script version: %v", - mtx.Version)) +// CheckTAdd verifies that the provided transaction satisfies the structural +// requirements to be a valid treasury add transaction. A treasury add +// transaction is one that sends existing funds to the decentralized treasury. +// +// A valid treasury add must have: +// - The transaction version set to [wire.TxVersionTreasury] +// - One or more normal inputs referencing the coins to spend +// - An output with a treasury add script (OP_TADD) +// - An optional second output that must be a stake change script +// (OP_SSTXCHANGE) when present +// - All script versions set to 0 +func CheckTAdd(tx *wire.MsgTx) error { + // The transaction version must be the required treasury version. + if tx.Version != wire.TxVersionTreasury { + str := fmt.Sprintf("treasury add transaction version is %d instead of %d", + tx.Version, wire.TxVersionTreasury) + return stakeRuleError(ErrTAddInvalidTxVersion, str) } - // A TADD consists of one OP_TADD in PkScript[0] followed by 0 or 1 - // stake change outputs. It also requires at least one input. - if !(len(mtx.TxOut) == 1 || len(mtx.TxOut) == 2) || len(mtx.TxIn) < 1 { - return stakeRuleError(ErrTAddInvalidCount, - fmt.Sprintf("invalid TADD script: TxIn %v TxOut %v", - len(mtx.TxIn), len(mtx.TxOut))) + // A treasury add must have at least one input and one or two outputs. + if len(tx.TxIn) < 1 { + const str = "treasury add transaction does not have any inputs" + return stakeRuleError(ErrTAddInvalidCount, str) + } + if len(tx.TxOut) != 1 && len(tx.TxOut) != 2 { + str := fmt.Sprintf("treasury add transaction has %d outputs instead "+ + "of 1 or 2", len(tx.TxOut)) + return stakeRuleError(ErrTAddInvalidCount, str) } // All output scripts must be version 0 and non-empty. const consensusScriptVer = 0 - for k := range mtx.TxOut { - if mtx.TxOut[k].Version != consensusScriptVer { - return stakeRuleError(ErrTAddInvalidVersion, - fmt.Sprintf("invalid script version found "+ - "in TADD TxOut: %v", k)) + for txOutIdx := range tx.TxOut { + txOut := tx.TxOut[txOutIdx] + if txOut.Version != consensusScriptVer { + str := fmt.Sprintf("treasury add transaction output %d script "+ + "version is %d instead of %d", txOutIdx, txOut.Version, + consensusScriptVer) + return stakeRuleError(ErrTAddInvalidVersion, str) } - - if len(mtx.TxOut[k].PkScript) == 0 { - return stakeRuleError(ErrTAddInvalidScriptLength, - fmt.Sprintf("zero script length found in "+ - "TADD: %v", k)) + if len(txOut.PkScript) == 0 { + str := fmt.Sprintf("treasury add transaction output %d script is "+ + "empty", txOutIdx) + return stakeRuleError(ErrTAddInvalidScriptLength, str) } } - // First output must be a TADD - if len(mtx.TxOut[0].PkScript) != 1 { - return stakeRuleError(ErrTAddInvalidLength, - fmt.Sprintf("TADD script length is not 1 byte, got %v", - len(mtx.TxOut[0].PkScript))) + // The first output must be a script that only consists of OP_TADD. + firstTxOut := tx.TxOut[0] + if len(firstTxOut.PkScript) != 1 { + str := fmt.Sprintf("treasury add transaction output 0 script length "+ + "is %d bytes instead of 1 byte", len(firstTxOut.PkScript)) + return stakeRuleError(ErrTAddInvalidLength, str) } - if mtx.TxOut[0].PkScript[0] != txscript.OP_TADD { - return stakeRuleError(ErrTAddInvalidOpcode, - fmt.Sprintf("first output must be a TADD, got 0x%x", - mtx.TxOut[0].PkScript[0])) + if firstTxOut.PkScript[0] != txscript.OP_TADD { + str := fmt.Sprintf("treasury add transaction output 0 script is 0x%x "+ + "instead of OP_TADD (0x%x)", firstTxOut.PkScript[0], + txscript.OP_TADD) + return stakeRuleError(ErrTAddInvalidOpcode, str) } - // Only 1 stake change output allowed. - if len(mtx.TxOut) == 2 { - // Script length has been already verified. - if !IsStakeChangeScript(mtx.TxOut[1].Version, mtx.TxOut[1].PkScript) { - return stakeRuleError(ErrTAddInvalidChange, - "second output must be an OP_SSTXCHANGE script") + // The second output must be a valid stake change output when present. + if len(tx.TxOut) == 2 { + changeTxOut := tx.TxOut[1] + if !IsStakeChangeScript(changeTxOut.Version, changeTxOut.PkScript) { + const str = "treasury add transaction output 1 is not a " + + "stake change script" + return stakeRuleError(ErrTAddInvalidChange, str) } } return nil } -// CheckTAdd exports checkTAdd for testing purposes. -func CheckTAdd(mtx *wire.MsgTx) error { - return checkTAdd(mtx) -} - -// IsTAdd returns true if the provided transaction is a proper TADD. +// IsTAdd returns whether or not the provided transaction satisfies the +// structural requirements to be a valid treasury add transaction. +// +// See the [CheckTAdd] documentation for more details. func IsTAdd(tx *wire.MsgTx) bool { - return checkTAdd(tx) == nil + return CheckTAdd(tx) == nil } // CheckTSpend verifies if a MsgTx is a valid TSPEND. diff --git a/blockchain/stake/treasury_test.go b/blockchain/stake/treasury_test.go index 525f354994..14ec3e0e77 100644 --- a/blockchain/stake/treasury_test.go +++ b/blockchain/stake/treasury_test.go @@ -584,7 +584,7 @@ func TestTreasuryAddErrors(t *testing.T) { }} for _, test := range tests { - err := checkTAdd(test.tx) + err := CheckTAdd(test.tx) if !errors.Is(err, test.expected) { t.Errorf("%q: unexpected error -- got %v, want %v", test.name, err, test.expected) From da5f2a2b477a504dcb516d1263fe53d88eec6f2c Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Wed, 22 Apr 2026 17:48:10 -0500 Subject: [PATCH 8/9] blockchain/stake: Cleanup treasury spend code. This cleans up the CheckTSpend method to make it much more consistent with the other code used in consensus throughout the rest of the code base. While there are no known exploitable issues with the func and it has worked well for a while now, it is highly inconsistent with the rest of the consensus code in style and polish and has various other issues. For example: - several of the reported error message are incorrect - most of the error message don't provide very helpful messages and reference internal names that are not visible to users - inconsistent errors - inconsistent variable names - uses less efficient and harder to read inverted logic tests - various misleading and inaccurate comments This is part of a larger overall effort to bring the treasury code up to the standards used throughout the rest of the blockchain consensus code. --- blockchain/stake/treasury.go | 145 ++++++++++++++++-------------- blockchain/stake/treasury_test.go | 2 +- 2 files changed, 79 insertions(+), 68 deletions(-) diff --git a/blockchain/stake/treasury.go b/blockchain/stake/treasury.go index 18d3ff4760..948e1c1902 100644 --- a/blockchain/stake/treasury.go +++ b/blockchain/stake/treasury.go @@ -36,7 +36,7 @@ const ( // // == Spend from treasury == // TxIn[0] OP_TSPEND -// TxOut[0] OP_RETURN +// TxOut[0] OP_RETURN OP_DATA_32 [8]{LE encoded input value} [24]{random} // TxOut[1..N] OP_TGEN // ----------------------------------------------------------------------------- @@ -122,101 +122,112 @@ func IsTAdd(tx *wire.MsgTx) bool { return CheckTAdd(tx) == nil } -// CheckTSpend verifies if a MsgTx is a valid TSPEND. +// CheckTSpend verifies that the provided transaction satisfies the structural +// requirements to be a valid treasury spend transaction. It returns the +// signature and public key encoded in the first input when the error is nil. +// // This function DOES NOT check the signature or if the public key is a well -// known PI key. This is a convenience function to obtain the signature and -// public key without iterating over the same MsgTx over and over again. The -// return values are signature, public key and an error. -func CheckTSpend(mtx *wire.MsgTx) ([]byte, []byte, error) { - // Require version TxVersionTreasury. - if mtx.Version != wire.TxVersionTreasury { - return nil, nil, stakeRuleError(ErrTSpendInvalidTxVersion, - fmt.Sprintf("invalid TSpend script version: %v", - mtx.Version)) +// known PI key. It also DOES NOT check the input value encoded in the data +// push of the first output matches the input value. +// +// A valid treasury spend must have: +// - The transaction version set to [wire.TxVersionTreasury] +// - A single input with a treasury spend script ( OP_TSPEND) +// - The first output with a 32 byte nulldata script +// (<8-byte LE encoded input value + 24-byte random>) +// - One or more remaining outputs that must be treasury gen scripts (OP_TGEN +// followed by pay-to-pubkey-hash or pay-to-script-hash) +// - All script versions set to 0 +func CheckTSpend(tx *wire.MsgTx) ([]byte, []byte, error) { + // The transaction version must be the required treasury version. + if tx.Version != wire.TxVersionTreasury { + str := fmt.Sprintf("treasury spend transaction version is %d instead "+ + "of %d", tx.Version, wire.TxVersionTreasury) + return nil, nil, stakeRuleError(ErrTSpendInvalidTxVersion, str) } - // A valid TSPEND consists of a single TxIn that contains a signature, - // a public key and an OP_TSPEND opcode. - // - // There must be at least two outputs. The first must contain an - // OP_RETURN followed by a 32 byte data push of a random number. This - // is used to randomize the transaction hash. - // The second output must be a TGEN tagged P2SH or P2PKH script. - if len(mtx.TxIn) != 1 || len(mtx.TxOut) < 2 { - return nil, nil, stakeRuleError(ErrTSpendInvalidLength, - fmt.Sprintf("invalid TSPEND script lengths in: %v "+ - "out: %v", len(mtx.TxIn), len(mtx.TxOut))) + // A treasury spend must have exactly one input and at least two outputs. + if len(tx.TxIn) != 1 { + str := fmt.Sprintf("treasury spend transaction has %d inputs instead "+ + "of 1", len(tx.TxIn)) + return nil, nil, stakeRuleError(ErrTSpendInvalidLength, str) + } + if len(tx.TxOut) < 2 { + str := fmt.Sprintf("treasury spend transaction does not have enough "+ + "outputs (min: %d, have: %d)", 2, len(tx.TxOut)) + return nil, nil, stakeRuleError(ErrTSpendInvalidLength, str) } // All output scripts must be version 0 and non-empty. const consensusScriptVer = 0 - for k, txOut := range mtx.TxOut { + for txOutIdx, txOut := range tx.TxOut { if txOut.Version != consensusScriptVer { - return nil, nil, stakeRuleError(ErrTSpendInvalidVersion, - fmt.Sprintf("invalid script version found in "+ - "TxOut: %v", k)) + str := fmt.Sprintf("treasury spend transaction output %d script "+ + "version is %d instead of %d", txOutIdx, txOut.Version, + consensusScriptVer) + return nil, nil, stakeRuleError(ErrTSpendInvalidVersion, str) } if len(txOut.PkScript) == 0 { - return nil, nil, stakeRuleError(ErrTSpendInvalidScriptLength, - fmt.Sprintf("invalid TxOut script length %v: "+ - "%v", k, len(txOut.PkScript))) + str := fmt.Sprintf("treasury spend transaction output %d script "+ + "is empty", txOutIdx) + return nil, nil, stakeRuleError(ErrTSpendInvalidScriptLength, str) } } - txIn := mtx.TxIn[0].SignatureScript - if !(len(txIn) == TSpendScriptLen && - txIn[0] == txscript.OP_DATA_64 && - txIn[65] == txscript.OP_DATA_33 && - txIn[99] == txscript.OP_TSPEND) { - return nil, nil, stakeRuleError(ErrTSpendInvalidScript, - "TSPEND invalid tspend script") - } + // The single input must have the exact treasury spend script format: + // + // DATA_64 <64-byte schnorr signature> DATA_33 <33-byte pubkey> OP_TSPEND + txIn := tx.TxIn[0].SignatureScript + if len(txIn) != TSpendScriptLen || txIn[0] != txscript.OP_DATA_64 || + txIn[65] != txscript.OP_DATA_33 || txIn[99] != txscript.OP_TSPEND { - // Pull out signature, pubkey. + const str = "treasury spend transaction input 0 script is malformed" + return nil, nil, stakeRuleError(ErrTSpendInvalidScript, str) + } signature := txIn[1 : 1+schnorr.SignatureSize] pubKey := txIn[66 : 66+secp256k1.PubKeyBytesLenCompressed] + + // The public key must adhere to the strict compressed public key encoding. if !txscript.IsStrictCompressedPubKeyEncoding(pubKey) { - return nil, nil, stakeRuleError(ErrTSpendInvalidPubkey, - "TSPEND invalid public key") + str := fmt.Sprintf("treasury spend transaction input 0 public key %x "+ + "does not use strict compressed encoding", pubKey) + return nil, nil, stakeRuleError(ErrTSpendInvalidPubkey, str) } - // Make sure TxOut[0] contains an OP_RETURN followed by a 32 byte data - // push. - if !txscript.IsStrictNullData(mtx.TxOut[0].Version, - mtx.TxOut[0].PkScript, 32) { - return nil, nil, stakeRuleError(ErrTSpendInvalidTransaction, - "First TSPEND output should have been an OP_RETURN "+ - "followed by a 32 byte data push") + // The first output must be an OP_RETURN followed by a 32 byte data push. + firstTxOut := tx.TxOut[0] + if !txscript.IsStrictNullData(firstTxOut.Version, firstTxOut.PkScript, 32) { + const str = "treasury spend transaction output 0 script is not an " + + "OP_RETURN followed by a 32 byte data push" + return nil, nil, stakeRuleError(ErrTSpendInvalidTransaction, str) } - // Verify that the TxOut's contains a P2PKH or P2PKH scripts. - for k, txOut := range mtx.TxOut[1:] { - // All tx outs are tagged with OP_TGEN - if txOut.PkScript[0] != txscript.OP_TGEN { - return nil, nil, stakeRuleError(ErrTSpendInvalidTGen, - fmt.Sprintf("Output %v is not tagged with "+ - "OP_TGEN", k+1)) + // All outputs after the first one must have OP_TGEN tagged p2pkh or p2sh + // scripts. + for txOutIdx, txOut := range tx.TxOut[1:] { + script := txOut.PkScript + if script[0] != txscript.OP_TGEN { + str := fmt.Sprintf("treasury spend transaction output %d script "+ + "is not tagged with OP_TGEN", txOutIdx+1) + return nil, nil, stakeRuleError(ErrTSpendInvalidTGen, str) } - if !(isPubKeyHashScript(txOut.PkScript[1:]) || - isScriptHashScript(txOut.PkScript[1:])) { - - return nil, nil, stakeRuleError(ErrTSpendInvalidSpendScript, - fmt.Sprintf("Output %v is not P2SH or P2PKH", k+1)) + if !isPubKeyHashScript(script[1:]) && !isScriptHashScript(script[1:]) { + str := fmt.Sprintf("treasury spend transaction output %d script "+ + "is not pay-to-script-hash or pay-to-pubkey-hash", txOutIdx+1) + return nil, nil, stakeRuleError(ErrTSpendInvalidSpendScript, str) } } return signature, pubKey, nil } -// checkTSpend verifies if a MsgTx is a valid TSPEND. -func checkTSpend(mtx *wire.MsgTx) error { - _, _, err := CheckTSpend(mtx) - return err -} - -// IsTSpend returns true if the provided transaction is a proper TSPEND. +// IsTSpend returns whether or not the provided transaction satisfies the +// structural requirements to be a valid treasury spend transaction. +// +// See the [CheckTSpend] documentation for more details. func IsTSpend(tx *wire.MsgTx) bool { - return checkTSpend(tx) == nil + _, _, err := CheckTSpend(tx) + return err == nil } // checkTreasuryBase verifies that the provided MsgTx is a treasury base. diff --git a/blockchain/stake/treasury_test.go b/blockchain/stake/treasury_test.go index 14ec3e0e77..034698960e 100644 --- a/blockchain/stake/treasury_test.go +++ b/blockchain/stake/treasury_test.go @@ -484,7 +484,7 @@ func TestTreasurySpendErrors(t *testing.T) { }} for _, test := range tests { - err := checkTSpend(test.tx) + _, _, err := CheckTSpend(test.tx) if !errors.Is(err, test.expected) { t.Errorf("%q: unexpected error -- got %v, want %v", test.name, err, test.expected) From 7cd4cbe1c112b2862deff6298bcbd6ce362d4188 Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Wed, 22 Apr 2026 17:48:11 -0500 Subject: [PATCH 9/9] blockchain/stake: Cleanup treasurybase code. This cleans up the CheckTreasuryBase method to make it much more consistent with the other code used in consensus throughout the rest of the code base. While there are no known exploitable issues with the func and it has worked well for a while now, it is highly inconsistent with the rest of the consensus code in style and polish and has various other issues. For example: - several of the reported error message are incorrect - most of the error message don't provide very helpful messages and reference internal names that are not visible to users - inconsistent variable names - some checks are not in the most logical order - various misleading and inaccurate comments - some checks are not making use of existing funcs This is part of a larger overall effort to bring the treasury code up to the standards used throughout the rest of the blockchain consensus code. --- blockchain/stake/treasury.go | 115 +++++++++++++++++------------- blockchain/stake/treasury_test.go | 2 +- 2 files changed, 67 insertions(+), 50 deletions(-) diff --git a/blockchain/stake/treasury.go b/blockchain/stake/treasury.go index 948e1c1902..8bd029445c 100644 --- a/blockchain/stake/treasury.go +++ b/blockchain/stake/treasury.go @@ -230,73 +230,90 @@ func IsTSpend(tx *wire.MsgTx) bool { return err == nil } -// checkTreasuryBase verifies that the provided MsgTx is a treasury base. -func checkTreasuryBase(mtx *wire.MsgTx) error { - // Require version TxVersionTreasury. - if mtx.Version != wire.TxVersionTreasury { - return stakeRuleError(ErrTreasuryBaseInvalidTxVersion, - fmt.Sprintf("invalid treasurybase script version: %v", - mtx.Version)) +// CheckTreasuryBase verifies that the provided transaction satisfies the +// structural requirements to be a valid treasurybase transaction. +// +// A valid treasurybase must have: +// - The transaction version set to [wire.TxVersionTreasury] +// - A single treasurybase input (no signature script, null prevout) +// - An output with a treasury add script (OP_TADD) +// - An output with a 12 byte nulldata script +// (<4-byte LE encoded height + 8-byte random>) +// - All script versions set to 0 +func CheckTreasuryBase(tx *wire.MsgTx) error { + // The transaction version must be the required treasury version. + if tx.Version != wire.TxVersionTreasury { + str := fmt.Sprintf("treasurybase transaction version is %d instead of %d", + tx.Version, wire.TxVersionTreasury) + return stakeRuleError(ErrTreasuryBaseInvalidTxVersion, str) } - // A TADD consists of one OP_TADD in PkScript[0] followed by an - // OP_RETURN in PkScript[1]. - if len(mtx.TxIn) != 1 || len(mtx.TxOut) != 2 { - return stakeRuleError(ErrTreasuryBaseInvalidCount, - fmt.Sprintf("invalid treasurybase in/out script "+ - "count: %v/%v", len(mtx.TxIn), - len(mtx.TxOut))) + // A treasurybase must have exactly one input and two outputs. + if len(tx.TxIn) != 1 { + str := fmt.Sprintf("treasurybase transaction has %d inputs instead of 1", + len(tx.TxIn)) + return stakeRuleError(ErrTreasuryBaseInvalidCount, str) + } + if len(tx.TxOut) != 2 { + str := fmt.Sprintf("treasurybase transaction has %d output(s) instead "+ + "of 2", len(tx.TxOut)) + return stakeRuleError(ErrTreasuryBaseInvalidCount, str) } - // Ensure that there is no SignatureScript on the zeroth input. - if len(mtx.TxIn[0].SignatureScript) != 0 { - return stakeRuleError(ErrTreasuryBaseInvalidLength, - "treasurybase input 0 contains a script") + // The first input signature script must be empty and its previous output + // must be a null outpoint (max value index, a zero hash, regular tx tree). + if len(tx.TxIn[0].SignatureScript) != 0 { + str := fmt.Sprintf("treasurybase input 0 signature script is %d "+ + "byte(s) instead of 0", len(tx.TxIn[0].SignatureScript)) + return stakeRuleError(ErrTreasuryBaseInvalidLength, str) + } + if !isNullOutpoint(tx) { + prevOut := &tx.TxIn[0].PreviousOutPoint + str := fmt.Sprintf("treasurybase input 0 previous output %s:%d:%d is "+ + "not a null outpoint", prevOut.Hash, prevOut.Index, prevOut.Tree) + return stakeRuleError(ErrTreasuryBaseInvalid, str) } // All output scripts must be version 0. const consensusScriptVer = 0 - for k := range mtx.TxOut { - if mtx.TxOut[k].Version != consensusScriptVer { - return stakeRuleError(ErrTreasuryBaseInvalidVersion, - fmt.Sprintf("invalid script version found in "+ - "treasurybase: output %v", k)) + for txOutIdx, txOut := range tx.TxOut { + if txOut.Version != consensusScriptVer { + str := fmt.Sprintf("treasurybase transaction output %d script "+ + "version is %d instead of %d", txOutIdx, txOut.Version, + consensusScriptVer) + return stakeRuleError(ErrTreasuryBaseInvalidVersion, str) } } - // First output must be a TADD - if len(mtx.TxOut[0].PkScript) != 1 || - mtx.TxOut[0].PkScript[0] != txscript.OP_TADD { - return stakeRuleError(ErrTreasuryBaseInvalidOpcode0, - "first treasurybase output must be a TADD") + // The first output must be a script that only consists of OP_TADD. + firstTxOut := tx.TxOut[0] + if len(firstTxOut.PkScript) != 1 { + str := fmt.Sprintf("treasurybase transaction output 0 script length "+ + "is %d bytes instead of 1 byte", len(firstTxOut.PkScript)) + return stakeRuleError(ErrTreasuryBaseInvalidOpcode0, str) } - - // Required OP_RETURN, OP_DATA_12 <4 bytes le encoded height> - // <8 bytes random> = 14 bytes total. - if len(mtx.TxOut[1].PkScript) != 14 || - mtx.TxOut[1].PkScript[0] != txscript.OP_RETURN || - mtx.TxOut[1].PkScript[1] != txscript.OP_DATA_12 { - return stakeRuleError(ErrTreasuryBaseInvalidOpcode1, - "second treasurybase output must be an OP_RETURN "+ - "OP_DATA_12 data script") + if firstTxOut.PkScript[0] != txscript.OP_TADD { + str := fmt.Sprintf("treasurybase transaction output 0 script is 0x%x "+ + "instead of OP_TADD (0x%x)", firstTxOut.PkScript[0], + txscript.OP_TADD) + return stakeRuleError(ErrTreasuryBaseInvalidOpcode0, str) } - if !isNullOutpoint(mtx) { - return stakeRuleError(ErrTreasuryBaseInvalid, - "invalid treasurybase constants") + // The second output must be an OP_RETURN followed by a 12 byte data push. + opRetTxOut := tx.TxOut[1] + if !txscript.IsStrictNullData(opRetTxOut.Version, opRetTxOut.PkScript, 12) { + const str = "treasurybase transaction output 1 is not an OP_RETURN " + + "followed by a 12 byte data push" + return stakeRuleError(ErrTreasuryBaseInvalidOpcode1, str) } return nil } -// CheckTreasuryBase verifies that the provided MsgTx is a treasury base. This -// is exported for testing purposes. -func CheckTreasuryBase(mtx *wire.MsgTx) error { - return checkTreasuryBase(mtx) -} - -// IsTreasuryBase returns true if the provided transaction is a treasury base -// transaction. +// IsTreasuryBase returns whether or not the provided transaction satisfies the +// structural requirements to be a valid treasurybase transaction. +// +// See the [CheckTreasuryBase] documentation for more details. func IsTreasuryBase(tx *wire.MsgTx) bool { - return checkTreasuryBase(tx) == nil + return CheckTreasuryBase(tx) == nil } diff --git a/blockchain/stake/treasury_test.go b/blockchain/stake/treasury_test.go index 034698960e..cb9dda4651 100644 --- a/blockchain/stake/treasury_test.go +++ b/blockchain/stake/treasury_test.go @@ -725,7 +725,7 @@ func TestTreasuryBaseErrors(t *testing.T) { expected: ErrTreasuryBaseInvalid, }} for _, test := range tests { - err := checkTreasuryBase(test.tx) + err := CheckTreasuryBase(test.tx) if !errors.Is(err, test.expected) { t.Errorf("%q: unexpected error -- got %v, want %v", test.name, err, test.expected)