Conversation
802cfce to
4bc39c8
Compare
| | `merkle_root` | `uint256` | 32 | Root of the Merkle tree computed over chunk hashes. | | ||
|
|
||
| A requesting node MUST ignore entries whose `serialized_hash` does not match a known | ||
| utxo set hash for the corresponding height. |
There was a problem hiding this comment.
I think it would be better for a client that supports this mechanism to hardcode the merkle root instead of the straight serialized hash, and to drop serialized_hash from this message.
There was a problem hiding this comment.
Hm, yeah, I think that makes sense, I was struggling with finding the right path between building on top of the existing assumeutxo data we already have and extending it. I am adding the merkle root to the assumeutxo data in my PR, so checking the serialized_hash is a belts and suspenders there, so it makes sense to make this explicit here as well.
There was a problem hiding this comment.
If you wanted to change the contents of the utxo dump as well, it would be nice to include the header chain for the block where the snapshot was taken. Then you could do an import without needing to connect to the network at all, I think.
| | `block_hash` | `uint256` | 32 | Block hash this data corresponds to. | | ||
| | `chunk_index` | `uint32_t` | 4 | Zero-based index of this chunk. | | ||
| | `proof_length` | `compact_size` | 1–9 | Number of hashes in the Merkle proof. | | ||
| | `proof_hashes` | `uint256[]` | 32 × `proof_length` | Sibling hashes from leaf to root. | |
There was a problem hiding this comment.
Rather than a proof, it might be better to just request getutxoset <height> <hash> 0xFFFFFFFF once to get the full list of chunk hashes -- that should be about 74kB for a 9GB utxo set, and should stay under 4MB until the utxo set is >450GB. Then each chunk is just <height> <hash> <number> <data>.
There was a problem hiding this comment.
Huh, interesting idea to get all the chunk hashes first, I didn't think of that. It might make sense to do this with a separate message type even instead of hacking getutxoset as you described. I will have to think about it a little more.
|
I was going to complain that I haven’t seen a discussion about this proposal on the mailing list… but you did that already for me. If you already know that you should send it to the mailing list first, I don’t know why you opened the PR first, though. 😛 |
It wasn't clear to me that a ML post was a prerequisit to open the PR. I just thought it was necessary to do this at some point before the bip could get merged/assigned a number. I think that having a place for more detail oriented commentary makes sense to have in addition to the high level discussion happening on the ML, if ML readers have such feedback but would rather use the more convenient inline commenting/suggestion features in GitHub. I was also looking for feedback on my assumeutxo related question, e.g. can I assume knowledge of a feature that is only implemented in Bitcoin Core or should I define this in the BIP. The ML doesn't seem like the right place to ask about this. I will close this for now and re-open when I have made the ML post and given it some reasonable time for discussion. |
|
Thanks, and sorry, I might have come off as more gruff than intended — tone is hard in written text. Obviously, you’ve been around the block and your proposal reads well-considered, but we have been getting a lot of premature submissions out of the blue here, where then BIP Editors become the first line of feedback. Personally, it’s been taking a growing part of my work hours to even just get through all submissions to the repository. So, we have become a bit more insistent on proposals actually being posted to the list first, and I’d like to avoid giving the impression that I’m playing favorites. A hypothetical optimal order of might be:
In your case it sounds like you’d be able to skip directly to 5, and we can of course reopen this PR, when there has been an ML discussion. |
| For each available UTXO set: | ||
|
|
||
| | Field | Type | Size | Description | | ||
| |-------|------|------|-------------| |
There was a problem hiding this comment.
Since the format has a version number, it would make sense to include it here.
| 1. The requesting node identifies peers advertising `NODE_UTXO_SET`. | ||
| 2. The requesting node sends `getutxosetinfo` to one or more of these peers. | ||
| 3. Each peer responds with `utxosetinfo`. The requesting node verifies that the advertised | ||
| `serialized_hash` matches a known UTXO set hash, compares `merkle_root` values across peers, | ||
| and selects a UTXO set whose Merkle root has agreement among multiple peers. |
There was a problem hiding this comment.
I think this process defeats the point of the service bit. Having "at least one" UTXO set only "works" while there are only a small number of UTXO set options to download. If it's not sufficient to just try peers until you find the UTXO set you want, then we probably need a way to advertise specific UTXO sets.
| before initiating the process. Including the serialized hash in the advertisement lets the requester | ||
| immediately filter out peers claiming a different UTXO set state before downloading any data. | ||
|
|
||
| **Discovery before download:** The `getutxosetinfo`/`utxosetinfo` exchange lets the requesting node |
There was a problem hiding this comment.
I'm not sure discovery is a useful feature. It enables a misguided implementation to blindly trust whatever nodes provide, and harms node privacy by adding a way to fingerprint nodes.
If you know what you'll accept in advance, you can simply request that snapshot, and the node will either provide it or (potentially) say it can't.
This BIP draft describes the sharing of a full UTXO set via the p2p network.
Design summary:
The one part I am not so sure about yet: This references Bitcoin Core and it’s features or RPCs in a few places now. I am aware that this is not ideal for specification that targets a wider audience but the reality is that assumeutxo seems to be only implemented in Bitcoin Core and mentioning RPCs from the workflow there seems the most clear way to describe this. But I am happy to generalize this further, I would be very happy to receive some guidance what level of referencing assumutxo is acceptable here since it is obviously the main current motivation. One concrete example: The Bitcoin Core PR that I will use as a reference implementation will rely on assumeutxo params rather than comparing multiple peer values of the merkle root. Is this discrepancy an issue?
Mailing List post
and reference implementationwill follow shortly and I will add the links here asap.