[Stream] Add downloader module #3584
Conversation
JackyWYX
force-pushed
the
stream_download
branch
3 times, most recently
from
a65e783
to
effcd6c
Compare
JackyWYX
force-pushed
the
stream_download
branch
from
eaeddf0
to
efe1f35
Compare
| } | ||
|
|
||
| // DownloadAsync triggers the download async. If there is already a download task that is | ||
| // in progress, return ErrDownloadInProgress. |
There was a problem hiding this comment.
I didn't see an error returned? is this comment outdated?
There was a problem hiding this comment.
Right. It's an out-of-date comment. Let me remove this
| if err != nil { | ||
| return nil, err | ||
| } | ||
| decider.UpdateParticipants(pubKeys) |
There was a problem hiding this comment.
No, I don't think lock is needed here. verifyAndInsertBlock is called sequentially in doLongRangeSync and doShortRangeSync. Moreover, decider is created for each epoch. So there shall be no race condition as far as I see.
| return nil | ||
| } | ||
|
|
||
| func computeBNMaxVote(votes map[sttypes.StreamID]uint64) uint64 { |
There was a problem hiding this comment.
no comments on the intention of this function?
There was a problem hiding this comment.
OK. Added comment, and changed function names.
| } | ||
| } | ||
|
|
||
| func validateGetBlocksResult(requested []uint64, result []*types.Block) error { |
There was a problem hiding this comment.
you have assumed the order of the number in the input list, how do you know the input list will be sorted based on the numberu64?
There was a problem hiding this comment.
This is the result get from remote node as the response for a request which well defines which number is expecting (as requested. If the remote node runs honestly, the response should always have the same number as requested. If the remote node does not give the exact block or mess up with the sequence, the remote stream is marked as dishonest and removed.
| return requestBNs | ||
| } | ||
|
|
||
| func (gbm *getBlocksManager) getBatchFromUnprocessed(cap int) []uint64 { |
There was a problem hiding this comment.
better add comments on the intention of this function and when to call it.
There was a problem hiding this comment.
OK. Let me add it.
| if len(blocks) != len(hashes) { | ||
| return errors.New("unexpected number of getBlocksByHashes result") | ||
| } | ||
| for i, block := range blocks { |
There was a problem hiding this comment.
assumed sorted list, how to guarantee that?
There was a problem hiding this comment.
Because the hashChain was assumed to be sorted. If the given hashChain is not sorted, remote node can be malicious and need to be killed.
Description
This is a break down pr of #3535 and is rebased on #3583.
1. downloader and downloaders
Downloaders is the aggregation of downloader. Each downloader is responsible for syncing for one specific shard. Each downloader will host its own
sync.Protocolon a specific topic.2. Downloader modes
Downloader has two modes:
3. beaconHelper
beaconHelperis the submodule of downloader, which aims to aid the block synchronization of beacon chain for shard chain nodes. Basically, it takes blocks from beacon client pub-sub, add to memory andInsertChainwhen it see appropriate.Additionally, the
InsertHookfunction is to broadcast crosslinks when the node is a leader (will be included in next PR).4. insertHelper
insertHelperis to do some extra verification onblock.commitSigAndBitmapfield. The block will only be inserted when current sig verification passes.