btcsuite / btcd

An alternative full node bitcoin implementation written in Go (golang)
https://github.com/btcsuite/btcd/blob/master/README.md
ISC License
6.11k stars 2.32k forks source link

blockchain: refactor and export header validation checks #1931

Closed Crypt-iQ closed 1 year ago

Crypt-iQ commented 1 year ago

This PR allows an external library like neutrino to be able to perform contextual and context-free header validation. This is achieved by introducing two new interfaces HeaderCtx (which replaces *blockNode in the relevant functions) and ChainCtx (which replaces *BlockChain in the relevant functions). A library calling into these functions needs to only satisfy the interface to be able to validate any received headers.

coveralls commented 1 year ago

Pull Request Test Coverage Report for Build 5415613990


Changes Missing Coverage Covered Lines Changed/Added Lines %
blockchain/chainio.go 0 1 0.0%
blockchain/thresholdstate.go 0 1 0.0%
blockchain/blockindex.go 20 29 68.97%
blockchain/validate.go 43 64 67.19%
blockchain/difficulty.go 6 31 19.35%
<!-- Total: 73 130 56.15% -->
Files with Coverage Reduction New Missed Lines %
blockchain/difficulty.go 3 48.78%
<!-- Total: 3 -->
Totals Coverage Status
Change from base Build 5379381875: 0.02%
Covered Lines: 26688
Relevant Lines: 48317

💛 - Coveralls
Crypt-iQ commented 1 year ago

Looks good! Only blocking comments are about the nil checks vs. interfaces (need to also read up on the specifics, when a normal nil check works and when not).

You can nil-check an interface, but it's a bit weird.

See here: https://go.dev/play/p/9NVSUoFo2oI If foo() returns innerFoo() which returns nil, then the nil is converted to a MyIface with a nil value. This will fail the nil-check.

If foo() just calls return nil, then this will succeed the nil-check. I'll double-check the call-sites you mentioned just to be sure though

guggero commented 1 year ago

Looks good! Only blocking comments are about the nil checks vs. interfaces (need to also read up on the specifics, when a normal nil check works and when not).

You can nil-check an interface, but it's a bit weird.

See here: https://go.dev/play/p/9NVSUoFo2oI If foo() returns innerFoo() which returns nil, then the nil is converted to a MyIface with a nil value. This will fail the nil-check.

If foo() just calls return nil, then this will succeed the nil-check. I'll double-check the call-sites you mentioned just to be sure though

Aaah, I see. Thanks for the example. I guess that makes sense. That's why you had that explicit if x == nil { return nil in there, gotcha.

Crypt-iQ commented 1 year ago

I do wonder if there's not a way to get around nil-checking and to instead use some dummy value to compare against

Crypt-iQ commented 1 year ago

Changed FindPreviousCheckpoint to check and return an error from findPreviousCheckpoint