Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

CHT calculations for full nodes #4181

Merged
merged 3 commits into from
Jan 19, 2017
Merged

CHT calculations for full nodes #4181

merged 3 commits into from
Jan 19, 2017

Conversation

rphmeier
Copy link
Contributor

@rphmeier rphmeier commented Jan 16, 2017

Closes #3458 (for now)

For now, Tries are always calculated on-demand. This will likely be changed in the future to maintain an LRU-Cache of recently-generated CHTs.

Also adds some more tests and documentation.

@rphmeier rphmeier added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Jan 16, 2017
@rphmeier
Copy link
Contributor Author

Note: this PR will be the first containing a divergence from LES as-specified. Perhaps we should choose another protocol identifier? plp for Parity Light Protocol?

@arkpar
Copy link
Collaborator

arkpar commented Jan 18, 2017

Does it break compatibility with go-ethereum implementation?

@rphmeier
Copy link
Contributor Author

yes, although there's no server/client capability exposed yet.

}
}
}
let needed_hdr = needed_hdr.expect("`needed_hdr` always set in loop, number checked before; qed");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not really clear that needed_hdr is always set. Is req.block_number guaranteed to be within the range?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's guaranteed by the check that req.cht_number is equal to the calculated CHT number based on the block number in the request. this means that the function has already returned in the case that the requested number wouldn't fall within the range of blocks we iterate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, if we don't have all the blocks in that range, we also return early. The CHT number should probably be omitted in the future because it is derivable from a block number and only serves to give misbehaving peers a method of misbehavior.

@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jan 18, 2017
@rphmeier rphmeier merged commit 5830e03 into master Jan 19, 2017
@rphmeier rphmeier deleted the cht-full-nodes branch January 19, 2017 14:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
Light Client
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants