celestia: pass submit options #31
Conversation
WalkthroughThe 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- go.mod
Files selected for processing (1)
- celestia.go (1 hunks)
Additional comments: 2
celestia.go (2)
77-85: The
Submitmethod has been correctly updated to include theoptions *da.SubmitOptionsparameter, which is used to pass fee and gas limit information to theclient.Blob.Submitcall. Ensure that there is proper validation for theoptionsparameter to handle cases where it might benilor contain invalid values.77-77: Ensure that the new
optionsparameter is documented in the method comment, and verify that there are tests covering the new functionality introduced by this parameter.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- go.mod
- go.sum
Files selected for processing (1)
- celestia.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- celestia.go
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- go.mod
- go.sum
Files selected for processing (1)
- celestia_test.go (2 hunks)
Additional comments: 3
celestia_test.go (3)
48-51: The image tag has been updated to "76be922". Please ensure that this tag points to a compatible and intended version of the image.
78-78: The command executed within the container has been changed to use the new binary "/bin/celestia-da". Please verify that the new binary is compatible and functions as expected within the test environment.
83-83: Trimming white spaces from the token is a good practice to avoid parsing issues. This change enhances the robustness of the code.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #31 +/- ##
==========================================
- Coverage 71.52% 69.79% -1.73%
==========================================
Files 2 2
Lines 144 149 +5
==========================================
+ Hits 103 104 +1
- Misses 28 31 +3
- Partials 13 14 +1 ☔ View full report in Codecov by Sentry. |
tuxcanfly
force-pushed
the
tux/submit-options
branch
from
c905a01 to
0a0c152
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- go.mod
- go.sum
Files selected for processing (2)
- celestia.go (1 hunks)
- celestia_test.go (2 hunks)
Additional comments: 4
celestia_test.go (2)
78-78: The change from
"/bin/celestia"to"/bin/celestia-da"is consistent with the PR's objective to update the test suite to reflect changes in the binary naming.83-83: Trimming white spaces from the
t.tokenassignment is a good practice to ensure that tokens are handled correctly in subsequent use.celestia.go (2)
77-77: The
Submitfunction has been correctly updated to accept an additional parameterdaOptionsof type*da.SubmitOptions. This change aligns with the PR objectives to allow customization of the submission process.82-86: The handling of the
daOptionsparameter to set theFeeandGasLimitis correctly implemented, allowing users to customize the submission process.
tuxcanfly
force-pushed
the
tux/submit-options
branch
from
0a0c152 to
8015bee
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- go.mod
- go.sum
Files selected for processing (2)
- celestia.go (1 hunks)
- celestia_test.go (2 hunks)
Additional comments: 4
celestia_test.go (2)
78-78: The change in the command executed within the container is consistent with the PR objectives.
83-83: Trimming white spaces from the
t.tokenvalue is a good practice to ensure clean data handling.celestia.go (2)
77-90: The addition of the
daOptionsparameter to theSubmitfunction allows for customizable submission options, aligning with the PR objectives.82-86: Properly setting
blobOptionsusing thedaOptionsproperties ensures that the submission options are applied as intended.
Manav-Aggarwal
force-pushed
the
tux/submit-options
branch
from
76a73f5 to
26afb07
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- go.mod
- go.sum
Files selected for processing (2)
- celestia.go (1 hunks)
- celestia_test.go (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- celestia.go
- celestia_test.go
tuxcanfly
force-pushed
the
tux/submit-options
branch
from
26afb07 to
60dc9ed
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- go.mod
- go.sum
Files selected for processing (2)
- celestia.go (1 hunks)
- celestia_test.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- celestia.go
Additional comments: 3
celestia_test.go (3)
48-48: The image tag "0dd21fb" used in
pool.Runshould be verified to ensure it corresponds to the correct and intended version of thelocal-celestia-devnetDocker image.78-78: The command executed within the container has been updated to
"/bin/celestia-da". Ensure this change aligns with the new functionality and the executable name change is intentional and correct.83-83: Trimming white spaces from
t.tokenis a good practice to avoid issues with token handling in subsequent requests.
tuxcanfly
force-pushed
the
tux/submit-options
branch
from
60dc9ed to
dbb6d7a
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- go.mod
- go.sum
Files selected for processing (2)
- celestia.go (1 hunks)
- celestia_test.go (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- celestia.go
- celestia_test.go
tuxcanfly
force-pushed
the
tux/submit-options
branch
from
a760002 to
1acad54
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- go.mod
- go.sum
Files selected for processing (3)
- celestia/celestia.go (1 hunks)
- celestia/celestia_test.go (2 hunks)
- celestia_test.go (2 hunks)
Additional comments: 5
celestia_test.go (2)
- 78-78: The command executed within the container has been changed to
/bin/celestia-da bridge auth admin --node.store /home/celestia/bridge. Verify that this command is correct and that the binarycelestia-daexists at the specified path in the container.Verification successful
The Dockerfile in the repository includes a step that copies the
celestia-dabinary to/bin/celestia-dain the final container image. This suggests that the binary should indeed exist at the specified path within the container, which aligns with the changes made in thecelestia_test.gofile. However, without the ability to execute Docker commands in the script environment, we cannot directly verify the binary's executability or the container's configuration to run the specified command.* 83-83: Trimming leading or trailing white spaces from the buffer content before assigning to `t.token` is a good practice to avoid unexpected issues due to white space.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the binary exists in the container and the command is executable. # Note: This script assumes that the container is running and accessible. docker exec $(docker ps -q --filter ancestor=ghcr.io/rollkit/local-celestia-devnet:0dd21fb) ls /bin/celestia-daLength of output: 261
Script:
#!/bin/bash # Attempt to find the Dockerfile or build scripts for the container image. # Since we don't know the exact location or name, we'll search for common patterns. fd Dockerfile --exec cat {} \; fd build.sh --exec cat {} \; fd setup.sh --exec cat {} \;Length of output: 1488
celestia/celestia_test.go (2)
82-82: The
Submitfunction now correctly passesnilfor the newdaOptionsparameter in the test caseSubmit_empty. This change is consistent with the updated function signature.121-121: The
Submitfunction also correctly passesnilfor the newdaOptionsparameter in the test caseSubmit_existing. This change is consistent with the updated function signature.celestia/celestia.go (1)
- 77-87: The
Submitfunction has been updated to accept a new parameterdaOptions. The code correctly checks ifdaOptionsis notnilbefore setting theFeeandGasLimitinblobOptions. This is a good practice to avoid null pointer dereferences.
tuxcanfly
force-pushed
the
tux/submit-options
branch
from
1acad54 to
054a046
Compare
tuxcanfly
force-pushed
the
tux/submit-options
branch
from
054a046 to
33156dd
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- go.mod
- go.sum
Files selected for processing (3)
- celestia/celestia.go (1 hunks)
- celestia/celestia_test.go (2 hunks)
- celestia_test.go (2 hunks)
Files skipped from review as they are similar to previous changes (3)
- celestia/celestia.go
- celestia/celestia_test.go
- celestia_test.go
| height, err := c.client.Blob.Submit(c.ctx, blobs, blob.DefaultSubmitOptions()) | ||
| blobOptions := blob.DefaultSubmitOptions() | ||
| if daOptions != nil { | ||
| blobOptions.Fee = daOptions.Fee |
|
Will be superseded by #46 |
|
Closing in favor of #46 |
Overview
This PR updates go-da to v0.0.1 which includes a breaking change in the DA interface.
The
Submitmethod now accepts a second param:SubmitOptions, which can benilto default toDefaultSubmitOptionsas before.SubmitOptionswhich is non-nil and includesGasandFeeparams will be passed through to the node.This PR adds
SubmitOptionsto theSubmitmethod.Checklist
Depends on:
Summary by CodeRabbit
New Features
Submitfunction to accept transaction fee and gas limit options.Refactor
Submitfunction parameters.Bug Fixes