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(zetaclient)!: support for runtime chain (de)provisioning #2497

Merged
merged 21 commits into from
Jul 22, 2024

Conversation

swift1337
Copy link
Contributor

@swift1337 swift1337 commented Jul 17, 2024

Description

This PR implements observers&signers runtime (de)provisioning. So then zetacore lacks $chain, $chainParams, or $chainParams.IsSupported == false zetaclient stops affected workers. Same for the opposite: then a net-new chain exists in the config of zetaclient AND it eventually pops up in chainParams or "becomes" supported, necessary workers are bootstrapped.

It also refactors code a bit and adds couple of useful packages, e.g. testrpc.

Context

In order to add a new chain, the following steps should happen:

  • gov proposal for new Chain creation
  • gov proposal for new ChainParam creation
  • zetaclient config update (e.g. RPC endpoint)
  • restart of zetaclient

This PR adds logic that periodically checks for updates in chains/chainParams and provisions signers & observers if net-new chain exists in zetaclient config. It also implements automatic deprovisioning

Changes

  • Refactor SQLite logic move it of observer package to db
  • Simplify appContext configuration, make it more streamlined
  • Move signers map creation to orchestrator. Support signers map sync based on AppContext
  • Move observers map creation to orchestrator. Support observers map sync based on AppContext
  • zetaclientd: program exits if operator is NOT an observer 🚨
  • Add BTC, EVM, and SOL RPCs mocks using httptest

How Has This Been Tested?

  • Tested CCTX in localnet
  • Tested in development environment
  • Go unit tests
  • Go integration tests
  • Tested via GitHub Actions

Summary by CodeRabbit

  • New Features

    • Added support for runtime chain (de)provisioning.
    • Introduced a database management system for handling SQLite connections and migrations.
  • Bug Fixes

    • Improved error handling and logging throughout various modules to provide clearer diagnostics.
  • Documentation

    • Updated changelog to reflect new features and changes.
  • Tests

    • Enhanced test cases for various functionalities, utilizing in-memory databases for improved reliability and performance.

@swift1337 swift1337 self-assigned this Jul 17, 2024
Copy link
Contributor

coderabbitai bot commented Jul 17, 2024

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Walkthrough

The recent updates to the Zeta Chain project significantly enhance its ability to manage blockchain chains dynamically at runtime. Key improvements include support for runtime chain (de)provisioning, streamlined observer and signer management, and a new, robust database interaction process. These changes foster greater flexibility and operational efficiency, enabling the system to adapt to new chain configurations without manual intervention or restarts.

Changes

