Skip to content
This repository has been archived by the owner on Aug 11, 2021. It is now read-only.

Add ethereum resolvers #78

Merged
merged 4 commits into from
Feb 11, 2017
Merged

Add ethereum resolvers #78

merged 4 commits into from
Feb 11, 2017

Conversation

kumavis
Copy link
Contributor

@kumavis kumavis commented Feb 9, 2017

replaces #71

@jbenet jbenet added the status/in-progress In progress label Feb 9, 2017
@kumavis
Copy link
Contributor Author

kumavis commented Feb 9, 2017

The security failure is from a dep of aegir in the various ipld resolvers -- will bump

@daviddias daviddias mentioned this pull request Feb 10, 2017
22 tasks
Copy link
Member

@daviddias daviddias left a comment

Choose a reason for hiding this comment

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

Could you add an extra test that adds the block through the block-service and fetches it with the resolver?

const expect = require('chai').expect
const rlp = require('rlp')
const BlockService = require('ipfs-block-service')
const cidForHash = require('ipld-eth-block/src/common').cidForHash
Copy link
Member

Choose a reason for hiding this comment

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

This is an anti-pattern, let's make avoid using paths and dots in conjunction. (preferably just dots)

Also, seems that cidForHash is used to calculate all the CIDs, can we have this as a module on npm eth-hash-to-cid, ideally even exports a CLI so that it is super easy to use.

Copy link
Member

Choose a reason for hiding this comment

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

@daviddias daviddias merged commit 9d1f98e into master Feb 11, 2017
@daviddias daviddias deleted the eth-resolvers branch February 11, 2017 22:41
@daviddias daviddias removed the status/in-progress In progress label Feb 11, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants