Closed guggero closed 3 years ago
I've cleaned up the PR and addressed all the comments so far.
Two questions remain:
ComputePkScript
(they're not detected and P2SH is assumed). Do we want to block this PR on the fix or do we want to continue with the witness only verification for now?It turned out that bare P2PK aren't supported by ComputePkScript (they're not detected and P2SH is assumed). Do we want to block this PR on the fix or do we want to continue with the witness only verification for now?
Ah I think we opted to ignore those as we needed this for stuff in the chain notifier, and nothing in lnd uses bare P2PK. I'm thinking we should either make it possible to ignore those, or update the upstream library to support it. Thoughts @wpaulino?
In the rescan logic (where we actually download the block and verify it), we don't know which peer we got the filter from originally
Yeah we definitely shouldn't continue with a bad filter there. An assumption here is that if we're able to do a full header chain sync w/o finding any divergence, then we do indeed have the correct set of filters. However, this may not be the case in practice assuming we had all bad peers to begin with. Only until we either download a filter, to go verify it and find out it's invalid or we get a proper peer will we discover that we had an invalid chain. However as is, we don't unwind our filter header chain (and wipe all filters) in order to start from scratch...
Fixes https://github.com/lightninglabs/neutrino/issues/207.
Whenever we download a block, we want to make sure that the filter we had for it was correct. It's trivial to check all output scripts of the block's transactions were included in the filter. The trickier part is to verify the scripts of the outputs being spent are also included in the filter. By looking at the signature script or witness we can derive the script that's being spent and from that can make sure they are all included in the filter.