feat: switch to zcash-block for decoding

[rvagg]

There's some renaming of properties going on in here. I've stuck with the Zcash API names so mostly things aren't camelCase, they are lowercase.

Transactions are supported, I've added some basic testing in for that. But since they sit outside of the header and have their own merkleroot hash, if we were doing this properly I imagine we'd have another multicodec for transactions. So let's not bother with that for now.

I haven't implemented a serializer and doubt I'll be able to justify the time investment to do so. How important is serialize() support? I've made it throw.

While doing this, I realised that our choice of a sync deserialize() bit me pretty hard. My zcash-block library was async, mainly for multihashing-async, because there's a bunch of hashing that gets done during decode. I had to switch that out for multihashing and unwind all of the async work. That's where the 2.0.0 of zcash-block comes from, 1.0.0 was async.

resolves #24

[codecov]

Codecov Report

Merging #46 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master    #46   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           3      3           
  Lines          65     62    -3     
=====================================
- Hits           65     62    -3

Impacted Files	Coverage Δ
src/resolver.js	100% <ø> (ø)	⬆️
src/util.js	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6a3869d...22bc170. Read the comment docs.

[vmx]

Thanks, the changes look good. Though I don't think we should change the field names. The reason to have the current names is to match the Go implementation: https://github.com/ipfs/go-ipld-zcash/blob/master/block.go

Hence I would also only return those fields on a tree() call, so that both implementations are equal.

[rvagg]

I've added aliases for timestamp, reserved and tx for compatibility purposes, even though I think all 3 are wrong and we should stick to compatibility with the Zcash API instead.

I haven't touched tree() because it'd be crippling what it's possible to retrieve. Are you suggesting we should omit certain properties that are available? Would that break the intended purpose of tree()? And is that worth it just because the Go version can't read all of the available fields?

[vmx]

I've added aliases for timestamp, reserved and tx for compatibility purposes, even though I think all 3 are wrong and we should stick to compatibility with the Zcash API instead.

Thanks!

I haven't touched tree() because it'd be crippling what it's possible to retrieve. Are you suggesting we should omit certain properties that are available? Would that break the intended purpose of tree()? And is that worth it just because the Go version can't read all of the available fields?

Good point. Let's keep tree() as it is, as then it's rather a bug on Go implementation.

[vmx]

So if you push it, I'm happy to approve it.

[alanshaw]

LGTM, thanks @rvagg ❤️

[vmx]

Thanks! Fix it up and merge!

[rvagg]

merged, this would normally justify semver-major bump, can we do that or are we stuck under 0? if not, a semver-minor would at least send a signal.

[vmx]

I prefer pushing it as 0.x.0. I forgot again that a "BREAKING CHANGES" commit would've made sense so that the changelog contains that information automatically. Well, I'll now do it manually.

[rvagg]

You could force push in an amendment, it hasn't been long since I merged. Does it just need "BREAKING CHANGES" in the summary text to be picked up by some tool?

[alanshaw]

AEgir picks it up in the commit footer and will add it to the changelog on release. https://github.com/ipfs/community/blob/master/CONTRIBUTING_JS.md#footer

This should really be documented in the AEgir README and linked to from there...

[vmx]

If you want to have it in the commit, the cleanest way if you don't want to do force pushes is reverting the commit, and committing it again with the changed commit message. Reverts don't show up in the changelog (I've done that before). Anyway, I now just created the Changelog entry manually.

[vmx]

I thought the GitHub release notes pick up the changelog, but they don't. So my recommendation for forgetting to add "BREAKING CHANGES" is to do the revert and re-commit dance.

[rvagg]

I consider force pushes on low traffic repos not a very big evil, there's a vaishingly small chance that anyone outside of us two even have the current HEAD chain. Up to you though.