Skip to content

Skale-contracts integration#839

Merged
dmytrotkk merged 25 commits intov4.1.xfrom
skale-contract-integration
Apr 14, 2025
Merged

Skale-contracts integration#839
dmytrotkk merged 25 commits intov4.1.xfrom
skale-contract-integration

Conversation

@Masterix0
Copy link
Copy Markdown
Contributor

@Masterix0 Masterix0 commented Mar 24, 2025

Overview

This pull request introduces a major overhaul of the environment configuration and validation in node-cli. The key changes include:

  • Simplified Environment Variable Handling:

    • Replaces the previous ABI JSON download mechanism (using MANAGER_CONTRACTS_ABI_URL and IMA_CONTRACTS_ABI_URL) with a new approach that uses the environment variables MANAGER_CONTRACTS and IMA_CONTRACTS.
    • The new approach forwards these values (which can be either direct contract addresses or recognized aliases) to other SKALE network components that use the skale-contracts library for dynamic ABI access.
  • Refactored Validation Logic:

    • The env.py file has been refactored to introduce helper functions to modularize the process of loading and validating environment variables.
    • Validation now explicitly checks the new variables (checking if alias is present in skale repo or if contract addresses have code in provided endpoint), ensuring that the environment is correctly configured before node-cli proceeds.
    • Deprecated validation code related to ABI JSON files has been removed.
  • Enhanced Testing:

    • A new test suite (tests/configs/configs_env_validate_test.py) has been created to extensively test the updated environment functions.
    • External HTTP requests are patched using requests_mock to simulate network responses, ensuring that the tests remain isolated.
  • Workflow and Documentation Improvements:

    • The README has been updated to reflect the new environment variable names and usage instructions.
    • The GitHub Actions workflow now includes a new step to run ruff for linting and code formatting, improving our code quality checks.
    • Adjustments were made to the build process (specifically the generation of info.py) to avoid development-time import errors.

Detailed Changes

  • Environment Configuration Updates:

    • New Variables:
      • Replace MANAGER_CONTRACTS_ABI_URL and IMA_CONTRACTS_ABI_URL with MANAGER_CONTRACTS and IMA_CONTRACTS.
      • These variables now contain either a direct contract address or an alias.
    • Refactoring:
      • Split the environment loading into three parts: checking and loading the file (load_env_file()), building a base parameters dictionary (build_params()), and populating it with environment values (populate_params()).
    • Validation:
      • validate_params() now calls validate_env_type() for environment type variable and validate_env_alias_or_address() for both the IMA and Manager contracts using the new variables.
      • Online validations (e.g., fetching the chain ID and network metadata) are performed only after basic checks have passed.
  • Testing Enhancements:

    • A dedicated test suite for the new environment configuration is added, covering:
      • Missing or unreadable env files.
      • Correct population of parameters from the environment.
      • Successful and failing validations for environment types, contract addresses, and aliases.
    • External HTTP calls are mocked using requests_mock to isolate tests from real network dependencies.
  • Workflow & Linting:

    • Integrated ruff into our CI/CD pipelines via GitHub Actions.
    • Updated the README with instructions regarding the new environment variables and the use of ruff.
  • Build Process Improvement:

    • The generation of info.py is now handled in a separate script (referenced in the README and CI), reducing development-time import errors.

Impact

  • Breaking Changes:
    • Scripts or integrations relying on the old ABI URL environment variables will need to be updated to use the new alias/contract address variables.
    • The deprecated validate command group has been removed.

Testing

  • All unit and integration tests for environment configuration now pass.
  • CI/CD has been updated to run ruff for code style and formatting checks and to only run test pipeline on push.

Masterix0 and others added 13 commits February 26, 2025 19:31
Added sample info.py file to fix pylance error since file doesn't existe.
File will be overwritten at build time.
…alidation

• Introduced dynamic fetching of contract ABIs from skale-contracts by allowing either an alias (e.g., “test-manager”) or a direct contract address (e.g., “0xabc123…”) in the environment.
• Renamed and replaced the old environment variables that directly pointed to local ABI files.
• Created new validation logic (validate_env_alias_or_address) for both Manager and IMA contracts in the CLI.
• Removed the obsolete tests that ensured local ABI files existed/were valid JSON and replaced them with new tests that check for valid aliases or addresses.
• Updated existing tests and mocks to patch or skip the new external validation calls where necessary.
• Added ruff configuration (ruff.toml) and reformatted code throughout the repository.
Added it to README dev preparation so there are no problems with missing info.py.
Added info.py to .gitignore.
…ate contract address validation logic to use this utility function.
…_OR_ADDRESS' to 'MANAGER_CONTRACTS' and 'IMA_CONTRACTS', respectively.
@Masterix0 Masterix0 self-assigned this Mar 24, 2025
@Masterix0 Masterix0 linked an issue Mar 24, 2025 that may be closed by this pull request
@dmytrotkk dmytrotkk requested a review from Copilot April 7, 2025 12:41
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 33 out of 33 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

node_cli/operations/skale_node.py:43

  • The 'else' branch in download_skale_node is unreachable because the function already errors out when both 'src' and 'stream' are not provided. Removing this redundant block would simplify the code.
else:

Comment thread node_cli/utils/helper.py
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread .github/workflows/test.yml Outdated
Comment thread node_cli/configs/env.py Outdated
Comment thread node_cli/configs/env.py Outdated
Comment thread node_cli/utils/helper.py Outdated
Comment thread node_cli/utils/helper.py Outdated
Comment thread node_cli/utils/print_formatters.py Outdated
Comment thread tests/configs/configs_env_validate_test.py Outdated
Comment thread tests/configs/configs_env_validate_test.py Outdated
Comment thread node_cli/configs/env.py Outdated
Comment thread node_cli/configs/env.py Outdated
Comment thread node_cli/configs/env.py Outdated
Comment thread node_cli/configs/env.py
Comment thread node_cli/configs/env.py
Comment thread node_cli/configs/env.py Outdated
Comment thread tests/configs/configs_env_validate_test.py Outdated
Comment thread tests/configs/configs_env_validate_test.py Outdated
Comment thread node_cli/core/host.py Outdated
Comment thread node_cli/utils/helper.py Outdated
Comment thread tests/configs/configs_env_validate_test.py
Comment thread tests/conftest.py Outdated
Comment thread node_cli/core/host.py
- Refactored environment validation into two files;
- Updated environment validation function names for improved clarity;
- Fixed minor docstring inconsistencies and formatting issues across multiple modules.
Comment thread node_cli/operations/skale_node.py Outdated
Comment thread node_cli/configs/alias_address_validation.py Outdated
Comment thread node_cli/utils/git_utils.py Outdated
Comment thread node_cli/utils/git_utils.py Outdated
@dmytrotkk dmytrotkk merged commit 3f434d3 into v4.1.x Apr 14, 2025
2 checks passed
@dmytrotkk dmytrotkk deleted the skale-contract-integration branch April 14, 2025 17:41
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.

Integrate 7.0 skale.py in skale-admin

4 participants