blocksync: introduce interfaces; rename to ChainExchange (abbrev. chainxchg) #3624
Conversation
|
cc @yiannisbot |
raulk
changed the title
| const ( | ||
| // BlockSyncProtocolID is the protocol ID of the former blocksync protocol. | ||
| // Deprecated. | ||
| BlockSyncProtocolID = "/fil/sync/blk/0.0.1" | ||
|
|
||
| // ChainExchangeProtocolID is the protocol ID of the chain exchange | ||
| // protocol. | ||
| ChainExchangeProtocolID = "/fil/chain/xchg/0.0.1" | ||
| ) |
There was a problem hiding this comment.
For consistency, I had to rename the p2p protocol. It's totally fine since we register the stream handler under both protocol IDs. And when opening an outbound stream, we try ChainExchange first, falling back to BlockSync. Before releasing v1, we can probably get rid of the deprecated one entirely.
There was a problem hiding this comment.
Can you open an issue so we can track this?
| // used by the Syncer. | ||
| type Client interface { | ||
| // GetBlocks fetches block headers from the network, from the provided | ||
| // tipset *backwards*, returning as many tipsets as the count parameter, |
There was a problem hiding this comment.
What does "backwards" mean here? Decending in height?
| GetBlocks(ctx context.Context, tsk types.TipSetKey, count int) ([]*types.TipSet, error) | ||
|
|
||
| // GetChainMessages fetches messages from the network, from the provided | ||
| // tipset *backwards*, returning the messages from as many tipsets as the |
| // FIXME: Should be reviewed and confirmed. | ||
| SuccessPeerTagValue = 25 | ||
| WriteReqDeadline = 5 * time.Second | ||
| ReadResDeadline = WriteReqDeadline |
There was a problem hiding this comment.
Yeah, interested to know if we actually want these to be linked or if we just did not want to see repetition 😅...
If no then we should unlink and set to 5 * time.Second and if yes then there should be a comment to explain the linking is intentional!
There was a problem hiding this comment.
I left all this logically unchanged, but yeah, this is a bit odd. Theoretically the response read timeout should be a bit larger than the write timeout, to account for the fact that this is an RPC and the remote party has work to do before they can actually return a response.
There was a problem hiding this comment.
I probably linked them when extracting the constants, fine to decouple if you want.
| // FIXME: Should be reviewed and confirmed. | ||
| SuccessPeerTagValue = 25 | ||
| WriteReqDeadline = 5 * time.Second | ||
| ReadResDeadline = WriteReqDeadline |
There was a problem hiding this comment.
I probably linked them when extracting the constants, fine to decouple if you want.
Fixes #2612.