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.

Sign up for GitHub

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

Jump to bottom

tests: round trip test for blob submission/retrieval #28

Merged
merged 2 commits into from
Jul 5, 2023
Merged

tests: round trip test for blob submission/retrieval #28

merged 2 commits into from
Jul 5, 2023

Conversation

tzdybal
Copy link
Member

@tzdybal tzdybal commented Jul 5, 2023 •
edited
Loading

Overview

Basic round trip test that submits a blob and then reads it back from DA.

Resolves #27.

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

@tzdybal tzdybal force-pushed the tzdybal/round_trip_test branch 3 times, most recently from a0a70b7 to 11423e4 Compare July 5, 2023 20:08
@tzdybal tzdybal mentioned this pull request Jul 5, 2023
Merged
5 tasks
Base automatically changed from tux/submit-pay-for-blob-api to main July 5, 2023 20:09
@tzdybal tzdybal marked this pull request as ready for review July 5, 2023 20:09
@tzdybal tzdybal changed the base branch from main to tux/shares-err-not-available July 5, 2023 20:10
@tzdybal tzdybal changed the base branch from tux/shares-err-not-available to main July 5, 2023 20:10
@tzdybal tzdybal requested a review from a team July 5, 2023 20:11
@tzdybal tzdybal enabled auto-merge July 5, 2023 20:11
tuxcanfly
tuxcanfly previously approved these changes Jul 5, 2023
View reviewed changes
Copy link
Contributor

@tuxcanfly tuxcanfly left a comment

Choose a reason for hiding this comment

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

OK.

client_test.go Show resolved Hide resolved
nashqueue
nashqueue approved these changes Jul 5, 2023
View reviewed changes
Copy link
Member

@nashqueue nashqueue left a comment

Choose a reason for hiding this comment

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

In a follow-up pr, we should calculate the commitment of the blob and test it against it. That means:

  • create commitment
  • get()
  • getProof()
  • Included(Proof)

This should check all the methods relevant to us

@nashqueue
Copy link
Member

Another follow-up is to test the submission of multiple blobs.

  1. 2 blobs the same namespace
  2. 2 blob different namespaces

@nashqueue
Copy link
Member

Another test is to ask for a namespace with no blob inside. Basically to catch if the error handling changes.

@tzdybal tzdybal added this pull request to the merge queue Jul 5, 2023
Merged via the queue into main with commit 8ac4b06 Jul 5, 2023
9 checks passed
@tzdybal tzdybal deleted the tzdybal/round_trip_test branch July 5, 2023 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gupadhyaya gupadhyaya gupadhyaya approved these changes

@Wondertan Wondertan Wondertan left review comments

@nashqueue nashqueue nashqueue approved these changes

@tuxcanfly tuxcanfly tuxcanfly left review comments

Assignees

@tzdybal tzdybal

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Roundtrip Test for OpenRPC
5 participants
@tzdybal @nashqueue @tuxcanfly @gupadhyaya @Wondertan