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.

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

Test(wasmd): add basic e2e tests for wasm #187

Merged
merged 3 commits into from
Mar 27, 2024
Merged

Test(wasmd): add basic e2e tests for wasm #187

merged 3 commits into from
Mar 27, 2024

Conversation

tuantran1702
Copy link
Collaborator

@tuantran1702 tuantran1702 commented Mar 27, 2024

Summary by CodeRabbit

  • New Features
    • Added support for .eved files and WebAssembly (.wasm) scripts in .gitignore to improve project confidentiality.
    • Introduced comprehensive guidelines for running end-to-end tests for Cosmwasm contracts.
    • Automated the process of downloading, uploading, and instantiating wasm contract files for enhanced efficiency in blockchain development.
  • Documentation
    • Provided detailed instructions for setting up a local node, downloading, uploading, and instantiating smart contracts on a blockchain network.
  • Chores
    • Streamlined blockchain development workflows with new automation scripts for contract management.

Copy link
Contributor

coderabbitai bot commented Mar 27, 2024

Walkthrough

The recent update introduces a comprehensive suite of tools and guidelines for working with Cosmwasm contracts. It covers everything from setting up a local node and downloading contract files to deploying and interacting with CW20 token contracts on a blockchain network. Additionally, it streamlines the governance process for submitting and managing wasm store proposals. These changes are encapsulated in modifications to the .gitignore file and the addition of scripts and documentation to aid developers in testing and deploying smart contracts efficiently.

Changes

