ipld / js-ipld-ethereum

JavaScript Implementation of All Ethereum IPLD formats
MIT License
40 stars 11 forks source link

feat: new IPLD Format API #52

Closed vmx closed 5 years ago

vmx commented 5 years ago

BREAKING CHANGE: The API is now async/await based

There are numerous changes, the most significant one is that the API is no longer callback based, but it using async/await.

For the full new API please see the IPLD Formats spec.

vmx commented 5 years ago

@rvagg This PR is based on the work at https://github.com/ipld/js-ipld-ethereum/pull/51 but it's now using the approach you proposed. I'm not just creating new objects and store the original object in a property called _ethObj.

I also improved the tests to check for the types of the fields when they are resolved.

It would be great if you could prioritise the review of this PR.

codecov[bot] commented 5 years ago

Codecov Report

Merging #52 into master will increase coverage by 2.52%. The diff coverage is 99.26%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #52      +/-   ##
==========================================
+ Coverage   96.84%   99.36%   +2.52%     
==========================================
  Files          22       19       -3     
  Lines         792      791       -1     
==========================================
+ Hits          767      786      +19     
+ Misses         25        5      -20
Impacted Files Coverage Δ
eth-state-trie/index.js 100% <100%> (ø) :arrow_up:
eth-tx/index.js 100% <100%> (ø) :arrow_up:
eth-block/index.js 100% <100%> (ø) :arrow_up:
eth-tx-trie/index.js 100% <100%> (ø) :arrow_up:
util/createUtil.js 100% <100%> (ø) :arrow_up:
eth-account-snapshot/index.js 100% <100%> (ø) :arrow_up:
eth-storage-trie/index.js 100% <100%> (ø) :arrow_up:
eth-block/test/resolver.spec.js 100% <100%> (+0.78%) :arrow_up:
util/cidFromHash.js 100% <100%> (ø) :arrow_up:
util/emptyCodeHash.js 100% <100%> (ø) :arrow_up:
... and 9 more

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 4eaa791...34263b6. Read the comment docs.

rvagg commented 5 years ago

tangential: why are we persisting with the term "ommer" when eth seems to be preferring "uncle" now and even the ethereumjs library looks like it's using "uncle" but we're turning it back in to "ommer". I don't know if this matters but it seems like an odd choice to be making here.

vmx commented 5 years ago

@rvagg About this "ommers" thing. I just didn't want to break backwards compatibility, it's also in the old code: https://github.com/ipld/js-ipld-ethereum/blob/4eaa7915c0842aac904553c47776fe87a2205bfb/eth-block/index.js#L40

vmx commented 5 years ago

New forced push (I always forget that GitHub doesn't notify on new pushes).

rvagg commented 5 years ago

yeah, I know the ommers thing is old, just wondering if anyone listening to this has context, just out of interest. This lgtm

vmx commented 5 years ago

I just force-pushed a better commit message.