From fe31eca5e7df4bae9b9626455af7cbcefda9dc18 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 5 Oct 2023 11:52:04 -0400 Subject: [PATCH 1/3] Merge bitcoin/bitcoin#28587: descriptors: disallow hybrid public keys c1e6c542af6d89a499e2a65465865aec651c4d67 descriptors: disallow hybrid public keys (Pieter Wuille) Pull request description: Fixes #28511 The descriptor documentation (`doc/descriptors.md`) and [BIP380](https://github.com/bitcoin/bips/blob/master/bip-0380.mediawiki) explicitly require that hex-encoded public keys start with 02 or 03 (compressed) or 04 (uncompressed). However, the current parsing/inference code permit 06 and 07 (hybrid) encoding as well. Fix this. ACKs for top commit: darosior: ACK c1e6c542af6d89a499e2a65465865aec651c4d67 achow101: ACK c1e6c542af6d89a499e2a65465865aec651c4d67 Tree-SHA512: 23b674fb420619b2536d12da10008bb87cf7bc0333ec59e618c0d02c3574b468cc71248475ece37f76658d743ef51e68566948e903bca79fda5f7d75416fea4d --- src/pubkey.h | 6 ++++++ src/script/descriptor.cpp | 6 +++++- src/test/descriptor_tests.cpp | 5 +++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/pubkey.h b/src/pubkey.h index 990a33ccef99..5de7fa9519a8 100644 --- a/src/pubkey.h +++ b/src/pubkey.h @@ -181,6 +181,12 @@ class CPubKey return size() > 0; } + /** Check if a public key is a syntactically valid compressed or uncompressed key. */ + bool IsValidNonHybrid() const noexcept + { + return size() > 0 && (vch[0] == 0x02 || vch[0] == 0x03 || vch[0] == 0x04); + } + //! fully validate whether this is a valid public key (more expensive than IsValid()) bool IsFullyValid() const; diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index e432ce3943fc..bdf6252d868e 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -869,6 +869,10 @@ std::unique_ptr ParsePubkeyInner(uint32_t key_exp_index, const S if (IsHex(str)) { std::vector data = ParseHex(str); CPubKey pubkey(data); + if (pubkey.IsValid() && !pubkey.IsValidNonHybrid()) { + error = "Hybrid public keys are not allowed"; + return nullptr; + } if (pubkey.IsFullyValid()) { if (permit_uncompressed || pubkey.IsCompressed()) { return std::make_unique(key_exp_index, pubkey); @@ -1096,7 +1100,7 @@ std::unique_ptr InferScript(const CScript& script, ParseScriptCo if (txntype == TxoutType::PUBKEY) { CPubKey pubkey(data[0]); - if (pubkey.IsValid()) { + if (pubkey.IsValidNonHybrid()) { return std::make_unique(InferPubkey(pubkey, ctx, provider)); } } diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp index 927d5551baea..69c03ccddf71 100644 --- a/src/test/descriptor_tests.cpp +++ b/src/test/descriptor_tests.cpp @@ -293,6 +293,11 @@ BOOST_AUTO_TEST_CASE(descriptor_test) Check("pkh(7sH936MDoVPFVk2VoaCM5yW8P3BfPyffnZECyaHrZwfLgWpS13e)", "pkh(04a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd5b8dec5235a0fa8722476c7709c02559e3aa73aa03918ba2d492eea75abea235)", "pkh(04a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd5b8dec5235a0fa8722476c7709c02559e3aa73aa03918ba2d492eea75abea235)", SIGNABLE, {{"76a914b5bd079c4d57cc7fc28ecf8213a6b791625b818388ac"}}, OutputType::LEGACY); Check("combo(7sH936MDoVPFVk2VoaCM5yW8P3BfPyffnZECyaHrZwfLgWpS13e)", "combo(04a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd5b8dec5235a0fa8722476c7709c02559e3aa73aa03918ba2d492eea75abea235)", "combo(04a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd5b8dec5235a0fa8722476c7709c02559e3aa73aa03918ba2d492eea75abea235)", SIGNABLE, {{"4104a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd5b8dec5235a0fa8722476c7709c02559e3aa73aa03918ba2d492eea75abea235ac"}}, OutputType::LEGACY); + // Equivalent single-key hybrid is not allowed + CheckUnparsable("", "combo(07a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd5b8dec5235a0fa8722476c7709c02559e3aa73aa03918ba2d492eea75abea235)", "combo(): Hybrid public keys are not allowed"); + CheckUnparsable("", "pk(0623542d61708e3fc48ba78fbe8fcc983ba94a520bc33f82b8e45e51dbc47af2726bcf181925eee1bdd868b109314f3ea92a6fc23d6b66057d3acfba04d6b08b58)", "pk(): Hybrid public keys are not allowed"); + CheckUnparsable("", "pkh(07a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd5b8dec5235a0fa8722476c7709c02559e3aa73aa03918ba2d492eea75abea235)", "pkh(): Hybrid public keys are not allowed"); + // Some unconventional single-key constructions Check("sh(pk(XJvEUEcFWCHCyruc8ZX5exPZaGe4UR7gC5FHrhwPnQGDs1uWCsT2))", "sh(pk(03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd))", "sh(pk(03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd))", SIGNABLE, {{"a9141857af51a5e516552b3086430fd8ce55f7c1a52487"}}, OutputType::LEGACY); Check("sh(pkh(XJvEUEcFWCHCyruc8ZX5exPZaGe4UR7gC5FHrhwPnQGDs1uWCsT2))", "sh(pkh(03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd))", "sh(pkh(03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd))", SIGNABLE, {{"a9141a31ad23bf49c247dd531a623c2ef57da3c400c587"}}, OutputType::LEGACY); From 743ac805d347e4b1a87fc8bff3db359424b08e15 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 19 Jul 2023 12:13:07 +0100 Subject: [PATCH 2/3] Merge bitcoin/bitcoin#28056: rpc: doc: Added `longpollid` and `data` params to `template_request` f6a26196cfb7e2c90e25f82b0e2f569a05013cae Added `longpollid` and `data` params to `template_request` #27998 (Rhythm Garg) Pull request description: This PR will add the optional parameters `longpollid` and `data` to `template_request` as they were missing when calling `help getblocktemplate` in RPCHelpMan. I request the maintainers to review this and let me know about any mistakes in the descriptions of the parameters. This PR refers to the issue #27998 ACKs for top commit: ItIsOHM: > tACK [f6a2619](https://github.com/bitcoin/bitcoin/commit/f6a26196cfb7e2c90e25f82b0e2f569a05013cae) russeree: tACK https://github.com/bitcoin/bitcoin/commit/f6a26196cfb7e2c90e25f82b0e2f569a05013cae stickies-v: tACK f6a26196cfb7e2c90e25f82b0e2f569a05013cae Tree-SHA512: 6c592db59cb11b2d031ce5265c547fa296266278f6c25f96afe18a420e0d547f4d483e0f66de75d52c0c319ac1585f3558b9f70c12ef208c96ec96a51f786c6a --- src/rpc/mining.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 24b63fe83eec..2474038e2211 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -559,6 +559,8 @@ static RPCHelpMan getblocktemplate() {"str", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "client side supported softfork deployment"}, }, }, + {"longpollid", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "delay processing request until the result would vary significantly from the \"longpollid\" of a prior template"}, + {"data", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED_NAMED_ARG, "proposed block data to check, encoded in hexadecimal; valid only for mode=\"proposal\""}, }, "\"template_request\""}, }, From 7e8d383d16d1857ef20fa6e086f971f4b5de7bd1 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 21 Jun 2023 13:27:54 +0100 Subject: [PATCH 3/3] Merge bitcoin/bitcoin#27905: validation: add missing insert to m_dirty_blockindex e639364495a26bd67dd08998fc7ec400747f9a15 validation: add missing insert to m_dirty_blockindex (Martin Zumsande) Pull request description: When the status of a block index is changed, we must add it to `m_dirty_blockindex` or the change might not get persisted to disk. This is missing from one spot in `FindMostWorkChain()`, where `BLOCK_FAILED_CHILD` is set. Since we have [code](https://github.com/bitcoin/bitcoin/blob/f0758d8a6696657269d9c057e7aa079ffa9e1c16/src/node/blockstorage.cpp#L284-L287) that later sets missing `BLOCK_FAILED_CHILD` during the next startup, I don't think that this can lead to bad block indexes in practice, but I still think it's worth fixing. ACKs for top commit: TheCharlatan: ACK e639364495a26bd67dd08998fc7ec400747f9a15 stickies-v: ACK e639364495a26bd67dd08998fc7ec400747f9a15 Tree-SHA512: a97af9c173e31b90b677a1f95de822e08078d78013de5fa5fe4c3bec06f45d6e1823b7694cdacb887d031329e4b4afc6a2003916e0ae131279dee71f43e1f478 --- src/validation.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/validation.cpp b/src/validation.cpp index 06d45ca61bcb..6073f5cc7d0e 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3111,9 +3111,11 @@ CBlockIndex* CChainState::FindMostWorkChain() while (pindexTest != pindexFailed) { if (fFailedChain) { pindexFailed->nStatus |= BLOCK_FAILED_CHILD; + m_blockman.m_dirty_blockindex.insert(pindexFailed); } else if (fConflictingChain) { // We don't need data for conflciting blocks pindexFailed->nStatus |= BLOCK_CONFLICT_CHAINLOCK; + m_blockman.m_dirty_blockindex.insert(pindexFailed); } else if (fMissingData) { // If we're missing data, then add back to m_blocks_unlinked, // so that if the block arrives in the future we can try adding