This is the first step to achieve what is described on #9728 (comment)

Ping @instagibbs

Will review.

Concept ACK, needs rebase

Concept NACK, I think this is a step in the wrong direction. The concept of watching should be removed from the keystore entirely.

@sipa what do you mean? this is precisely the intent of this PR: Removing WatchOnlyStore from CKeyStore. (see the CWatchOnlyStore class)

@NicolasDorier There's no need for a separate store for watch-only, watching should be determined by the wallet, not the keys or anything related to signing.

I'm working on a design doc for how to evolve the watching/signing code and segwit/HD/... support. I'll let you know when it's in a usable state.

I'm working on a design doc for how to evolve the watching/signing code and segwit/HD/... support. I'll let you know when it's in a usable state.

Doc was posted as https://gist.github.com/sipa/125cfa1615946d0c3f3eec2ad7f250a2#future-design

It does make this PR look like a step in a different direction if the goal is to have unified representation and handling of watchonly and signing keys.

It also isn't obvious to me how this PR would simplify https://github.com/instagibbs/bitcoin/commits/externalhd2 or #9728. @NicolasDorier it would be good if you could explain more why this change is useful, and whether you think anything in sipa's design might make it more difficult to support external hd wallets.

I've implemented hww support, and I believe the design document posted goes more in the right direction to make it sane and palatable for upstream. Obviously it will be a much larger project...

@ryanofsky this PR was an attempt to fix my immediate problem on externalhd2. I was trying to fix the mess incrementally, one PR after another.

The design doc has a different approach: Complete redesign + one time migration. I agree with the general direction it takes (I need to re read it later though), it just seems to me it will be quite hard to do incrementally.

@ryanofsky Just additional information about what I was thinking. This is long time ago so I might not remember all details...

Currently externalhd2 works fine but generate the address as "watch-only".
The problem is that core consider an address "IS_MINE" only if you can sign. (which is not the case for addresses generated by external hd)

But nevertheless, the watchonly addresses generated by externalhd2 should be considered IS_MINE even if I can't sign. Because I want Core to behave exactly as if it was a normal wallet, except without it the ability to sign with the generated keys. (Another PR related to that that I had was separating IS_MINE from IS_SIGNABLE)

My solution was to create a new implementation of CKeyStore for external hd keys.
The old CKeyStore would still be used in non-external hd mode.
And I would keep the WatchOnlyStore separately from those classes. (As the WatchOnlyStore still need to exist independently from the CKeyStore implementation use)

Because generated addresses would come from the CKeyStore, it would be considered "IS_MINE".

This PR was the first step: Separating CWatchStore from CKeyStore.

I was attempting to slowly refactor the current flawed design until it get better. The design document though takes the bulldozer approach. (which might be OK, just harder)

closing this, my approach of slowly refactoring does not seem to get traction.