htlc hook not working on startup

[renepickhardt]

it seems like the @plugin.hook("htlc_accepted") hook is not working as excpected when starting a node which has an unresolved htlc.

from the logfile I copy

2019-08-08T11:02:01.889Z plugin-htlchook.py on_htlc_accepted called
2019-08-08T11:02:01.889Z lightningd(12424): Plugin for htlc_accepted returned non-result response {"jsonrpc": "2.0", "id": 3, "error": "Error while processing htlc_accepted: AttributeError(\"'NoneType' object has no attribute 'getinfo'\",)"}

this error is invoked because I called plugin.rpc.getinf() it seems like on startup plugin.rpc is still None but the hook is already being invoked.

Issue and Steps to Reproduce

create a payment channel between two nodes. Initiate the payment process and let the other node crash during the payment process. then restart the node and experience this behavior.

getinfo output

ln1 getinfo
{
   "id" : "03d1bba2403b9901096f565cec8f274805adfba9fcf80ea693360d54cf711b7bb6",
   "alias" : "alice",
   "color" : "03d1bb",
   "num_peers" : 1,
   "num_pending_channels" : 0,
   "num_active_channels" : 0,
   "num_inactive_channels" : 1,
   "address" : [],
   "binding" : [
      {
         "type" : "ipv4",
         "address" : "127.0.0.1",
         "port" : 1337
      }
   ],
   "version" : "v0.7.1",
   "blockheight" : 649,
   "network" : "regtest",
   "msatoshi_fees_collected" : 0,
   "fees_collected_msat" : "0msat"
}

[rustyrussell]

Yuck. @cdecker changed htlcs to be retried, rather than failed, on startup. But plugins aren't ready yet.

Completing the HTLCs later is the obvious solution, but I'm not sure what the effect of having "half-finished" HTLCs lying around in the interim would be (which is why we failed on startup!).

Assigning to @cdecker for this thoughts.

[cdecker]

I'm surprised calling any RPC call while handling a hook is actually working at all. Remember that the hooks are supposed to be synchronous and no other action is allowed to make any progress, including RPC calls. This is the main motivation when I felt ok moving the call to htlc_accepted hook calls before init calls in the first place (you're not supposed to interact with the RPC when handling a hook, though I could have made that more clear in the documentation).

TL;DR: Your hook handling logic simply should not rely on the fact that the RPC is available at all, since they may be called before init, and the RPC will likely not reply, since hook calls are synchronous.

FWIW db_write suffers from the same issue, i.e., returning writes before calling init, and we solve it by postponing the write to the backup.

A clean solution would be to introduce a hold state (which we'll need eventually anyway), then holding on to HTLCs until we are initialized, and only then returning the verdict that may depend on RPC calls.

[darosior]

I'm surprised calling any RPC call while handling a hook is actually working at all. Remember that the hooks are supposed to be synchronous and no other action is allowed to make any progress, including RPC calls..

FWIW I noticed they are not while working on https://github.com/ElementsProject/lightning/pull/2925. That surprised me and that's why I extended the API in this PR.

[rustyrussell]

Ok, we'll defer this until next version, since rc1 is due.

The DB hook is a special case, and documented as such: generally we shouldn't be calling into a plugin before init.