Watch-Only Addresses: Separating Transaction Signing From Transaction Processing

CodeShark commented at 7:46 am on December 21, 2012: contributor

A major issue many have faced in using bitcoind to build applications is the lack of RPC support for tracking transactions and balances without having to know and keep associated private keys in the wallet. Oftentimes one might want to watch other people’s transactions or to keep signing nodes behind tighter security while using separate relay nodes to service all nonsigning, non-key-generating application functionality such as sending payment and confirmation alerts.

In order to achieve these objectives, I have had to build a bunch of custom software - much of which duplicates functionality that is already present in the Satoshi Client. This pull request is an attempt at addressing some of these concerns without having to fundamentally restructure the client architecture. (Thanks, sipa!)

The proposal is to add another kind of object to the wallet database - a bitcoin address sans private key - which the client treats as if it were any other wallet adddress except for when it comes to signing and privkey export operations. This means RPC calls such as getreceivedbyaddress and listtransactions can be used on arbitrary bitcoin addresses.

I’ve added an RPC call:

0importaddress <bitcoinaddress> [label] [rescan=true]

The address is added as a new type of serialized object in wallet.dat and loads into the key maps of the CKeyStore instances with the key set to the CKeyID and the secret set to an empty vector.

Please test it out and let me know what you think.

Cheers,

-Eric Lombrozo

CodeShark commented at 7:59 am on December 21, 2012: contributor

Sorry, guys - I changed the branch names and it closed the original pull request #2117

The pull request remains open but under a new name.

CodeShark commented at 5:58 am on December 24, 2012: contributor

A multiple wallet version of bitcoind is underway: #2124

That branch will eventually be merged with this one. In order to distinguish spendable wallets from watch-only wallets, a new member will be added to the CWallet class as well as a new corresponding value to the database file. The CWallet methods as well as RPC calls will then be able to distinguish between the two cases and exhibit the correct behavior.

mikegogulski commented at 0:15 am on December 25, 2012: none

OMG this is great!

BitcoinPullTester commented at 6:11 am on January 24, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/89f806c1d5e171ab48b29eeea8937fb2078e917a for binaries and test log.

jgarzik commented at 6:51 pm on April 8, 2013: contributor

Neat. I’m surprised it was this easy.

Separating Transaction Signing From Transaction Processing

==========================================================

A major issue many have faced in using bitcoind to build applications is the lack of RPC support for tracking transactions and balances without having to know and keep associated private keys in the wallet. Oftentimes I want to watch other people's transactions or to keep my signing nodes behind tighter security while using separate relay nodes to service all nonsigning, non-key-generating application functionality such as sending payment and confirmation alerts.

In order to achieve these objectives, I have had to build a bunch of custom software - much of which duplicates functionality that is already present in the Satoshi Client. This fork is an attempt at addressing some of these concerns without having to fundamentally restructure the client architecture. (Thanks, sipa!)

The proposal is to add another kind of object to the wallet database - a bitcoin address sans private key - which the client treats as if it were any other wallet adddress except for when it comes to signing and privkey export operations. This means RPC calls such as getreceivedbyaddress and listtransactions can be used on arbitrary bitcoin addresses.

I've added an RPC call:

	importaddress <bitcoinaddress> [label] [rescan=true]

The address is added as a new type of serialized object in wallet.dat and loads into the key maps of the CKeyStore instances with the key set to the CKeyID and the secret set to an empty vector.

Please test it out and let me know what you think.

Cheers,

-Eric Lombrozo

007a283741

sipa commented at 5:05 pm on April 27, 2013: member

Just a remark so we don’t forget (I haven’t checked whether it’s possible with this implementation): it’d be nice to have a way to include arbitrary P2SH addresses as watch-only

namecoin-qt referenced this in commit 79f84511db on May 31, 2013

jgarzik commented at 6:50 pm on June 19, 2013: contributor

Looks pretty good. Minor nit: “label” is deprecated in favor of “account.” The help text must be changed. And if you are going to change the help text, go ahead and name the var strAccount as other, new code does.

Would like to see this merged.

sipa commented at 7:17 pm on June 19, 2013: member

Yes, please rebase.

Regarding labels vs. accounts - I consider them separate things, which incidentally (and regrettably) overlap (if an address A has label L, payments to A will credit the account also called L). In this case, similarity with the existing API (importprivkey) is more important in any case, so I prefer seeing it called label.