Files Change Summary
.gitignore Added .eved and scripts/wasm/*.wasm
scripts/wasm/README.md Guidelines for Cosmwasm contracts testing, including setup and instantiation instructions
scripts/wasm/get_contract.sh, scripts/wasm/gov_upload_code.sh,
scripts/wasm/instantiate_cw20.sh, scripts/wasm/upload_code.sh
Scripts for contract downloading, governance, instantiation, and deployment processes

🐰✨

In the realm of code, where smart contracts abide,
A rabbit hopped in, with a toolbox wide.
From tests to deployment, it leaped with grace,
Guiding the bytes in their rightful place.
"To the blockchain!" it cheered, with a merry stride,
Leaving trails of wisdom, far and wide.
🚀🌕

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?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit-tests for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit tests for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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: 10

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 5d22450 and 4d4ace0.
Files selected for processing (6)
  • .gitignore (1 hunks)
  • scripts/wasm/README.md (1 hunks)
  • scripts/wasm/get_contract.sh (1 hunks)
  • scripts/wasm/gov_upload_code.sh (1 hunks)
  • scripts/wasm/instantiate_cw20.sh (1 hunks)
  • scripts/wasm/upload_code.sh (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • .gitignore
Additional comments: 2
scripts/wasm/upload_code.sh (1)
  • 9-13: Variables DENOM and TEST0_ADDRESS are defined but appear unused in the script. If these are intended for future use or external scripts, consider documenting their purpose or exporting them. Otherwise, you might want to remove them to clean up the script.
scripts/wasm/gov_upload_code.sh (1)
  • 6-7: Variables VOTER and VAL are defined but appear unused in the script. If these are intended for future use or external scripts, consider documenting their purpose or exporting them. Otherwise, you might want to remove them to clean up the script.

scripts/wasm/get_contract.sh Outdated Show resolved Hide resolved
scripts/wasm/get_contract.sh Outdated Show resolved Hide resolved
scripts/wasm/README.md Outdated Show resolved Hide resolved
scripts/wasm/upload_code.sh Outdated Show resolved Hide resolved
scripts/wasm/upload_code.sh Outdated Show resolved Hide resolved
scripts/wasm/gov_upload_code.sh Outdated Show resolved Hide resolved
scripts/wasm/gov_upload_code.sh Outdated Show resolved Hide resolved
scripts/wasm/instantiate_cw20.sh Outdated Show resolved Hide resolved
scripts/wasm/instantiate_cw20.sh Outdated Show resolved Hide resolved
scripts/wasm/instantiate_cw20.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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: 9

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 4d4ace0 and d8c7510.
Files selected for processing (5)
  • scripts/wasm/README.md (1 hunks)
  • scripts/wasm/get_contract.sh (1 hunks)
  • scripts/wasm/gov_upload_code.sh (1 hunks)
  • scripts/wasm/instantiate_cw20.sh (1 hunks)
  • scripts/wasm/upload_code.sh (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • scripts/wasm/get_contract.sh
Additional comments: 1
scripts/wasm/README.md (1)
  • 1-1: The spelling of "CosmWasm" is correct as per the previous comment. No action needed here.


CONTRACT=cw20_base
HOME=mytestnet
DENOM="ueve"
Copy link
Contributor

Choose a reason for hiding this comment

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

The variable DENOM appears unused in this script. If it's intended for external use, consider exporting it. Otherwise, verify if it's needed.

KEY="test0"

KEYRING="test"
TEST0_ADDRESS=$($BINARY keys show $KEY -a --keyring-backend $KEYRING --home $HOME)
Copy link
Contributor

Choose a reason for hiding this comment

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

The variable TEST0_ADDRESS appears unused. If it's intended for use outside this script, you might want to export it. Otherwise, please check its necessity.


CHAIN_ID=local-eve
VAL_KEY=test0
VOTER=
Copy link
Contributor

Choose a reason for hiding this comment

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

The variable VOTER appears unused in this script. If it's intended for external use, consider exporting it. Otherwise, verify if it's needed.

CHAIN_ID=local-eve
VAL_KEY=test0
VOTER=
VAL=$($BINARY keys show -a $VAL_KEY --keyring-backend test)
Copy link
Contributor

Choose a reason for hiding this comment

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

The variable VAL appears unused. If it's intended for use outside this script, you might want to export it. Otherwise, please check its necessity.


# check initial balance
echo "Validator Balance:"
$BINARY query wasm contract-state smart $CONTRACT "$QUERY"
Copy link
Contributor

Choose a reason for hiding this comment

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

To prevent globbing and word splitting, consider double quoting the variable expansion in this command.

- $BINARY query wasm contract-state smart $CONTRACT "$QUERY"
+ $BINARY query wasm contract-state smart "$CONTRACT" "$QUERY"

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
$BINARY query wasm contract-state smart $CONTRACT "$QUERY"
$BINARY query wasm contract-state smart "$CONTRACT" "$QUERY"

scripts/wasm/instantiate_cw20.sh Show resolved Hide resolved
scripts/wasm/instantiate_cw20.sh Outdated Show resolved Hide resolved
scripts/wasm/instantiate_cw20.sh Show resolved Hide resolved
scripts/wasm/instantiate_cw20.sh Show resolved Hide resolved
@hoank101
Copy link
Collaborator

hoank101 commented Mar 27, 2024

@tropicaldog can you add make test-wasm? add run test to Makefile

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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: 2

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between d8c7510 and 65a6910.
Files selected for processing (4)
  • Makefile (1 hunks)
  • scripts/run-node.sh (1 hunks)
  • scripts/wasm/instantiate_cw20.sh (1 hunks)
  • scripts/wasm/test_wasm.sh (1 hunks)
Additional comments: 3
scripts/wasm/test_wasm.sh (1)
  • 44-44: Using pkill -9 to terminate the eved process might be too forceful and could lead to data corruption or other issues if the process is in the middle of writing to disk. Consider using a gentler signal first (e.g., SIGTERM) and then escalating to SIGKILL (-9) if the process does not terminate after a timeout period.
scripts/run-node.sh (1)
  • 14-14: Changing the process kill command to pkill -9 eved forces the termination of the eved process. While this ensures the process is terminated, it might be too abrupt and could lead to data corruption or other unintended consequences. Consider using a more graceful shutdown approach if possible.
Makefile (1)
  • 157-159: The addition of the test-wasm target to the Makefile is a positive enhancement that streamlines the process of running Wasm tests. This change supports the PR's objectives to improve the testing framework for Wasm contracts. Ensure that the test_wasm.sh script is properly maintained and updated to reflect any changes in the testing process.

if [[ "$OSTYPE" == "darwin"* ]]; then
screen -L -dmS node1 bash scripts/run-node.sh
else
screen -L -Logfile $HOME/log-screen.txt -dmS node1 bash scripts/run-node.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

To enhance the security and reliability of your script, it's recommended to double-quote variables to prevent globbing and word splitting. This is particularly important when variables could potentially contain spaces or special characters.

-    screen -L -Logfile $HOME/log-screen.txt -dmS node1 bash scripts/run-node.sh
+    screen -L -Logfile "$HOME/log-screen.txt" -dmS node1 bash scripts/run-node.sh

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
screen -L -Logfile $HOME/log-screen.txt -dmS node1 bash scripts/run-node.sh
screen -L -Logfile "$HOME/log-screen.txt" -dmS node1 bash scripts/run-node.sh

Comment on lines 11 to 17
fi

rm -rf mytestnet
pkill eved
pkill -9 eved

# check DENOM is set. If not, set to ueve
DENOM=${2:-ueve}
Copy link
Contributor

Choose a reason for hiding this comment

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

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [65-68]

To prevent globbing and word splitting, it's recommended to double-quote variables when they are expanded. This ensures that the variables are treated as a single argument even if they contain spaces or special characters.

- $BINARY start --home $HOME_DIR --log_level debug
+ $BINARY start --home "$HOME_DIR" --log_level debug

Copy link
Collaborator

@hoank101 hoank101 left a comment

Choose a reason for hiding this comment

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

LGTM

@hoank101 hoank101 merged commit 4600867 into main Mar 27, 2024
10 checks passed
@hoank101 hoank101 deleted the tuan/add-wasm-e2e branch March 27, 2024 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants