-
Notifications
You must be signed in to change notification settings - Fork 23
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!: GetProofs introduction #38
Conversation
WalkthroughThe recent update introduces the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #38 +/- ##
==========================================
- Coverage 79.77% 78.01% -1.77%
==========================================
Files 3 3
Lines 178 191 +13
==========================================
+ Hits 142 149 +7
- Misses 28 32 +4
- Partials 8 10 +2 ☔ View full report in Codecov by Sentry. |
13f9042
to
7886008
Compare
* adding namespaces * changing submitoptions to gasprice,ns * adding namespace to remaining endpoints * updating readme w note * nit: lint md013 - skip tables --------- Co-authored-by: Javed Khan <tuxcanfly@gmail.com> updating readme w note feat: GetProofs introduction updating readme
7886008
to
760dfcf
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
types/pb/da/da.pb.go
is excluded by:!**/*.pb.go
Files selected for processing (7)
- README.md (1 hunks)
- da.go (1 hunks)
- proto/da/da.proto (3 hunks)
- proxy/client.go (3 hunks)
- proxy/server.go (1 hunks)
- test/dummy.go (2 hunks)
- test/test_suite.go (4 hunks)
Additional comments: 14
da.go (2)
- 19-21: The addition of the
GetProofs
method aligns with the PR objectives to separate the retrieval of proofs from theSubmit
method. This change enhances flexibility and compliance with the JSON-RPC specification.- 27-29: The modification to the
Submit
method, which now returns only the IDs identifying blobs in the DA, is correctly implemented. This change simplifies the method's return type, improving language compatibility and user flexibility as intended.proxy/server.go (2)
- 59-66: The implementation of the
GetProofs
method in theproxySrv
struct correctly delegates the call to the underlyingDA
interface'sGetProofs
method. This ensures that the proxy server can retrieve proofs as per the new interface requirements.- 71-77: The modification to the
Submit
method, removing theproofs
return value and its associated logic, is correctly implemented. This change aligns with the PR objectives to streamline theSubmit
method's return type.README.md (1)
- 20-24: The documentation in the README.md file has been updated to include the new
GetProofs
method and modify theSubmit
method's return type. These updates accurately reflect the changes made to the interface, ensuring that the documentation remains consistent with the codebase.proto/da/da.proto (2)
- 15-17: The addition of the
GetProofs
RPC method to theDAService
service is correctly defined. This change enables the retrieval of inclusion proofs for all blobs at a given height, aligning with the PR objectives.- 84-93: The corresponding request and response message types for the
GetProofs
method (GetProofsRequest
andGetProofsResponse
) are correctly defined. These message types facilitate the correct exchange of data for the retrieval of proofs.proxy/client.go (2)
- 78-87: The implementation of the
GetProofs
method in theClient
struct correctly interacts with the gRPC client to retrieve proofs from the DA layer. This addition is consistent with the interface changes and the PR objectives.- 111-125: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [105-122]
The modification to the
Submit
method, removing the[]da.Proof
slice from the return signature, is correctly implemented. This change reflects the updated interface requirements and simplifies the method's return type.test/dummy.go (2)
- 93-103: The implementation of the
GetProofs
method in theDummyDA
struct correctly simulates the retrieval of inclusion proofs for all blobs at a given height. This method is essential for testing the new interface functionality.- 115-126: The modification to the
Submit
method, removing the return of proofs and the namespace parameter, is correctly implemented. This change aligns with the updated interface and simplifies the method's functionality for testing purposes.test/test_suite.go (3)
- 63-81: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [39-78]
The
BasicDATest
function has been updated to remove proof variables from theSubmit
function calls and to separate the retrieval and validation of proofs into distinct steps. These changes correctly reflect the updated interface and ensure that the tests remain valid.
- 114-114: In the
GetIDsTest
function, the removal of theproofs
variable from theSubmit
function's return values and the adjustment of corresponding assertions are correctly implemented. This change aligns with the interface modifications.- 167-167: The
ConcurrentReadWriteTest
function correctly removes theproof
variable from theSubmit
function call, aligning with the updated interface. This ensures that the test accurately reflects the changes made to theSubmit
method.
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
types/pb/da/da.pb.go
is excluded by:!**/*.pb.go
Files selected for processing (7)
- README.md (1 hunks)
- da.go (1 hunks)
- proto/da/da.proto (3 hunks)
- proxy/client.go (3 hunks)
- proxy/server.go (1 hunks)
- test/dummy.go (2 hunks)
- test/test_suite.go (4 hunks)
Files skipped from review as they are similar to previous changes (7)
- README.md
- da.go
- proto/da/da.proto
- proxy/client.go
- proxy/server.go
- test/dummy.go
- test/test_suite.go
This PR is stacked on top of #35, so keeping in draft.
The contents of this PR:
There are three reasons for the removal of proofs as a return value from Submit and the introduction of the GetProofs method:
Summary by CodeRabbit
GetProofs
method to obtain inclusion proofs for blobs at a specific blockchain height.Submit
method to return only blob IDs upon successful submission, removing the inclusion proofs from the return type.