-
Notifications
You must be signed in to change notification settings - Fork 925
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!(rpc): implementing go-da interface #3146
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #3146 +/- ##
==========================================
+ Coverage 51.88% 52.06% +0.18%
==========================================
Files 178 178
Lines 11262 11284 +22
==========================================
+ Hits 5843 5875 +32
+ Misses 4918 4908 -10
Partials 501 501 ☔ View full report in Codecov by Sentry. |
kind of a lot going on here and i realize it's mostly just copied over but....main question from me before getting into weeds is: "is da and the Only other thing that jumps out at me and could use a comment before deeper review is the Marshall func on blob...i vaguely sense we maybe had a bug around this not that long ago.... |
@Wondertan and I discussed naming before I started this PR because I had the same concerns. It is very confusing to have both "da" and "das" package. But we came to the conclusion it's the best name for now - but if you have any suggestions I am very open to it, because I hate "da" as well As per the blob proof marshalling: Yes, there was a bug that nmt.Proof was not json serializable. We added json serialization in the nmt package to solve this. Is this what you mean? |
Reminder that the point of this PR is simply to move celestia-da code into node, so that we can debug the existing problems more easily - fixes will be expected, but it needs to get released as soon as possible |
I am OK with calling this da if we change |
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.
utACK
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.
new test fails, can we fix or remove then otherwise 👍
Ahhh it's flaky |
Integrates go-da directly to eliminate the need for [celestia-da](https://github.com/rollkit/celestia-da).
…celestiaorg#3204) Reverts a change introduced in celestiaorg#3146 that unintentionally broke `blob.Proof` serialisation. Resolves celestiaorg#3173
…celestiaorg#3204) Reverts a change introduced in celestiaorg#3146 that unintentionally broke `blob.Proof` serialisation. Resolves celestiaorg#3173
Integrates go-da directly to eliminate the need for celestia-da.