-
Notifications
You must be signed in to change notification settings - Fork 108
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
Conversation
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughWalkthroughThe 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
Assessment against linked issues
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ 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
|
There was a problem hiding this 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 togranterAddress
.- granterAddreess, err := sdk.AccAddressFromBech32(cfg.AuthzGranter) + granterAddress, err := sdk.AccAddressFromBech32(cfg.AuthzGranter)
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
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 anEVMServer
instance.
21-25
: LGTM!The
SetBlockNumber
function correctly sets the block number for theEVMServer
.
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 theSetupAuthZSignerList
method.zetaclient/testutils/testrpc/rpc_btc.go (4)
13-16
: LGTM! Struct definition is straightforward and correct.The
BtcServer
struct correctly uses embedding to extendServer
.
18-37
: LGTM! Function is well-structured with necessary error handling.The function correctly initializes a new
BtcServer
and returns aBTCConfig
struct. Consider adding documentation for therpc.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 theNew
function, but many of them are unrelated to theNew
function inzetaclient/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 inzetaclient/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
andNewBtcServer
.
The
NewEVMServer
andNewBtcServer
functions are used in thezetaclient/orchestrator/bootstap_test.go
file. Thet.Cleanup(testWeb.Close)
line in theNew
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.goLength 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 theNewFromSqlite
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 5Length 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
: Usesassert.NoError(t, db.Close())
for error handling in tests.zetaclient/chains/base/observer.go
: Logs the error withob.Logger().Chain.Error().Err(err).Msgf("unable to close db for chain %d", ob.Chain().ChainId)
.server/start.go
: Logs the error withctx.Logger.Error("error closing db", "error", err.Error())
andctx.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 usingrequire.NoError
andrequire.NotNil
.zetaclient/orchestrator/bootstrap.go
: Errors are logged and handled with acontinue
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" doneLength 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 ifbtcEnabled
is false.cmd/zetaclientd/keygen_tss.go
: Operations are performed only ifbtcEnabled
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.goLength 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 thefound
variable and updates parameters iffound
is true.- In
bootstrap.go
, it uses a switch statement to handle different cases based on thefound
andIsSupported
variables.- In
tx_script.go
,headers.go
,chain.go
, andaddress.go
, the function checks for errors returned byGetBTCChainParams
and handles them appropriately.- The test files (
app_test.go
) verify the correct behavior ofGetBTCChainParams
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.goLength 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
incmd/zetaclientd/start.go
.The function
CreateChainObserverMap
is correctly defined and tested. However, please verify the implementation incmd/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 theobserverMap
and appropriately logs and returns any errors that occur.
cmd/zetaclientd/start.go
: Correctly initializesobserverMap
usingCreateChainObserverMap
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.goLength 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 adb.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 theMockEVMObserver
.
64-64
: LGTM! Simplified observer instantiation.The removal of the
memDBPath
parameter simplifies the instantiation of theMockEVMObserver
.
80-80
: LGTM! Simplified observer instantiation.The removal of the
memDBPath
parameter simplifies the instantiation of theMockEVMObserver
.
134-134
: LGTM! Simplified observer instantiation.The removal of the
memDBPath
parameter simplifies the instantiation of theMockEVMObserver
.
150-150
: LGTM! Simplified observer instantiation.The removal of the
memDBPath
parameter simplifies the instantiation of theMockEVMObserver
.
166-166
: LGTM! Simplified observer instantiation.The removal of the
memDBPath
parameter simplifies the instantiation of theMockEVMObserver
.
220-220
: LGTM! Simplified observer instantiation.The removal of the
memDBPath
parameter simplifies the instantiation of theMockEVMObserver
.
230-230
: LGTM! Simplified observer instantiation.The removal of the
memDBPath
parameter simplifies the instantiation of theMockEVMObserver
.
240-240
: LGTM! Simplified observer instantiation.The removal of the
memDBPath
parameter simplifies the instantiation of theMockEVMObserver
.
251-251
: LGTM! Simplified observer instantiation.The removal of the
memDBPath
parameter simplifies the instantiation of theMockEVMObserver
.
262-262
: LGTM! Simplified observer instantiation.The removal of the
memDBPath
parameter simplifies the instantiation of theMockEVMObserver
.
279-279
: LGTM! Simplified observer instantiation.The removal of the
memDBPath
parameter simplifies the instantiation of theMockEVMObserver
.
328-328
: LGTM! Simplified observer instantiation.The removal of the
memDBPath
parameter simplifies the instantiation of theMockEVMObserver
.
386-386
: LGTM! Simplified observer instantiation.The removal of the
memDBPath
parameter simplifies the instantiation of theMockEVMObserver
.
463-463
: LGTM! Simplified observer instantiation.The removal of the
memDBPath
parameter simplifies the instantiation of theMockEVMObserver
.
472-472
: LGTM! Simplified observer instantiation.The removal of the
memDBPath
parameter simplifies the instantiation of theMockEVMObserver
.
478-478
: LGTM! Simplified observer instantiation.The removal of the
memDBPath
parameter simplifies the instantiation of theMockEVMObserver
.zetaclient/orchestrator/orchestrator.go (15)
8-8
: New imports are necessary and correctly used.The
sync
package is used for theRWMutex
, and theerrors
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
andloggerSamplingRate
are well-defined and used in the code for configuring lookback factors and logger sampling rates.
66-69
: New fields inOrchestrator
struct are necessary and correctly used.The fields
logger
,stop
, andmu
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 theOrchestrator
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 thesignerMap
.
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 theobserverMap
.
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 fordb
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 inObserver
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 theClient()
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 theClient()
method for database interactions, enhancing robustness.zetaclient/chains/evm/observer/outbound_test.go (1)
63-63
: Changes inTest_IsOutboundProcessed
function are correct and do not affect the overall logic.The
memDBPath
constant has been replaced with the integer value1
in theMockEVMObserver
function calls, simplifying the function calls without affecting the test logic.
There was a problem hiding this 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 ?
I would say it closes it only partially as we still lots of |
# 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
There was a problem hiding this 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.
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:
Chain
creationChainParam
creationzetaclient
config update (e.g. RPC endpoint)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
observer
package todb
orchestrator
. Support signers map sync based onAppContext
orchestrator
. Support observers map sync based onAppContext
zetaclientd
: program exits if operator is NOT an observer 🚨httptest
How Has This Been Tested?
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests