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

feat: added mainnet chain configs #689

Merged
merged 8 commits into from
Dec 4, 2024
Merged

Conversation

crnbarr93
Copy link
Contributor

@crnbarr93 crnbarr93 commented Dec 4, 2024

Motivation

These changes add configs for all active mainnet chains. There are also changes to chain names to differentiate their type and a uniqueness check to prevent overlaps.

Implementation

  • Added mainnets.rs file

Summary by CodeRabbit

  • New Features

    • Introduced a new publish method for the Operating System Deployment, enhancing contract publishing capabilities.
    • Added new public function os_contracts to return deployable contracts specific to the operating system context.
    • New constants for various mainnet and devnet blockchain networks, improving network configuration accessibility.
  • Bug Fixes

    • Updated naming conventions for testnet chain names to ensure consistency.
  • Chores

    • Modified release workflow to create draft and prerelease versions, improving release management.

@crnbarr93 crnbarr93 requested a review from SlayerAnsh December 4, 2024 14:01
Copy link
Contributor

coderabbitai bot commented Dec 4, 2024

Walkthrough

The pull request introduces modifications to several files, primarily focusing on the release workflow and the implementation of blockchain network constants. The .github/workflows/release.yml file is updated to mark releases as drafts and prereleases. Additionally, new constants for various development and mainnet networks are added in the devnets.rs and mainnets.rs files. The KernelContract struct is enhanced with a new trait implementation for metadata, and a new function for retrieving chain information is introduced in the mod.rs file.

Changes

File Path Change Summary
.github/workflows/release.yml Updated Create Release step to set draft: true and prerelease: true.
contracts/os/andromeda-kernel/src/interface.rs Added implementation of ADOMetadata trait for KernelContract, including name() and version() methods.
packages/deploy/src/chains/devnets.rs Introduced constants for OSMOSIS_DEVNET, WASM_DEVNET, and ANDROMEDA_DEVNET, along with DEVNET_CHAINS.
packages/deploy/src/chains/mainnets.rs Added constants for multiple mainnet networks, including ANDROMEDA_MAINNET, STARGAZE_MAINNET, etc., and MAINNET_CHAINS.
packages/deploy/src/chains/mod.rs Added sub-modules for devnets, mainnets, and testnets, and a new function get_chain.
packages/deploy/src/chains/testnets.rs Updated testnet constants to include "-testnet" suffix, replaced ALL_CHAINS with TESTNET_CHAINS, and removed get_chain function.
packages/deploy/src/contracts.rs Added imports for new contracts and introduced os_contracts function for OS deployable contracts.
packages/deploy/src/os.rs Added publish method to OperatingSystemDeployment struct and updated instantiate method to call publish.

Possibly related PRs

Suggested labels

ci: skip-changelog, backport/release/3

Suggested reviewers

  • joemonem

🐰 "In the land of code where the rabbits play,
Changes abound in a bright new way.
Drafts and prereleases, oh what a sight,
With constants and contracts, everything feels right!
Hops of joy for the updates we see,
In our coding burrow, happy as can be!" 🐇


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 96bbaff and d63ebc9.

📒 Files selected for processing (8)
  • .github/workflows/release.yml (1 hunks)
  • contracts/os/andromeda-kernel/src/interface.rs (2 hunks)
  • packages/deploy/src/chains/devnets.rs (1 hunks)
  • packages/deploy/src/chains/mainnets.rs (1 hunks)
  • packages/deploy/src/chains/mod.rs (1 hunks)
  • packages/deploy/src/chains/testnets.rs (3 hunks)
  • packages/deploy/src/contracts.rs (4 hunks)
  • packages/deploy/src/os.rs (4 hunks)
🔇 Additional comments (9)
packages/deploy/src/chains/mod.rs (1)

1-8: LGTM: Clean module organization

The modular structure cleanly separates different chain types (devnets, mainnets, testnets) while maintaining a unified interface through the common ChainInfo type.

packages/deploy/src/chains/testnets.rs (2)

7-7: LGTM: Clear chain type identification

The addition of "-testnet" suffix to chain names (andromeda-testnet, stargaze-testnet) improves clarity and aligns with the PR objective of differentiating chain types.

Also applies to: 24-24


40-40: LGTM: Clean constant organization

The TESTNET_CHAINS constant provides a clean way to group and access all testnet configurations.

.github/workflows/release.yml (1)

61-62: LGTM: Safer release process for mainnet configs

Setting releases as drafts and prereleases is a good safety measure, especially when introducing mainnet chain configurations. This ensures proper review before production deployment.

packages/deploy/src/chains/devnets.rs (1)

6-55: LGTM: Well-structured devnet configurations

The devnet configurations are well-structured with clear separation between network info and chain info. Good practice using constants for maintainability.

packages/deploy/src/contracts.rs (1)

2-2: LGTM: Clean separation of OS contracts

Good architectural decision to separate core OS contracts from application contracts. The implementation is clean and uses consistent patterns.

Also applies to: 14-16, 31-31, 88-96

packages/deploy/src/chains/mainnets.rs (1)

16-16: Verify gas price configurations across chains

The gas prices vary significantly between chains:

  • Andromeda: 0.025
  • Stargaze: 0.025
  • Neutron: 0.0053
  • Migaloo: 1.0

Also applies to: 33-33, 50-50, 67-67

packages/deploy/src/os.rs (2)

141-167: LGTM! Well-implemented publishing functionality

The new publish function is well-implemented with:

  • Version checking to avoid duplicates
  • Proper error handling
  • Informative logging
  • Integration into the deployment process

Also applies to: 184-186


130-130: Verify backward compatibility of module name change

The module name change from "ibc_registry" to "ibc-registry" aligns with the naming convention but might affect existing deployments.


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>, please review it.
    • Generate unit testing code 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 testing code 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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • 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/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@crnbarr93 crnbarr93 added ci: skip-changelog Skips the changelog check in CI ci: skip-build Skip artifact uploads backport/release/3 backport to release/3 labels Dec 4, 2024
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.

Caution

Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.

Actionable comments posted: 4

🧹 Outside diff range and nitpick comments (6)
contracts/os/andromeda-kernel/src/interface.rs (1)

41-50: Add documentation for ADOMetadata implementation

The implementation looks good but would benefit from documentation explaining the purpose of these metadata fields and their usage.

Consider adding documentation:

+/// Implements ADOMetadata for KernelContract to provide standardized
+/// access to contract metadata such as name and version.
 impl<Chain> ADOMetadata for KernelContract<Chain> {
+    /// Returns the contract identifier
     fn name() -> String {
         CONTRACT_ID.to_string()
     }

+    /// Returns the contract version from Cargo.toml
     fn version() -> String {
         let version = env!("CARGO_PKG_VERSION");
         version.to_string()
     }
 }
packages/deploy/src/chains/devnets.rs (2)

13-13: Fix inconsistent chain ID naming pattern

The chain IDs have inconsistent naming:

  • localosmosisa-1
  • localwasma-1
  • localandromedaa-1 (has double 'a')

Consider standardizing to a consistent pattern.

Also applies to: 30-30, 47-47


17-18: Consider infrastructure redundancy for devnets

All devnet services (GRPC, LCD) are hosted on a single IP (164.90.212.168). While acceptable for development, consider:

  1. Adding fallback endpoints
  2. Documenting the infrastructure setup
  3. Adding health check endpoints

This helps prevent development bottlenecks if the single endpoint fails.

Also applies to: 34-35, 51-52

packages/deploy/src/contracts.rs (1)

88-96: Add documentation for OS contracts function

Consider adding documentation to explain:

  • Purpose of OS contracts vs regular contracts
  • When to use os_contracts() vs all_contracts()
  • Requirements or dependencies between these contracts

Suggested documentation:

