Sync with multiple peers #37
Conversation
|
@NikVolf currently sync/src/local_chain.rs is used as in-memory storage && there is some code in sync/src/synchronization.rs which links (well, it should :) ) together storage && verification queue.
|
| self.orphan_blocks.insert(block.block_header.previous_header_hash.clone(), block.clone()); | ||
| return; | ||
| } | ||
| assert!(self.blocks_map.contains_key(&block.block_header.previous_header_hash)); |
There was a problem hiding this comment.
is it because block is already verified?
There was a problem hiding this comment.
Yes - currently LocalChain is what is database is supposed to be (without fork support) - only verified in-order blocks are here.
|
|
||
| // process unknown blocks | ||
| let unknown_blocks: Vec<_> = message.inventory.iter() | ||
| .filter(|item| InventoryType::from_u32(item.inv_type) == Some(InventoryType::MessageBlock)) |
There was a problem hiding this comment.
not an issue. Maybe item.inv_type could be InventoryType instead of u32?
There was a problem hiding this comment.
This would be great. I doubt anybody will ever need to have access to underlying values of enums + check is moved to deserialization step (which is good, because we can drop connections without even interacting with sync module)
There was a problem hiding this comment.
I'll add TODO for this
| ///! 2) no forks support | ||
| ///! | ||
| ///! When new peer is connected: | ||
| ///! 1) send `getdata` message with full block locator hashes |
There was a problem hiding this comment.
few questions :)
in the docs getdata message description says
getdata is used in response to inv, to retrieve the content of a specific object, and is usually sent after receiving an inv packet, after filtering known elements
how does it correspond with the docs?
There was a problem hiding this comment.
Mistake here - sorry :) Actually inv message is sent. Will fix this
| } | ||
| SynchronizationTask::RequestInventory(peer_index) => { | ||
| let getblocks = types::GetBlocks { | ||
| version: 0, |
There was a problem hiding this comment.
Is it ok to send 0 instead of real protocol version? What if some implementations expect real protocol version here? :)
There was a problem hiding this comment.
Good point, thank you :) I think I should look into it, because for now it is not clear to me - which version should I pass here. Is it negotiated version (during handshake)? Or is it version of the protocol, this particular message was defined in? For example: in PV 0...1000 message1 had 5 fields and since that it have 10 fields.
In protocol documentation it is described as "the protocol version". But if we have negotiated version earlier, then why should I pass it everytime with the message? And if it is this particular message version, then 0 is OK because structure of getblocks message (and other messages I use here) hasn't changed since bitcoin beginning.
I'll add TODO here.
There was a problem hiding this comment.
I found an answer! :)
https://github.com/libbitcoin/libbitcoin/blob/master/src/message/get_blocks.cpp#L113
|
lgtm |
Started work on multiple peers support in sync + added several basic sync tests.
Synchronization strategy is described in sync/src/synchronization.rs, however there are a lot TODOs.
Current functionality:
able to read blocks from single (potentially - many, however, not tested this scenario) peer && store in in-memory-storage (without any verification, except for header.parent_hash = hash(parent))
tested with bitcoind
Next steps: