From ae3b9c9ed77db5a78c0d307200e8bd65ee9319af Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Sun, 9 Oct 2022 17:03:31 -0700 Subject: [PATCH 1/6] wire: remove erroneous witness size check in wire parsing In this commit, we fix a bug that would cause nodes to be unable to parse a given block from the wire. The block would be properly accepted if fed in via other mechanisms. The issue here is that the old checks for the maximum witness size, circa segwit v0 where placed in the wire package _as well_ as the tx engine. This check should only be in the engine, since it's properly gated by other related scrip validation flags. The fix itself is simple: limit witnesses only based on the maximum block size in bytes, or ~4MB. --- wire/msgtx.go | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/wire/msgtx.go b/wire/msgtx.go index 1e2f69fad4..dbc379cf30 100644 --- a/wire/msgtx.go +++ b/wire/msgtx.go @@ -103,10 +103,9 @@ const ( maxWitnessItemsPerInput = 500000 // maxWitnessItemSize is the maximum allowed size for an item within - // an input's witness data. This number is derived from the fact that - // for script validation, each pushed item onto the stack must be less - // than 10k bytes. - maxWitnessItemSize = 11000 + // an input's witness data. This value is bounded by the largest + // possible block size, post segwit v1 (taproot). + maxWitnessItemSize = 4_000_000 ) // TxFlagMarker is the first byte of the FLAG field in a bitcoin tx @@ -114,16 +113,18 @@ const ( // transaction from one that would require a different parsing logic. // // Position of FLAG in a bitcoin tx message: -// ┌─────────┬────────────────────┬─────────────┬─────┐ -// │ VERSION │ FLAG │ TX-IN-COUNT │ ... │ -// │ 4 bytes │ 2 bytes (optional) │ varint │ │ -// └─────────┴────────────────────┴─────────────┴─────┘ +// +// ┌─────────┬────────────────────┬─────────────┬─────┐ +// │ VERSION │ FLAG │ TX-IN-COUNT │ ... │ +// │ 4 bytes │ 2 bytes (optional) │ varint │ │ +// └─────────┴────────────────────┴─────────────┴─────┘ // // Zooming into the FLAG field: -// ┌── FLAG ─────────────┬────────┐ -// │ TxFlagMarker (0x00) │ TxFlag │ -// │ 1 byte │ 1 byte │ -// └─────────────────────┴────────┘ +// +// ┌── FLAG ─────────────┬────────┐ +// │ TxFlagMarker (0x00) │ TxFlag │ +// │ 1 byte │ 1 byte │ +// └─────────────────────┴────────┘ const TxFlagMarker = 0x00 // TxFlag is the second byte of the FLAG field in a bitcoin tx message. @@ -586,8 +587,9 @@ func (msg *MsgTx) BtcDecode(r io.Reader, pver uint32, enc MessageEncoding) error // item itself. txin.Witness = make([][]byte, witCount) for j := uint64(0); j < witCount; j++ { - txin.Witness[j], err = readScript(r, pver, - maxWitnessItemSize, "script witness item") + txin.Witness[j], err = readScript( + r, pver, maxWitnessItemSize, "script witness item", + ) if err != nil { returnScriptBuffers() return err From c4e78568b4f1498221151b1e55448c0313bb8a22 Mon Sep 17 00:00:00 2001 From: eugene Date: Mon, 2 Aug 2021 11:14:52 -0400 Subject: [PATCH 2/6] mempool: export isDust for use in other projects This changes isDust to IsDust so other golang projects (btcwallet or lnd) can use the precise dust calculation used by btcd. --- mempool/policy.go | 6 +++--- mempool/policy_test.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/mempool/policy.go b/mempool/policy.go index 7e97329319..74142bf60c 100644 --- a/mempool/policy.go +++ b/mempool/policy.go @@ -172,12 +172,12 @@ func checkPkScriptStandard(pkScript []byte, scriptClass txscript.ScriptClass) er return nil } -// isDust returns whether or not the passed transaction output amount is +// IsDust returns whether or not the passed transaction output amount is // considered dust or not based on the passed minimum transaction relay fee. // Dust is defined in terms of the minimum transaction relay fee. In // particular, if the cost to the network to spend coins is more than 1/3 of the // minimum transaction relay fee, it is considered dust. -func isDust(txOut *wire.TxOut, minRelayTxFee btcutil.Amount) bool { +func IsDust(txOut *wire.TxOut, minRelayTxFee btcutil.Amount) bool { // Unspendable outputs are considered dust. if txscript.IsUnspendable(txOut.PkScript) { return true @@ -351,7 +351,7 @@ func checkTransactionStandard(tx *btcutil.Tx, height int32, // "dust". if scriptClass == txscript.NullDataTy { numNullDataOutputs++ - } else if isDust(txOut, minRelayTxFee) { + } else if IsDust(txOut, minRelayTxFee) { str := fmt.Sprintf("transaction output %d: payment "+ "of %d is dust", i, txOut.Value) return txRuleError(wire.RejectDust, str) diff --git a/mempool/policy_test.go b/mempool/policy_test.go index 9dd618ad6e..b9cdbac47a 100644 --- a/mempool/policy_test.go +++ b/mempool/policy_test.go @@ -204,7 +204,7 @@ func TestCheckPkScriptStandard(t *testing.T) { } } -// TestDust tests the isDust API. +// TestDust tests the IsDust API. func TestDust(t *testing.T) { pkScript := []byte{0x76, 0xa9, 0x21, 0x03, 0x2f, 0x7e, 0x43, 0x0a, 0xa4, 0xc9, 0xd1, 0x59, 0x43, 0x7e, 0x84, 0xb9, @@ -268,7 +268,7 @@ func TestDust(t *testing.T) { }, } for _, test := range tests { - res := isDust(&test.txOut, test.relayFee) + res := IsDust(&test.txOut, test.relayFee) if res != test.isDust { t.Fatalf("Dust test '%s' failed: want %v got %v", test.name, test.isDust, res) From f30a73d573a4a4379dd57252a6c07ed52964c8de Mon Sep 17 00:00:00 2001 From: eugene Date: Mon, 16 Aug 2021 12:40:38 -0400 Subject: [PATCH 3/6] mempool: introduce GetDustThreshold to export dust limit calculation This commit modifies no behavior and would allow other projects to retrieve the dust limit for a particular output type before the amount of the output is known. This is particularly useful in the Lightning Network for channel negotiation. --- mempool/policy.go | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/mempool/policy.go b/mempool/policy.go index 74142bf60c..1fe850797f 100644 --- a/mempool/policy.go +++ b/mempool/policy.go @@ -172,17 +172,10 @@ func checkPkScriptStandard(pkScript []byte, scriptClass txscript.ScriptClass) er return nil } -// IsDust returns whether or not the passed transaction output amount is -// considered dust or not based on the passed minimum transaction relay fee. -// Dust is defined in terms of the minimum transaction relay fee. In -// particular, if the cost to the network to spend coins is more than 1/3 of the -// minimum transaction relay fee, it is considered dust. -func IsDust(txOut *wire.TxOut, minRelayTxFee btcutil.Amount) bool { - // Unspendable outputs are considered dust. - if txscript.IsUnspendable(txOut.PkScript) { - return true - } - +// GetDustThreshold calculates the dust limit for a *wire.TxOut by taking the +// size of a typical spending transaction and multiplying it by 3 to account +// for the minimum dust relay fee of 3000sat/kvb. +func GetDustThreshold(txOut *wire.TxOut) int64 { // The total serialized size consists of the output and the associated // input script to redeem it. Since there is no input script // to redeem it yet, use the minimum size of a typical input script. @@ -253,6 +246,20 @@ func IsDust(txOut *wire.TxOut, minRelayTxFee btcutil.Amount) bool { totalSize += 107 } + return 3 * int64(totalSize) +} + +// IsDust returns whether or not the passed transaction output amount is +// considered dust or not based on the passed minimum transaction relay fee. +// Dust is defined in terms of the minimum transaction relay fee. In +// particular, if the cost to the network to spend coins is more than 1/3 of the +// minimum transaction relay fee, it is considered dust. +func IsDust(txOut *wire.TxOut, minRelayTxFee btcutil.Amount) bool { + // Unspendable outputs are considered dust. + if txscript.IsUnspendable(txOut.PkScript) { + return true + } + // The output is considered dust if the cost to the network to spend the // coins is more than 1/3 of the minimum free transaction relay fee. // minFreeTxRelayFee is in Satoshi/KB, so multiply by 1000 to @@ -265,7 +272,7 @@ func IsDust(txOut *wire.TxOut, minRelayTxFee btcutil.Amount) bool { // // The following is equivalent to (value/totalSize) * (1/3) * 1000 // without needing to do floating point math. - return txOut.Value*1000/(3*int64(totalSize)) < int64(minRelayTxFee) + return txOut.Value*1000/GetDustThreshold(txOut) < int64(minRelayTxFee) } // checkTransactionStandard performs a series of checks on a transaction to From 4f64fe9bd79b8b14cf1da20dcba73cedb20917c2 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 10 Oct 2022 14:57:13 -0700 Subject: [PATCH 4/6] mempool: fix linter error --- mempool/mempool_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mempool/mempool_test.go b/mempool/mempool_test.go index 96d5054417..de192781dc 100644 --- a/mempool/mempool_test.go +++ b/mempool/mempool_test.go @@ -560,7 +560,7 @@ func TestOrphanReject(t *testing.T) { // Ensure no transactions were reported as accepted. if len(acceptedTxns) != 0 { - t.Fatal("ProcessTransaction: reported %d accepted "+ + t.Fatalf("ProcessTransaction: reported %d accepted "+ "transactions from failed orphan attempt", len(acceptedTxns)) } From 744680237d4de70bfec5d9722714e656fa6ddd22 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Mon, 13 Sep 2021 21:11:26 +0200 Subject: [PATCH 5/6] btcec: check if recovered pk is at point of infinity --- btcec/signature.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/btcec/signature.go b/btcec/signature.go index cdd7cedfb8..8a8f8301b7 100644 --- a/btcec/signature.go +++ b/btcec/signature.go @@ -353,6 +353,10 @@ func recoverKeyFromSignature(curve *KoblitzCurve, sig *Signature, msg []byte, // step to prevent the jacobian conversion back and forth. Qx, Qy := curve.Add(sRx, sRy, minuseGx, minuseGy) + if Qx.Sign() == 0 && Qy.Sign() == 0 { + return nil, errors.New("point (Qx, Qy) equals the point at infinity") + } + return &PublicKey{ Curve: curve, X: Qx, From ba2fba510c864b6cd15512889c64919df9ffedc2 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 10 Oct 2022 11:49:11 -0700 Subject: [PATCH 6/6] build: bump version to v0.22.2 --- version.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.go b/version.go index f723b5c6e7..e9a86b3bad 100644 --- a/version.go +++ b/version.go @@ -18,7 +18,7 @@ const semanticAlphabet = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqr const ( appMajor uint = 0 appMinor uint = 22 - appPatch uint = 1 + appPatch uint = 2 // appPreRelease MUST only contain characters from semanticAlphabet // per the semantic versioning spec.