WIP: Pollard hashes up to populated nodes, not all the way to roots #226
Conversation
same thing, just a little easier to read
instead of giving it a function to read locations simpler, and we don't have multiple callers of verifyBatchProof()
This also has an upside because the csn can tell if the bridge sends to much. So it could disconnect if the bridge makes it download more than needed. |
…ashes need to clean up some other things first before removing the targets
it all happens without needing to query any csn state (except for the blockhash which we're not using yet)
it was just returning the targets slice given to the function
doesn't make any sense as the targets aren't included in the proof now so you can only tell if the proof works with respect to a pollard, can't check a proof's self-consistency
it was a cool idea but wasn't any faster and we have forest more built out now would still be nice to merge a lot of the overlapping accumulator code, but that wasn't how
| @@ -91,7 +89,7 @@ func ProofPositions( | |||
| targets = nextTargets | |||
| } | |||
|
|
|||
| return proofPositions, computedPositions | |||
There was a problem hiding this comment.
The computedPostions slice held the targets and all the positions of the hashes that are computed when verifying.
Removing it is probably still fine because only the length of the slice was used to allocate enough miniTrees.
| // set dataDir | ||
| if *dataDirCmd == "" { // No custom datadir given by the user | ||
| dataDir = btcutil.AppDataDir("bitcoin", true) | ||
| } else { | ||
| dataDir = *dataDirCmd // set dataDir to the one set by the user | ||
| cfg.blockDir = *dataDirCmd // use user sepcified as the full blockdir |
There was a problem hiding this comment.
Don't we want this to point to the dataDir?
We know where the blocks are if get the datadir and the network type.
| @@ -525,6 +526,7 @@ func (ub *UBlock) SerializeCompactSize() int { | |||
| // rebuilt from the block data. Note that this leaves the blockhash | |||
There was a problem hiding this comment.
The blockhash is not yet being restored right? at least i can't find it in any of the commits.
This also assumes a height to blockhash index on the client side exists, might not be worth it if the saving aren't huge.
looks like there's an off by 1 error somewhere
It chokes if the proof is empty (no hashes) But that's correct now; if you're proving everything, there is no proof needed!
this will eliminate populate, verifyBatchProof, and ProofPositions, which is getting rid of a lot of complex code. So that's good. It does need a grabPos variant that returns a slice of the whole branch. Then we can dedupe the nodes in those slices when inserting them into a 2d slice. modify grabPos, new function, or inline it all into ingestAndCheck..?
…t with nodes should be more efficient since we don't need to calculate all the way up to roots tricky part will be skipping proof hashes that we don't need though.
Gives exact same output as previous proofPositions but shorter, and a bit easier to understand. Also it can output branch-at-a-time proofs which feel more suited to hash-to-known
hash to known isn't enough, you have to hash past known values...
Previously
(p *Pollard) IngestBatchProof()would verify the batch proof, then apply it to the pollard; the verification step verified proofs all the way up to the roots.This verification is inefficient as the pollard can already have hashes near the top of the tree. Also if you need to hash up to roots, that means you can't have partial proofs and can't use the TTL values.
This PR will change pollard's
IngestBatchProof()to only hash up to known values, and not require proofs up to the roots.The simplest way is that it will require proofs up to exactly it's lowest populated position, which is fragile as it won't be able to accept a larger than needed proof. But that seems like an OK first step, we can deal with excess proofs in a later PR.