Files Change Summary
changelog.md Added entry for support for runtime chain (de)provisioning (PR #2497).
cmd/zetaclientd/start.go, cmd/zetaclientd/utils.go Refactored observer node handling, improved logging, and removed redundant functions for enhanced clarity and error handling.
zetaclient/chains/base/observer.go, .../observer_test.go Modified database handling and concurrency management, improved error handling, and added test utilities for improved testing clarity.
zetaclient/chains/bitcoin/... Adjusted observer functionality, enhanced error handling, removed redundant parameters, and updated tests for streamlined database interactions.
zetaclient/context/app.go, .../app_test.go Simplified initialization and configuration management for AppContext, enhancing clarity and robustness in tests.
zetaclient/db/db.go, .../db_test.go Established a new database API with SQLite support, including in-memory capabilities and unit tests for validation.
zetaclient/orchestrator/... Created framework for managing signers and observers, introduced synchronization mechanisms, and improved logging and error handling.
zetaclient/testutils/... Introduced mock JSON-RPC servers for testing Bitcoin and EVM interactions, enhancing testing capabilities without external dependencies.

Assessment against linked issues

Objective Addressed Explanation
Dynamically add a new chain at runtime (1904)
Enable detection of newly added chains dynamically (1904)
Improve observer and signer management (1904)

Poem

🐇 In the meadow, new chains bloom,
As ZetaClient sheds old gloom.
With observers swift and signers bright,
It dances in the joyful light!
Dynamic changes, hopping free,
A lively world for you and me! 🌼✨


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 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 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 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 as 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.

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

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.

@swift1337 swift1337 added the zetaclient Issues related to ZetaClient label Jul 17, 2024
Copy link

codecov bot commented Jul 18, 2024

Codecov Report

Attention: Patch coverage is 59.75855% with 200 lines in your changes missing coverage. Please review.

Project coverage is 46.64%. Comparing base (e730d3a) to head (16fd762).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2497      +/-   ##
===========================================
+ Coverage    46.53%   46.64%   +0.11%     
===========================================
  Files          456      459       +3     
  Lines        30296    30590     +294     
===========================================
+ Hits         14098    14270     +172     
- Misses       15371    15473     +102     
- Partials       827      847      +20     
Files Coverage Δ
zetaclient/chains/bitcoin/signer/signer.go 17.57% <0.00%> (ø)
zetaclient/orchestrator/mapping.go 90.47% <90.47%> (ø)
zetaclient/context/app.go 85.00% <88.46%> (-0.81%) ⬇️
zetaclient/chains/evm/observer/observer.go 42.23% <42.85%> (-2.81%) ⬇️
pkg/ptr/ptr.go 28.57% <28.57%> (ø)
zetaclient/chains/solana/observer/observer.go 50.00% <61.53%> (+3.65%) ⬆️
zetaclient/chains/bitcoin/observer/observer.go 30.27% <33.33%> (-3.36%) ⬇️
zetaclient/db/db.go 71.42% <71.42%> (ø)
zetaclient/chains/base/observer.go 85.71% <47.82%> (-3.74%) ⬇️
zetaclient/orchestrator/bootstrap.go 72.47% <72.47%> (ø)
... and 1 more

... and 2 files with indirect coverage changes

@swift1337 swift1337 marked this pull request as ready for review July 19, 2024 16:39
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.

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (1)
cmd/zetaclientd/utils.go (1)

Line range hint 14-35:
Fix the typographical error in the variable name.

The variable granterAddreess should be renamed to granterAddress.

-	granterAddreess, err := sdk.AccAddressFromBech32(cfg.AuthzGranter)
+	granterAddress, err := sdk.AccAddressFromBech32(cfg.AuthzGranter)
Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 8e1cfa7 and 03e87c3.

Files selected for processing (29)
  • changelog.md (1 hunks)
  • cmd/zetaclientd/start.go (6 hunks)
  • cmd/zetaclientd/utils.go (2 hunks)
  • pkg/ptr/ptr.go (1 hunks)
  • zetaclient/chains/base/observer.go (10 hunks)
  • zetaclient/chains/base/observer_test.go (13 hunks)
  • zetaclient/chains/bitcoin/observer/observer.go (8 hunks)
  • zetaclient/chains/bitcoin/observer/observer_test.go (12 hunks)
  • zetaclient/chains/bitcoin/observer/outbound_test.go (2 hunks)
  • zetaclient/chains/bitcoin/rpc/rpc_live_test.go (2 hunks)
  • zetaclient/chains/bitcoin/signer/signer.go (2 hunks)
  • zetaclient/chains/evm/observer/inbound_test.go (18 hunks)
  • zetaclient/chains/evm/observer/observer.go (6 hunks)
  • zetaclient/chains/evm/observer/observer_test.go (10 hunks)
  • zetaclient/chains/evm/observer/outbound_test.go (6 hunks)
  • zetaclient/chains/evm/signer/signer_test.go (2 hunks)
  • zetaclient/context/app.go (6 hunks)
  • zetaclient/context/app_test.go (2 hunks)
  • zetaclient/db/db.go (1 hunks)
  • zetaclient/db/db_test.go (1 hunks)
  • zetaclient/orchestrator/bootstap_test.go (1 hunks)
  • zetaclient/orchestrator/bootstrap.go (1 hunks)
  • zetaclient/orchestrator/mapping.go (1 hunks)
  • zetaclient/orchestrator/orchestrator.go (17 hunks)
  • zetaclient/orchestrator/orchestrator_test.go (3 hunks)
  • zetaclient/testutils/constant.go (1 hunks)
  • zetaclient/testutils/testrpc/rpc.go (1 hunks)
  • zetaclient/testutils/testrpc/rpc_btc.go (1 hunks)
  • zetaclient/testutils/testrpc/rpc_evm.go (1 hunks)
Files skipped from review due to trivial changes (1)
  • zetaclient/testutils/constant.go
Additional context used
Path-based instructions (27)
pkg/ptr/ptr.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/testutils/testrpc/rpc_evm.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

cmd/zetaclientd/utils.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/testutils/testrpc/rpc_btc.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/orchestrator/mapping.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/db/db_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/testutils/testrpc/rpc.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/db/db.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/context/app.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/chains/bitcoin/observer/observer_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/chains/base/observer_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/chains/base/observer.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/orchestrator/bootstrap.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

cmd/zetaclientd/start.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/orchestrator/orchestrator_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/chains/evm/observer/observer_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/chains/bitcoin/signer/signer.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/context/app_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/orchestrator/bootstap_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/chains/bitcoin/rpc/rpc_live_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/chains/evm/signer/signer_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/chains/evm/observer/observer.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/chains/bitcoin/observer/outbound_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/chains/evm/observer/inbound_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/orchestrator/orchestrator.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/chains/bitcoin/observer/observer.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/chains/evm/observer/outbound_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

Learnings (4)
zetaclient/orchestrator/bootstrap.go (2)
Learnt from: ws4charlie
PR: zeta-chain/node#2411
File: zetaclient/orchestrator/chain_activate.go:184-247
Timestamp: 2024-07-05T00:02:31.446Z
Learning: The `CreateSignerObserverBTC` function in `zetaclient/orchestrator/chain_activate.go` is covered by unit tests in `zetaclient/orchestrator/chain_activate_test.go`.
Learnt from: ws4charlie
PR: zeta-chain/node#2411
File: zetaclient/orchestrator/chain_activate.go:116-181
Timestamp: 2024-07-05T00:02:36.493Z
Learning: The `CreateSignerObserverEVM` function in `zetaclient/orchestrator/chain_activate.go` is covered by unit tests in `zetaclient/orchestrator/chain_activate_test.go`.
zetaclient/orchestrator/orchestrator_test.go (4)
Learnt from: ws4charlie
PR: zeta-chain/node#2411
File: zetaclient/orchestrator/orchestrator.go:192-217
Timestamp: 2024-07-04T23:46:38.428Z
Learning: The `GetUpdatedSigner` method in `zetaclient/orchestrator/orchestrator.go` is covered by unit tests in `zetaclient/orchestrator/chain_activate_test.go` and `zetaclient/orchestrator/orchestrator_test.go`.
Learnt from: ws4charlie
PR: zeta-chain/node#2411
File: zetaclient/orchestrator/orchestrator.go:222-237
Timestamp: 2024-07-04T23:47:56.072Z
Learning: The `GetUpdatedChainObserver` method in the `Orchestrator` class is covered by unit tests in `zetaclient/orchestrator/orchestrator_test.go` and `zetaclient/orchestrator/chain_activate_test.go`.
Learnt from: ws4charlie
PR: zeta-chain/node#2411
File: zetaclient/orchestrator/chain_activate.go:116-181
Timestamp: 2024-07-05T00:02:36.493Z
Learning: The `CreateSignerObserverEVM` function in `zetaclient/orchestrator/chain_activate.go` is covered by unit tests in `zetaclient/orchestrator/chain_activate_test.go`.
Learnt from: ws4charlie
PR: zeta-chain/node#2411
File: zetaclient/orchestrator/chain_activate.go:184-247
Timestamp: 2024-07-05T00:02:31.446Z
Learning: The `CreateSignerObserverBTC` function in `zetaclient/orchestrator/chain_activate.go` is covered by unit tests in `zetaclient/orchestrator/chain_activate_test.go`.
zetaclient/orchestrator/bootstap_test.go (3)
Learnt from: ws4charlie
PR: zeta-chain/node#2411
File: zetaclient/orchestrator/chain_activate.go:184-247
Timestamp: 2024-07-05T00:02:31.446Z
Learning: The `CreateSignerObserverBTC` function in `zetaclient/orchestrator/chain_activate.go` is covered by unit tests in `zetaclient/orchestrator/chain_activate_test.go`.
Learnt from: ws4charlie
PR: zeta-chain/node#2411
File: zetaclient/orchestrator/chain_activate.go:116-181
Timestamp: 2024-07-05T00:02:36.493Z
Learning: The `CreateSignerObserverEVM` function in `zetaclient/orchestrator/chain_activate.go` is covered by unit tests in `zetaclient/orchestrator/chain_activate_test.go`.
Learnt from: ws4charlie
PR: zeta-chain/node#2411
File: zetaclient/orchestrator/orchestrator.go:192-217
Timestamp: 2024-07-04T23:46:38.428Z
Learning: The `GetUpdatedSigner` method in `zetaclient/orchestrator/orchestrator.go` is covered by unit tests in `zetaclient/orchestrator/chain_activate_test.go` and `zetaclient/orchestrator/orchestrator_test.go`.
zetaclient/chains/evm/signer/signer_test.go (3)
Learnt from: ws4charlie
PR: zeta-chain/node#2411
File: zetaclient/orchestrator/chain_activate.go:116-181
Timestamp: 2024-07-05T00:02:36.493Z
Learning: The `CreateSignerObserverEVM` function in `zetaclient/orchestrator/chain_activate.go` is covered by unit tests in `zetaclient/orchestrator/chain_activate_test.go`.
Learnt from: ws4charlie
PR: zeta-chain/node#2411
File: zetaclient/orchestrator/chain_activate.go:184-247
Timestamp: 2024-07-05T00:02:31.446Z
Learning: The `CreateSignerObserverBTC` function in `zetaclient/orchestrator/chain_activate.go` is covered by unit tests in `zetaclient/orchestrator/chain_activate_test.go`.
Learnt from: ws4charlie
PR: zeta-chain/node#2411
File: zetaclient/orchestrator/orchestrator.go:192-217
Timestamp: 2024-07-04T23:46:38.428Z
Learning: The `GetUpdatedSigner` method in `zetaclient/orchestrator/orchestrator.go` is covered by unit tests in `zetaclient/orchestrator/chain_activate_test.go` and `zetaclient/orchestrator/orchestrator_test.go`.
Additional comments not posted (126)
pkg/ptr/ptr.go (2)

4-7: LGTM!

The Ptr function is correctly implemented to return a pointer to the value passed in.


9-17: LGTM!

The Deref function is correctly implemented to return the value of the pointer passed in, or the zero value of the type if the pointer is nil.

zetaclient/testutils/testrpc/rpc_evm.go (4)

8-12: LGTM!

The EVMServer struct is correctly defined to represent an EVM RPC server for testing.


14-19: LGTM!

The NewEVMServer function correctly initializes and returns an EVMServer instance.


21-25: LGTM!

The SetBlockNumber function correctly sets the block number for the EVMServer.


27-29: LGTM!

The hex function correctly returns the hexadecimal representation of a value.

cmd/zetaclientd/utils.go (1)

10-12: LGTM!

The CreateAuthzSigner function correctly sets up the authorization signer list using the SetupAuthZSignerList method.

zetaclient/testutils/testrpc/rpc_btc.go (4)

13-16: LGTM! Struct definition is straightforward and correct.

The BtcServer struct correctly uses embedding to extend Server.


18-37: LGTM! Function is well-structured with necessary error handling.

The function correctly initializes a new BtcServer and returns a BTCConfig struct. Consider adding documentation for the rpc.On method to clarify its purpose.


39-43: LGTM! Function is straightforward and correct.

The function correctly sets the block count for the mocked Bitcoin RPC.


45-51: LGTM! Function correctly formats the Bitcoin RPC host URL.

The function includes necessary error handling and correctly formats the host and port.

zetaclient/orchestrator/mapping.go (4)

5-9: LGTM! Function is straightforward and correct.

The function correctly checks for the presence of a key in the map.


11-19: LGTM! Function is well-structured with an optional callback mechanism.

The function correctly sets a value for a given key in the map and optionally runs a callback.


21-35: LGTM! Function is well-structured with an optional callback mechanism.

The function correctly removes a value for a given key from the map and optionally runs a callback.


37-49: LGTM! Function is well-structured and correct.

The function correctly removes keys from the map that are not present in the provided list and includes an optional callback mechanism.

zetaclient/db/db_test.go (2)

11-45: LGTM! Test function is well-structured and correct.

The function correctly tests the creation of new SQLite databases, both in-memory and file-based, and includes necessary assertions. The use of subtests enhances clarity and organization.


47-67: LGTM! Test function is well-structured and correct.

The function correctly tests the set and get operations for the database and includes necessary assertions.

zetaclient/testutils/testrpc/rpc.go (5)

34-37: LGTM!

The code changes are approved.


14-19: LGTM!

The code changes are approved.

Also applies to: 39-49


78-86: LGTM! Verify the error handling.

The code changes are approved.

However, ensure that the error handling is correctly implemented and that all edge cases are covered.


21-31: LGTM! Verify the cleanup logic.

The code changes are approved.

However, ensure that the cleanup logic is correctly implemented and that the test server is properly closed after tests.

Verification successful

The rg command output shows multiple occurrences of the New function, but many of them are unrelated to the New function in zetaclient/testutils/testrpc/rpc.go. We need to filter out the relevant occurrences and check if the cleanup logic is properly implemented.

Let's refine our search to focus on the New function in zetaclient/testutils/testrpc/rpc.go and inspect how it is used in the test files.


To ensure that the cleanup logic is correctly implemented, we need to verify the test files that use NewEVMServer and NewBtcServer.


The NewEVMServer and NewBtcServer functions are used in the zetaclient/orchestrator/bootstap_test.go file. The t.Cleanup(testWeb.Close) line in the New function should ensure that the test server is properly closed after the tests.

Based on the information gathered, the cleanup logic seems to be correctly implemented, as the t.Cleanup(testWeb.Close) ensures that the test server is closed after the tests. Therefore, the review comment is verified.

LGTM! The cleanup logic is correctly implemented.

The code changes are approved.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the cleanup logic in the tests.

# Test: Search for the usage of the `New` function and verify the cleanup logic.
rg --type go 'New\('

Length of output: 35622


Script:

#!/bin/bash
# Description: Verify the cleanup logic in the tests for the specific `New` function in `zetaclient/testutils/testrpc/rpc.go`.

# Find all test files that call the `New` function from `zetaclient/testutils/testrpc/rpc.go`.
rg --type go --context 5 'New\(t, "EVM"\)' zetaclient/testutils/testrpc/rpc_evm.go
rg --type go --context 5 'New\(t, "bitcoin"\)' zetaclient/testutils/testrpc/rpc_btc.go

Length of output: 657


Script:

#!/bin/bash
# Description: Verify the cleanup logic in the tests for the specific `New` function in `zetaclient/testutils/testrpc/rpc.go`.

# Find all test files that call `NewEVMServer`.
rg --type go --context 5 'NewEVMServer\('

# Find all test files that call `NewBtcServer`.
rg --type go --context 5 'NewBtcServer\('

Length of output: 3582


51-76: LGTM! Verify the error handling and logging.

The code changes are approved.

However, ensure that the error handling and logging are correctly implemented and that all edge cases are covered.

zetaclient/db/db.go (7)

53-56: LGTM!

The code changes are approved.


75-78: LGTM!

The code changes are approved.


17-35: LGTM!

The code changes are approved.


58-73: LGTM! Verify the error handling.

The code changes are approved.

However, ensure that the error handling is correctly implemented and that all edge cases are covered.


94-110: LGTM! Verify the error handling.

The code changes are approved.

However, ensure that the error handling is correctly implemented and that all edge cases are covered.

Verification successful

Verified: Error handling in ensurePath is correctly implemented.

The ensurePath function's errors are properly checked and wrapped with additional context in the NewFromSqlite function, ensuring robust error handling.

  • zetaclient/db/db.go:
    • Line 94: ensurePath function definition.
    • Line 98: Error handling in NewFromSqlite function.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the error handling in the `ensurePath` function.

# Test: Search for the usage of the `ensurePath` function and verify the error handling.
rg --type go 'ensurePath'

Length of output: 169


Script:

#!/bin/bash
# Extract the surrounding lines of code where `ensurePath` is called to review error handling.
rg --type go 'ensurePath' -A 5 -B 5

Length of output: 1001


80-91: LGTM! Verify the error handling.

The code changes are approved.

However, ensure that the error handling is correctly implemented and that all edge cases are covered.

Verification successful

LGTM! Verify the error handling.

The code changes are approved. The error handling in the Close function calls is correctly implemented in the following instances:

  • zetaclient/db/db_test.go: Uses assert.NoError(t, db.Close()) for error handling in tests.
  • zetaclient/chains/base/observer.go: Logs the error with ob.Logger().Chain.Error().Err(err).Msgf("unable to close db for chain %d", ob.Chain().ChainId).
  • server/start.go: Logs the error with ctx.Logger.Error("error closing db", "error", err.Error()) and ctx.Logger.With("error", err).Error("error closing db").
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the error handling in the `Close` function.

# Test: Search for the usage of the `Close` function and verify the error handling.
rg --type go 'Close'

Length of output: 11285


Script:

#!/bin/bash
# Description: Extract context around the `Close` function calls to review error handling.

# Extract 5 lines before and after each occurrence of `db.Close()`.
rg --type go -A 5 -B 5 'db.Close()'

# Extract 5 lines before and after each occurrence of `db.Close();`.
rg --type go -A 5 -B 5 'db.Close();'

Length of output: 2499


43-51: LGTM! Verify the error handling.

The code changes are approved.

However, ensure that the error handling is correctly implemented and that all edge cases are covered.

Verification successful

Error handling is correctly implemented.

The error handling for the NewFromSqlite function is properly managed in both test and non-test files. Errors are checked and handled appropriately.

  • zetaclient/db/db_test.go: Errors are checked using require.NoError and require.NotNil.
  • zetaclient/orchestrator/bootstrap.go: Errors are logged and handled with a continue statement.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the error handling in the `NewFromSqlite` function.

# Test: Search for the usage of the `NewFromSqlite` function and verify the error handling.
rg --type go 'NewFromSqlite'

Length of output: 1715


Script:

#!/bin/bash
# Description: Verify the error handling in the `NewFromSqlite` function.

# Test: Check how the errors returned by `NewFromSqlite` are handled in the identified files.

# List of files to check
files=(
  "zetaclient/db/db_test.go"
  "zetaclient/orchestrator/bootstrap.go"
)

# Search for error handling in the identified files
for file in "${files[@]}"; do
  echo "Checking file: $file"
  rg --type go 'NewFromSqlite' -A 5 "$file"
done

Length of output: 1184

zetaclient/context/app.go (10)

39-47: LGTM!

The code changes are approved.


Line range hint 111-119:
LGTM!

The code changes are approved.


Line range hint 121-129:
LGTM!

The code changes are approved.


Line range hint 131-136:
LGTM!

The code changes are approved.


Line range hint 138-145:
LGTM!

The code changes are approved.


Line range hint 181-185:
LGTM!

The code changes are approved.


Line range hint 197-206:
LGTM!

The code changes are approved.


64-74: LGTM! Verify the error handling.

The code changes are approved.

However, ensure that the error handling is correctly implemented and that all edge cases are covered.

Verification successful

The error handling for the GetBTCChainAndConfig function is correctly implemented.

The btcEnabled flag is appropriately checked in the relevant files, ensuring that operations are only performed if the BTC chain and config are enabled.

  • zetaclient/orchestrator/bootstrap.go: The loop continues if btcEnabled is false.
  • cmd/zetaclientd/keygen_tss.go: Operations are performed only if btcEnabled is true.
  • zetaclient/context/app_test.go: The function's behavior is validated through assertions in a test case.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the error handling in the `GetBTCChainAndConfig` function.

# Test: Search for the usage of the `GetBTCChainAndConfig` function and verify the error handling.
rg --type go 'GetBTCChainAndConfig'

Length of output: 596


Script:

#!/bin/bash
# Extract relevant code snippets to verify error handling for `GetBTCChainAndConfig` usage.

# Extract usage in `zetaclient/orchestrator/bootstrap.go`
rg --type go -A 5 'GetBTCChainAndConfig' zetaclient/orchestrator/bootstrap.go

# Extract usage in `zetaclient/context/app_test.go`
rg --type go -A 5 'GetBTCChainAndConfig' zetaclient/context/app_test.go

# Extract usage in `cmd/zetaclientd/keygen_tss.go`
rg --type go -A 5 'GetBTCChainAndConfig' cmd/zetaclientd/keygen_tss.go

Length of output: 763


Line range hint 168-174:
LGTM! Verify the error handling.

The code changes are approved.

However, ensure that the error handling is correctly implemented and that all edge cases are covered.

Verification successful

The error handling for GetBTCChainParams is correctly implemented.

The function's return values are checked, and appropriate actions are taken based on whether the chain parameters are found and supported. Here are some key points:

  • In orchestrator.go, the function checks for the found variable and updates parameters if found is true.
  • In bootstrap.go, it uses a switch statement to handle different cases based on the found and IsSupported variables.
  • In tx_script.go, headers.go, chain.go, and address.go, the function checks for errors returned by GetBTCChainParams and handles them appropriately.
  • The test files (app_test.go) verify the correct behavior of GetBTCChainParams by asserting the expected outputs.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the error handling in the `GetBTCChainParams` function.

# Test: Search for the usage of the `GetBTCChainParams` function and verify the error handling.
rg --type go 'GetBTCChainParams'

Length of output: 2220


Script:

#!/bin/bash
# Extract lines around the usage of `GetBTCChainParams` to verify error handling

# Extract usage from orchestrator.go
echo "== orchestrator.go =="
rg --context 5 'GetBTCChainParams' zetaclient/orchestrator/orchestrator.go

# Extract usage from bootstrap.go
echo "== bootstrap.go =="
rg --context 5 'GetBTCChainParams' zetaclient/orchestrator/bootstrap.go

# Extract usage from app_test.go
echo "== app_test.go =="
rg --context 5 'GetBTCChainParams' zetaclient/context/app_test.go

# Extract usage from tx_script.go
echo "== tx_script.go =="
rg --context 5 'GetBTCChainParams' zetaclient/chains/bitcoin/tx_script.go

# Extract usage from headers.go
echo "== headers.go =="
rg --context 5 'GetBTCChainParams' pkg/proofs/headers.go

# Extract usage from chain.go
echo "== chain.go =="
rg --context 5 'GetBTCChainParams' pkg/chains/chain.go

# Extract usage from address.go
echo "== address.go =="
rg --context 5 'GetBTCChainParams' pkg/chains/address.go

Length of output: 5243


Line range hint 231-279:
LGTM! Verify the error handling and logging.

The code changes are approved.

However, ensure that the error handling and logging are correctly implemented and that all edge cases are covered.

zetaclient/chains/bitcoin/observer/observer_test.go (3)

Line range hint 37-62:
LGTM!

The database initialization using db.NewFromSqliteInMemory(true) is correctly implemented, and the function populates the database as expected.


Line range hint 77-86:
LGTM!

The database initialization using db.NewFromSqliteInMemory(true) is correctly implemented, and the function creates the observer as expected.


153-184: LGTM!

The database initialization using db.NewFromSqliteInMemory(true) is correctly implemented, and the test cases cover the expected scenarios.

zetaclient/chains/base/observer_test.go (3)

Line range hint 31-43:
LGTM!

The use of the new helper function createDatabase is correctly implemented, and the function creates the observer as expected.


Line range hint 61-124:
LGTM!

The use of the new helper function createDatabase is correctly implemented, and the test cases cover the expected scenarios.


351-356: LGTM!

The function correctly creates an in-memory SQLite database and handles errors appropriately.

zetaclient/chains/base/observer.go (6)

Line range hint 89-119:
LGTM!

The use of the new database type db.DB is correctly implemented, and the function initializes the observer as expected.


124-137: LGTM!

The mutex locking is correctly implemented, and the function handles the observer's state as expected.


141-153: LGTM!

The mutex locking is correctly implemented, and the function handles the observer's state and database closing as expected.


248-249: LGTM!

The new return type db.DB is correctly used.


Line range hint 308-326:
LGTM!

The enhanced error handling is correctly implemented, and the function loads the last scanned block as expected.


335-336: LGTM!

The new database type db.DB is correctly used, and the function saves the last scanned block as expected.

zetaclient/chains/evm/observer/observer_test.go (3)

109-111: LGTM! The use of in-memory SQLite improves test isolation.

The changes enhance test reliability by avoiding dependencies on file system paths.


Line range hint 136-189:
LGTM! The updated error handling and use of in-memory SQLite improve test reliability.

The changes ensure that the tests are less prone to environmental issues and more focused on the core functionality.


Line range hint 244-267:
LGTM! The use of in-memory SQLite improves test isolation.

The changes enhance test reliability by avoiding dependencies on file system paths.

zetaclient/chains/bitcoin/signer/signer.go (1)

19-19: LGTM! The enhanced error reporting improves debugging and error tracking.

Using errors.Wrap provides more descriptive context to the error messages.

Also applies to: 81-81

zetaclient/context/app_test.go (1)

9-9: LGTM! The use of mock chain parameters improves test robustness and maintainability.

The changes ensure that the test accurately validates the behavior of the appContext when handling EVM chain parameters.

Also applies to: 73-114

zetaclient/orchestrator/bootstap_test.go (5)

419-445: LGTM!

The chainParams function correctly generates the chain parameters for supported chains. The code changes are approved.


447-450: LGTM!

The mustUpdateAppContextChainParams function correctly updates the application context with chain parameters. The code changes are approved.


452-470: LGTM!

The mustUpdateAppContext function correctly updates the application context with various parameters. The code changes are approved.


22-217: LGTM! But verify the function usage in the codebase.

The test cases for CreateSignerMap are comprehensive and cover various scenarios. The code changes are approved.

However, ensure that all function calls to CreateSignerMap are correctly implemented in the codebase.


219-417: LGTM! But verify the function usage in the codebase.

The test cases for CreateChainObserverMap are comprehensive and cover various scenarios. The code changes are approved.

However, ensure that all function calls to CreateChainObserverMap are correctly implemented in the codebase.

Verification successful

Verify the implementation of CreateChainObserverMap in cmd/zetaclientd/start.go.

The function CreateChainObserverMap is correctly defined and tested. However, please verify the implementation in cmd/zetaclientd/start.go to ensure it is correctly handling the function.


The function CreateChainObserverMap is correctly implemented in the codebase.

The implementation in cmd/zetaclientd/start.go correctly handles the creation of the observerMap and appropriately logs and returns any errors that occur.

  • cmd/zetaclientd/start.go: Correctly initializes observerMap using CreateChainObserverMap and handles errors.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `CreateChainObserverMap` are correctly implemented.

# Test: Search for the function usage. Expect: Only occurances of the correct implementation.
rg --type go -A 5 $'CreateChainObserverMap'

Length of output: 2529


Script:

#!/bin/bash
# Description: Verify the implementation of `CreateChainObserverMap` in `cmd/zetaclientd/start.go`.

# Test: Search for the function usage and retrieve the surrounding context.
rg --type go -A 10 -B 10 $'CreateChainObserverMap' cmd/zetaclientd/start.go

Length of output: 843

zetaclient/chains/bitcoin/rpc/rpc_live_test.go (3)

58-63: LGTM!

The changes to SetupTest improve test isolation and performance by using an in-memory SQLite database. The code changes are approved.


Line range hint 73-96:
LGTM!

The createRPCClient function correctly creates a new Bitcoin RPC client based on the chain ID. The code changes are approved.


Line range hint 98-115:
LGTM!

The getFeeRate function correctly retrieves the fee rate from the Bitcoin RPC client. The code changes are approved.

zetaclient/chains/evm/signer/signer_test.go (3)

84-94: LGTM!

The changes to getNewEvmChainObserver improve test isolation and performance by using an in-memory SQLite database. The code changes are approved.


Line range hint 46-62:
LGTM!

The getNewEvmSigner function correctly creates a new EVM signer for testing. The code changes are approved.


Line range hint 142-154:
LGTM!

The TestSigner_SetGetConnectorAddress function correctly tests setting and getting the connector address for the EVM signer. The code changes are approved.

zetaclient/chains/evm/observer/observer.go (3)

170-173: LGTM! Improved startup routine.

The new check to verify if the observer is already running improves the robustness of the startup routine and enhances logging for better traceability.


96-97: LGTM! Enhanced error handling and logging.

The enhanced error handling and logging in the LoadLastBlockScanned method improve maintainability and traceability.


Line range hint 65-97:
LGTM! Improved database handling.

The change to replace the dbpath string parameter with a pointer to a db.DB instance enhances the handling of database connections and resource management.

However, ensure that all calls to NewObserver are updated to match the new function signature.

zetaclient/chains/bitcoin/observer/outbound_test.go (1)

31-35: LGTM! Improved database initialization.

The change to initialize a new database using db.NewFromSqliteInMemory(true) enhances the test's reliance on an actual database instance, improving the accuracy and reliability of the tests.

zetaclient/chains/evm/observer/inbound_test.go (17)

48-48: LGTM! Simplified observer instantiation.

The removal of the memDBPath parameter simplifies the instantiation of the MockEVMObserver.


64-64: LGTM! Simplified observer instantiation.

The removal of the memDBPath parameter simplifies the instantiation of the MockEVMObserver.


80-80: LGTM! Simplified observer instantiation.

The removal of the memDBPath parameter simplifies the instantiation of the MockEVMObserver.


134-134: LGTM! Simplified observer instantiation.

The removal of the memDBPath parameter simplifies the instantiation of the MockEVMObserver.


150-150: LGTM! Simplified observer instantiation.

The removal of the memDBPath parameter simplifies the instantiation of the MockEVMObserver.


166-166: LGTM! Simplified observer instantiation.

The removal of the memDBPath parameter simplifies the instantiation of the MockEVMObserver.


220-220: LGTM! Simplified observer instantiation.

The removal of the memDBPath parameter simplifies the instantiation of the MockEVMObserver.


230-230: LGTM! Simplified observer instantiation.

The removal of the memDBPath parameter simplifies the instantiation of the MockEVMObserver.


240-240: LGTM! Simplified observer instantiation.

The removal of the memDBPath parameter simplifies the instantiation of the MockEVMObserver.


251-251: LGTM! Simplified observer instantiation.

The removal of the memDBPath parameter simplifies the instantiation of the MockEVMObserver.


262-262: LGTM! Simplified observer instantiation.

The removal of the memDBPath parameter simplifies the instantiation of the MockEVMObserver.


279-279: LGTM! Simplified observer instantiation.

The removal of the memDBPath parameter simplifies the instantiation of the MockEVMObserver.


328-328: LGTM! Simplified observer instantiation.

The removal of the memDBPath parameter simplifies the instantiation of the MockEVMObserver.


386-386: LGTM! Simplified observer instantiation.

The removal of the memDBPath parameter simplifies the instantiation of the MockEVMObserver.


463-463: LGTM! Simplified observer instantiation.

The removal of the memDBPath parameter simplifies the instantiation of the MockEVMObserver.


472-472: LGTM! Simplified observer instantiation.

The removal of the memDBPath parameter simplifies the instantiation of the MockEVMObserver.


478-478: LGTM! Simplified observer instantiation.

The removal of the memDBPath parameter simplifies the instantiation of the MockEVMObserver.

zetaclient/orchestrator/orchestrator.go (15)

8-8: New imports are necessary and correctly used.

The sync package is used for the RWMutex, and the errors package is used for improved error handling.

Also applies to: 13-13


Line range hint 35-39:
New constants are appropriately defined.

The constants evmOutboundLookbackFactor and loggerSamplingRate are well-defined and used in the code for configuring lookback factors and logger sampling rates.


66-69: New fields in Orchestrator struct are necessary and correctly used.

The fields logger, stop, and mu enhance logging, graceful shutdown, and concurrency control.


72-75: multiLogger struct is correctly defined and used.

The multiLogger struct encapsulates a standard logger and a sampled logger, improving logging flexibility.


77-119: New function improvements are correct and beneficial.

The New function now initializes the Orchestrator with the new fields and includes improved error handling, enhancing robustness.


Line range hint 122-147:
Start method improvements are correct and enhance operations.

The Start method now better reflects its role in initiating the orchestrator's operations, with improved error handling and logging.


148-183: resolveSigner method improvements are correct and beneficial.

The resolveSigner method now includes better logic for updating chain parameters and improved error handling.


185-195: getSigner method is correctly defined and used.

The getSigner method provides a thread-safe way to retrieve signers from the signerMap.


197-223: resolveObserver method improvements are correct and beneficial.

The resolveObserver method now includes better logic for updating chain parameters and improved error handling.


227-237: getObserver method is correctly defined and used.

The getObserver method provides a thread-safe way to retrieve observers from the observerMap.


Line range hint 290-381:
runScheduler method improvements are correct and beneficial.

The runScheduler method now includes better logic for scheduling keysigns and improved error handling.


Line range hint 405-474:
ScheduleCctxEVM method improvements are correct and beneficial.

The ScheduleCctxEVM method now includes better error handling and logging, enhancing the scheduling process.


Line range hint 508-555:
ScheduleCctxBTC method improvements are correct and beneficial.

The ScheduleCctxBTC method now includes better error handling and logging, enhancing the scheduling process.


569-586: runObserverSignerSync method is correctly defined and used.

The runObserverSignerSync method provides a periodic synchronization mechanism for observers and signers.


588-622: syncObserverSigner method is correctly defined and used.

The syncObserverSigner method provides a mechanism to synchronize and provision observers and signers, enhancing the orchestrator's functionality.

zetaclient/chains/bitcoin/observer/observer.go (7)

30-30: New import for db is necessary and correctly used.

The db package is used for handling database interactions, which is a core part of the observer's functionality.


Line range hint 72-74:
Changes in Observer struct are correct and beneficial.

The Observer struct now includes a direct database object reference, improving database interactions.


Line range hint 120-171:
NewObserver function improvements are correct and beneficial.

The NewObserver function now accepts a direct database object reference and includes improved error handling, enhancing the initialization process.


203-206: Start method improvements are correct and beneficial.

The Start method now includes a check for whether the observer is already running, improving the control flow and user experience.


543-543: SaveBroadcastedTx method improvements are correct and beneficial.

The SaveBroadcastedTx method now uses the Client() method for database interactions, enhancing robustness.


615-615: LoadLastBlockScanned method improvements are correct and beneficial.

The LoadLastBlockScanned method now includes improved error handling, enhancing robustness.


615-615: LoadBroadcastedTxMap method improvements are correct and beneficial.

The LoadBroadcastedTxMap method now uses the Client() method for database interactions, enhancing robustness.

zetaclient/chains/evm/observer/outbound_test.go (1)

63-63: Changes in Test_IsOutboundProcessed function are correct and do not affect the overall logic.

The memDBPath constant has been replaced with the integer value 1 in the MockEVMObserver function calls, simplifying the function calls without affecting the test logic.

Copy link
Member

@lumtis lumtis left a comment

Choose a reason for hiding this comment

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

Overall changes look good to me, just a few comments

Seems like this issue can close #2019 ?

zetaclient/orchestrator/bootstap_test.go Show resolved Hide resolved
zetaclient/orchestrator/bootstrap.go Outdated Show resolved Hide resolved
zetaclient/orchestrator/bootstrap.go Outdated Show resolved Hide resolved
zetaclient/orchestrator/bootstrap.go Outdated Show resolved Hide resolved
zetaclient/orchestrator/mapping.go Show resolved Hide resolved
zetaclient/orchestrator/orchestrator.go Show resolved Hide resolved
@swift1337
Copy link
Contributor Author

Seems like this issue can close #2019 ?

I would say it closes it only partially as we still lots of todo: revamp

# Conflicts:
#	changelog.md
#	cmd/zetaclientd/start.go
#	cmd/zetaclientd/utils.go
#	zetaclient/chains/base/observer.go
#	zetaclient/chains/base/observer_test.go
#	zetaclient/context/app.go
Copy link
Contributor

@ws4charlie ws4charlie left a comment

Choose a reason for hiding this comment

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

looks good. Just a few clarifications.

@swift1337 swift1337 enabled auto-merge July 22, 2024 17:54
@swift1337 swift1337 added this pull request to the merge queue Jul 22, 2024
Merged via the queue into develop with commit b2bc8a9 Jul 22, 2024
28 checks passed
@swift1337 swift1337 deleted the feat/zetaclient-runtime-chains branch July 22, 2024 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking:cli zetaclient Issues related to ZetaClient
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: Zetaclient should be able to enable newly added chain dynamically at runtime
3 participants