Repository navigation
bitcoin: add sign-only and getrawtransaction RPCs - #279
Merged
Merged
Conversation
morehouse
reviewed
Oct 14, 2026
| let signed = self | ||
| .sign_raw_transaction_with_wallet(tx) | ||
| .inspect_err(|stderr| { | ||
| log::debug!("bitcoin-cli signrawtransactionwithwallet failed: {stderr}"); |
Collaborator
There was a problem hiding this comment.
Under what circumstances would we expect an error?
Should we still panic for errors that are unexpected?
Member
Author
There was a problem hiding this comment.
It only makes sense for the interactive-tx. Since the fuzzer and target might build invalid transactions that we can't sign. E.g a transaction without any input.
Collaborator
There was a problem hiding this comment.
I asked Claude under what circumstances signrawtransactionwithwallet can return an error, and it gave me this table.
┌─────────────────────────────────────────────┬──────────────────────────────────────────────────────────────────────────────┐
│ Error │ Can smite hit it? │
├─────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────────────────┤
│ GetWalletForJSONRPCRequest: no wallet │ │
│ loaded, or several loaded without │ Only if the environment is broken │
│ -rpcwallet │ │
├─────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────────────────┤
│ DecodeHexTx fails ("TX decode failed. Make │ Only with zero inputs. rust-bitcoin always produces a valid serialization; │
│ sure the tx has at least one input.") │ with zero inputs it adds a segwit marker but no witnesses, which Core │
│ │ rejects as "Superfluous witness record" │
├─────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────────────────┤
│ EnsureWalletIsUnlocked │ No, the wallet isn't encrypted │
├─────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────────────────┤
│ ParsePrevouts / ParseSighashString │ No, smite passes neither argument │
├─────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────────────────┤
│ "Missing amount" (the one per-input error │ No. It needs amount == MAX_MONEY, which only happens when prevtxs is passed │
│ that becomes an RPC error) │ without an amount (rawtransaction_util.cpp:244) │
├─────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────────────────┤
│ bitcoind down, still starting up, or │ Only if the environment is broken │
│ failing auth │ │
└─────────────────────────────────────────────┴──────────────────────────────────────────────────────────────────────────────┘
The only one I can imagine us hitting is "zero inputs". But even then it seems like something we shouldn't ever hit unless there's a bug in smite.
- splicing always has the funding input
- for dual funding smite should only ever do the RPC if it added its own input, otherwise there's no reason to sign. And if the target allowed this, it seems like a bug we should detect and report early, before we ever try to sign.
Member
Author
There was a problem hiding this comment.
Makes sense. We can check it before calling the RPC. I've updated the code to panic.
erickcestari
force-pushed
the
bitcoin-raw-tx-rpcs
branch
from
October 15, 2026 17:02
3feef63 to
5ad1310
Compare
morehouse
reviewed
Oct 15, 2026
Lets a sign-only caller that does not require complete signing reuse it.
Channel establishment v2 needs our own witnesses for tx_signatures, which the wallet can only produce as a partial signing of the shared transaction.
Channel establishment v2 needs the previous transaction bytes for tx_add_input's prevtx.
erickcestari
force-pushed
the
bitcoin-raw-tx-rpcs
branch
from
October 15, 2026 19:00
5ad1310 to
88b50f1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Channel establishment v2 needs the previous transaction bytes for
tx_add_inputand our own witnesses fortx_signatures, which the wallet can only produce as a partial signing of the shared transaction.This is part of breaking down PR #239 into smaller PRs to make them easier to review.