-
Notifications
You must be signed in to change notification settings - Fork 36.2k
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
fundrawtransaction fails to add watchonly addresses imported with importaddress #7879
Comments
I was able to fund a raw transaction using watchonly outputs for which I don't have a local copy of the public keys using this workaround: dooglus@0b29c31 I have the public keys on an airgapped machine, where I intend to sign the funded raw transaction. I don't want to have to have the public keys on my online machine. It's probably not the correct fix, but it demonstrates where the problem is. |
@dooglus Is this still an issue? If so, you might also write a functional test that exercises the logic. |
@MarcoFalke It is still an issue. It's quite easy to reproduce. I'm starting with two brand new wallets in the following. If I use
But if I use
I would like to be able to keep my public keys offline and still be able to use |
I don't really understand why this should work. |
Fair enough. I thought that when I first created this issue I was able to get |
Ah I see; for P2PKH you can't solve without knowing the pubkey, but you can know ahead of time what the size of a scriptSog would be. I'm not sure we should bother with that. |
So is there a way so mark an imported address as solvable?
|
You can't mark an imported address as solvable, there's no flag for that. Rather every output considered for coin selection goes through the solvability test which checks to see whether the wallet has enough information to construct the final input for that output if it knew what the private keys were. The whole point of this is to allow for fee estimation. If you don't know what needs to be in the inputs for a given output, you won't be able to know how big that input will be so you cannot reliably estimate the fee for the transaction. This then effects what coins you select because you need to know how much you will pay in fees for each input so that you can be sure that there are enough inputs to cover the outgoing amounts and the transaction fees. |
@stevenroose > what's the point of that if watched addresses are not considered? I'm guessing the point is that you can have a watchonly address that was imported using |
And on the other side, addresses watched by using importaddress (or non-solvable importmulti) can still work in a wallet that just needs to track incoming payments etc, but not participate in spending. |
I agree just clarifying this in fundrawtransaction is sufficient. |
8003104 Clarify includeWatching for fundrawtransaction (Steven Roose) Pull request description: Might be sufficient to solve #16396, #7879 and #14405. ACKs for top commit: Sjors: ACK 8003104. This will always be confusing, but at least it gives a bunch more clues for the user to google. Tree-SHA512: 9b8002c259c50f93d89fc5574105aae6152858d8d45c07b4c3d5b7023adafe73c7a98a290874ff3fbbb7dfad2ac1bdf4acb8769a2a1c14e38484922f44e84e54
This has been fixed in 79aeed8? |
8003104 Clarify includeWatching for fundrawtransaction (Steven Roose) Pull request description: Might be sufficient to solve bitcoin#16396, bitcoin#7879 and bitcoin#14405. ACKs for top commit: Sjors: ACK 8003104. This will always be confusing, but at least it gives a bunch more clues for the user to google. Tree-SHA512: 9b8002c259c50f93d89fc5574105aae6152858d8d45c07b4c3d5b7023adafe73c7a98a290874ff3fbbb7dfad2ac1bdf4acb8769a2a1c14e38484922f44e84e54
Summary: Clarify includeWatching for fundrawtransaction (Steven Roose) Pull request description: Might be sufficient to solve bitcoin/bitcoin#16396, bitcoin/bitcoin#7879 and bitcoin/bitcoin#14405. --- Backport of Core [[bitcoin/bitcoin#16397 | PR16397]] Test Plan: ninja bitcoin-cli -regtest help fundrawtransaction Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D7381
8003104 Clarify includeWatching for fundrawtransaction (Steven Roose) Pull request description: Might be sufficient to solve bitcoin#16396, bitcoin#7879 and bitcoin#14405. ACKs for top commit: Sjors: ACK 8003104. This will always be confusing, but at least it gives a bunch more clues for the user to google. Tree-SHA512: 9b8002c259c50f93d89fc5574105aae6152858d8d45c07b4c3d5b7023adafe73c7a98a290874ff3fbbb7dfad2ac1bdf4acb8769a2a1c14e38484922f44e84e54
8003104 Clarify includeWatching for fundrawtransaction (Steven Roose) Pull request description: Might be sufficient to solve bitcoin#16396, bitcoin#7879 and bitcoin#14405. ACKs for top commit: Sjors: ACK 8003104. This will always be confusing, but at least it gives a bunch more clues for the user to google. Tree-SHA512: 9b8002c259c50f93d89fc5574105aae6152858d8d45c07b4c3d5b7023adafe73c7a98a290874ff3fbbb7dfad2ac1bdf4acb8769a2a1c14e38484922f44e84e54
8003104 Clarify includeWatching for fundrawtransaction (Steven Roose) Pull request description: Might be sufficient to solve bitcoin#16396, bitcoin#7879 and bitcoin#14405. ACKs for top commit: Sjors: ACK 8003104. This will always be confusing, but at least it gives a bunch more clues for the user to google. Tree-SHA512: 9b8002c259c50f93d89fc5574105aae6152858d8d45c07b4c3d5b7023adafe73c7a98a290874ff3fbbb7dfad2ac1bdf4acb8769a2a1c14e38484922f44e84e54
I have a wallet with nothing but a bunch of addresses I imported using
importaddress
.I tried using
fundrawtransaction
withincludeWatching
set totrue
but it always tells me I have insufficient funds.It turns out that for this to work I need to have added the watchonly address using
importpubkey
notimportaddress
because otherwiseIsMine()
on the output'sscriptPubKey
fails because the dummy signer'sscriptSig
has 0 for the public key, causing theHASH160 ... EQUALVERIFY
part to fail when testing the signature in here:That evaluates to
ISMINE_WATCH_UNSOLVABLE
even though it is actually solvable, and so stops any of my outputs being considered for inclusion.output.fSpendable
isfalse
for each output whenCWallet::SelectCoinsMinConf()
runs.The text was updated successfully, but these errors were encountered: