Skip to content

Conversation

@khodzha
Copy link
Contributor

@khodzha khodzha commented May 2, 2020

This is a bug fix:

ensure! macro was mutating the range so by the end of the macro evaluation
the range was in wrong state

Checklist

  • tests pass
  • tests and/or benchmarks are included (im not sure how to test this)
  • documentation is changed or added (is this needed?)

Context

this fixes test in #108 if my comment in that PR is addressed

Semver Changes

no changes? idk 🤷‍♂️

ensure! macro was mutating the range so by the end of the macro evaluation
the range was in wrong state
@Frando
Copy link
Member

Frando commented May 4, 2020

Wow, great find @khodzha! I can confirm that this fixes the put method. #108 passes if the proof call is changed to not include hashes. And I can now successfully put data into a hypercore through hypercore-protocol-rs (in the dev branch).

This was referenced May 4, 2020
@Frando
Copy link
Member

Frando commented May 4, 2020

I fixed and rebased the put test in #111, which I think should also be a fine test for this (so the box above can be ticket IMO).

bltavares added a commit that referenced this pull request May 4, 2020
bltavares added a commit to bltavares/hypercore that referenced this pull request May 4, 2020
@bltavares
Copy link
Member

I'm trying to test on my local prototype, but I'm not finding the correct combination of forks between hypercore, bitfield-rle and this branch that compiles yet.

I'll try again on the weekend, but it looks like a good changeset in combination with the tests of #111

@Frando If you want, I could try to do a octopus merge of both and run locally the tests to merge both on master.

@Frando Frando mentioned this pull request May 16, 2020
5 tasks
@bltavares bltavares merged commit f56a7c6 into datrs:master May 17, 2020
@bltavares
Copy link
Member

Thanks @khodzha and @Frando. Both PRs are merged on master and released as the beta release.

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.

3 participants