chainntnfs: fix nil pointer on address reuse

[guggero]

When a confirmation notification for a script hash is registered, the notifier panics with a nil pointer dereference in case two transactions send funds to this script and are confirmed at different block heights.

The test that triggers the panic is added in the first commit with a fix for it in the second commit.

[wpaulino]

Perhaps the better fix here is to ignore the reuse when detecting it within a transaction? This currently prevents the panic, but a notification won't be dispatched for the reuse anyway.

[guggero]

I now implemented the version that will not notify again on reuse.
Turns out, it's also less code.
Tested it with my regtest setup and was unable to reproduce it after the fix.

The unit test setup is still a bit peculiar with the event listener in the goroutine since we still get multiple messages on the Event.Update channel.
The reason for this is that because both confRequests are identical, there are two ntfns created for the same request and they both get notified. And since the request is referenced from two block heights, we get a total of 4 Update messages (two per TX).
And because the chan buffer is only 1, the test would block if we didn't read in a goroutine.

Not sure if this can still cause problems on the receiver end?

[wpaulino]

The reason for this is that because both confRequests are identical, there are two ntfns created for the same request and they both get notified. And since the request is referenced from two block heights, we get a total of 4 Update messages (two per TX).

I think we can get around this by adding an additional check to ensure no details exist for the notification before notifying the transaction at:

lnd/chainntnfs/txnotifier.go

Line 1494 in 6765b86

if _, ok := n.confNotifications[confRequest]; ok {

[guggero]

I was able to get around the problem by not notifying ntfns that have already been marked as dispatched. Now the test is simpler too.

[guggero]

Ok, I think I was finally able to fix it the way you suggested, @wpaulino.

[wpaulino]

Travis is reporting a linter failure: chainntnfs/txnotifier.go:1510: File is not goimports-ed (goimports)

[halseth]

LGTM ✅