-
Notifications
You must be signed in to change notification settings - Fork 2
Match Core generateblock multipath, ranged, and Expand errors #612
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -333,6 +333,9 @@ pub(crate) fn deriveaddresses(ctx: &Arc<Context>, params: &Value) -> Result<Valu | |
| fn descriptor_error(error: DescriptorError) -> RpcError { | ||
| match error { | ||
| DescriptorError::Range(message) => RpcError::InvalidParameter(message.to_owned()), | ||
| DescriptorError::PrivateKeys => { | ||
| RpcError::InvalidAddressOrKey(GENERATEBLOCK_NEEDS_PRIVATE_KEYS.to_owned()) | ||
| } | ||
| DescriptorError::Parse(message) => RpcError::InvalidAddressOrKey(message), | ||
| } | ||
| } | ||
|
|
@@ -532,13 +535,16 @@ enum DescriptorError { | |
| Parse(String), | ||
| /// The derivation range does not match the descriptor. | ||
| Range(&'static str), | ||
| /// `Expand` needs a private key (hardened path from an xpub, …). | ||
| PrivateKeys, | ||
| } | ||
|
|
||
| impl core::fmt::Display for DescriptorError { | ||
| fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { | ||
| match self { | ||
| Self::Parse(message) => write!(f, "{message}"), | ||
| Self::Range(message) => write!(f, "{message}"), | ||
| Self::PrivateKeys => write!(f, "{GENERATEBLOCK_NEEDS_PRIVATE_KEYS}"), | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -972,17 +978,25 @@ fn strip_checksum(text: &str) -> &str { | |
| } | ||
|
|
||
| pub(crate) const GENERATEBLOCK_INVALID_OUTPUT: &str = "Error: Invalid address or descriptor"; | ||
| pub(crate) const GENERATEBLOCK_MULTIPATH: &str = "Multipath descriptor not accepted"; | ||
| pub(crate) const GENERATEBLOCK_RANGED: &str = | ||
| "Ranged descriptor not accepted. Maybe pass through deriveaddresses first?"; | ||
| pub(crate) const GENERATEBLOCK_NEEDS_PRIVATE_KEYS: &str = | ||
| "Cannot derive script without private keys"; | ||
|
|
||
| /// Coinbase script for `generateblock`'s `output` argument (`API-05`). | ||
| /// | ||
| /// CONTRACT: docs/contracts/external-api.md#API-26 | ||
| /// CONTRACT: docs/contracts/external-api.md#API-28 | ||
| pub(crate) fn generateblock_payout_script( | ||
| text: &str, | ||
| network: bitcoin::Network, | ||
| ) -> Result<Vec<u8>, RpcError> { | ||
| match script_from_descriptor(text, network) { | ||
| Ok(script) => Ok(script), | ||
| Err(error @ DescriptorError::Range(_)) => Err(descriptor_error(error)), | ||
| Err(error @ (DescriptorError::Range(_) | DescriptorError::PrivateKeys)) => { | ||
| Err(descriptor_error(error)) | ||
| } | ||
| Err(_) => payout_script_from_address(text, network, GENERATEBLOCK_INVALID_OUTPUT), | ||
| } | ||
| } | ||
|
|
@@ -1028,38 +1042,62 @@ fn script_from_descriptor( | |
| let (descriptor, keys) = | ||
| MiniscriptDescriptor::<DescriptorPublicKey>::parse_descriptor(&secp, &checksummed) | ||
| .map_err(|error| DescriptorError::Parse(error.to_string()))?; | ||
| if descriptor.has_wildcard() || descriptor.is_multipath() { | ||
| if descriptor.is_multipath() { | ||
| return Err(multipath_descriptor_rejected()); | ||
| } | ||
| if descriptor.has_wildcard() { | ||
| return Err(ranged_descriptor_rejected()); | ||
| } | ||
| ensure_keys_match_network(&descriptor, network)?; | ||
| ensure_secret_keys_match_network(keys, network)?; | ||
| reject_hardened_xpub(&descriptor)?; | ||
| let derived = descriptor | ||
| .at_derivation_index(0) | ||
| .map_err(|error| DescriptorError::Parse(error.to_string()))?; | ||
| .map_err(|_| DescriptorError::PrivateKeys)?; | ||
| Ok(derived.script_pubkey().as_bytes().to_vec()) | ||
| } | ||
|
|
||
| fn combo_payout_script(key: &str, network: bitcoin::Network) -> Result<Vec<u8>, DescriptorError> { | ||
| let combo = parse_combo_info(key, network)?; | ||
| if combo.is_range || combo.paths.len() != 1 { | ||
| if combo.paths.len() != 1 { | ||
| return Err(multipath_descriptor_rejected()); | ||
| } | ||
| if combo.is_range { | ||
| return Err(ranged_descriptor_rejected()); | ||
| } | ||
| let path = combo | ||
| .paths | ||
| .first() | ||
| .ok_or_else(|| DescriptorError::Parse("Invalid combo descriptor".into()))?; | ||
| reject_hardened_xpub(path)?; | ||
| let derived = path | ||
| .at_derivation_index(0) | ||
| .map_err(|error| DescriptorError::Parse(error.to_string()))?; | ||
| .map_err(|_| DescriptorError::PrivateKeys)?; | ||
| // Core's combo Expand emits P2PK first and generateblock uses scripts[0]. | ||
| let pk = MiniscriptDescriptor::new_pk(combo_key(&derived)?); | ||
| Ok(pk.script_pubkey().as_bytes().to_vec()) | ||
| } | ||
|
|
||
| fn multipath_descriptor_rejected() -> DescriptorError { | ||
| DescriptorError::Range(GENERATEBLOCK_MULTIPATH) | ||
| } | ||
|
|
||
| fn ranged_descriptor_rejected() -> DescriptorError { | ||
| DescriptorError::Range( | ||
| "Ranged descriptor not accepted. Maybe pass through deriveaddresses first?", | ||
| ) | ||
| DescriptorError::Range(GENERATEBLOCK_RANGED) | ||
| } | ||
|
|
||
| /// rust-miniscript panics in `at_derivation_index` on an xpub hardened step. | ||
| /// Core `Expand` returns false, which `getScriptFromDescriptor` maps to | ||
| /// `Cannot derive script without private keys`. An xprv is converted to an | ||
| /// xpub with those steps already applied during parse, so it never hits this. | ||
|
Comment on lines
+1089
to
+1092
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1. reject_hardened_xpub duplicates api-28 The new comment restates API-28’s Core Expand failure behavior and exact error text without referencing the authoritative contract. This creates another textual representation that can drift from docs/contracts/external-api.md. Agent Prompt
|
||
| fn reject_hardened_xpub( | ||
| descriptor: &MiniscriptDescriptor<DescriptorPublicKey>, | ||
| ) -> Result<(), DescriptorError> { | ||
| if descriptor.for_any_key(DescriptorPublicKey::has_hardened_step) { | ||
| Err(DescriptorError::PrivateKeys) | ||
| } else { | ||
| Ok(()) | ||
| } | ||
| } | ||
|
|
||
| fn descriptor_text_with_optional_checksum(text: &str) -> Result<String, DescriptorError> { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -141,7 +141,7 @@ declare_rows! { | |
| "submitheader", SurfaceKind::Rpc, Status::Implemented, "", CORE_VERSION, "Header-only admission through the same tree path as inbound P2P headers. Decode failures are -22; missing previous or invalid headers are -25.", "0.4.0", Some(mining::submitheader); | ||
| "prioritisetransaction", SurfaceKind::Rpc, Status::Implemented, "", CORE_VERSION, "Dummy (params[1]) must be 0 or null; fee_delta is params[2]. Non-zero dummy is Core -8. Pooled dust outputs are -8 except on regtest.", "0.4.0", Some(mining::prioritisetransaction); | ||
| "generatetoaddress", SurfaceKind::Rpc, Status::Implemented, "", CORE_VERSION, "Assembles, solves, and submits n blocks paying the given address through the mining coordinator. A non-address is Core -5 Error: Invalid address.", "0.4.0", Some(mining::generatetoaddress); | ||
| "generateblock", SurfaceKind::Rpc, Status::Implemented, "", CORE_VERSION, "Assembles and solves one block paying an address or descriptor from the listed mempool txids or raw txs in that order; third param is Core's submit flag. Unknown 64-hex txids are -5; raw-tx decode failures are -22. Invalid output is -5 Error: Invalid address or descriptor. Consensus failure before solve is -25 TestBlockValidity failed: {reason}.", "0.4.0", Some(mining::generateblock); | ||
| "generateblock", SurfaceKind::Rpc, Status::Implemented, "", CORE_VERSION, "Assembles and solves one block paying an address or descriptor from the listed mempool txids or raw txs in that order; third param is Core's submit flag. Unknown 64-hex txids are -5; raw-tx decode failures are -22. Invalid output is -5 Error: Invalid address or descriptor. Consensus failure before solve is -25 TestBlockValidity failed: {reason}. Multipath is -8 Multipath descriptor not accepted; ranged is -8 Ranged descriptor not accepted…; Expand without private keys is -5 Cannot derive script without private keys.", "0.4.0", Some(mining::generateblock); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1. generateblock rules duplicated in registry The registry note fully restates API-28’s error codes, messages, and conditions instead of linking to the canonical contract. This creates a parallel textual representation that can drift from docs/contracts/external-api.md. Agent Prompt
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Document the complete ranged error message. Line 144 replaces the Core message with an ellipsis. 🤖 Prompt for AI Agents |
||
| "getnetworkhashps", SurfaceKind::Rpc, Status::Implemented, "", CORE_VERSION, "Estimated hashes/s over a caller-chosen lookback ending at a caller-chosen height; default lookback 120, height the applied tip.", "0.4.0", Some(mining::getnetworkhashps); | ||
| "getprioritisedtransactions", SurfaceKind::Rpc, Status::Implemented, "", CORE_VERSION, "Projects the mempool's signed fee-delta overlay, including txids not currently pooled. modified_fee is satoshis like Core mining RPCs, not BTC.", "0.4.0", Some(mining::getprioritisedtransactions); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2. Network mismatch masked by multipath
🐞 Bug≡ CorrectnessAgent Prompt
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools