Skip to content

Conversation

@bltavares
Copy link
Member

@bltavares bltavares commented Feb 7, 2020

The network code requires to send compressed bitfields to the wire with the size of the feed.
This commit exposes the bitfield as a public method, and adds the compress method.

bitfield_rle is not encoding the same as node's version.
Tests are broken because of that.

This is a draft while working on other codes, so I can pull from other machines. After that, I will cleanup the changes to have it easier to review, depending on less PRs.

Choose one: a 🙋 feature

Checklist

  • tests pass
  • tests and/or benchmarks are included
  • documentation is changed or added

Context

Semver Changes

minor

@bltavares bltavares marked this pull request as ready for review February 19, 2020 22:09
// TODO: use the index to speed this up *a lot*
/// https://github.com/mafintosh/hypercore/blob/06f3a1f573cb74ee8cfab2742455318fbf7cc3a2/lib/bitfield.js#L111-L126
pub fn compress(&self, start: usize, length: usize) -> std::io::Result<Vec<u8>> {
if start == 0 && length == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

A comment here could be useful?

Copy link
Contributor

@yoshuawuyts yoshuawuyts left a comment

Choose a reason for hiding this comment

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

LGTM!

new dalek pre version has some changes, and requires a new rand version.

This commit bumps both, including changes needed on quickcheck to use
new rand.
The network code requires to send compressed bitfields to the wire with the size of the feed.
This commit exposes the bitfield as a public method, and adds the compress method.

bitfield_rle is not encoding the same as node's version.
Tests are broken because of that.
@bltavares bltavares force-pushed the bitfield-compress branch from 58d8c25 to 9c6812d Compare March 3, 2020 01:33
@bltavares bltavares merged commit 4136866 into datrs:master Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants