-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add hash validation and support for HTTP(S) and IPFS to command hash anchor-data
#895
Conversation
44caf5e
to
cf9cf24
Compare
eeb9869
to
02d8367
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Quite elegant solution.
11ea69c
to
dda90ab
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 . Couple of comments and have somebody from devx look at your nix changes.
dda90ab
to
e7e8a8a
Compare
47d4f27
to
4f7cdac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
0d48517
to
d0df4c0
Compare
Changelog
Context
This PR is a first step towards solving: #882
But it doesn't completely address it because the check should be in the commands mentioned and not in a separate command. That will be addressed in a separate PR.
How to trust this PR
The tests should provide a lot of assurance. I think the most unverified bit is the URL structure for IPFS_GATEWAYS, but I have tried it against existing gateways and seems to work. Other than that, code style, and maybe code organisation? We may need to move the verification code somewhere else in order to be able to use it in the next PR.
The PR adds quite a few packages, but it was mainly for testing. For querying it uses
http-client
(andhttp-client-tls
for HTTPS support).Checklist