SatoshiPortal / cyphernode

Modular Bitcoin full-node microservices API server architecture and utilities toolkit to build scalable, secure and featureful apps and services without trusted third parties
MIT License
363 stars 68 forks source link

watchxpub error v0.4.0 #193

Open Tomtibo opened 4 years ago

Tomtibo commented 4 years ago

Since v0.4.0, a wathxpub call return error "Can't insert xpub watcher in DB".The watch call is ok, only with watchxpub. The return code of the DB request( watchrequest.sh line 184) is : Error: ON CONFLICT clause does not match any PRIMARY KEY or UNIQUE constraint This happen to me on an updated CN instance, and on a fresh CN install.

Kexkey commented 4 years ago

Thanks for the report! I will fix that, I have a good idea what the problem is.

Are you using the label arg in your call? If not, in the meantime, can you try something? Remove , label on line 184 in the ON CONFLICT clause. The line would look like this:

sql "INSERT INTO watching_by_pub32 (pub32, label, derivation_path, watching, callback0conf, callback1conf, last_imported_n) VALUES (\"${pub32}\", \"${label}\", \"${path}\", 1, ${cb0conf_url}, ${cb1conf_url}, ${last_n}) ON CONFLICT(pub32) DO UPDATE SET watching=1, callback0conf=${cb0conf_url}, callback1conf=${cb1conf_url}"

If this happened, it's because you called watchxpub more than once on the same xpub or with the same label. If you don't use labels, you can remove it from the CONFLICT clause. I will find a better fix for the next release.

Thanks!

Tomtibo commented 4 years ago

Calls without label, or with an others xpub, or with label removed from ON CONFLICT, return the same error. Files authorisation and ownership look fine. BTW, as a suggestion, the watchxpub call could be way much powerfull, if it behave as follow:

xpub1 + label1 + derivPath1(receiving) and derivPath2(Change) | (2 wachxpub request with same xpub1 and label1 but different derivpath)

xpub1 + label2 + derivPath3(nstart=0) and devivPath3(nstart=100) | (2 wachxpub request with same xpub1 and label2 and derivpath but different nstart)

xpub1 + label3 + derivPath5 | (1 watchxpub same xpub1 but with an other label and different derivPath)

xpub2 + label4 + derivPath1

xpub2 + label5 + derivPath2

etc...

Then it would be easy to track wallets balance and tx:

getbalancebyxpublabel label1

getbalancebyxpub xpub1

get_txns_by_watchlabel label2

Also, the unwatchxpubbylabel label4 should not unwatch label5 address because of the shared xpub2 (curent behavior i think), but using unwatchxpubbyxpub xpub2 remove label4 and label5 watchs. I see no downside of implementing this, beside the codework of doing so. I've no idea if it can be implemnent or how to implement this but it would be a nice improvement! Sorry, I've just receive my coinkite stuff, so I derive like a crazy man! hahaha

Tomtibo commented 4 years ago

Hey Kexkey, In fact you were absolutly right, removing label from ON CONFLICT work! I was changing code in the wrong place, Sorry for that! Thanks!

Kexkey commented 3 years ago

Hi @Tomtibo !

I pushed improvements and fixes for the xpub watching features. Can you try it out?

https://github.com/SatoshiPortal/cyphernode/tree/bugfixes/watchxpubconflict

Do you use the webhooks?

Thanks!

@gabidi if you could try that branch with your project it would be great!

Tomtibo commented 3 years ago

Hi @Kexkey ! I've tried your changes, and the conflict error is gone. I've also tried callback, inspected proxyDB and all look fine! What do you mean by webhooks? I use libcn CallbackServer class to handle callback, I suppose this is my webhook. Anyway, the bug look resolved. The behavior described in my previous post was a little off topic since is more a features improvement than a bug. I think the branch can be merged and issue closed. Will try to better describe the behavior improvement in an other issue. Thanks!

Kexkey commented 3 years ago

Yes I focused on fixing the bug and I noticed some other problems in doing so that I also fixed. Your suggestions are very welcome though, it would be cool to open another issue with them. Specify "Feature Request" in the title/description.

Thanks for the bug report and for testing my patch, very appreciated. The fix will be part of the v0.5.0 release that I'm working on (as well as enhanced batching capabilities).

Kexkey commented 3 years ago

It definitely makes sense to have more flexible watching keys, of course for the xpub watcher but also it is possible to have multiple applications requesting a watch on the same address or txid but they would have different webhook Urls...

We in fact have this specific case at Bull Bitcoin and I need to work around that with hacks but I have to think about a permanent solution in Cyphernode.

Kexkey commented 2 years ago

Hey @Tomtibo ! Just to let you know that I'm currently improving the watchxpub functionality to support watching the same xpub multiple times with different derivation paths and callback URLs. The simplest way to make it work is to make label unique and to remove the unique constraint on pub32. So the driver here will be the label.

Also, if you call unwatchxpubbylabel, it will only unwatch the given label. But when calling unwatchxpubbyxpub, it will unwatch everything with that xpub.

I think this covers what you were suggesting above. Several cypherapps watching the same xpub will be able to coexist without interference. What do you think?