feat(zetaclient)!: support for runtime chain (de)provisioning #2497
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
|
swift1337
requested review from
brewmaster012,
fbac,
kingpinXD,
lumtis,
skosito and
ws4charlie
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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
granterAddreessshould 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
Ptrfunction is correctly implemented to return a pointer to the value passed in.
9-17: LGTM!The
Dereffunction 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
EVMServerstruct is correctly defined to represent an EVM RPC server for testing.
14-19: LGTM!The
NewEVMServerfunction correctly initializes and returns anEVMServerinstance.
21-25: LGTM!The
SetBlockNumberfunction correctly sets the block number for theEVMServer.
27-29: LGTM!The
hexfunction correctly returns the hexadecimal representation of a value.cmd/zetaclientd/utils.go (1)
10-12: LGTM!The
CreateAuthzSignerfunction correctly sets up the authorization signer list using theSetupAuthZSignerListmethod.zetaclient/testutils/testrpc/rpc_btc.go (4)
13-16: LGTM! Struct definition is straightforward and correct.The
BtcServerstruct correctly uses embedding to extendServer.
18-37: LGTM! Function is well-structured with necessary error handling.The function correctly initializes a new
BtcServerand returns aBTCConfigstruct. Consider adding documentation for therpc.Onmethod 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
rgcommand output shows multiple occurrences of theNewfunction, but many of them are unrelated to theNewfunction 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
Newfunction inzetaclient/testutils/testrpc/rpc.goand 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
NewEVMServerandNewBtcServer.
The
NewEVMServerandNewBtcServerfunctions are used in thezetaclient/orchestrator/bootstap_test.gofile. Thet.Cleanup(testWeb.Close)line in theNewfunction 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
ensurePathis correctly implemented.The
ensurePathfunction's errors are properly checked and wrapped with additional context in theNewFromSqlitefunction, ensuring robust error handling.
zetaclient/db/db.go:
- Line 94:
ensurePathfunction definition.- Line 98: Error handling in
NewFromSqlitefunction.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
Closefunction 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
NewFromSqlitefunction 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.NoErrorandrequire.NotNil.zetaclient/orchestrator/bootstrap.go: Errors are logged and handled with acontinuestatement.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
GetBTCChainAndConfigfunction is correctly implemented.The
btcEnabledflag 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 ifbtcEnabledis false.cmd/zetaclientd/keygen_tss.go: Operations are performed only ifbtcEnabledis 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
GetBTCChainParamsis 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 thefoundvariable and updates parameters iffoundis true.- In
bootstrap.go, it uses a switch statement to handle different cases based on thefoundandIsSupportedvariables.- In
tx_script.go,headers.go,chain.go, andaddress.go, the function checks for errors returned byGetBTCChainParamsand handles them appropriately.- The test files (
app_test.go) verify the correct behavior ofGetBTCChainParamsby 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
createDatabaseis correctly implemented, and the function creates the observer as expected.
Line range hint
61-124:
LGTM!The use of the new helper function
createDatabaseis 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.DBis 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.DBis 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.DBis 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.Wrapprovides 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
appContextwhen handling EVM chain parameters.Also applies to: 73-114
zetaclient/orchestrator/bootstap_test.go (5)
419-445: LGTM!The
chainParamsfunction correctly generates the chain parameters for supported chains. The code changes are approved.
447-450: LGTM!The
mustUpdateAppContextChainParamsfunction correctly updates the application context with chain parameters. The code changes are approved.
452-470: LGTM!The
mustUpdateAppContextfunction 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
CreateSignerMapare comprehensive and cover various scenarios. The code changes are approved.However, ensure that all function calls to
CreateSignerMapare correctly implemented in the codebase.
219-417: LGTM! But verify the function usage in the codebase.The test cases for
CreateChainObserverMapare comprehensive and cover various scenarios. The code changes are approved.However, ensure that all function calls to
CreateChainObserverMapare correctly implemented in the codebase.Verification successful
Verify the implementation of
CreateChainObserverMapincmd/zetaclientd/start.go.The function
CreateChainObserverMapis correctly defined and tested. However, please verify the implementation incmd/zetaclientd/start.goto ensure it is correctly handling the function.
The function
CreateChainObserverMapis correctly implemented in the codebase.The implementation in
cmd/zetaclientd/start.gocorrectly handles the creation of theobserverMapand appropriately logs and returns any errors that occur.
cmd/zetaclientd/start.go: Correctly initializesobserverMapusingCreateChainObserverMapand 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
SetupTestimprove test isolation and performance by using an in-memory SQLite database. The code changes are approved.
Line range hint
73-96:
LGTM!The
createRPCClientfunction correctly creates a new Bitcoin RPC client based on the chain ID. The code changes are approved.
Line range hint
98-115:
LGTM!The
getFeeRatefunction 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
getNewEvmChainObserverimprove test isolation and performance by using an in-memory SQLite database. The code changes are approved.
Line range hint
46-62:
LGTM!The
getNewEvmSignerfunction correctly creates a new EVM signer for testing. The code changes are approved.
Line range hint
142-154:
LGTM!The
TestSigner_SetGetConnectorAddressfunction 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
LoadLastBlockScannedmethod improve maintainability and traceability.
Line range hint
65-97:
LGTM! Improved database handling.The change to replace the
dbpathstring parameter with a pointer to adb.DBinstance enhances the handling of database connections and resource management.However, ensure that all calls to
NewObserverare 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
memDBPathparameter simplifies the instantiation of theMockEVMObserver.
64-64: LGTM! Simplified observer instantiation.The removal of the
memDBPathparameter simplifies the instantiation of theMockEVMObserver.
80-80: LGTM! Simplified observer instantiation.The removal of the
memDBPathparameter simplifies the instantiation of theMockEVMObserver.
134-134: LGTM! Simplified observer instantiation.The removal of the
memDBPathparameter simplifies the instantiation of theMockEVMObserver.
150-150: LGTM! Simplified observer instantiation.The removal of the
memDBPathparameter simplifies the instantiation of theMockEVMObserver.
166-166: LGTM! Simplified observer instantiation.The removal of the
memDBPathparameter simplifies the instantiation of theMockEVMObserver.
220-220: LGTM! Simplified observer instantiation.The removal of the
memDBPathparameter simplifies the instantiation of theMockEVMObserver.
230-230: LGTM! Simplified observer instantiation.The removal of the
memDBPathparameter simplifies the instantiation of theMockEVMObserver.
240-240: LGTM! Simplified observer instantiation.The removal of the
memDBPathparameter simplifies the instantiation of theMockEVMObserver.
251-251: LGTM! Simplified observer instantiation.The removal of the
memDBPathparameter simplifies the instantiation of theMockEVMObserver.
262-262: LGTM! Simplified observer instantiation.The removal of the
memDBPathparameter simplifies the instantiation of theMockEVMObserver.
279-279: LGTM! Simplified observer instantiation.The removal of the
memDBPathparameter simplifies the instantiation of theMockEVMObserver.
328-328: LGTM! Simplified observer instantiation.The removal of the
memDBPathparameter simplifies the instantiation of theMockEVMObserver.
386-386: LGTM! Simplified observer instantiation.The removal of the
memDBPathparameter simplifies the instantiation of theMockEVMObserver.
463-463: LGTM! Simplified observer instantiation.The removal of the
memDBPathparameter simplifies the instantiation of theMockEVMObserver.
472-472: LGTM! Simplified observer instantiation.The removal of the
memDBPathparameter simplifies the instantiation of theMockEVMObserver.
478-478: LGTM! Simplified observer instantiation.The removal of the
memDBPathparameter simplifies the instantiation of theMockEVMObserver.zetaclient/orchestrator/orchestrator.go (15)
8-8: New imports are necessary and correctly used.The
syncpackage is used for theRWMutex, and theerrorspackage is used for improved error handling.Also applies to: 13-13
Line range hint
35-39:
New constants are appropriately defined.The constants
evmOutboundLookbackFactorandloggerSamplingRateare well-defined and used in the code for configuring lookback factors and logger sampling rates.
66-69: New fields inOrchestratorstruct are necessary and correctly used.The fields
logger,stop, andmuenhance logging, graceful shutdown, and concurrency control.
72-75:multiLoggerstruct is correctly defined and used.The
multiLoggerstruct encapsulates a standard logger and a sampled logger, improving logging flexibility.
77-119:Newfunction improvements are correct and beneficial.The
Newfunction now initializes theOrchestratorwith the new fields and includes improved error handling, enhancing robustness.
Line range hint
122-147:
Startmethod improvements are correct and enhance operations.The
Startmethod now better reflects its role in initiating the orchestrator's operations, with improved error handling and logging.
148-183:resolveSignermethod improvements are correct and beneficial.The
resolveSignermethod now includes better logic for updating chain parameters and improved error handling.
185-195:getSignermethod is correctly defined and used.The
getSignermethod provides a thread-safe way to retrieve signers from thesignerMap.
197-223:resolveObservermethod improvements are correct and beneficial.The
resolveObservermethod now includes better logic for updating chain parameters and improved error handling.
227-237:getObservermethod is correctly defined and used.The
getObservermethod provides a thread-safe way to retrieve observers from theobserverMap.
Line range hint
290-381:
runSchedulermethod improvements are correct and beneficial.The
runSchedulermethod now includes better logic for scheduling keysigns and improved error handling.
Line range hint
405-474:
ScheduleCctxEVMmethod improvements are correct and beneficial.The
ScheduleCctxEVMmethod now includes better error handling and logging, enhancing the scheduling process.
Line range hint
508-555:
ScheduleCctxBTCmethod improvements are correct and beneficial.The
ScheduleCctxBTCmethod now includes better error handling and logging, enhancing the scheduling process.
569-586:runObserverSignerSyncmethod is correctly defined and used.The
runObserverSignerSyncmethod provides a periodic synchronization mechanism for observers and signers.
588-622:syncObserverSignermethod is correctly defined and used.The
syncObserverSignermethod 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 fordbis necessary and correctly used.The
dbpackage is used for handling database interactions, which is a core part of the observer's functionality.
Line range hint
72-74:
Changes inObserverstruct are correct and beneficial.The
Observerstruct now includes a direct database object reference, improving database interactions.
Line range hint
120-171:
NewObserverfunction improvements are correct and beneficial.The
NewObserverfunction now accepts a direct database object reference and includes improved error handling, enhancing the initialization process.
203-206:Startmethod improvements are correct and beneficial.The
Startmethod now includes a check for whether the observer is already running, improving the control flow and user experience.
543-543:SaveBroadcastedTxmethod improvements are correct and beneficial.The
SaveBroadcastedTxmethod now uses theClient()method for database interactions, enhancing robustness.
615-615:LoadLastBlockScannedmethod improvements are correct and beneficial.The
LoadLastBlockScannedmethod now includes improved error handling, enhancing robustness.
615-615:LoadBroadcastedTxMapmethod improvements are correct and beneficial.The
LoadBroadcastedTxMapmethod now uses theClient()method for database interactions, enhancing robustness.zetaclient/chains/evm/observer/outbound_test.go (1)
63-63: Changes inTest_IsOutboundProcessedfunction are correct and do not affect the overall logic.The
memDBPathconstant has been replaced with the integer value1in theMockEVMObserverfunction calls, simplifying the function calls without affecting the test logic.
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
ws4charlie
left a comment
ws4charlie
left a comment
There was a problem hiding this comment.
looks good. Just a few clarifications.
3 participants
Description
This PR implements observers&signers runtime (de)provisioning. So then zetacore lacks
$chain,$chainParams, or$chainParams.IsSupported == falsezetaclient 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:
ChaincreationChainParamcreationzetaclientconfig update (e.g. RPC endpoint)zetaclientThis 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
observerpackage todborchestrator. Support signers map sync based onAppContextorchestrator. Support observers map sync based onAppContextzetaclientd: program exits if operator is NOT an observer 🚨httptestHow Has This Been Tested?
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests