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

chore: removes the usage of the merkletree package #126

Merged
merged 3 commits into from
Mar 9, 2023

Conversation

staheri14
Copy link
Contributor

@staheri14 staheri14 commented Mar 8, 2023

Overview

Inline with #125.
For more context, please see the following comment.

Checklist

  • New and updated code has appropriate documentation
  • New and updated code has new and/or updated testing
  • Required CI checks are passing
  • Visual proof for any user facing features like CLI or documentation updates
  • Linked issues closed with keywords

@staheri14 staheri14 self-assigned this Mar 8, 2023
@staheri14 staheri14 added the enhancement New feature or request label Mar 8, 2023
@codecov
Copy link

codecov bot commented Mar 8, 2023

Codecov Report

Merging #126 (f9f65fa) into master (e8ad2aa) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #126   +/-   ##
=======================================
  Coverage   96.33%   96.33%           
=======================================
  Files           6        6           
  Lines         464      464           
=======================================
  Hits          447      447           
  Misses         11       11           
  Partials        6        6           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@staheri14 staheri14 marked this pull request as ready for review March 9, 2023 00:13
Copy link
Collaborator

@rootulp rootulp left a comment

Choose a reason for hiding this comment

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

Awesome, this LGTM after we remove the first line from

nmt/README.md

Lines 98 to 99 in e8ad2aa

This implementation (currently) uses NebulousLabs' [merkletree][NebulousLabs'] implementation
and was heavily inspired by the initial implementation in the celestiaorg [prototype].

liamsi
liamsi previously approved these changes Mar 9, 2023
Copy link
Member

@liamsi liamsi left a comment

Choose a reason for hiding this comment

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

👏🏼 Nice!
Agree on this: #126 (review)

@liamsi liamsi dismissed their stale review March 9, 2023 15:44

readme needs to be updated 1st

@staheri14
Copy link
Contributor Author

Awesome, this LGTM after we remove the first line from

nmt/README.md

Lines 98 to 99 in e8ad2aa

This implementation (currently) uses NebulousLabs' [merkletree][NebulousLabs'] implementation
and was heavily inspired by the initial implementation in the celestiaorg [prototype].

Sure, good point, please see the revised version in f9f65fa.

@staheri14 staheri14 requested a review from rootulp March 9, 2023 15:49
@staheri14 staheri14 merged commit 8be15c4 into master Mar 9, 2023
@staheri14 staheri14 deleted the removes-merkletree-dep branch March 9, 2023 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants