New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Gossip verify block to return block if no parent #1028
Comments
Could you just clone the EDIT: oh you need to store the whole block to process it later. |
I'm going to un-assign myself from this one since it's a relatively minor optimization. Someone else can come pick it up if they like, otherwise I'll get to it at some point. Returning the parent block means making |
Sounds good to me |
Working on this now. |
@AgeManning Haven't contributed to your repo before, so excuse the dumb questions. I assume the team is generally busy and don't expect fast response |
Hey @thor314. Thanks for the interest. We get a As you can see, we then call this function to determine if we should propagate the block or not. Now it can happen that the block we received references a block that we do not know about. In this case the In this case, we want to go and find the parent block. As you can see, the returned error You'll have to track down the processing in Hopefully this is enough information to get you started :) |
Thanks for bearing with me @AgeManning. I'm going to talk to you like a rubber duck for a bit, sprinkling questions as they arise, then restate the questions at the end. Commence rubber ducking:
We want to make BlockError generic over Now once that's handled, we skedaddle back on over to After we clear up making
|
Let me answer these in some arbitrary high level order and then go back to some more specifics. The whole EthSpec thing seems to be a point of confusion, and i don't blame you, it's pretty funky. At the crux of it, a Now, Hopefully this answers your question 0 above.
pub fn to_block_error<T:EthSpec>(self) -> Result<(), BlockError<T>> {
match self {
ChainSegmentResult::Failed { error, .. } => Err(error),
ChainSegmentResult::Successful { .. } => Ok(()),
}
} However depending on the logic, perhaps you want to make
enum Thing {
A(BeaconBlock),
B(usize),
Now, I think the core of this issue, is finding out where the block is being consumed when it fails with parent unknown. A quick look points me to the check starting here It's here and it errors here It seems to me, the solution would be to set |
Okay, so I'm struggling in tracking down all the changes needed for making
done again in
I'm interpreting this to mean that I need to change the following:
and in fact that does solve both instances of Some other questions that came up along the way:
|
Hey, to resolve all the issues with the generic type parameter, I threaded the generic parameter through to help you start. Its in #1174. Hope this gives a helping hand. |
Sorry on the delay, balancing this with finals.
More explicitly: Edit: ignore just now, still sorting myself out |
Yes you're right. Make the load_parent accept a |
Okay, I'm getting stuck, but I think I can resolve that with some input. Here's what I've done:
This compiles. Now if I plug in Here's the problem: where
And presently is only looking at the parent of the borrowed block, so it's dropping the block I just gave it ownership of when it returns the snapshot. Here's what I think I need to do: If I do that, I'm only using |
Hi @thor314 , where are you working on this if may I ask? went lurking to your fork but I couldn't get to a branch, and most links on your last comment go to the |
@divagant-martian Ah, I'm still getting used to a git workflow. It should be up to date now. I'm updating all the links from my prev comment now, to point to the right stuff. K, links updated to be in my fork of the repo. |
Hi, answering your questions:
Also, if you check, all calls to Let me know if you run into issues |
Also probably wrap the block in a Box since those are thick boys |
## Issue Addressed #1028 A bit late, but I think if `BlockError` had a kind (the current `BlockError` minus everything on the variants that comes directly from the block) and the original block, more clones could be removed
Description
When verifying a block for gossip propagation, one error that can occur is
ParentUnknown
. In this case, we want to store the block and lookup its parents before processing a chain of blocks.Currently this return type doesn't return the block and we must clone the block before checking the verification in case we need to do a parent lookup.
To avoid the clone, the
load_parent
function should return theSignedBeaconBlock
itself in the error message.The text was updated successfully, but these errors were encountered: