Skip to content
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

Feat: Create dart implementation of multihash for use in dart_cid #1

Closed
6 tasks done
nelsonic opened this issue Nov 25, 2022 · 7 comments
Closed
6 tasks done
Assignees
Labels
awaiting-review An issue or pull request that needs to be reviewed documentation Improvements or additions to documentation enhancement New feature or enhancement of existing functionality good first issue Good for newcomers help wanted If you can help make progress with this issue, please comment! priority-2 Second highest priority, should be worked on as soon as the Priority-1 issues are finished research Research required; be specific T1d Time Estimate 1 Day technical A technical issue that requires understanding of the code, infrastructure or dependencies user-feedback Feedback from people using the App

Comments

@nelsonic
Copy link
Member

nelsonic commented Nov 25, 2022

As noted in /dwyl/learn-ipfs we believe that distributed networks are the future.
At the time of writing there is no dart implementation of multihash listed in: github.com/multiformats/multihash/#implementations

Our objective is to create one so that we can use it in our Flutter Project.
Putting in the effort up-front to do create a well-documented, tested & maintained package
will help save other people building "DApps" time.

Todo ✅

  • Create World Class Documentation for how to use dart_multihash in a dart or Flutter project up-front. 📝
  • Write tests for the function with reference to the Go, Rust or JS implementation (a known working example) 🧪
  • Implement the code with all dart best practices for inline docs 🧑‍💻
  • Publish the package to pub.dev under the @dwyl namespace/ownership for business continuity. 📦
  • Open Pull Request to https://github.com/multiformats/multihash#implementations to add to list 🚀

Note: it's not a truly Decentralised App if people have to get it from a proprietary App Store ... but you get the idea. 💭

https://pub.dev/packages/multihash
image

https://pub.dev/packages?q=multihash
image

@nelsonic nelsonic added enhancement New feature or enhancement of existing functionality good first issue Good for newcomers help wanted If you can help make progress with this issue, please comment! documentation Improvements or additions to documentation research Research required; be specific T1d Time Estimate 1 Day technical A technical issue that requires understanding of the code, infrastructure or dependencies priority-2 Second highest priority, should be worked on as soon as the Priority-1 issues are finished labels Nov 25, 2022
@nelsonic nelsonic changed the title Feat: Create dart implementation of multihash for us in dart_cid Feat: Create dart implementation of multihash for use in dart_cid Nov 25, 2022
LuchoTurtle added a commit that referenced this issue Nov 25, 2022
LuchoTurtle added a commit that referenced this issue Nov 25, 2022
Added constants for supported hash functions. Added encoding function that uses varint encoding.
LuchoTurtle added a commit that referenced this issue Nov 25, 2022
LuchoTurtle added a commit that referenced this issue Nov 25, 2022
LuchoTurtle added a commit that referenced this issue Nov 25, 2022
LuchoTurtle added a commit that referenced this issue Nov 25, 2022
LuchoTurtle added a commit that referenced this issue Nov 25, 2022
Using `listEquals` that is enough in our case, since we are comparing first-level arrays (array of bytes) and not objects.
LuchoTurtle added a commit that referenced this issue Nov 29, 2022
@nelsonic
Copy link
Member Author

@LuchoTurtle the package should be called just multihash not dart_multihash
We don't need the word dart in any of the filenames:

dart pub publish --dry-run

Publishing dart_multihash 0.0.1 to https://pub.dartlang.org:
|-- .github
|   '-- ci.yml
|-- .gitignore
|-- CHANGELOG.md
|-- LICENSE
|-- README.md
|-- analysis_options.yaml
|-- dart_multihash.iml
|-- lib
|   |-- dart_multihash.dart
|   '-- src
|       |-- constants.dart
|       |-- models.dart
|       '-- varintUtils.dart
|-- pubspec.yaml
'-- test
    |-- dart_multihash_test.dart
    '-- dart_varint_test.dart

Please rename lib/dart_multihash.dart to lib/multihash.dart etc. (before we publish to pub.dev...) 🙏

@nelsonic nelsonic added the user-feedback Feedback from people using the App label Nov 29, 2022
@LuchoTurtle
Copy link
Member

We shouldn't change it to multihash because we should follow the multiformat's convention. Every implementation has the language suffixed and I believe we should maintain this so we could successfully add our own to the list.

@nelsonic
Copy link
Member Author

Good spot. 👍
Reluctantly. Agree. 👌
Please feel free to publish so you can continue making progress. :shipit:
Then when you feel it's "stable" (V1.0), transfer to the newly created @dwyl verified publisher dwyl/learn-dart#22 (comment) 📦

@nelsonic
Copy link
Member Author

@LuchoTurtle if you need to push directly to main to figure out the CI please do it. :shipit:
This project is in "setup phase" so it's acceptable. 🔧
Once it's published we will protect main branch to avoid destruction. 🔥

@LuchoTurtle
Copy link
Member

LuchoTurtle commented Nov 30, 2022

The package has been published and a PR has been opened to add it to the multiformats canonical repo.
Submitting for review so @nelsonic can close it.

@LuchoTurtle LuchoTurtle added the awaiting-review An issue or pull request that needs to be reviewed label Nov 30, 2022
@LuchoTurtle LuchoTurtle assigned nelsonic and unassigned LuchoTurtle Nov 30, 2022
@nelsonic
Copy link
Member Author

@LuchoTurtle stoked you got your implementation published and PR adding it to the list merged. 🎉

@nelsonic
Copy link
Member Author

Well done again. 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review An issue or pull request that needs to be reviewed documentation Improvements or additions to documentation enhancement New feature or enhancement of existing functionality good first issue Good for newcomers help wanted If you can help make progress with this issue, please comment! priority-2 Second highest priority, should be worked on as soon as the Priority-1 issues are finished research Research required; be specific T1d Time Estimate 1 Day technical A technical issue that requires understanding of the code, infrastructure or dependencies user-feedback Feedback from people using the App
Projects
Status: Done
Development

No branches or pull requests

2 participants