+/// Returns a list of core Operating System contracts that are fundamental to the Andromeda protocol.
+/// These contracts (ADODB, Kernel, VFS, Economics, IBC Registry) provide the base infrastructure
+/// and should be deployed before application-specific contracts.
 pub fn os_contracts() -> Vec<DeployableContract> {
packages/deploy/src/chains/mainnets.rs (2)

14-14: Consider adding documentation for gas denomination units

Add comments to clarify the units for gas prices (e.g., price per gas unit) to help maintainers understand and update these values correctly.

Also applies to: 31-31, 48-48, 65-65


193-205: Consider documenting the significance of chain order

Add a comment above the MAINNET_CHAINS array to explain if the order of chains has any significance for the system.

🛑 Comments failed to post (4)
packages/deploy/src/chains/mod.rs (1)

10-25: ⚠️ Potential issue

Critical: Improve error handling in get_chain function

The current implementation has two potential panic points:

  1. Unwrap on chain lookup (line 23)
  2. Panic on duplicate chain names (line 17)

Consider returning a Result type instead to handle errors gracefully.

Here's a suggested implementation:

-pub fn get_chain(chain: String) -> ChainInfo {
+pub fn get_chain(chain: String) -> Result<ChainInfo, String> {
     let all_chains: Vec<ChainInfo> = [MAINNET_CHAINS, TESTNET_CHAINS, DEVNET_CHAINS].concat();
     let unique_chain_names: std::collections::HashSet<&str> = all_chains
         .iter()
         .map(|c| c.network_info.chain_name)
         .collect();
     if unique_chain_names.len() != all_chains.len() {
-        panic!("Duplicate chain names found in ChainInfo");
+        return Err("Duplicate chain names found in ChainInfo".to_string());
     }

     all_chains
         .iter()
         .find(|c| c.chain_id == chain || c.network_info.chain_name == chain)
-        .unwrap()
-        .clone()
+        .map(|c| c.clone())
+        .ok_or_else(|| format!("Chain '{}' not found", chain))
 }
📝 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.

pub fn get_chain(chain: String) -> Result<ChainInfo, String> {
    let all_chains: Vec<ChainInfo> = [MAINNET_CHAINS, TESTNET_CHAINS, DEVNET_CHAINS].concat();
    let unique_chain_names: std::collections::HashSet<&str> = all_chains
        .iter()
        .map(|c| c.network_info.chain_name)
        .collect();
    if unique_chain_names.len() != all_chains.len() {
        return Err("Duplicate chain names found in ChainInfo".to_string());
    }

    all_chains
        .iter()
        .find(|c| c.chain_id == chain || c.network_info.chain_name == chain)
        .map(|c| c.clone())
        .ok_or_else(|| format!("Chain '{}' not found", chain))
}
packages/deploy/src/chains/mainnets.rs (3)

119-119: ⚠️ Potential issue

Fix typo in Chihuahua GRPC URL

The GRPC URL contains a typo: "chiahuahua" instead of "chihuahua"

-    grpc_urls: &["https://chiahuahua-grpc.polkachu.com:21290"],
+    grpc_urls: &["https://chihuahua-grpc.polkachu.com:21290"],
📝 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.

    grpc_urls: &["https://chihuahua-grpc.polkachu.com:21290"],

98-98: 💡 Codebase verification

Chain ID should be updated to "desmos-mainnet"

The current chain ID "desmos" is incorrect. The API endpoint shows that the correct chain ID for Desmos mainnet is "desmos-mainnet".

  • Update line 98 in packages/deploy/src/chains/mainnets.rs to use chain_id: "desmos-mainnet"
🔗 Analysis chain

Verify Desmos chain ID format

Other chains follow the "-1" suffix pattern (e.g., "juno-1", "chihuahua-1"), but Desmos uses just "desmos". Please verify if this is the correct chain ID.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify Desmos chain ID
curl -s "https://api.mainnet.desmos.network/cosmos/base/tendermint/v1beta1/node_info" | jq -r '.default_node_info.network'

Length of output: 135


186-186: 💡 Codebase verification

Based on the search results from the codebase, I can now provide a final response:

Archway's gas price appears to be incorrectly configured

Comparing gas prices across all chains in the codebase:

  • Most chains have gas prices between 0.001 and 1.0
  • Archway's value (140000000000.0) is orders of magnitude higher than any other chain
  • Similar chains like Nibiru and Coreum use 0.025 and 0.0625 respectively

The current value appears to be a misconfiguration as it's significantly out of range compared to all other chains in the codebase. Consider adjusting it to be in line with other Cosmos chains.

🔗 Analysis chain

Verify Archway's unusually high gas price

The gas price for Archway (140000000000.0) is significantly higher than other chains. Please verify if this is the correct value and consider adding a comment explaining why it differs so much from other chains.


Let me try another approach to verify the gas price by checking other files in the codebase for context and comparing with other chains.


Let me try one more verification by checking Archway's documentation or configuration files in the repository.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify Archway's gas price
curl -s "https://archway-api.polkachu.com/cosmos/base/node/v1beta1/params" | jq '.min_gas_price'

Length of output: 99


Script:

#!/bin/bash
# Search for gas price configurations across the codebase
rg -A 2 "gas_price:" 

# Also check for any comments or documentation about Archway gas prices
rg -i "archway.*gas.*price"

Length of output: 4651


Script:

#!/bin/bash
# Search for any Archway-related configuration files or documentation
fd -e md -e txt -e rs "archway" --exec grep -i -A 3 -B 3 "gas|price|fee"

# Also check for any commit messages or configuration changes related to Archway gas price
git log --oneline -S "140000000000" -- packages/deploy/src/chains/mainnets.rs

Length of output: 72

@crnbarr93 crnbarr93 merged commit 6533d5d into development Dec 4, 2024
8 checks passed
@crnbarr93 crnbarr93 deleted the connor/mainnet-chains branch December 4, 2024 14:09
mergify bot pushed a commit that referenced this pull request Dec 4, 2024
crnbarr93 added a commit that referenced this pull request Dec 4, 2024
This was referenced Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/release/3 backport to release/3 ci: skip-build Skip artifact uploads ci: skip-changelog Skips the changelog check in CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants