Breakup RPCBlock into LookupBlock & RangeSyncBlock#8860
Breakup RPCBlock into LookupBlock & RangeSyncBlock#8860ethDreamer wants to merge 7 commits intosigp:unstablefrom
Conversation
ethDreamer
changed the title
| ) | ||
| .await? | ||
| .try_into() | ||
| .expect("block blobs are available") |
There was a problem hiding this comment.
the expect message is a bit misleading since we are not expecting a block w/ blobs
eserilev
left a comment
eserilev
left a comment
There was a problem hiding this comment.
This looks great. Really makes things much clearer imo
I just had a few questions about a few test functions
| ) | ||
| .await? | ||
| .try_into() | ||
| .expect("block blobs are available") |
There was a problem hiding this comment.
the expect message is a bit misleading since we are not expecting a block w/ blobs
| RangeSyncBlock::new( | ||
| block, | ||
| None, | ||
| AvailableBlockData::NoData, |
There was a problem hiding this comment.
I believe this should be a LookupSync block, since we were previously passing None as BlockData in RpcBlock::new
| let block_data = if let Some(blobs) = blobs { | ||
| AvailableBlockData::new_with_blobs(blobs) | ||
| } else { | ||
| RpcBlock::new( | ||
| block, | ||
| None, | ||
| &self.chain.data_availability_checker, | ||
| self.chain.spec.clone(), | ||
| )? | ||
| } | ||
| AvailableBlockData::NoData | ||
| }; | ||
|
|
||
| RangeSyncBlock::new( | ||
| block, | ||
| block_data, | ||
| &self.chain.data_availability_checker, | ||
| self.chain.spec.clone(), | ||
| )? |
There was a problem hiding this comment.
I dont think this keeps the previous test logic intact. I think that passing None to BlockData during RpcBlock construction should be equivalent to constructing a LookupSync block
2 participants
This should make sync and the DA checker much more clear.