ryny24 commented at 0:26 am on June 27, 2013: none

is it possible to download and test this modification? I’d love to try it. Can’t seem to select this branch

kyledrake commented at 3:32 pm on July 10, 2013: none

I would love to see this rebased and merged! I’m going to be using this functionality for an upcoming new version of Coinpunk, to support client-side encryption for private keys, so that bitcoind becomes a listener, and cannot spend any of the user’s money. This is a huge linchpin in my new version, and it enables this capability for people developing web services and offline clients that use a central bitcoind instance to keep up with the blockchain while they are offline. And of course cold storage wallets that generate the private keys on their own end.

I won’t be able to release the new Coinpunk until this is released, so it would be amazing if this commit could get into the next release. I don’t think it’s too dangerous, just because I believe it’s not handling anything that could potentially lose bitcoins, but I could be mistaken. But it would be awesome.

Let me know if I can help in any way. Thanks, you guys are awesome. :-)

kiaya commented at 7:15 am on July 11, 2013: none

I’d also love to see this make the next release. I have a project where I need to watch a large number of addresses without having the private keys on the server. I think the pull request would solve my biggest current development challenge.

tstranex commented at 7:32 am on July 11, 2013: none

Just thinking about an alternative to this: Set a wallet passphrase so that all the private keys are encrypted and then write a separate utility to overwrite the encrypted private keys with zeros in the wallet file. Since bitcoind is not going to access the keys anyway while they’re encrypted, it shouldn’t matter that they’re actually zeros. So that will effectively produce watch-only addresses. However, it still wouldn’t be possible to add new watch-only addresses via rpc, which this patch can do.

By the way, could a slightly cleaner implementation of this be to store watch-only addresses as normal keys (except where the encrypted private key is cleared) instead of adding the new “address” data type in the walletdb?

kiaya commented at 7:40 am on July 11, 2013: none

Thanks for this, tstranex. I hadn’t considered that. I might look into this as a fall back. In my case, I’m using vanity addresses so actually don’t need to create addresses via RPC.

luke-jr commented at 3:50 am on July 17, 2013: member

@CodeShark Needs rebase.

Although I don’t think this makes sense to merge until we have HD wallets and it can be an entire read-only address-chain…

kyledrake commented at 6:41 am on July 17, 2013: none

I compiled and tested this code, and it seems to work fine. Does it collide with HD wallets? If not, I would really like to advocate for getting this in. I strongly believe that this code solves a critically urgent security problem.

Without this code, I’m concerned that we may get more services that put private keys on servers simply because they have no choice, which could lead to more major thefts, which could contribute to undermining the soundness of the Bitcoin ecosystem. The last thing Bitcoin needs right now is another high profile wallet theft fiasco.

This is a very important and very significant security upgrade, that I believe will dramatically lead to the improvement of security across the board, for many different types of applications. To provide analogies, this could be as important to bitcoind security as ProPolice was to buffer overflow prevention, or PBKDF2 was to password hashing.

Please consider including this in the next release of bitcoind. Do it for the people that have lost a lot of money to server wallet thefts. Do it for the children.

:wink:

luke-jr commented at 6:50 am on July 17, 2013: member

@kyledrake HD wallets should solve all of those issues, that’s my point ;)

kyledrake commented at 2:12 pm on July 18, 2013: none

@luke-jr I’m cool with HD Wallets, but HD Wallets are a long way away. This is a light, simple patch to solve a very big problem, that enables some major new infrastructure for bitcoin. I tried to rebase it and I got very close, I think someone with knowledge of the bitcoin source would very easily be able to rebase this. The biggest hurdle was the @sipa (I think?) refactor of keystore.cpp. The rest was just really simple merges.

kyledrake commented at 2:25 pm on July 18, 2013: none

So here is my rebase attempt:

https://github.com/kyledrake/bitcoin/commit/317183993364feaf9833446612f91d776c244698

It fails to compile on this:

