Note: untested for now.

THANK YOU SO MUCH! @sipa you are my hero (and probably a bunch of other people's too)!

I will pull this and test it tomorrow, and try to do a more extensive test this weekend. I'll report back if I find any issues.

Again, you are awesome, and I think this is hugely important for bitcoin. Thank you, thank you, thank you.

@sipa is there any chance this could make it into master for the next release? I'm not sure if it's out of scope for the next release or not, just curious on timing so I can plan accordingly. I would love to see in the next release of course, but I don't want to derail all the other important stuff you guys are working on.

Thanks for giving this pull request some love, sipa!

I think having both spendable and watch-only accounts in the same wallet is problematic. For instance, how do we deal with getbalance, account labels, etc...?

I had delayed attempting this merge until multiwallet capability had been merged, where we could simply have a wallet be entirely watch-only or entirely spendable. I'm glad some people are finding it useful as it is, though.

@CodeShark The approach that this pullreq takes is that for almost everything, watch-only is considered equal to normal keys. You'll see it in getbalance, in listtransactions, in listunspent, ...

However, these are not considered when creating a transaction. Presumably we'll need a getspendablebalance too, and some GUI special-casing (perhaps only shown when watch-only addresses are present at all).

Thanks @sipa and others who have contributed to this! I am so insanely happy to see this functionality get added.

I have tested this patch by importing an address withimportaddress, and getbalance, listtransactions, getreceivedbyaddress, listreceivedbyaddress work as expected (and they agree with blockchain.info).

sendtoaddress from a wallet that contains only a single watch-only address fails gracefully with:

error: {"code":-4,"message":"Signing transaction failed"}

One interesting thing to test would be having a mixed wallet, and checking that you can spend the amount in normal keys, but not more.

Summarizing IRC review comments:

• AvailableCoins() needs an additional arg. It now takes two optional bool args.
• Would be nice if 'listunspent' indicated watch-only or not
• Agree that testing a mixed wallet would be useful

ACK once issues are fixed

I just tested sending transactions on testnet. Sending amounts higher than the balance available for spending (from non-watch-only addresses) results in a "Signing transaction failed" error.

mq7se9wy2egettFxPbmn99cK8v5AFq55Lx is the watch-only address and mv7VTUiBNmNSNrWaXrySbZcEzQ9C6ZNTNu I have the private key for:

rune@rune-desktop:~/Programming/bitcoin-sipa/src$ ./bitcoind listunspent
[
    {
        "txid" : "98278b9eb6cde5d3124ffc6c0ff75fa1d813b41383c35414315ce47f85fc45cc",
        "vout" : 1,
        "address" : "mv7VTUiBNmNSNrWaXrySbZcEzQ9C6ZNTNu",
        "scriptPubKey" : "76a914a019eab435cd509dce95353febe6b48e7b15075388ac",
        "amount" : 0.39800000,
        "confirmations" : 5
    },
    {
        "txid" : "bd9a149d58d9d2eff00e9bb1c0f463a2ce213f99affa8261bdbe8e85d60f5158",
        "vout" : 1,
        "address" : "mq7se9wy2egettFxPbmn99cK8v5AFq55Lx",
        "account" : "",
        "scriptPubKey" : "76a9146953ce65058e5e68125a9163d74b277d6a7f4a9e88ac",
        "amount" : 23.49990000,
        "confirmations" : 101
    }
]
rune@rune-desktop:~/Programming/bitcoin-sipa/src$ ./bitcoind sendtoaddress mq7se9wy2egettFxPbmn99cK8v5AFq55Lx 0.4
error: {"code":-4,"message":"Signing transaction failed"}
rune@rune-desktop:~/Programming/bitcoin-sipa/src$ ./bitcoind sendtoaddress mq7se9wy2egettFxPbmn99cK8v5AFq55Lx 0.398
error: {"code":-4,"message":"Signing transaction failed"}
rune@rune-desktop:~/Programming/bitcoin-sipa/src$ ./bitcoind sendtoaddress mq7se9wy2egettFxPbmn99cK8v5AFq55Lx 0.397
14ce96d4809b18a7a7bc7f6501801572d3e673f6eb2ea645ad731a056e953221
rune@rune-desktop:~/Programming/bitcoin-sipa/src$ ./bitcoind getbalance 
33.69660000

Significant changes, re-testing would be welcome :)

Instead of fRequireSpendable, the IsMine family of functions now return MINE_NO, MINE_WATCH_ONLY or MINE_SPENDABLE (instead of a boolean). listunspend should show whether a coin is spendable.

Overall ACK of updated code. Did not test, just code review. Minor taste-based nits:

• RPC: 'true' seems a better return value than 'null'
• perhaps due to my b/g I prefer the "_t" suffix to the more verbose "type". POSIX standard naming for such situations.

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/02e10bf92bcb693288740ff7af0422d4fa0572d8 for binaries and test log.
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

Some changes:

• Fixed a bug that caused rescans to sometimes be skipped.
• Adapt the output of validateaddress for watch-only addresses.

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/80f264464ff2f424cc65e6c910411b7ba059f893 for binaries and test log.
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

I just tested the new changes and now re-scanning works both on testnet and mainnet for me. I can send and receive on testnet, and I get a

error: {"code":-4,"message":"Insufficient funds"}

error message when I try to send more than I have in spendable funds (according to listunspent) but less than I have in unspendable funds.

On mainnet, the listransactions output agrees with blockchain.info for the tested address.

@gavinandresen You've voiced some objections to this on IRC, and prefer watch-only wallets over individual watch-only addresses IIRC, care to mention them here?

I don't feel like working on this further if it is controversial, but I think it's extremely useful and pretty much necessary to enable managing P2SH addresses without having all keys available. To get that same functionality with watch-only wallets, this becomes dependent on multiwallet, which doesn't seem to be making any progress.

IMO there is a strong user-based case for merging. There is clear user interest. People are building websites based on this branch, among other things.

I'd like to add that it increases decentralization, thus increasing the resilience of the Bitcoin ecosystem. Instead of having to rely on blockchain.info to get information about addresses not owned by myself, I can monitor these addresses using my own node. The same goes for other Bitcoin services. I suspect a lot of people depend on the blockchain.info JSON API simply because they have no other option right now. I know at least one service whose developer I spoke with using the blockchain.info API because they're the only ones offering a way to pull data about addresses for which one doesn't own the private key.

Every time blockchain.info has problems, or gets DDOSed, my service is unable to continue because I have no way of retrieving this information in a decentralized manner. It becomes a single point of failure for many of the smaller Bitcoin-related services.

I agree that separate, watch-only wallets would be preferable, but at the same time I don't see why it has to be an either-or issue.

It might be useful to have 'getinfo' or another RPC return a boolean indication of watch-only addresses in the wallet.

We'd love to have this feature. It is nonsense that we have to rely on blockexplorer.com and blockchain.info while having the entire blockchain stored on our own node.

Currently bitcoind doesn't have a friendly API for merchants and payment services providers. It is crucial to be able to handle tens of thousands of addresses for which we don't want to share the private keys with a certain copy of bitcoind.

It would be really cool to have this merged ASAP without creating dependencies on other not yet developed features.

To show how people overcomplicate this: The best would be to simply have an RPC call which queries the database for a certain address, as a one-off query. No need to add it to any wallets, manage it, etc. Just query the database for it. Make getreceivedbyaddress work with any address. That's all we need.

I really appreciate the work put into this branch, in fact we live off it for quite some time now. Would love to see this merged.

Thanks for listening!

It also seems opportune, with mentions of blockexplorer/blockchain, to plug my related pull request,

