Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bad response to getutxout (bad sats amount) #3610

Closed
kirelagin opened this issue Mar 30, 2020 · 11 comments · Fixed by #3605
Closed

bad response to getutxout (bad sats amount) #3610

kirelagin opened this issue Mar 30, 2020 · 11 comments · Fixed by #3605

Comments

@kirelagin
Copy link

Issue and Steps to Reproduce

I have the following setup:

  • A “public” lightning node (v0.8.1) connected to public nodes with (currently) one public channel open.
  • A “private” lightning node connected (not yet) only to my public node.
  • Each lightning has its own bitcoin node (Bitcoin Core version v0.19.0.1-g1bc9988993ee84bc814e5a7f33cc90f670a19f6a).
  • All this on Bitcoin testnet3.

The setup was performed as follows:

  • Fully sync the first BTC node, start the first lightning node.
  • Connect the first lightning node to a public node, fund the channel.
  • Fully sync the second BTC node, start the second lightning node.
  • Connect the first lightning node to the second one.

Output from the second (“private”) node:

{
   "peers": [
      {
         "id": "021c38ebb30aa17901aec609e98da063b4e433bfbe45656bddb34ea688192bd734",
         "connected": true,
         "netaddr": [
            "<ip>:53160"
         ],
         "globalfeatures": "",
         "localfeatures": "02aaa2",
         "features": "02aaa2",
         "channels": []
      }
   ]
}

The problem is that after a couple of seconds the second node crashes with:

<path>/bin/../libexec/c-lightning/plugins/bcli error: bad response to getutxout (bad sats amount), response was {"jsonrpc":"2.0","id":1853,"result":{"amount":"0msat","script":"0020c07569ccab2b"} }
2020-03-30T11:27:08.482Z **BROKEN** lightningd: <path>/bin/../libexec/c-lightning/plugins/bcli error: bad response to getutxout (bad sats amount), response was {"jsonrpc":"2.0","id":1853,"result":{"amount":"0msat","script":"0020c07569ccab2b"} }
FATAL SIGNAL 6 (version v0.8.1)

The same happens if I try to connect in the opposite direction, that is from the second node to the first.

I also tried wiping the state of c-lightnind and bitcoind on the second server and resyncing from scratch, same result.

getinfo output

First lightning node:

{
   "id": "021c38ebb30aa17901aec609e98da063b4e433bfbe45656bddb34ea688192bd734",
   "alias": "STRANGESET",
   "color": "021c38",
   "num_peers": 1,
   "num_pending_channels": 0,
   "num_active_channels": 1,
   "num_inactive_channels": 0,
   "address": [],
   "binding": [
      {
         "type": "ipv4",
         "address": "0.0.0.0",
         "port": 9735
      }
   ],
   "version": "v0.8.1",
   "blockheight": 1692037,
   "network": "testnet",
   "msatoshi_fees_collected": 0,
   "fees_collected_msat": "0msat",
   "lightning-dir": "/var/lib/clightning/testnet"
}

Second lightning node:

{
   "id": "03d44f0feafb17d8ad785ea7e9a96b591bcf8a727c735d51ee16d564f779dbc7b5",
   "alias": "LOUDCALENDAR",
   "color": "03d44f",
   "num_peers": 0,
   "num_pending_channels": 0,
   "num_active_channels": 0,
   "num_inactive_channels": 0,
   "address": [],
   "binding": [
      {
         "type": "ipv4",
         "address": "0.0.0.0",
         "port": 9735
      }
   ],
   "version": "v0.8.1",
   "blockheight": 1692040,
   "network": "testnet",
   "msatoshi_fees_collected": 0,
   "fees_collected_msat": "0msat",
   "lightning-dir": "/var/lib/clightning/testnet"
}

I am still trying to figure out what exactly is going on, any pointers would be very appreciated. For now I can’t even make sense of the error message as 0msat sounds like a valid amount, although I am not really sure what format it expects.

@kirelagin
Copy link
Author

This is the request that causes it to crash:

$ bitcoin-cli gettxout "9112fdaee5566f27343b466b868041e33c539dbac8ee9e4e50b6af7bc60d255e" 0
{
  "bestblock": "0000000000000e961f7fe3f8100282a7117285124a67c40977d08e3619ee8e24",
  "confirmations": 399396,
  "value": 0.00000000,
  "scriptPubKey": {
    "asm": "0 5e3fdfa2ac033fb24c31ae0fd92a6bdfe313bb7c687f4d25f58a7581bb328128",
    "hex": "00205e3fdfa2ac033fb24c31ae0fd92a6bdfe313bb7c687f4d25f58a7581bb328128",
    "reqSigs": 1,
    "type": "witness_v0_scripthash",
    "addresses": [
      "tb1qtclalg4vqvlmynp34c8aj2ntml338wmudpl56f043f6crwejsy5qnd79kg"
    ]
  },
  "coinbase": false
}

@kirelagin
Copy link
Author

The weirdest thing of all is that there is no packet in the capture that would contain amount 0msat as the error says.

@kirelagin
Copy link
Author

Putting the misleading error aside, according to the comment at parse_amount_sat, 0.00000000 is indeed not a valid input:

/* Valid strings:
 *  [0-9]+ => satoshi.
 *  [0-9]+sat => satoshi.
 *  [0-9]+000msat => satoshi.
 *  [0-9]+.[0-9]{1,8}btc => satoshi.
 */

@kirelagin
Copy link
Author

cc @rustyrussell since you wrote this code

@kirelagin
Copy link
Author

Ah, ok, so bcli.c converts this bitcoind amount to some other form. So the problem seems to be somewhere around json_to_bitcoin_amount and json_add_amount_sat_only.

@kirelagin
Copy link
Author

Probably cc @darosior as it seems like the bug might have been introduced in #3488, although I am not 100% sure.

@darosior
Copy link
Collaborator

@kirelagin you are right, Bitcoin #3605 fixes this.

@darosior
Copy link
Collaborator

And I'm currently testing Rusty's latest patch on testnet ;)

@kirelagin
Copy link
Author

kirelagin commented Mar 30, 2020

Alright, so tldr:

bcli.c receives amount in sat and then calls json_add_amount_sat_only, which converts it to msat and adds the msat suffix. Now, when parsing it, the parse_amount_sat function sees the msat suffix and verifies that the number ends with three zeroes, aka, it is a whole number of satoshis.

Which works almost great, except that when you multiply 0 by whatever, you still get 0. So it sounds like adding a special case for 0msat is the way to go.

@kirelagin
Copy link
Author

Oh ok cool.

@darosior
Copy link
Collaborator

Will commit his patch and push it so you can pull this branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants