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

getTransaction bug in for confirmation() in watcher context. #153

Closed gabidi closed 4 years ago

gabidi commented 4 years ago

When a transaction received for a watching wallet, cyphernode's get_transaction will attempt to first locate the transaction in spender wallet, then iterate over watcher wallets. This results in a series of JSON objects:

2019-12-12T23:57:01+0000 1777 [confirmation] tx_details={"result":null,"error":{"code":-5,"message":"Invalid or non-wallet transaction
id"},"id":null}                              
{"result":{"amount":0.00100000,"confirmations":0,...l}

This creates an issue when the transaction is being parsed assuming only one JSON object has been returned, which results in the failure of a watching wallet's txn to be inserted into the watching_tx table:

2019-12-12T23:59:22+0000 2061 [confirmation] fees=0.00000145 
sh: 0                                                               
0: bad number                                                                                                                          
2019-12-12T23:59:22+0000 2061 sqlite3 -cmd ".timeout 20000" /proxy/db/proxydb "INSERT OR IGNORE INTO tx (txid, hash, confirmations, tim
ereceived, fee, size, vsize, is_replaceable, blockhash, blockheight, blocktime, raw_tx) VALUES ("4a10e28a2bb4b5b9dda2b038aec8357733d506
8171bbcf254c4274f81cc9e3c8", "98484b169015889c7a940a4af266ad888cde87f7e54398a1ce05b7423cf5615b", 0 
0, null                                                             
1576195162, 0.00000145, 226, 145, 0, null, null, null, readfile('rawtx-4a10e28a2bb4b5b9dda2b038aec8357733d5068171bbcf254c4274f81cc9e3c8
.blob'))"                                                                                                                              
Error: near "0": syntax error     

Issue has already been fixed in PR https://github.com/SatoshiPortal/cyphernode/pull/150 This issue is opened for commit reference

Kexkey commented 4 years ago

I think that all this is caused by one small addition you made in the code. Here are some details:

Let me know what you think! Thanks!

gabidi commented 4 years ago

Hey @Kexkey , Thanks for taking the time to reply. Please excuse me for not having supplied detailed information and written up a proper ticket about the issue. You are right , the issue has nothing to do with Spending wallet but with watching wallets. I have no idea why i said spending in that ticket. If you would be kind enough to allow me to provide more detail and context to the issue:

So assuming the context of a transaction comes in where we have multiple watcher wallets and one of them is concerned with the txn. confirmation.sh will eventually call get_transaction which will call send_to_watcher_node: https://github.com/SatoshiPortal/cyphernode/blob/760c9954cacb7e8f6c5215327c2e82185f9a92d3/proxy_docker/app/script/sendtobitcoinnode.sh#L5

  1. send_to_watcher_node will First try the default watcher wallet by calling: https://github.com/SatoshiPortal/cyphernode/blob/760c9954cacb7e8f6c5215327c2e82185f9a92d3/proxy_docker/app/script/sendtobitcoinnode.sh#L7

  2. send_to_bitcoin_node will echo it's result, meaning: a. it finds the txn within the first watcher wallet attempted return code will ne 0 and we go back up the stack (get_transaction <-send_to_watcher_node <- sendtobitcoinnode) with only ONE json output, everything works great. b. However if the txn is not found in the default watcher wallet, send_to_bitcoin_node will output the usual not found :

    {"result":null,"error":{"code":-5,"message":"Invalid or non-wallet transaction
    id"},"id":null}

    but then send_to_watcher_node will detect a return code ne 0 and call https://github.com/SatoshiPortal/cyphernode/blob/760c9954cacb7e8f6c5215327c2e82185f9a92d3/proxy_docker/app/script/sendtobitcoinnode.sh#L14 send_to_watcher_node_wallet , which will call send_to_bitcoin_node, which will output another json object with the txn if found or not, and thus we get the TWO json objects send back to the get_transaction call , described in this issue.

    {"result":null,"error":{"code":-5,"message":"Invalid or non-wallet transaction
    id"},"id":null}                              
    {"result":{"amount":0.00100000,"confirmations":0,...etc..}
  3. Now since tx_details has two JSON objects (one on each line) https://github.com/SatoshiPortal/cyphernode/blob/43ddfc4acb0b63aa03e9716b6a1b19b75f5c8f3a/proxy_docker/app/script/confirmation.sh#L35 all functions down stream expect , as you aptly put it, only one object. So for example: https://github.com/SatoshiPortal/cyphernode/blob/43ddfc4acb0b63aa03e9716b6a1b19b75f5c8f3a/proxy_docker/app/script/confirmation.sh#L73

given the output returned by get_transaction call in this issue (two objects, first being not found result , the second being found) will result in an output of

null\n
0

Since the first JSON object has nothing in result and the second does indeed have 0 in the .result.confirmations field. Even if we do add a jq -rc -c flag for compact the output will be 'null0'

This will cause a bunch of insert SQL statements to fail down stream and the rest is history.

Thus adding https://github.com/SatoshiPortal/cyphernode/commit/43ddfc4acb0b63aa03e9716b6a1b19b75f5c8f3a#diff-1efdbdd6c16c9f9d14a6c0441e0a7702R35 Make sure to only take the correct output the rest of the script is expecting.

I hope i didn't cause even more confusion lol.

You are 100% right about breaking the return code by adding JQ and will push a commit that moves the jq select statement to after the return code of get_transaction is checked. Just want to make sure i'm completely wrong here.

Let me know what you think about the above. Thank you so much again ! G

Kexkey commented 4 years ago

Ok I got it! We should capture the output of send_to_bitcoin_node in a variable (inside send_to_watcher_node) instead of displaying it. If returncode is 0, then we can echo it.

Thanks so much!

Kexkey commented 4 years ago

Was this fixed with PR#150 ?

gabidi commented 4 years ago

Yes sir. I'll close the issue.