"Add unauthenticated HTTP REST interface" (req #2844 )

@sipa is the importaddress code expected to update the account balance when a transaction goes out? I just dropped a generated raw transaction onto blockchain.info's tx send with one of the unspent transactions, and it removed the unspent from listunspent, but it didn't change the balance for the account that the address is associated with.

I didn't send it through my bitcoind instance, so that may be why that's happening, I need to do another test and send through my local bitcoind. Just wanted to mention incase it was a bug:

~ $ bitcoind listunspent
[
    {
        "txid" : "d266240586fc70f4e3927f3b0a70351179a5f2dbd189aecd31907f8596f0ffc3",
        "vout" : 1,
        "address" : "134GKGyWFftj2m4ZFKsBuCbm3GgXfDmuxX",
        "account" : "RErMnwLZqmdGXIiWeJ83frHs+FG/CVCQhPXZ14USk9I=",
        "scriptPubKey" : "76a914168e48aa5551a3ce7339dd55048b976edea3687288ac",
        "amount" : 0.06000000,
        "confirmations" : 2668,
        "spendable" : false
    }
]
~ $ bitcoind getbalance
0.06000000
~ $ bitcoind getbalance RErMnwLZqmdGXIiWeJ83frHs+FG/CVCQhPXZ14USk9I=
0.12000000
~ $ bitcoind listaccounts
{
    "" : -0.06000000,
    "RErMnwLZqmdGXIiWeJ83frHs+FG/CVCQhPXZ14USk9I=" : 0.12000000
}

And the transaction that was sent: https://blockchain.info/tx/87eb6a89ad7c53bc597a9c8431a22072e83d5e4b4ecbeb071b651aa5b5e964bd?show_adv=true

@kyledrake Accounts are not addresses, and do not have anything to do with the coins available to an address. The abstraction provided by the reference client has at no point any concept of a "balance of an address", only "balance of a wallet".

Addresses are associated with accounts, and coins received by those address will credit the account. For debits, you need to explicitly use "sendfrom".

Accounts are just virtual counters, and they can even go negative. I don't see a useful way to combine them with watch-only addresses. I think in general that their use is very limited, and in most cases what people really want is multiple individual wallets.

I'll be happy with watch-only addresses in the wallet if their meaning is
clear and that meaning doesn't break people's existing code.

If I understand, as implemented their meaning is "Exactly like any other
address in your wallet (shown as part of total balance, shown in
listtransactions, ??shown in listunspent??), except you cannot spend them
without constructing a raw transaction yourself".

That breaks two assumptions people may have:

1. getbalace shows the total number of bitcoins available to spend. I
   think that is an important assumption not to break.

2. If all your receive transactions are confirmed, sum(listtransactions) ==
   getbalance '*' == sum(listunspent) .

Gavin Andresen

They are included in listtransactions, listreceivedbyaddress, and balances. They are also included in listunspent, but marked as watch-only there. Adding a flag to balance, or a separate RPC to query only spendable / watch-only addresses is probably useful.

More thoughts:

I agree that there is consensus that SOME solution is needed.

I'll (grudgingly) go along with a watch-only address breaking the "getbalance == coins you can spend" assumption, as long as that is well documented.

RE: flag to balance or separate RPC: "meh" -- if people want to keep track of watch-only balances separately, they should give them all a "watchonly" label. In fact, I'd vote that the default label for importaddress be "watchonly" (or "imported" -- don't care about the name) instead of "".

RE: documentation: How about: importaddress help says something like:

Adds an address that can be watched as if it were in your wallet, but cannot
be used to spend. Transactions to or from watch-only addresses will appear
in listtransactions and listunspent and will be counted in getbalance, but
are not spendable without the private key. Watch-only addresses may be
made spendable by importing the corresponding private key using
importprivkey.

One more random thought:

Will the typical use case be a merchant importing a bunch of addresses? If yes, maybe this should be:
importaddresses [address1,address2,...] <label> <rescan>

"six of one, half-dozen of the other" -- JSON-RPC 2.0 batches work just as well, for multiple importaddress.

@laanwj People are already relying on this interface, because they desperately needed it. Rejecting this pull request doesn't prevent that, it causes more problems. Sometimes we can't control how open source software is used. The street finds its own use for things, and the street is using watch-only addresses. We should recognize that this is happening and help people solve the most important problem with Bitcoin right now: security.

Getting the RPC interface right (I'm using it right now, and I have no changes, it's perfect for what I'm doing anyway), and splitting off the RPC are different goals, and I really don't think we should reject this pull request because of an ongoing purity debate regarding the wallet code in bitcoind. There's always warts in APIs, but I actually think the RPC interface in Bitcoin is pretty good. I've worked with HTTP APIs for years, and I would say it's one of the best interfaces I've ever worked with (The Facebook API was a nightmare in comparison). @gavinandresen's improvements to raw transaction were well done and very helpful.

I agree with you that splitting off is a good thing and I support that, but how does holding back on this pull request right now prevent you from splitting it off later? Let's not let bitcoind fall into disrepair while having this discussion. Let's make it better right now, and split off the code later. This merge is so simple it could be put in the next release of Bitcoind, but the code splitting probably needs another major version, so it's a good partition to do it this way.

Tomorrow I'm going to be at the Subway in Allentown, PA that accepts Bitcoin, and I'm going to demo the first-ever pure HTML QR-based POS transaction ever done, and it's going to be based on Coinpunk, which is based on this pull request. This is significant, because it's a mobile Bitcoin wallet that supports QR that Apple can't ban, which is another major problem we've been having. If that's not a good use case of this pull request, I don't know what is. Please help me solve these problems. I can write the code on my side, but I can't merge this code in. Throw me a bone here, I'll make it worth your time. Thank you.

@kyledrake
I think we realize how important it is. Just 1) the code is broken right now 2) it needs a fair bit more work to be acceptable in mainline.

@wtogami I appreciate the workaround and it's interesting, but if I understand it, it can't be dynamically changed efficiently without exposing private keys and restarting servers. A hacker with a packet sniffer would be able to read all of the incoming keys unless it was being transferred remotely. For my use case, the private key must never touch the server.

I can't speak for everyone, but a "fully watch-only mode where privkeys are disabled" is a perfectly fine solution for my needs and I could use that.

No disrespect intended for the great work on Coin Control, but IMHO, this pull request is more important than Coin Control. Lack of anonymity hasn't caused nearly as many problems as private keys on servers. The list of thefts from Bitcoin servers is long and terrifying to read: https://bitcointalk.org/index.php?topic=83794.0

I wonder -- would it be possible to simulate "importaddress" in an always-encrypted wallet that is guaranteed to never by decrypted? (ie, by providing fake private key data)

@laanwj Can't speak for everyone of course, but that would also work great for me! I'm totally indifferent as to the mechanism (as long as it doesn't involve a real private key). Just adding an address, and being able to optionally scan the full blockchain (so I can skip if the key is new) solves my use case. Being able to put in a fake private key with a real address wouldn't be an issue for me at all.

Uh, didn't mean to do that, wrong button

So essentially:

• Introduce a new wallet state "read only" (apart from unencrypted, encrypted, unlocked)
  • Can be switched using a "setwalletreadonly" RPC command
  • This is a one way trip!
    • Probably needs a safety catch to prevent people from doing this to their only copy of their wallet.
    • Make an automatic backup copy of the old wallet file
• importaddress command will only work in this mode.

(The reason I wonder whether it's possible to do this minimally invasive using the current "encrypted wallet" logic is that it may require the full pubkey, not just an address, and it'd not work for pay-to-P2SH or cases in which the pubkey is not known)

Making watch-only addresses in a wallet turn the wallet watch-only entirely removes the use case of implementing true multisig through it (you need to be able to watch payments to the multisig address, even though you have only keys for some). Also, wallets can already be mixed spendable and non-spendable (through lockunspent).

@sipa Okay, good point, so in that case we need separate spendable and non-spendable balances per wallet either way.

What about my previous idea of having GetBalance (as well as other API entry points) take a IsMineType parameter? This would make it possible to show to the user that they have both spendable and unspendable coins in their wallet, and how much.

@laanwj Sounds good.

Do we really need wallets that contain a mix of both multisig and non-multisig coins? Seems to me that use case would be best served by having separate wallets too?

@luke-jr I think the best user experience for multisig coins would be indeed via a separate wallet create-multsig-wallet -> enter in extended pubkeys or an extended redeemscript (embeds the pubkeys) -> use wallet. But that perhaps doesn't preclude having mutt wallets that mix types.

@gavinandresen for all the reasons mentioned above. In particular avoiding the use of blockexplorer.com and trying out coinpunk.

Sounds like a -readonlywallet mode is what is wanted, with just public keys and no private keys. Is that correct? What wallet operations is CoinPunk doing? bitcoind really isn't designed to be a generic "Tell me the confirmed/unconfirmed balance of THIS set of public keys, please" service-- it seems like that is the functionality you really want (and isn't CoinPunk switching to libbitcoin anyway???).

The problem with this pull request is the mixing keys-that-can-be-spent with keys-that-cannot-be-spent.

I like the idea of a readonly wallet. This is a mode where you don't need to spend, don't need multisig, etc., because you're running it on a server as a validating node that can be queried instead of blockexplorer/blockchain. I wouldn't mix this with the GUI and personal use cases.

"Tell me the confirmed/unconfirmed balance of THIS set of public keys, please" is the exact thing bitcoind needs right now to be able to be used in production at wallets, merchants, PSPs, etc.

libbitcoin is buggy, and so are the others. When you want to verify a transaction and want to make sure, you need your own bitcoind node which doesn't contain any private keys.

This is is THE most important functionality that holds back hundreds of not thousands of businesses from being able to safely verify transactions. Having a safe and stable workaround for this problem is a huge cost right now.

"Tell me the confirmed/unconfirmed balance of THIS set of public keys, please" is the exact thing bitcoind needs right now to be able to be used in production at wallets, merchants, PSPs, etc.

^ I agree 100%. Having to setup and run your own block explorer to find unspent outputs for addresses that are not in your wallet is very bad.

By being able to query confirmed/unconfirmed balance and find unspent outputs..etc of addresses that you do not directly own... you can then build more secure apps around bitcoin handling spending and such in the browser.. only using bitcoind to push the transactions.

I also prefer the idea of a read-only wallet.

@gavinandresen importaddress, listunspent, gettransaction, sendrawtransaction. That's it.

I have never said I will be using libbitcoin, and haven't even considered the possibility until you mentioned. Upon quick inspection, it appears to be also not designed for my use (I'm not writing an anarchist-themed C++ application), but trying to implement it via FFI might be an interesting experiment. If I did consider doing that, I would still like to design Coinpunk to be usable by both clients. There's other issues with using an alternative implementation, such as stability and blockchain fork concerns and the like.

@kyledrake It is not necessary for you use libbitcoin directly, you could use sx (https://github.com/spesmilo/sx) which is built on top of libbitcoin.

However I think these projects are not yet mature enough and Coinpunk should aim at using both in the future.

@np I conveniently happen to be in the same room as the author of libbitcoin right now, and apparently there is a 0MQ interface for working with it I will investigate.

RE this pull request, it would be great to be able to continue supporting the importaddress approach as well. I realize that bitcoind isn't perfect, but nothing will be for a long time, and it's a known quantity that I'm comfortable using until a better solution comes out in the future.

I am working on a project that requires watch only addresses, I previously got this code working perfectly using the importaddress rpc command a few months ago, but seems to fail every way I try now. I documented my steps which was really just "git checkout -b remotes/origin/importaddress" before compiling. No matter what I try now, I get method not found.

Desperately need this command to build a POS system for restaurants which are waiting on me. Cannot have private key on the terminal. Help?

EDIT: Nevermind, I think I got it. Now see importaddress in the src files. Just want to say thanks for this feature. There will soon be several restaurants accepting bitcoin without any worry that the private key can be stolen by employees/customers. To anyone needing this amazing feature, use:
"git clone -b watchonly https://github.com/sipa/bitcoin.git"

See #3383 for a rebased version

In case anyone is still looking for this functionality, here is our project that creates a REST/Websocket API for any address:
https://github.com/bitpay/insight | insight Homepage
The project also includes a blockexplorer, but that is 100% on client-side JavaScript.
The index is stored on leveldb, and the data is pulled from bitcoind's .dat files or RPC getrawtransaction calls, and updated via p2p.

Still under heavy development, but it is working. Patches are welcomed.