0llvm-g++ -c -Wall -Wextra -Wformat -Wformat-security -Wno-unused-parameter -g -DMAC_OSX -DMSG_NOSIGNAL=0 -DBOOST_SPIRIT_THREADSAFE -DUSE_UPNP=1 -DUSE_IPV6=1 -I/Users/kyledrake/bitcoin/src/leveldb/include -I/Users/kyledrake/bitcoin/src/leveldb/helpers -DHAVE_BUILD_INFO -I"/Users/kyledrake/bitcoin/src" -I"/Users/kyledrake/bitcoin/src"/obj -I"/opt/local/include" -I"/opt/local/include/db48" -MMD -MF obj/keystore.d -o obj/keystore.o keystore.cpp
1keystore.cpp: In member function ‘bool CBasicKeyStore::AddKey(const CKeyID&)’:
2keystore.cpp:34: error: ‘CSecret’ was not declared in this scope
3keystore.cpp:34: error: ‘make_pair’ was not declared in this scope
4make: *** [obj/keystore.o] Error 1

Apparently CSecret has gone away.. any ideas on how to fix this?

luke-jr commented at 2:49 pm on July 18, 2013: member

I think CSecret became CKey (and the old CKey become CPubKey).

HD wallets IIRC are planned for the next release.

sipa commented at 2:53 pm on July 18, 2013: member

CKey and CSecret were merged. CPubKey is still CPubKey but inherited some of the pubkey-only functionality of the former CKey.

I consider this a very useful feature, and it’s orthogonal to HD wallets. Sure, BIP32 specifies a derivation that can be useful for watch-only wallets, but it still needs to be implemented.

kyledrake commented at 3:10 pm on July 18, 2013: none

@sipa Do you have any free time this week to look at this? I think it’s a really super easy fix for you because you are very familiar with the code (and awesome). I think I got pretty close, I just don’t know any C++ and I’m completely unfamiliar with the bitcoin keystore plumbing. http://i.imgur.com/xVyoSl.jpg

runeksvendsen commented at 3:55 pm on July 18, 2013: contributor

FYI this patch compiles successfully against ec0004aca0a2bf11f99c9587ddb2bf8ea818d3bb.

I’ve tested importaddress with listtransactions and getbalance and it agrees with blockchain.info.

BitcoinPullTester commented at 11:21 am on July 20, 2013: none

Automatic sanity-testing: FAILED MERGE, see http://jenkins.bluematt.me/pull-tester/007a28374165f67d713a318bc37ef1286684cad5 for test log.

This pull does not merge cleanly onto current master 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.

kyledrake commented at 3:40 pm on July 25, 2013: none

For those of us in the bitcoind camp using an old unstable master so we can get importaddress, I have merged importaddress with @runeksvendsen’s hint of ec0004a here: https://github.com/kyledrake/bitcoin/tree/importaddressupdate

Since I posted on this pull request, I have gotten emails from people using this commit in production, because there is no other alternative. Which makes me excited about its potential to improve bitcoin security, but also terrified because they are using it with unstable bitcoind. It would be really great if someone was able to look at this, I really doubt it would take more than a few hours for a seasoned member of the glorious, highly talented bitcoin development team to resolve the merge conflicts on it (@sipa? @jgarzik?).

sipa commented at 11:12 pm on July 25, 2013: member

@kyledrake Have a look at #2861.

meighti commented at 2:38 pm on July 27, 2013: none

It would be nice to add a feature in the code to easily or automatically LOCKUNSPENT all unspent coins of the imported watch-only address. Not to mention that those coins are not spendable without privkey.

In shell I run this: bitcoind lockunspent false $(bitcoind listunspent 0 9999999 ‘[“12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX”]’ |tr -d ’ \n’)

jgarzik commented at 2:08 pm on July 30, 2013: contributor

Superceded by #2861, closing.

jgarzik closed this on Jul 30, 2013

davispuh referenced this in commit 6e0f77e982 on Feb 18, 2014

cyssxt commented at 7:03 am on March 20, 2018: none

Perfect!

HashUnlimited referenced this in commit 0548008a14 on Jul 2, 2018

HashUnlimited referenced this in commit d69403072a on Jul 2, 2018

HashUnlimited referenced this in commit d2c844e038 on Jul 2, 2018

HashUnlimited referenced this in commit 6f75b42788 on Jul 2, 2018

HashUnlimited referenced this in commit 9490d6cbf5 on Jul 3, 2018

sidhujag referenced this in commit fbb08f0e2c on Jul 6, 2018

owlhooter referenced this in commit b98643c27d on Oct 11, 2018

DrahtBot locked this on Sep 8, 2021

Watch-Only Addresses: Separating Transaction Signing From Transaction Processing #2121