Livenet tests now run on local docker node and in CI #352
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThe changes involve the addition of a new service Changes
Related issues
Poem
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 (
|
596b38e to
71cf5ee
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (4)
- .github/workflows/odra-ci.yml (2 hunks)
- examples/bin/erc20_on_livenet.rs (3 hunks)
- examples/bin/livenet_tests.rs (2 hunks)
- justfile (1 hunks)
Additional comments: 9
.github/workflows/odra-ci.yml (2)
- 21-30: Ensure the
DEPLOY_DELAYenvironment variable is appropriately configured for the expected deployment times in your CI environment to avoid potential timeouts or race conditions.- 43-43: Confirm that
just test-livenetis integrated correctly with the rest of the CI pipeline, especially in terms of dependencies between steps and potential impacts on test execution times.examples/bin/erc20_on_livenet.rs (1)
- 15-15: Renaming
deploy_newtodeploy_erc20improves clarity regarding the function's purpose. Ensure all references to this function are updated accordingly.examples/bin/livenet_tests.rs (4)
- 3-6: The addition of
U256andErc20InitArgsimports is appropriate for the updated functionality. Ensure these are used correctly throughout the script.- 14-16: The update to deploy and log both
contractanderc20in themainfunction enhances the test's comprehensiveness. Verify that the gas settings are optimized for the deployment and interaction with these contracts.- 58-66: The
deploy_newfunction's update to return a tuple of(LivenetContractHostRef, Erc20HostRef)and deploy an ERC20 contract is logical. Ensure that thetransfermethod call is correctly handling errors and exceptions.- 80-94: The
deploy_erc20function is well-implemented for deploying ERC20 contracts with specified parameters. Confirm that the gas limit set is sufficient and optimized for the deployment of these contracts.justfile (2)
- 74-89: The
test-livenettarget is correctly set up for running tests against a local Casper node. Ensure that the sleep duration in line 81 is sufficient for the node to start and stabilize before proceeding with tests.- 89-89: The
run-example-erc20-on-livenettarget description is missing in the provided context. Confirm that the modifications align with the objectives of running ERC20 examples on the live network.
d2f3368 to
74ae89a
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (5)
- .github/workflows/odra-ci.yml (2 hunks)
- examples/bin/erc20_on_livenet.rs (3 hunks)
- examples/bin/livenet_tests.rs (2 hunks)
- examples/src/lib.rs (1 hunks)
- justfile (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- .github/workflows/odra-ci.yml
- examples/bin/erc20_on_livenet.rs
- justfile
Additional comments: 6
examples/bin/livenet_tests.rs (6)
- 1-6: Imports are correctly added for
U256andErc20InitArgs. Ensure these imports are used effectively within the file.- 14-16: The
deploy_newfunction is called with&env, which is correct. However, ensure that the gas limit set withenv.set_gas(30_000_000_000u64);is appropriate for the deployment of both contracts.- 43-95: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [19-52]
The logic for deploying, loading, and interacting with contracts appears sound. However, ensure that the gas limits set (e.g.,
env.set_gas(1_000_000_000u64);) are sufficient for the operations performed.
- 58-66: The
deploy_newfunction correctly deploys an ERC20 contract and a Livenet contract, linking them via theerc20_address. Ensure that the gas limit set before deploying the Livenet contract is appropriate.- 69-77: The
loadfunction is updated to accept anerc20_addressparameter, correctly loading both contract references. This change is logical and maintains consistency.- 80-94: The
deploy_erc20function is well-structured, with clear initialization arguments for deploying an ERC20 contract. Ensure that the gas limit set is sufficient for the deployment.
74ae89a to
5e07331
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (5)
- .github/workflows/odra-ci.yml (2 hunks)
- examples/bin/erc20_on_livenet.rs (3 hunks)
- examples/bin/livenet_tests.rs (2 hunks)
- examples/src/lib.rs (1 hunks)
- justfile (2 hunks)
Files skipped from review as they are similar to previous changes (3)
- .github/workflows/odra-ci.yml
- examples/bin/erc20_on_livenet.rs
- examples/src/lib.rs
Additional comments: 9
examples/bin/livenet_tests.rs (6)
- 1-6: Imports are correctly added for
U256andErc20InitArgs. Ensure these are used appropriately in the subsequent code.- 14-16: The
set_gascall before deploying contracts is a good practice for ensuring sufficient gas. However, verify that the gas limit is aligned with the expected cost of deploying ERC20 contracts on the Casper network.- 43-95: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [19-52]
The logic for deploying, loading contracts, and executing various contract calls is clear and follows a logical sequence. Ensure that the gas limits set are appropriate for the operations being performed, especially for mutable calls and crosscalls.
- 58-66: The
deploy_newfunction correctly deploys an ERC20 contract and initializes aLivenetContractwith its address. Ensure that thetransfercall to seed theLivenetContractwith tokens is intended and that the amount1000.into()aligns with test requirements.- 69-77: The
loadfunction's update to accept anerc20_addressparameter and return both contract references is correctly implemented. This change supports the enhanced testing capabilities by allowing both contract and ERC20 instances to be loaded for interaction.- 80-94: The
deploy_erc20function is well-structured, with clear initialization arguments for deploying an ERC20 contract. Ensure that theset_gascall before deployment is aligned with the expected deployment cost on the Casper network.justfile (3)
- 42-44: The
start-casper-nodetarget correctly uses Docker to run a Casper node. Ensure that the ports exposed match the Casper node's requirements and that the Docker imagemakesoftware/casper-nctlis the correct and latest version for testing purposes.- 77-87: The
test-livenettarget is well-structured for setting up a local Casper node, extracting secret keys, and running tests. Ensure that the paths and commands used for interacting with Docker and setting environment variables are correct and secure. Also, verify that the cleanup steprm -rf examples/.node-keysadequately addresses any potential security concerns regarding the handling of secret keys.- 87-87: The modification to
run-example-erc20-on-livenetdoes not show any changes in the provided code. If there were intended modifications, ensure they are correctly implemented and aligned with the objectives of running ERC20 examples on the live network.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release/0.9.0 #352 +/- ##
==============================================
Coverage 80.72% 80.72%
==============================================
Files 127 127
Lines 13902 13902
==============================================
Hits 11223 11223
Misses 2679 2679 ☔ View full report in Codecov by Sentry. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes: #188
Summary by CodeRabbit