New guide: Use Hyperlane TIA as gas token #390
Conversation
Also add `--depth` 1 to all `git clone`s
|
Note Reviews pausedUse the following commands to manage reviews:
WalkthroughThe changes introduce a Token Factory module into the application, enhancing its functionality for managing tokens. Additionally, a comprehensive configuration for deploying a Hyperlane application is established, detailing services, network settings, and validator configurations. The integration of TIA as a gas token is emphasized, with updates to documentation and configuration files to support its use in transactions across multiple blockchain networks. Changes
Possibly related PRs
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@assafmo what's missing? when ready for review? |
|
Still on it. Will tag you when it's ready. |
…/app.go from 902-904 to 898-900
…tions for deploying hyperlane contracts
… domain id, and add explanations for domain id and contract deployment process
…onfig setup instructions with detailed example
• Add headers for each step in the guide • Update command for creating config.yaml • Update command for funding cw-hyperlane signer • Update commands for uploading and deploying contracts • Add setup for relayer and validator configs
`exchange_rate` cannot be a floating point because in the contract it's a Uint128: https://github.com/many-things/cw-hyperlane/blob/f6a428a/packages/interface/src/igp/oracle.rs#L14
…e 0.0.0.0 instead of 127.0.0.1 for accessibility from Hyperlane docker agents
…les and docker compose settings for Hyperlane guides
…porary fix for cw-hyperlane#133
…dd tip to check tx status
… prompts to avoid manual interaction
…ys set to yes for apt-get install
fix(wasmd.Dockerfile): remove hardcoded wasmvm download and install from Dockerfile
… restart-wasm.sh script if available before running setup from scratch
…e correct installation of stride repository
RollkitBot
requested review from
jcstein,
Manav-Aggarwal and
yarikbratashchuk
|
Hey guys, this is ready for a review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
🧹 Outside diff range and nitpick comments (11)
guides/assets/hyperlane-use-tia-for-gas/validator.strideinternal1.json (1)
1-6: Consider using relative paths and evaluating checkpoint storage method.
The database and checkpoint paths are currently set as absolute paths. Consider using relative paths to improve portability across different environments.
The checkpoint syncer is set to use local storage. While this is simple, it might not be suitable for distributed or high-availability setups. Consider evaluating if this meets your scalability and reliability requirements.
guides/assets/hyperlane-use-tia-for-gas/hyperlane-config-2.yaml (1)
38-66: Document IBC token usage and consider parameterizing the owner.The hooks configuration is well-structured and includes important features like the pausable hook. However, there are a couple of points that could be improved:
The IBC token "ibc/1A7653323C1A9E267FF7BEBF40B3EEA8065E8F069F47F2493ABC3E0B621BF793" is used for both the IGP and fee hooks. It would be helpful to document what this token represents.
The owner is set to "" for all hooks. While this is consistent, it might limit flexibility in cases where different owners are needed for different hooks.
Consider the following improvements:
- Add a comment explaining what the IBC token represents and why it's used for both IGP and fee hooks.
- Consider parameterizing the owner to allow for more flexibility. For example:
deploy: default_owner: <signer> hooks: default: type: aggregate owner: ${HYPERLANE_DEFAULT_HOOK_OWNER:-<default_owner>} # ... rest of the configuration required: type: aggregate owner: ${HYPERLANE_REQUIRED_HOOK_OWNER:-<default_owner>} # ... rest of the configurationThis allows setting different owners through environment variables while maintaining the current behavior as a default.
guides/assets/hyperlane-use-tia-for-gas/docker-compose.yml (2)
61-80: Reviewvalidator-localwasmservice configurationThe configuration for this service is consistent with the
relayerservice, which is good for maintainability. A few points to note:
As with the
relayerservice, consider if you want to enable therestart: alwayspolicy for production use.The volume mounts are appropriate, but ensure the
./validator/localwasmdirectory exists.To reduce duplication, you might consider using YAML anchors and aliases to share common configuration between this service and the
relayerservice. This could make the docker-compose file more maintainable.Here's an example of how you could use YAML anchors to reduce duplication:
x-common-config: &common-config image: gcr.io/abacus-labs-dev/hyperlane-agent:8a66544-20240530-111322 platform: linux/amd64 user: root # restart: always volumes: - ./hyperlane:/etc/hyperlane - ./validator:/etc/validator services: relayer: <<: *common-config # ... rest of relayer config ... validator-localwasm: <<: *common-config # ... rest of validator-localwasm config ...
81-99: Reviewvalidator-strideinternal1service configurationThe configuration for this service is consistent with the
validator-localwasmservice, which is good for maintainability. Here are some observations and suggestions:
As with the other services, consider if you want to enable the
restart: alwayspolicy for production use.The volume mounts are appropriate, but ensure the
./validator/strideinternal1directory exists.Given the similarity between this service and
validator-localwasm, you could further reduce duplication by using YAML anchors and aliases, as suggested in the previous comment.Consider if you need both validator services running simultaneously, or if they serve different purposes in your setup.
Here's an example of how you could use YAML anchors to reduce duplication for both validator services:
x-validator-config: &validator-config <<: *common-config entrypoint: ["sh", "-c"] command: - | rm -rf /app/config/* && \ cp "/etc/hyperlane/agent-config.docker.json" "/app/config/agent-config.json" && \ CONFIG_FILES="/etc/hyperlane/validator.$VALIDATOR_TYPE.json" \ ./validator ports: - ${VALIDATOR_PORT}:9090 services: validator-localwasm: <<: *validator-config container_name: hpl-validator-localwasm environment: - VALIDATOR_TYPE=localwasm - VALIDATOR_PORT=9120 volumes: - ./validator/localwasm:/etc/data validator-strideinternal1: <<: *validator-config container_name: hpl-validator-strideinternal1 environment: - VALIDATOR_TYPE=strideinternal1 - VALIDATOR_PORT=9121 volumes: - ./validator/strideinternal1:/etc/dataThis approach would make it easier to add or modify validator configurations in the future.
guides/use-tia-for-gas.md (3)
7-15: Excellent addition of the warning disclaimer.The new warning effectively communicates the current limitations of IBC and provides clear guidance to users. It's great that you've included links to relevant GitHub issues for more information.
Consider adding a brief explanation of what Hyperlane is and why it's recommended over IBC for now. This would provide more context for users who might not be familiar with Hyperlane.
Line range hint
142-324: Excellent addition of IBC connection instructions.The new section "Connecting to Celestia Mocha testnet using IBC" is a valuable addition to the guide. It provides comprehensive, step-by-step instructions for setting up the IBC connection, which is crucial for using TIA as a gas token in the rollup.
Consider adding a brief explanation of what IBC is and why it's necessary for this setup. This would provide more context for users who might not be familiar with IBC.
Also, it might be helpful to add a note about the expected duration of some of these steps, especially the relayer setup and initial IBC transfers, as they might take some time to complete.
325-325: Consider expanding the conclusion.While the celebratory emoji is a nice touch, the removal of the concluding text might reduce the sense of accomplishment for the reader.
Consider adding back a brief concluding paragraph that:
- Congratulates the reader on completing the tutorial.
- Summarizes what they've accomplished.
- Suggests possible next steps or further resources for learning.
This would provide a more satisfying end to the tutorial and guide readers on how to build upon what they've learned.
guides/hyperlane-use-tia-for-gas.md (4)
107-108: Address the TODO comment about updating to a forked version.There's a TODO comment about updating to a forked version. This should be addressed before finalizing the guide.
Would you like me to create a GitHub issue to track this task?
187-188: Address the TODO comment about updating the fee denomination.There's a TODO comment about updating the fee to be denominated in the tokenfactory TIA. This should be addressed before finalizing the guide.
Would you like me to create a GitHub issue to track this task?
232-234: Address the TODO comments about Stride deployment configuration.There are TODO comments about not redeploying all Stride contracts and changing the config.yaml layout. These should be addressed to ensure the guide is complete and optimized.
Would you like me to create a GitHub issue to track these tasks?
1-437: Address minor formatting inconsistencies.To improve the document's consistency and adhere to markdown best practices:
- Remove trailing punctuation from headings (e.g., lines 27, 33, 185, 211, etc.).
- Specify a language for all fenced code blocks (e.g., lines 308, 325).
- Use proper sentence structure for headings starting with "Setup" (e.g., lines 263, 269).
- Add a comma after "Alternatively" on line 85 for clarity.
- Properly separate "therefore" as a conjunction on line 396.
These minor changes will enhance the overall readability and consistency of the document.
🧰 Tools
🪛 LanguageTool
[grammar] ~56-~56: This sentence should probably be started with a verb instead of the noun ‘Config’. If not, consider inserting a comma for better clarity.
Context: ...stall ``` ### Configure Local Clients Config thewasmdand `strided` CLI for ease ...(SENT_START_NN_DT)
[typographical] ~85-~85: Consider adding a comma after ‘Alternatively’ for more clarity.
Context: ... predefined seed phrase for simplicity. Alternatively you can generate and use your own: ```...(RB_LY_COMMA)
[grammar] ~263-~263: This sentence should probably be started with a verb instead of the noun ‘Setup’. If not, consider inserting a comma for better clarity.
Context: ...-use-tia-for-gas/relayer.json ``` #### Setup the validator config on the Stride side...(SENT_START_NN_DT)
[grammar] ~269-~269: This sentence should probably be started with a verb instead of the noun ‘Setup’. If not, consider inserting a comma for better clarity.
Context: ...alidator.strideinternal1.json ``` #### Setup the validator config on the localwasm r...(SENT_START_NN_DT)
[uncategorized] ~380-~380: “you” seems less likely than “your” (belonging to you).
Context: ...n9wxzg6vhc0370lcem8939zc3uexj ``` Fund you account with the [Celestia Mocha Testne...(AI_HYDRA_LEO_CP_YOU_YOUR)
[typographical] ~396-~396: The word “therefore” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...ds and routes transfers directly to the rollup, therefore we only need to sign one transaction on...(THUS_SENTENCE)
🪛 Markdownlint
27-27: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
33-33: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
185-185: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
211-211: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
217-217: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
239-239: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
257-257: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
263-263: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
269-269: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
275-275: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
290-290: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
168-168: null
Bare URL used(MD034, no-bare-urls)
308-308: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
325-325: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (14)
- guides/assets/hyperlane-use-tia-for-gas/add-tokenfactory.diff (1 hunks)
- guides/assets/hyperlane-use-tia-for-gas/docker-compose.yml (1 hunks)
- guides/assets/hyperlane-use-tia-for-gas/hyperlane-config-2.yaml (1 hunks)
- guides/assets/hyperlane-use-tia-for-gas/hyperlane-config.yaml (1 hunks)
- guides/assets/hyperlane-use-tia-for-gas/relayer.json (1 hunks)
- guides/assets/hyperlane-use-tia-for-gas/stride-internal-1.json (1 hunks)
- guides/assets/hyperlane-use-tia-for-gas/utia-localwasm.json (1 hunks)
- guides/assets/hyperlane-use-tia-for-gas/utia-stride.json (1 hunks)
- guides/assets/hyperlane-use-tia-for-gas/validator.localwasm.json (1 hunks)
- guides/assets/hyperlane-use-tia-for-gas/validator.strideinternal1.json (1 hunks)
- guides/assets/hyperlane-use-tia-for-gas/wasmd.Dockerfile (1 hunks)
- guides/hyperlane-use-tia-for-gas.md (1 hunks)
- guides/use-tia-for-gas.md (4 hunks)
- tutorials/cosmwasm.md (8 hunks)
🧰 Additional context used
🪛 Gitleaks
guides/assets/hyperlane-use-tia-for-gas/relayer.json
20-20: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
27-27: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
guides/assets/hyperlane-use-tia-for-gas/validator.localwasm.json
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
guides/assets/hyperlane-use-tia-for-gas/validator.strideinternal1.json
10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 LanguageTool
guides/hyperlane-use-tia-for-gas.md
[grammar] ~56-~56: This sentence should probably be started with a verb instead of the noun ‘Config’. If not, consider inserting a comma for better clarity.
Context: ...stall ``` ### Configure Local Clients Config thewasmdand `strided` CLI for ease ...(SENT_START_NN_DT)
[typographical] ~85-~85: Consider adding a comma after ‘Alternatively’ for more clarity.
Context: ... predefined seed phrase for simplicity. Alternatively you can generate and use your own: ```...(RB_LY_COMMA)
[grammar] ~263-~263: This sentence should probably be started with a verb instead of the noun ‘Setup’. If not, consider inserting a comma for better clarity.
Context: ...-use-tia-for-gas/relayer.json ``` #### Setup the validator config on the Stride side...(SENT_START_NN_DT)
[grammar] ~269-~269: This sentence should probably be started with a verb instead of the noun ‘Setup’. If not, consider inserting a comma for better clarity.
Context: ...alidator.strideinternal1.json ``` #### Setup the validator config on the localwasm r...(SENT_START_NN_DT)
[uncategorized] ~380-~380: “you” seems less likely than “your” (belonging to you).
Context: ...n9wxzg6vhc0370lcem8939zc3uexj ``` Fund you account with the [Celestia Mocha Testne...(AI_HYDRA_LEO_CP_YOU_YOUR)
[typographical] ~396-~396: The word “therefore” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...ds and routes transfers directly to the rollup, therefore we only need to sign one transaction on...(THUS_SENTENCE)
tutorials/cosmwasm.md
[style] ~170-~170: Consider a shorter alternative to avoid wordiness.
Context: ...requires Docker in order to compile. <!-- markdownlint-enable MD05...(IN_ORDER_TO_PREMIUM)
🪛 Markdownlint
guides/hyperlane-use-tia-for-gas.md
27-27: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
33-33: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
185-185: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
211-211: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
217-217: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
239-239: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
257-257: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
263-263: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
269-269: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
275-275: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
290-290: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
168-168: null
Bare URL used(MD034, no-bare-urls)
308-308: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
325-325: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
tutorials/cosmwasm.md
215-215: Expected: 80; Actual: 197
Line length(MD013, line-length)
🔇 Additional comments (29)
guides/assets/hyperlane-use-tia-for-gas/utia-localwasm.json (2)
1-11: LGTM! The configuration structure looks correct.The JSON configuration for the native bridged asset "TIA.stride-localwasm" is well-structured and aligns with the PR objective of using TIA as a gas token. The "utia" denomination in the bridged config is appropriate for TIA.
4-4: Verify the asset ID and consider adding a comment for clarity.The asset ID "TIA.stride-localwasm" seems to be specific to a Stride blockchain implementation in a local Wasm environment. To improve clarity and maintainability:
- Please verify that this ID is correct and consistent with other parts of the system.
- Consider adding a comment above the JSON object to explain the purpose and context of this configuration, especially regarding its use in Hyperlane and as a gas token.
To ensure consistency, let's check for other occurrences of this asset ID:
✅ Verification successful
Asset ID "TIA.stride-localwasm" is consistent across the codebase.
The asset ID "TIA.stride-localwasm" is consistently used in multiple files, ensuring its correctness and consistency within the system. To enhance clarity and maintainability:
- Consider adding a comment above the JSON object to explain the purpose and context of this configuration, especially regarding its use in Hyperlane and as a gas token.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other occurrences of the asset ID rg "TIA\.stride-localwasm" --type json --type mdLength of output: 375
guides/assets/hyperlane-use-tia-for-gas/utia-stride.json (3)
2-3: LGTM: Asset type and mode are correctly defined.The asset is properly configured as a native type in collateral mode, which aligns with the intended use of TIA as a gas token in the Hyperlane protocol.
6-10: Collateral configuration structure is correct, but verify the IBC token hash.The collateral configuration is structured correctly. However, please ensure the following:
- Verify that the IBC token hash "ibc/1A7653323C1A9E267FF7BEBF40B3EEA8065E8F069F47F2493ABC3E0B621BF793" correctly represents the TIA token on the Stride network.
- Consider if additional properties are needed in the collateral configuration, such as exchange rates or limits, based on Hyperlane protocol requirements.
To verify the IBC token hash, you can run the following command to search for its usage in other configuration files:
#!/bin/bash # Search for the IBC token hash in all JSON files rg --type json '1A7653323C1A9E267FF7BEBF40B3EEA8065E8F069F47F2493ABC3E0B621BF793' -C 3
4-5: Asset ID looks good, but verify the owner placeholder.The asset ID "TIA.stride-localwasm" appears to be correctly formatted. However, the "" placeholder for the owner needs to be replaced with an actual signer address or identifier before deployment.
To ensure this placeholder is not used elsewhere, run the following command:
✅ Verification successful
Owner placeholder is uniquely used and ready for replacement.
The "" placeholder for the owner is only found in
guides/assets/hyperlane-use-tia-for-gas/utia-stride.json. Ensure it is replaced with an actual signer address or identifier before deployment.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for "<signer>" placeholder in all JSON files rg --type json '<signer>' -C 3Length of output: 640
guides/assets/hyperlane-use-tia-for-gas/wasmd.Dockerfile (3)
1-3: LGTM: Appropriate base image and package installation.The use of a specific Go version (1.22.5) on Debian Bullseye is good for reproducibility. The installation of git and make is done efficiently in a single RUN command, following Docker best practices.
8-9: Clarify LEDGER_ENABLED flag and consider multi-stage build.The build process disables the ledger functionality. This might have implications for the application's features.
Could you clarify why LEDGER_ENABLED is set to false? Are there any functional limitations this introduces?
Also, consider using a multi-stage build to reduce the final image size. This would involve using a separate build stage and only copying the necessary binary to the final image.
11-15: Clarify working directory change and port usage.The working directory is changed to
/opt, which is unusual. Could you explain the reasoning behind this?Also, the Dockerfile exposes ports 1317, 36656, and 36657. Could you confirm the purpose of each port?
- Port 1317: Is this for the API server?
- Port 36656: Is this for P2P communication?
- Port 36657: Is this for the RPC server?
It would be helpful to add comments in the Dockerfile explaining the purpose of each exposed port for better maintainability.
guides/assets/hyperlane-use-tia-for-gas/stride-internal-1.json (3)
1-25: LGTM: Well-structured JSON fileThe overall structure of the JSON file is correct and follows best practices. It contains two main objects: "artifacts" and "deployments", which are properly formatted.
2-23: Please clarify the purpose and scope of the artifactsThe "artifacts" object contains a mapping of Hyperlane-related components to numeric identifiers. Could you provide more context on:
- The purpose of these identifiers and how they are used in the system?
- Are there other artifacts not listed here (given the sequential nature of the identifiers)?
- The significance of the "hpl_test_" prefix for some artifacts - are these meant for testing purposes only?
This information would help in understanding the role of this configuration in the broader system.
24-24: Clarify the purpose of the empty "deployments" objectThe "deployments" object is currently empty. Could you provide more information on:
- Is this intentional, or will it be populated in the future?
- If it's meant to be populated, what kind of information will it contain?
- Is there a reason for including an empty object rather than omitting it entirely at this stage?
Understanding the role of the "deployments" object will help in assessing its relationship with the "artifacts" and its importance in the overall configuration.
guides/assets/hyperlane-use-tia-for-gas/relayer.json (2)
6-15: Whitelist configuration looks goodThe whitelist configuration allows bidirectional communication between domains 963 and 1651, which seems appropriate for the intended use case.
16-31:⚠️ Potential issueCritical: Remove hardcoded private keys and review key usage
The configuration contains hardcoded private keys for both "localwasm" and "strideinternal1" chains. This is a significant security risk.
Both chains are using the same private key, which may not be intended and could pose additional security risks.
To address these issues:
- Remove the hardcoded private keys from the configuration file.
- Use environment variables or a secure secret management system to provide the keys at runtime.
- Consider using different keys for each chain unless there's a specific reason to use the same key.
Example of using environment variables:
"signer": { "type": "cosmosKey", "key": "${CHAIN_PRIVATE_KEY}", "prefix": "wasm" }Then, set the environment variable
CHAIN_PRIVATE_KEYsecurely before running the application.To ensure no other hardcoded keys exist in the codebase, run the following command:
#!/bin/bash # Search for potential hardcoded private keys in JSON files rg --type json -e '"key":\s*"0x[a-fA-F0-9]{64}"'🧰 Tools
🪛 Gitleaks
20-20: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
27-27: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
guides/assets/hyperlane-use-tia-for-gas/hyperlane-config.yaml (1)
38-65: Hooks configuration looks good, but replace placeholders.The hooks configuration is well-structured and comprehensive. However, there are a few points to consider:
- The owner for all hooks is set to "", which appears to be a placeholder. Ensure this is replaced with the actual signer address before deployment.
- The IGP configuration seems reasonable, but verify that the exchange rate (1000) and gas price (10000) are appropriate for your use case.
- The fee hook defines a fee in "uwasm", which matches the denomination used in the "localwasm" network. Ensure this is the intended denomination for fees across all networks.
To verify the consistency of token denominations across the configuration, run the following script:
#!/bin/bash # Description: Check token denominations used in different parts of the configuration echo "Gas token denominations:" yq '.networks[].gas.denom' guides/assets/hyperlane-use-tia-for-gas/hyperlane-config.yaml echo "Fee hook denomination:" yq '.deploy.hooks.required.hooks[] | select(.type == "fee") | .fee.denom' guides/assets/hyperlane-use-tia-for-gas/hyperlane-config.yamlThis will help ensure that the token denominations are consistent and as intended throughout the configuration.
guides/assets/hyperlane-use-tia-for-gas/docker-compose.yml (4)
14-40: Improvelocalwasmservice configurationThe service setup looks good overall. Here are some suggestions for improvement:
- Consider adding an explicit dependency on the
daservice:depends_on: - daThis will ensure that Docker Compose starts the services in the correct order.
The use of
netcatfor health checking is a good practice. It ensures that the service waits for the DA layer to be ready before starting.The script modifications seem appropriate for adjusting network configurations in a containerized environment.
1-99: Overall assessment of the Docker Compose configurationThis Docker Compose file provides a comprehensive setup for a Hyperlane application, including data availability, CosmWasm, relayer, and validator services. The configuration is well-structured and generally follows best practices. Here's a summary of the key points:
Consistency: The use of the same Hyperlane agent image across multiple services promotes consistency and ease of maintenance.
Security: There are some security considerations, particularly in the
daservice, that should be addressed (see earlier comments).Dependency Management: Consider adding explicit
depends_ondeclarations to ensure services start in the correct order.Configuration Reuse: There's an opportunity to reduce duplication and improve maintainability by using YAML anchors and aliases (see suggestions in earlier comments).
Environment Specificity: Some commented-out configurations (like
restart: always) suggest that this file might be used across different environments. Consider using environment variables or multiple compose files for different deployment scenarios.Volume Management: Ensure all referenced local directories exist in the context where this compose file will be run.
Overall, this is a solid configuration that sets up a complex multi-service application. With a few tweaks, it can be made even more robust and maintainable.
41-60: Reviewrelayerservice configurationThe service configuration looks solid. Here are a few points to consider:
The
restart: alwayspolicy is commented out. Consider if you want to enable this for production use to ensure the service restarts automatically in case of failures.The volume mounts seem appropriate, providing necessary configuration and data persistence. However, ensure that the paths
./hyperlane,./relayer, and./validatorexist in the context of where this docker-compose file will be run.The use of a specific image tag (
gcr.io/abacus-labs-dev/hyperlane-agent:8a66544-20240530-111322) is good for reproducibility. However, consider if you want to implement a strategy for updating this image in the future.To ensure the mounted directories exist, you can run:
#!/bin/bash for dir in hyperlane relayer validator; do if [ ! -d "./$dir" ]; then echo "Warning: Directory ./$dir does not exist" fi done
2-13: Consider security implications in thedaservice configuration.While the service setup appears functional, there are a couple of points to consider:
The script modification to use
-listen-all(line 10) might expose the service more broadly than necessary. Ensure this is intentional and doesn't introduce unnecessary security risks.Downloading and executing a script directly from the internet (line 9) can be a security risk. Consider vendoring the script or using a verified, pinned version.
To ensure the script hasn't been modified unexpectedly, you can run:
Compare the output with a known good hash to verify the script's integrity.
guides/use-tia-for-gas.md (3)
Line range hint
16-30: Introduction changes improve clarity.The rephrasing in the Introduction section enhances the clarity of the tutorial's focus without altering its overall message. This improvement helps readers better understand the purpose and scope of the guide.
Line range hint
326-336: Transaction command successfully updated to use IBC TIA.The changes in the Transactions section correctly implement the use of IBC TIA as the gas token, which is the primary objective of this tutorial. The gas price is consistent with the earlier change in the "Starting your rollup" section.
137-140: Verify gas price change and block height update.Two important changes have been made in this section:
- The gas prices have been updated to "0.02ibc/C3E53D20BC7A4CC993B17C7971F8ECD06A433C10B6A96F4C4C3714F0624C56DA,0.025stake".
- The log output now shows a block height of 1 instead of 22.
Please confirm that:
- The new gas prices are correct and consistent with the rest of the document.
- The change in block height from 22 to 1 is intentional and doesn't affect the tutorial's flow.
✅ Verification successful
Gas price and block height updates are consistent and intentional.
gas-pricesare correctly set with appropriate denominations:
0.02ibc/C3E53D20BC7A4CC993B17C7971F8ECD06A433C10B6A96F4C4C3714F0624C56DA,0.025stake0.25utia- Block height has been uniformly updated to
1, indicating a fresh initial state for the rollup.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify gas prices consistency grep -n "gas-prices" guides/use-tia-for-gas.md # Verify block height references grep -n "height=" guides/use-tia-for-gas.mdLength of output: 964
Script:
# Extract line 207 to inspect the context of gas-prices sed -n '207p' guides/use-tia-for-gas.mdLength of output: 72
Script:
# Search for all instances of 'utia' to verify consistency grep -n "utia" guides/use-tia-for-gas.md # Review lines around line 207 for additional context sed -n '200,210p' guides/use-tia-for-gas.mdLength of output: 821
guides/hyperlane-use-tia-for-gas.md (1)
1-437: Overall assessment: Comprehensive guide with room for improvements.This guide provides detailed instructions for integrating Hyperlane as a gas token mechanism in a CosmWasm rollup. The step-by-step approach is commendable and covers the necessary components thoroughly.
Key strengths:
- Comprehensive coverage of the setup and deployment process.
- Clear instructions with code examples.
- Integration of multiple technologies (CosmWasm, Hyperlane, Celestia, Stride).
Areas for improvement:
- Enhance security practices, especially regarding seed phrase handling.
- Address TODO comments and pending updates.
- Provide more context about Hyperlane and its components.
- Improve formatting consistency.
- Expand the conclusion and resources section.
Implementing the suggested changes will significantly enhance the guide's quality, security, and educational value.
🧰 Tools
🪛 LanguageTool
[grammar] ~56-~56: This sentence should probably be started with a verb instead of the noun ‘Config’. If not, consider inserting a comma for better clarity.
Context: ...stall ``` ### Configure Local Clients Config thewasmdand `strided` CLI for ease ...(SENT_START_NN_DT)
[typographical] ~85-~85: Consider adding a comma after ‘Alternatively’ for more clarity.
Context: ... predefined seed phrase for simplicity. Alternatively you can generate and use your own: ```...(RB_LY_COMMA)
[grammar] ~263-~263: This sentence should probably be started with a verb instead of the noun ‘Setup’. If not, consider inserting a comma for better clarity.
Context: ...-use-tia-for-gas/relayer.json ``` #### Setup the validator config on the Stride side...(SENT_START_NN_DT)
[grammar] ~269-~269: This sentence should probably be started with a verb instead of the noun ‘Setup’. If not, consider inserting a comma for better clarity.
Context: ...alidator.strideinternal1.json ``` #### Setup the validator config on the localwasm r...(SENT_START_NN_DT)
[uncategorized] ~380-~380: “you” seems less likely than “your” (belonging to you).
Context: ...n9wxzg6vhc0370lcem8939zc3uexj ``` Fund you account with the [Celestia Mocha Testne...(AI_HYDRA_LEO_CP_YOU_YOUR)
[typographical] ~396-~396: The word “therefore” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...ds and routes transfers directly to the rollup, therefore we only need to sign one transaction on...(THUS_SENTENCE)
🪛 Markdownlint
27-27: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
33-33: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
185-185: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
211-211: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
217-217: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
239-239: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
257-257: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
263-263: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
269-269: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
275-275: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
290-290: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
168-168: null
Bare URL used(MD034, no-bare-urls)
308-308: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
325-325: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
guides/assets/hyperlane-use-tia-for-gas/add-tokenfactory.diff (7)
9-11: Ensure correct import paths and module versions.The imports for the Token Factory module have been added:
"github.com/Stride-Labs/tokenfactory/tokenfactory" tokenfactorykeeper "github.com/Stride-Labs/tokenfactory/tokenfactory/keeper" tokenfactorytypes "github.com/Stride-Labs/tokenfactory/tokenfactory/types"Please verify that the import paths are correct and that the module versions are compatible with your current Cosmos SDK and other dependencies.
23-27: Addtokenfactorytypes.ModuleNametomaccPermswith appropriate permissions.The
tokenfactorytypes.ModuleNamehas been added to themaccPermsmap withauthtypes.Minterandauthtypes.Burnerpermissions:tokenfactorytypes.ModuleName: {authtypes.Minter, authtypes.Burner},Ensure that granting these permissions is necessary and does not introduce security risks. Verify that the Token Factory module requires both minting and burning capabilities and that it properly handles these operations.
35-35: AddTokenFactoryKeeperto theWasmAppstruct.The
TokenFactoryKeeperfield is added to theWasmAppstruct:TokenFactoryKeeper tokenfactorykeeper.KeeperThis addition integrates the Token Factory module into the application. Confirm that this keeper is correctly utilized throughout the application where needed.
51-59: InitializeTokenFactoryKeeperwith correct dependencies.The
TokenFactoryKeeperis initialized in theNewWasmAppfunction:app.TokenFactoryKeeper = tokenfactorykeeper.NewKeeper( keys[tokenfactorytypes.StoreKey], app.GetSubspace(tokenfactorytypes.ModuleName), maccPerms, app.AccountKeeper, app.BankKeeper, app.DistrKeeper, )Ensure that all dependencies (
AccountKeeper,BankKeeper,DistrKeeper,maccPerms) are properly initialized before being passed. Also, verify that the subspace fortokenfactorytypes.ModuleNamehas been set correctly.
68-68: RegisterTokenFactorymodule with the module manager.The
TokenFactorymodule is added to the module manager:tokenfactory.NewAppModule(app.TokenFactoryKeeper, app.AccountKeeper, app.BankKeeper),Confirm that the module is correctly registered and that it integrates smoothly with other modules in the application lifecycle.
76-77: Update module initialization and execution order to includeTokenFactory.The
tokenfactorytypes.ModuleNameis added toSetOrderInitGenesis,SetOrderBeginBlockers, andSetOrderEndBlockers:app.ModuleManager.SetOrderInitGenesis( // ... tokenfactorytypes.ModuleName, )Verify that the placement of the
TokenFactorymodule in the initialization and blocker order is appropriate and does not conflict with other modules' dependencies.Also applies to: 84-85, 92-93
110-110: Initialize parameter subspace forTokenFactorymodule.In the
initParamsKeeperfunction, the parameter subspace fortokenfactorytypes.ModuleNameis added:paramsKeeper.Subspace(tokenfactorytypes.ModuleName)Ensure that this subspace is properly configured and that any necessary parameter key tables are set up for the
TokenFactorymodule.
| celestia-appd keys add my-key --recover # celestia133xh839fjn9wxzg6vhc0370lcem8939zc3uexj | ||
| ``` | ||
|
|
||
| Fund you account with the [Celestia Mocha Testnet Faucet](https://docs.celestia.org/nodes/mocha-testnet#mocha-testnet-faucet). |
There was a problem hiding this comment.
Fix typo in funding instruction.
There's a minor typo in the instruction for funding the account.
-Fund you account with the [Celestia Mocha Testnet Faucet](https://docs.celestia.org/nodes/mocha-testnet#mocha-testnet-faucet).
+Fund your account with the [Celestia Mocha Testnet Faucet](https://docs.celestia.org/nodes/mocha-testnet#mocha-testnet-faucet).📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Fund you account with the [Celestia Mocha Testnet Faucet](https://docs.celestia.org/nodes/mocha-testnet#mocha-testnet-faucet). | |
| Fund your account with the [Celestia Mocha Testnet Faucet](https://docs.celestia.org/nodes/mocha-testnet#mocha-testnet-faucet). |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~380-~380: “you” seems less likely than “your” (belonging to you).
Context: ...n9wxzg6vhc0370lcem8939zc3uexj ``` Fund you account with the [Celestia Mocha Testne...(AI_HYDRA_LEO_CP_YOU_YOUR)
|
please review and resolve Coderabbit suggestions as you see fit @assafmo ! some are junk and can definitely be thrown out, but some may be useful. |
|
@coderabbitai ignore |
|
@coderabbitai ignore ✅ Actions performedReviews paused. |
| #### Add token factory to app.go: | ||
|
|
||
| ```bash | ||
| curl -sSL https://raw.githubusercontent.com/Stride-Labs/Rollkit-docs/hyperlane-v2/guides/assets/hyperlane-use-tia-for-gas/add-tokenfactory.diff | git apply |
There was a problem hiding this comment.
As opposed to blinding applying a git diff, it would be great to understand what the diff is so that people understand it. Similar to how we walk the user through building the wordle tutorial.
There was a problem hiding this comment.
There are tons of parts like this during the guide, it'll be super long and boring if we explain all of these, it's like going through the commit history and explaining commits. Would you rather we fork the repo, apply the patch and have people clone our fork?
There was a problem hiding this comment.
Can we instead have a tag or branch that we are targeting?
A branch would be nice because then it would be easier to know how to update in the future.
Then the steps would be:
git clone https://github.com/Stride-Labs/wasmd
cd wasmd
git checkout rollkit-hyperlane
That would be my preferred way.
| cd ../wasmd | ||
| ``` | ||
|
|
||
| Add the signer accounts to each keyring |
There was a problem hiding this comment.
Can we explain why we are setting an explicit keyring vs just generating a new one?
| ``` | ||
|
|
||
| :::tip | ||
| In this guide, we're using a predefined seed phrase for simplicity. Alternatively you can generate and use your own: |
There was a problem hiding this comment.
Let's break this into 2.
The first about simplicity should be above the steps for using that seed phrase.
The second that can stay here is about generating your own.
| <!-- TODO: Update the fee to be denominated in the tokenfactory TIA --> | ||
|
|
||
| ```bash | ||
| wget -O config.yaml https://raw.githubusercontent.com/Stride-Labs/Rollkit-docs/hyperlane-v2/guides/assets/hyperlane-use-tia-for-gas/hyperlane-config.yaml |
There was a problem hiding this comment.
For all these wget commands I think we should just inline the content and have the user copy into a local file.
This eliminates the need for all the asset files that are part of this PR.
Also these wget commands are problematic in that they aren't targeting the actual files being upload but some branch on the Strife-Labs fork. A little chicken and egg situation.
This is going to lead to the tutorial potentially breaking without warning.
Co-authored-by: Matthew Sevey <[email protected]>
Yeah, there are a lot of new configs that we introduce here, and initially we had it inline but decided to do it using wget because the entire guide was becoming Lmk if you want me to restore the old way, keep the wget way, or do anything else. We'll fix all the code review comments in the next few days. |
I think all the assets should live on the branch of the fork I recommended. i.e. |
Mainly add "Use Hyperlane TIA as gas token" as an alternative to IBC guide, but also:
guides/use-tia-for-gas.mdtutorials/cosmwasm.md(and add--depth 1to all thegit clones)Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation