Skip to content

backport: Merge bitcoin#28587, 28056, 27905 #7356

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
vijaydasmp wants to merge 3 commits into
base: develop
Choose a base branch
from
+20 −1
Open

backport: Merge bitcoin#28587, 28056, 27905 #7356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fe31eca
Merge bitcoin/bitcoin#28587: descriptors: disallow hybrid public keys
achow101 Oct 5, 2023
743ac80
Merge bitcoin/bitcoin#28056: rpc: doc: Added `longpollid` and `data` …
fanquake Jul 19, 2023
7e8d383
Merge bitcoin/bitcoin#27905: validation: add missing insert to m_dirt…
fanquake Jun 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/pubkey.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 2 additions & 0 deletions src/rpc/mining.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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\""},
},
Expand Down
6 changes: 5 additions & 1 deletion src/script/descriptor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -869,6 +869,10 @@ std::unique_ptr<PubkeyProvider> ParsePubkeyInner(uint32_t key_exp_index, const S
if (IsHex(str)) {
std::vector<unsigned char> data = ParseHex(str);
CPubKey pubkey(data);
if (pubkey.IsValid() && !pubkey.IsValidNonHybrid()) {
error = "Hybrid public keys are not allowed";
return nullptr;
}
Comment on lines +872 to +875

@thepastaclaw thepastaclaw Jun 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Blocking: Backport bitcoin#24148 before bitcoin#28587

bitcoin#28587 modifies three descriptor parsing paths upstream: direct hex pubkey parsing, descriptor inference, and the Miniscript descriptor KeyParser path. This backport only applies the first two because Dash does not yet have struct KeyParser / Miniscript descriptor support in src/script/descriptor.cpp, which was introduced upstream by bitcoin#24148. That absence is a missing prerequisite, not a Dash-specific adaptation: upstream's pre-bitcoin#28587 state already contained the parser and its descriptor tests, and bitcoin#28587 changes it from pubkey.IsValid() to pubkey.IsValidNonHybrid() while adding corresponding wsh(...)/Miniscript coverage in src/test/descriptor_tests.cpp. Please backport the prerequisite descriptor/Miniscript chain first, or drop bitcoin#28587 from this PR until that chain is present.

source: ['codex']

@thepastaclaw thepastaclaw Jun 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Resolved in this update — Backport bitcoin#24148 before bitcoin#28587 no longer present.

Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.

if (pubkey.IsFullyValid()) {
if (permit_uncompressed || pubkey.IsCompressed()) {
return std::make_unique<ConstPubkeyProvider>(key_exp_index, pubkey);
Expand Down Expand Up @@ -1096,7 +1100,7 @@ std::unique_ptr<DescriptorImpl> InferScript(const CScript& script, ParseScriptCo

if (txntype == TxoutType::PUBKEY) {
CPubKey pubkey(data[0]);
if (pubkey.IsValid()) {
if (pubkey.IsValidNonHybrid()) {
return std::make_unique<PKDescriptor>(InferPubkey(pubkey, ctx, provider));
Comment on lines 1101 to 1104

@thepastaclaw thepastaclaw Jun 15, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Blocking: Missing prerequisite: bitcoin#24148

bitcoin#28587 changes all descriptor public-key inference/parsing paths from accepting CPubKey::IsValid() to rejecting hybrid encodings via IsValidNonHybrid(). Upstream also changes the Miniscript descriptor KeyParser::FromPKBytes path (CPubKey pubkey(begin, end); if (pubkey.IsValidNonHybrid()) ...). Dash's starting state has no KeyParser or Miniscript descriptor tests, so the local cherry-pick only applies the non-Miniscript ParsePubkeyInner and InferScript parts.

Tracing the missing struct KeyParser section with git log bitcoin/master -S "struct KeyParser" -- src/script/descriptor.cpp points to 85b601e043 Merge bitcoin/bitcoin#24148: Miniscript support in Output Descriptors. Because this PR claims to backport bitcoin#28587 as a whole, the omitted upstream hunk is a missing prerequisite rather than a harmless Dash adaptation. Please backport the prerequisite descriptor/Miniscript chain first, or drop bitcoin#28587 from this PR until that chain is present.

source: ['codex-backport-reviewer'], promoted to blocking by backport-prereq policy gate

}
}
Expand Down
5 changes: 5 additions & 0 deletions src/test/descriptor_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions src/validation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading