For wallets with WALLET_FLAG_DISABLE_PRIVATE_KEYS this makes the watch-only balance available on the send screen (including coin selection). Instead of sending a transaction it generates a PSBT.
The user can take this PSBT and process it with HWI or put it an SD card for hardware wallets that support that.
The PSBT is copied to the clipboard. This was the easiest approach; we can add a dialog later to display it, as well as an option to save to disk.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
concept ACK.
I do however think a proper dialogue will be required to let the user know what’s going on at a minimum rather than magic resulting in a clipboard result.
very light testing also shows it appears it does what’s on the tin. I’ll do lots more testing later once approach is settled.
So as some of you saw, I did some related work quite awhile ago. I have a dialog with support for creating/saving/loading/signing/sending these. It’s somewhat modeled after the offline signing UI in Armory. I was never happy with the UI, and I think designing this to be intuitive is really hard, but I think it may already be an improvement over clipboard-only.
Let me spend a few minutes figuring out what the most recent version is that builds cleanly and works, and I will link it here so people can try it.
Ok, if you’d like to see my work on this, please have a look at https://github.com/gwillen/bitcoin/tree/feature-offline-v2 .
You will need to check preferences->wallet->enable offline transaction features. Then you’ll get an ‘Offline’ menu, a new offline transactions dialog box, and a new pair of checkboxes in the ‘send’ dialog: “Include watchonly coins” and “create offline transaction” (which is forced on if you include watchonly). If you select the latter, the ‘send’ button becomes “Create unsigned” and will take you to the new dialog.
There is still some amount of jank to be ironed out, there are some XXX comments in the source and some missing error handling.
I’ve had this vaguely shelved for awhile, but I’d be happy to bring it out and work aggressively on it now if desired; I’d also be happy to bring it out less aggressively and start prepping it as a possible sequel to the PR under discussion here.
@gwillen can you make a (draft) PR with that branch? If you rebase on top of this, you can probably drop the “Enable offline transaction features” checkbox as well as the “include watchonly coins” and “create unsigned “checkboxes, in favor of only allowing the functionality with watch-only wallets.
I like how the send button text changes, so I’ll steal that for this PR :-)
Once we have descriptor wallets and better HWI integration, the simple distinction between watch-only and wallets with private keys will no longer work. For example you might be part of a 2 of 3 multisig, with the keys in the wallet. Or maybe you don’t have keys in the Core wallet, but your hardware wallet can sign its part via USB.
So in terms of your wizard, I would split into two wizards: one for composing & exporting, and one for loading.
There’s probably a step 0 where you attempt to (partially) sign with your own keys and with any connected devices. If that’s not enough, then you go to step 1 where you can export the PSBT via clipboard or file system.
I would introduce a fresh dialog for loading, maybe co-signing and then broadcasting a transaction.
I’m afraid the current version of this PR introduces a bug in CWallet::ListCoins.
First, note that == has higher precedence than the ternary conditional:
0[cling]$ 1 == 2 ? 3 : 4
1(int) 4
2[cling]$ (1 == 2) ? 3 : 4
3(int) 4
4[cling]$ 1 == (2 ? 3 : 4)
5(bool) false
Second, some context:
0unsigned int IsMine(...);
1bool IsWalletFlagSet(...);
2int ISMINE_WATCH_ONLY = 1;
3int ISMINE_SPENDABLE = 2;
This is being suggested in this PR:
Shortened to highlight the issue:
0if (... &&
1 IsMine(it->second.tx->vout[output.n]) ==
2 IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ?
3 ISMINE_WATCH_ONLY : ISMINE_SPENDABLE) {
Which – given the precedence rules – is equivalent to:
0if (... &&
1 (IsMine(it->second.tx->vout[output.n]) ==
2 IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) ?
3 ISMINE_WATCH_ONLY : ISMINE_SPENDABLE) {
Which – given that ISMINE_WATCH_ONLY and ISMINE_SPENDABLE are both non-zero integer constants (1 and 2) that will be evaluated in a boolean context – is equivalent to:
0if (... && true) {
Or simply:
0if (...) {
:)
I think the intention was:
0if (... &&
1 IsMine(it->second.tx->vout[output.n]) ==
2 (IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ?
3 ISMINE_WATCH_ONLY : ISMINE_SPENDABLE)) {
Could that be the case? :)
2589@@ -2590,7 +2590,7 @@ std::map<CTxDestination, std::vector<COutput>> CWallet::ListCoins(interfaces::Ch
2590 if (it != mapWallet.end()) {
2591 int depth = it->second.GetDepthInMainChain(locked_chain);
2592 if (depth >= 0 && output.n < it->second.tx->vout.size() &&
2593- IsMine(it->second.tx->vout[output.n]) == ISMINE_SPENDABLE) {
2594+ IsMine(it->second.tx->vout[output.n]) == IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE) {
394+ if (model->privateKeysDisabled()) {
395+ done = true;
396+ CMutableTransaction mtx = CMutableTransaction{*(currentTransaction.getWtx())};
397+ PartiallySignedTransaction psbtx(mtx);
398+ bool complete = false;
399+ const TransactionError err = model->wallet().fillPSBT(psbtx, complete, SIGHASH_ALL, false, true);
385@@ -383,17 +386,36 @@ void SendCoinsDialog::on_sendButton_clicked()
386 return;
387 }
388
389- // now send the prepared transaction
390- WalletModel::SendCoinsReturn sendStatus = model->sendCoins(currentTransaction);
391- // process sendStatus and on error generate message shown to user
392- processSendCoinsReturn(sendStatus);
393+ bool done = false;
done isn’t really helping me parse what is going on here, maybe call it send_failure and reverse the meaning?
The final dialogue should have the “Yes” text replace with “Copy unsigned tx to clipboard” or something self-explanatory.
Really “Yes” should be “Send” in the average case anyways(think I might PR that change separately)
407+ assert(err == TransactionError::OK);
408+ // Serialize the PSBT
409+ CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
410+ ssTx << psbtx;
411+ GUIUtil::setClipboard(EncodeBase64(ssTx.str()).c_str());
412+ Q_EMIT message(tr("PSBT copied"), "Copied to clipbboard", CClientUIInterface::MSG_INFORMATION);
Fixed the precedence bug @practicalswift found.
I added some additional copy:
With that I don’t think it’s necessary to show a dialog with the PSBT text. See also #16966 for a more rigorous approach (I’m trying to keep that an easy rebase), where it will be easier to integrate @gwillen’s PSBT management changes (save to disk, check progress, etc).
“e.g., and offline Bitcoin Core wallet, or a PSBT-compatible hardware wallet”
I think that change would make me happy.
tACK https://github.com/bitcoin/bitcoin/pull/16944/commits/da869592ecf82f8a03eb4ab2339d6bffa3dcea58
Ran both watch-only and non-watch-only modes, tested coin control in watch-only mode, tested a Core -> HWI -> Core round-trip and successfully finalized a PSBT with the desired properties.
437+ Q_EMIT coinsSent(currentTransaction.getWtx()->GetHash());
438+ } else {
439+ send_failure = true;
440+ }
441+ }
442+ if (!send_failure) {
else block since it isn’t used in the no private keys case.
assert) that PSBT creation can never fail, which is why send_failure is always false.
Conecept ACK
It would be preferable if clicking “Copy to Clipboard” wouldn’t close the dialog. Also, maybe show the psbt in a text box in the dialog too?
What am I missing?
0getbalances
1{
2 "mine": {
3 "trusted": 0.00000000,
4 "untrusted_pending": 0.00000000,
5 "immature": 0.00000000
6 },
7 "watchonly": {
8 "trusted": 11202.50000000,
9 "untrusted_pending": 0.00000000,
10 "immature": 1250.00003320
11 }
12}
And then I get “The amount exceeds your balance.”
My experience was that coin selection refused to select watchonly coins when I wanted it to, even if I set watchonly to true in the coincontrol object passed to preparetransaction (and hence createtransaction), unless I made some more-invasive changes, linked below. I’m not sure if that’s what you’re seeing, @promag.
In particular, see my changes to CWallet::ListCoins: https://github.com/gwillen/bitcoin/compare/35991b162fd142d76d83e672002846c8a40eb794...gwillen:feature-offline-v2#diff-b2bb174788c7409b671c46ccc86034bdR2362
which aren’t quite the same as the ones in this PR. When I call AvailableCoins, I force on fWatchOnly in the CCoinControl object that I hand it, which the code did not previously do even if it was called for (I had to add a flag to ListCoins.) Curiously, it seems like it worked for me without the spendability-related changes that this PR makes. Not sure what’s up there.
@promag how did you insert the watch-only keys into that wallet? I’ve only tested with HWI which imports as follows:
[{"desc": "wpkh([8...4P/0/*)#36sal9a4", "internal": false, "range": [0, 1000], "timestamp": "now", "keypool": true, "watchonly": true}, {"desc": "wpkh([80..P/1/*)#nl2rc26w", "internal": true, "range": [0, 1000], "timestamp": "now", "keypool": true, "watchonly": true}]
What does getaddressinfo tell you about the coins you’re trying to spend?
Does walletcreatefundedpsbt fail the same way?
@Sjors result of getaddressinfo:
0{
1 "address": "2N9wHFC4dSkZMu3BCspQkApJ9Pf4coHQWUF",
2 "scriptPubKey": "a914b7155eadf2015c0fe4d1e17c4e29d5d58659a9d987",
3 "ismine": false,
4 "solvable": true,
5 "desc": "sh(wpkh([528979b9]03dce9cd688fd0a9f6dac92627691eb609b8096123455676b5b57db6abaa32f666))#w8dysyn3",
6 "iswatchonly": true,
7 "isscript": true,
8 "iswitness": false,
9 "script": "witness_v0_keyhash",
10 "hex": "0014528979b9bff424bdefb6e7d6cf98f692eb5a9d05",
11 "pubkey": "03dce9cd688fd0a9f6dac92627691eb609b8096123455676b5b57db6abaa32f666",
12 "embedded": {
13 "isscript": false,
14 "iswitness": true,
15 "witness_version": 0,
16 "witness_program": "528979b9bff424bdefb6e7d6cf98f692eb5a9d05",
17 "pubkey": "03dce9cd688fd0a9f6dac92627691eb609b8096123455676b5b57db6abaa32f666",
18 "address": "bcrt1q22yhnwdl7sjtmmakultvlx8kjt4448g9nrg59w",
19 "scriptPubKey": "0014528979b9bff424bdefb6e7d6cf98f692eb5a9d05"
20 },
21 "label": "",
22 "ischange": false,
23 "timestamp": 1,
24 "labels": [
25 {
26 "name": "",
27 "purpose": "receive"
28 }
29 ]
30}
I’ve used importpubkey.
Does
walletcreatefundedpsbtfail the same way?
I’ll try it.
@promag able to reproduce issue? @instagibbs no, currently it’s working 👀
code review and tACK 428bdc8041e2c1479b650207110de081a91a19eb
Tested:
@Sjors and @gwillen thanks for working on this! I use both the GUI and a Bitcoin Core-based cold wallet, so I’m very appreciative of this type of improvement.
Nit-ish. If I try to create an unsigned spend by clicking “Use available balance” and checking “Subtract fee from amount”, my transaction shouldn’t have a change output. However, on a wallet where I haven’t created a keypool, I get an error message about not having a change address.
I also get a second error after that error (“Transaction creation failed”) which seems like error message overkill.
After importing a key to the keypool, this generated the prompt as expected. However, I hit cancel, tried again, and it failed with the above error message. It seems it marked the change address it allocated as used even though (1) the transaction didn’t include a change address and (2) the PSBT was never sent to the clipboard (so, even if there had been a change address, it would never have been user-accessible).
After importing another single key into the keypool and this time not cancelling, it worked as expected. I was able to take the PSBT from the clipboard, run it through my other wallet’s walletprocesspsbt, copy it back, run finalizepsbt, run sendrawtransaction, and verify it entered the mempool of my connected regtest node.
I also tried this on a mixed wallet with some private keys and some watch-only keys, and I can confirm that it doesn’t work there (as it shouldn’t, per the OP). For reference, I ran everything manually (no HWI) and only tested single-sig on regtest.
Thanks again for working on this!
--nokeypool feature). Added a commit to address this.
2968@@ -2969,7 +2969,9 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std
2969 // Create change script that will be used if we need change
2970 // TODO: pass in scriptChange instead of reservedest so
2971 // change transaction isn't always pay-to-bitcoin-address
2972- CScript scriptChange;
2973+ CScript scriptChange = CScript();
1cb1381ca51d1967146a0a75fd96c1b4800ec378
Why was this changed?
2968@@ -2969,7 +2969,9 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std
2969 // Create change script that will be used if we need change
2970 // TODO: pass in scriptChange instead of reservedest so
2971 // change transaction isn't always pay-to-bitcoin-address
2972- CScript scriptChange;
2973+ CScript scriptChange = CScript();
2974+ bool failed_to_get_change_address = false;
1cb1381ca51d1967146a0a75fd96c1b4800ec378
nit bool failed_to_get_change_address{false}.
2968@@ -2969,7 +2969,9 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std
2969 // Create change script that will be used if we need change
2970 // TODO: pass in scriptChange instead of reservedest so
2971 // change transaction isn't always pay-to-bitcoin-address
2972- CScript scriptChange;
2973+ CScript scriptChange = CScript();
2974+ bool failed_to_get_change_address = false;
2975+ std::string failed_to_get_change_address_reason = "";
1cb1381ca51d1967146a0a75fd96c1b4800ec378
I think you don’t need this, you can write the reason in strFailReason.
100+
101+ # creating a transaction with change should not be possible
102+ assert_raises_rpc_error(-4, "Can't generate a change-address key. No keys in the internal keypool and can't generate any keys.", w2.walletcreatefundedpsbt, inputs=[], outputs=[{addr.pop(): 0.05}], options={"subtractFeeFromOutputs": [0]})
103+
104+ # creating a transaction without change should still be possible
105+ res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{addr.pop(): 0.1}], options={"subtractFeeFromOutputs": [0]})
1cb1381ca51d1967146a0a75fd96c1b4800ec378
Could also test it works with changeAddress option?
3007+ bool ret = reservedest.GetReservedDestination(change_type, dest, true);
3008+ if (ret) {
3009+ scriptChange = GetScriptForDestination(dest);
3010+ } else {
3011+ failed_to_get_change_address = true;
3012+ failed_to_get_change_address_reason = "Keypool ran out, please call keypoolrefill first";
1cb1381ca51d1967146a0a75fd96c1b4800ec378
unrelated, I wonder why this is not translated.
186@@ -186,6 +187,13 @@ void SendCoinsDialog::setModel(WalletModel *_model)
187 // set default rbf checkbox state
188 ui->optInRBF->setCheckState(Qt::Checked);
189
190+ if (model->privateKeysDisabled()) {
191+ ui->sendButton->setText(tr("Create Unsigned"));
192+ ui->sendButton->setToolTip(tr("Creates a Partially Unsigned Bitcoin Transaction (PSBT) for use with e.g. an offline Bitcoin Core wallet, or a PSBT-compatible hardware wallet."));
54@@ -55,6 +55,7 @@ public Q_SLOTS:
55
56 Q_SIGNALS:
57 void coinsSent(const uint256& txid);
58+ void psbtCopied(const std::string& psbt);
3209@@ -3207,6 +3210,11 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std
3210 coin_selection_params.use_bnb = false;
3211 continue;
3212 }
3213+
3214+ // Give up if change keypool ran out and coin selection failed to find a solution without change:
3215+ if (failed_to_get_change_address && nChangePosInOut != -1) {
coin_selection_params.use_knapsack which is set to false if failed_to_get_change_address , in order to skip knapsack.
This seems hopeless. There’s a few scenarios where BNB isn’t used:
This is the scenario @gwillen ran into: #16944 (comment)
Reverted to just letting the knapsack solver do its thing.
Code Review ACK 6429af6e4a41d93a632770e050effc8e3e74b023
Looked over the code and did a manual test.
@promag able to reproduce issue?
@instagibbs no, currently it’s working eyes
I ran into this issue too, and in the past with other watch only wallet things. I think it’s just due to balance caching and it should go away after closing and reopening the wallet. I’ll investigate as it is not caused by this PR. edit: maybe not. I was not able to reliably reproduce afterwards.
2982@@ -2982,21 +2983,23 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std
2983 // rediscover unknown transactions that were written with keys of ours to recover
2984 // post-backup change.
2985
2986- // Reserve a new key pair from key pool
2987+ // Reserve a new key pair from key pool. If it fails, provide a dummy
2988+ // destination in case we don't need change.
GetReservedDestination to check if CanGetAddress is true or not. You also can just not store ret and use it directly in the conditional.
re-code-review ACK https://github.com/bitcoin/bitcoin/pull/16944/commits/9808f7867d9c557ba4bc253104f47fbd7f02b668
only changes from last tested review:
0e081a91a19eb 9808f7867d9c557ba4bc253104f47fbd7f02b668
1diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp
2index de701dbdd..3fd63a1b7 100644
3--- a/src/qt/sendcoinsdialog.cpp
4+++ b/src/qt/sendcoinsdialog.cpp
5@@ -189,7 +189,7 @@ void SendCoinsDialog::setModel(WalletModel *_model)
6
7 if (model->privateKeysDisabled()) {
8 ui->sendButton->setText(tr("Create Unsigned"));
9- ui->sendButton->setToolTip(tr("Creates a Partially Unsigned Bitcoin Transaction (PSBT) for use with e.g. an offline Bitcoin Core wallet, or a PSBT-compatible hardware wallet."));
10+ ui->sendButton->setToolTip(tr("Creates a Partially Signed Bitcoin Transaction (PSBT) for use with e.g. an offline Bitcoin Core wallet, or a PSBT-compatible hardware wallet."));
11 } else {
12 ui->sendButton->setText(tr("Send"));
13 }
14diff --git a/src/qt/sendcoinsdialog.h b/src/qt/sendcoinsdialog.h
15index df1d96576..ccd849461 100644
16--- a/src/qt/sendcoinsdialog.h
17+++ b/src/qt/sendcoinsdialog.h
18@@ -55,7 +55,6 @@ public Q_SLOTS:
19
20 Q_SIGNALS:
21 void coinsSent(const uint256& txid);
22- void psbtCopied(const std::string& psbt);
23
24 private:
25 Ui::SendCoinsDialog *ui;
Code review ACK 9808f7867d9c557ba4bc253104f47fbd7f02b668
I’ll wait for one more tested ACK before merging, just to make sure.
2589@@ -2590,7 +2590,9 @@ std::map<CTxDestination, std::vector<COutput>> CWallet::ListCoins(interfaces::Ch
2590 if (it != mapWallet.end()) {
2591 int depth = it->second.GetDepthInMainChain(locked_chain);
2592 if (depth >= 0 && output.n < it->second.tx->vout.size() &&
2593- IsMine(it->second.tx->vout[output.n]) == ISMINE_SPENDABLE) {
2594+ IsMine(it->second.tx->vout[output.n]) ==
a16d2eb0d593633d6ba733d761c56c835b0e30dc
Does it make sense to do something like:
0 // some comment here
1 isminetype is_mine_filter = IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)
2 ? ISMINE_WATCH_ONLY
3 : ISMINE_SPENDABLE;
4
5…
6 IsMine(it->second.tx->vout[output.n]) == is_mine_filter) {
629@@ -630,6 +630,9 @@ void SendCoinsDialog::useAvailableBalance(SendCoinsEntry* entry)
630 coin_control = *CoinControlDialog::coinControl();
631 }
632
633+ // Include watch-only for wallets without private key
634+ coin_control.fAllowWatchOnly = model->privateKeysDisabled();
eef2b6350684f2675989b48a2d7f79c0d8a7be6e
Does this have to be here? Couldn’t this be checked in CWallet::AvailableCoins?
includeWatching, so then we’d have to make it an Optional.
195@@ -195,6 +196,13 @@ class Wallet
196 bool& in_mempool,
197 int& num_blocks) = 0;
198
199+ //! Fill PSBT
200+ virtual TransactionError fillPSBT(PartiallySignedTransaction& psbtx,
201+ bool& complete,
202+ int sighash_type = 1 /* SIGHASH_ALL */,
2bc10122874421ba7ce379325ebc2420955b1b9b
Looks like default arguments aren’t used in these interfaces, @ryanofsky coincidence or is there a reason for that?
psbtx and complete arguments for now.
unsignedFilllPSBT(psbtx) which assert(complete == false) and makes the GUI code simpler?
195@@ -195,6 +196,13 @@ class Wallet
196 bool& in_mempool,
197 int& num_blocks) = 0;
198
199+ //! Fill PSBT
2bc10122874421ba7ce379325ebc2420955b1b9b
nit, comments have period here.
13@@ -14,6 +14,7 @@
14 #include <functional>
15 #include <map>
16 #include <memory>
17+#include <psbt.h>
2bc10122874421ba7ce379325ebc2420955b1b9b
I think you should forward declare what’s necessary.
I tried, but it results in having to import stuff all over the place, and errors like this:
0interfaces/wallet.cpp:354:20: error: no matching function for call to 'FillPSBT'
1 return FillPSBT(m_wallet.get(), psbtx, complete, sighash_type, sign, bip32derivs);
2 ^~~~~~~~
3./wallet/psbtwallet.h:27:28: note: candidate function not viable: cannot convert argument of incomplete type 'interfaces::PartiallySignedTransaction' to 'PartiallySignedTransaction &' for 2nd argument
4NODISCARD TransactionError FillPSBT(const CWallet* pwallet,
5 ^
61 error generated.
20@@ -21,6 +21,7 @@
21 #include <chainparams.h>
22 #include <interfaces/node.h>
23 #include <key_io.h>
24+#include <wallet/psbtwallet.h>
9808f7867d9c557ba4bc253104f47fbd7f02b668
nit sort.
337+
338 questionString.append("<br /><span style='font-size:10pt;'>");
339- questionString.append(tr("Please, review your transaction."));
340+ QString pleaseReview;
341+ if (!model->privateKeysDisabled()) {
342+ pleaseReview = tr("Please, review your transaction.");
9808f7867d9c557ba4bc253104f47fbd7f02b668
Avoid pleaseReview by calling questionString.append here and below? - like L332 above.
392@@ -373,7 +393,12 @@ void SendCoinsDialog::on_sendButton_clicked()
393 questionString = questionString.arg("<br /><br />" + formatted.at(0));
394 }
395
396- SendConfirmationDialog confirmationDialog(tr("Confirm send coins"), questionString, informative_text, detailed_text, SEND_CONFIRM_DELAY, this);
397+ const QString confirmation = model->privateKeysDisabled() ? tr("Confirm transaction proposal") : tr("Confirm send coins");
398+ QString confirmButtonText = tr("Send");
9808f7867d9c557ba4bc253104f47fbd7f02b668
Use ternary like above or just one if (model->privateKeysDisabled()) for these.
349+ bool& complete,
350+ int sighash_type = 1 /* SIGHASH_ALL */,
351+ bool sign = true,
352+ bool bip32derivs = false) override
353+ {
354+ return FillPSBT(m_wallet.get(), psbtx, complete, sighash_type, sign, bip32derivs);
d474156f242dc900c221924765210ac4f7e97f7a
Wrong indentation.
Mmm, this in an odd Travis error:
02019-11-06T17:49:26.056000Z TestFramework (INFO): sendrawtransaction with missing input
1198162019-11-06T17:49:42.869000Z TestFramework (ERROR): JSONRPC error
219817Traceback (most recent call last):
319818 File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 111, in main
419819 self.run_test()
519820 File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-i686-pc-linux-gnu/test/functional/rpc_rawtransaction.py", line 354, in run_test
619821 self.nodes[2].sendrawtransaction(rawTxComb)
719822 File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/coverage.py", line 47, in __call__
819823 return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
919824 File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/authproxy.py", line 141, in __call__
1019825 raise JSONRPCException(response['error'], status)
1119826test_framework.authproxy.JSONRPCException: non-mandatory-script-verify-flag (Signature must be zero for failed CHECK(MULTI)SIG operation) (-26)
186@@ -186,6 +187,13 @@ void SendCoinsDialog::setModel(WalletModel *_model)
187 // set default rbf checkbox state
188 ui->optInRBF->setCheckState(Qt::Checked);
189
190+ if (model->privateKeysDisabled()) {
191+ ui->sendButton->setText(tr("Create Unsigned"));
192+ ui->sendButton->setToolTip(tr("Creates a Partially Signed Bitcoin Transaction (PSBT) for use with e.g. an offline Bitcoin Core wallet, or a PSBT-compatible hardware wallet."));
PACKAGE_NAME
186@@ -186,6 +187,13 @@ void SendCoinsDialog::setModel(WalletModel *_model)
187 // set default rbf checkbox state
188 ui->optInRBF->setCheckState(Qt::Checked);
189
190+ if (model->privateKeysDisabled()) {
191+ ui->sendButton->setText(tr("Create Unsigned"));
192+ ui->sendButton->setToolTip(tr("Creates a Partially Signed Bitcoin Transaction (PSBT) for use with e.g. an offline Bitcoin Core wallet, or a PSBT-compatible hardware wallet."));
193+ } else {
194+ ui->sendButton->setText(tr("Send"));
SendCoinsDialog, and can’t change privateKeysDisabled AFAIK…
setModel(WalletModel *_model). That’s the first time we know privateKeysDisabled. The constructor doesn’t know.
s keyboard shortcut back, though I can’t test that on macOS.
.ui file).
312@@ -305,9 +313,19 @@ void SendCoinsDialog::on_sendButton_clicked()
313 formatted.append(recipientElement);
314 }
315
316- QString questionString = tr("Are you sure you want to send?");
317+ QString questionString;
318+ if (!model->privateKeysDisabled()) {
324 questionString.append("<br /><span style='font-size:10pt;'>");
325- questionString.append(tr("Please, review your transaction."));
326+ if (!model->privateKeysDisabled()) {
327+ questionString.append(tr("Please, review your transaction."));
328+ } else {
329+ questionString.append(tr("Please, review your transaction proposal. This will produce a Partially Signed Bitcoin Transaction (PSBT) which you can copy and then sign with e.g. an offline Bitcoin Core wallet, or a PSBT-compatible hardware wallet."));
949@@ -908,11 +950,11 @@ void SendConfirmationDialog::updateYesButton()
950 if(secDelay > 0)
951 {
952 yesButton->setEnabled(false);
953- yesButton->setText(tr("Send") + " (" + QString::number(secDelay) + ")");
954+ yesButton->setText(confirmButtonText + " (" + QString::number(secDelay) + ")");
This makes them available in GUI coin selection.
For wallets with WALLET_FLAG_DISABLE_PRIVATE_KEYS.
2159@@ -2160,7 +2160,7 @@ std::map<CTxDestination, std::vector<COutput>> CWallet::ListCoins(interfaces::Ch
2160
2161 for (const COutput& coin : availableCoins) {
2162 CTxDestination address;
2163- if (coin.fSpendable &&
2164+ if ((coin.fSpendable || (IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && coin.fSolvable)) &&
const bool include_watch_only to check the wallet flag once outside the loops…
Something like this above:
0 // Include watch-only for wallets without private keys
1 const bool include_watch_only = IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS);
2 const isminetype is_mine_filter = include_watch_only ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE;
186@@ -186,6 +187,14 @@ void SendCoinsDialog::setModel(WalletModel *_model)
187 // set default rbf checkbox state
188 ui->optInRBF->setCheckState(Qt::Checked);
189
190+ if (model->privateKeysDisabled()) {
191+ ui->sendButton->setText(tr("&Create Unsigned"));
192+ ui->sendButton->setToolTip(tr("Creates a Partially Signed Bitcoin Transaction (PSBT) for use with e.g. an offline %1 wallet, or a PSBT-compatible hardware wallet.").arg(PACKAGE_NAME));
193+ } else {
194+ ui->sendButton->setText(tr("&Send"));
"S&end"
186@@ -186,6 +187,11 @@ void SendCoinsDialog::setModel(WalletModel *_model)
187 // set default rbf checkbox state
188 ui->optInRBF->setCheckState(Qt::Checked);
189
190+ if (model->privateKeysDisabled()) {
191+ ui->sendButton->setText(tr("C&reate Unsigned"));
"Cr&eate Unsigned" so the “E” shortcut matches with "S&end"
I’ve used the “Use available balance” which filled the amount, but below balance is still zero.
Something that can be fixed in a follow up - the button could change and/or the balance info on the bottom too.
Tested ACK c6dd565c8820aa8a98b190621e10c6e2821a9ecc.