feat(zetaclient): minor code improvements#4014
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 📝 WalkthroughWalkthroughThis update refactors several interfaces and method signatures to use Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant ZetacoreClient
participant RPCClient
Caller->>ZetacoreClient: GetOutboundTracker(ctx, chainID, nonce)
ZetacoreClient->>RPCClient: GetOutboundTracker(ctx, chainID, nonce)
RPCClient-->>ZetacoreClient: OutboundTracker/err
ZetacoreClient-->>Caller: OutboundTracker/err
sequenceDiagram
participant Client
participant AccountRetriever
participant Logger
Client->>AccountRetriever: GetAccountNumberAndSequence(address)
AccountRetriever-->>Client: accountNumber, sequence
Client->>Logger: Log account setup
Note over Client: Account and sequence stored in Client maps
sequenceDiagram
participant Client
participant TxFactory
participant Logger
Client->>TxFactory: Setup (WithGasAdjustment, etc.)
Client->>Client: SignTx(ctx, ...)
Client->>Client: Broadcast transaction
alt Success
Client->>Logger: Log success, increment sequence
else Account sequence mismatch
Client->>Logger: Parse error, reset sequence, log warning
else Other error
Client->>Logger: Log error
end
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
zetaclient/zetacore/client_monitor.go (1)
183-197: Robust retry implementation with validation.The parameter type change to
uint64prevents negative values, and the backoff configuration with functional options is elegant. The zero-check validation ensures proper usage.Consider a more descriptive error message:
- return errors.New("attempts must be positive") + return errors.New("retry attempts must be greater than zero")
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
cmd/zetatool/cctx/cctx_details.go(1 hunks)pkg/bg/bg.go(1 hunks)pkg/rpc/clients_crosschain.go(1 hunks)pkg/rpc/clients_test.go(1 hunks)zetaclient/chains/bitcoin/signer/signer.go(1 hunks)zetaclient/chains/interfaces/interfaces.go(1 hunks)zetaclient/chains/ton/observer/inbound.go(2 hunks)zetaclient/testutils/mocks/zetacore_client.go(1 hunks)zetaclient/zetacore/broadcast.go(4 hunks)zetaclient/zetacore/broadcast_test.go(2 hunks)zetaclient/zetacore/client.go(3 hunks)zetaclient/zetacore/client_fungible_test.go(1 hunks)zetaclient/zetacore/client_monitor.go(5 hunks)zetaclient/zetacore/client_start.go(0 hunks)zetaclient/zetacore/client_test.go(3 hunks)zetaclient/zetacore/constant.go(0 hunks)zetaclient/zetacore/tx.go(2 hunks)zetaclient/zetacore/tx_test.go(3 hunks)
💤 Files with no reviewable changes (2)
- zetaclient/zetacore/constant.go
- zetaclient/zetacore/client_start.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧠 Learnings (17)
📓 Common learnings
Learnt from: gartnera
PR: zeta-chain/node#3228
File: cmd/zetaclientd/start.go:222-231
Timestamp: 2024-11-27T22:02:48.034Z
Learning: In the `zetaclient` codebase, port numbers like `26657` are hardcoded, and changing them to be configurable via the config struct is considered unrelated refactoring.
Learnt from: gartnera
PR: zeta-chain/node#3277
File: cmd/zetae2e/get_zetaclient_bootstrap.go:62-66
Timestamp: 2024-12-11T17:10:18.193Z
Learning: In the `getZetaclientBootstrap` function within `cmd/zetae2e/get_zetaclient_bootstrap.go`, `Validator` and `Description` in `validatorRes` are not pointers. An error is returned if `validatorRes` is nil (not found), so additional nil checks for these fields are unnecessary.
Learnt from: gartnera
PR: zeta-chain/node#3070
File: cmd/zetae2e/init.go:0-0
Timestamp: 2024-10-30T17:56:16.341Z
Learning: In code reviews for Go files like `cmd/zetae2e/init.go` in the ZetaChain project, avoid suggesting unrelated refactoring. Focus comments on changes relevant to the PR objectives.
Learnt from: gartnera
PR: zeta-chain/node#3489
File: pkg/chains/chains.go:0-0
Timestamp: 2025-02-06T15:31:32.723Z
Learning: SuiMainnet uses chain ID 105 (temporarily) instead of 101 to avoid collision with ZetaChainPrivnet. This is tracked in issue #3491 for proper resolution.
Learnt from: ws4charlie
PR: zeta-chain/node#2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-10-08T15:34:48.217Z
Learning: In the `mineTxNSetNonceMark` function within `bitcoin/observer/outbound_test.go`, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.
Learnt from: ws4charlie
PR: zeta-chain/node#2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-09-23T19:51:43.360Z
Learning: In the `mineTxNSetNonceMark` function within `bitcoin/observer/outbound_test.go`, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.
Learnt from: ws4charlie
PR: zeta-chain/node#3461
File: x/observer/types/chain_params.go:59-62
Timestamp: 2025-02-04T06:12:41.760Z
Learning: The legacy `ConfirmationCount` field in the ChainParams struct is planned to be deprecated after a full upgrade to the new confirmation count system that uses SafeInboundCount, FastInboundCount, SafeOutboundCount, and FastOutboundCount fields.
Learnt from: skosito
PR: zeta-chain/node#3939
File: go.mod:52-52
Timestamp: 2025-05-30T16:31:30.275Z
Learning: The ethermint dependency updates in the zeta-chain/node repository are typically moves between feature branches and main branch of the same fork, not breaking API changes. CI status should be verified before assuming compilation issues.
pkg/bg/bg.go (1)
Learnt from: gartnera
PR: zeta-chain/node#3031
File: server/json_rpc.go:47-64
Timestamp: 2024-10-23T18:36:52.781Z
Learning: When using the Tendermint logger in Go, which lacks a warning level, it's acceptable to map both warning and info logs to `g.logger.Info`.
zetaclient/chains/bitcoin/signer/signer.go (11)
Learnt from: gartnera
PR: zeta-chain/node#3632
File: zetaclient/chains/solana/signer/signer.go:304-304
Timestamp: 2025-03-04T22:39:58.395Z
Learning: The Solana signer implementation in zetaclient/chains/solana/signer/signer.go has limited test coverage, particularly for the transaction broadcasting logic with fallback scenarios. Adding this coverage has been acknowledged as a potential future improvement outside the scope of immediate fixes.
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`.
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: gartnera
PR: zeta-chain/node#3228
File: zetaclient/orchestrator/orchestrator.go:388-401
Timestamp: 2024-11-27T22:01:49.732Z
Learning: When reviewing code changes in `zetaclient/orchestrator/orchestrator.go`, avoid suggesting refactoring that is unrelated to the current PR.
Learnt from: ws4charlie
PR: zeta-chain/node#2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-10-08T15:34:48.217Z
Learning: In the `mineTxNSetNonceMark` function within `bitcoin/observer/outbound_test.go`, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.
Learnt from: ws4charlie
PR: zeta-chain/node#2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-09-23T19:51:43.360Z
Learning: In the `mineTxNSetNonceMark` function within `bitcoin/observer/outbound_test.go`, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.
Learnt from: gartnera
PR: zeta-chain/node#3406
File: cmd/zetae2e/local/local.go:199-199
Timestamp: 2025-01-23T20:29:05.877Z
Learning: The function `monitorBlockProductionExit` in cmd/zetae2e/local/local.go uses context cancellation to handle errors, not os.Exit, and should ideally be named `monitorBlockProductionCancel` to better reflect its behavior.
Learnt from: gartnera
PR: zeta-chain/node#3070
File: cmd/zetae2e/init.go:0-0
Timestamp: 2024-10-30T17:56:16.341Z
Learning: In code reviews for Go files like `cmd/zetae2e/init.go` in the ZetaChain project, avoid suggesting unrelated refactoring. Focus comments on changes relevant to the PR objectives.
Learnt from: gartnera
PR: zeta-chain/node#3079
File: e2e/e2etests/test_pause_erc20_custody.go:65-68
Timestamp: 2024-11-01T17:07:59.584Z
Learning: In the file `e2e/e2etests/test_pause_erc20_custody.go`, prefer verbosity and avoid extracting common event handling logic into helper functions to enhance readability and ease future refactoring.
Learnt from: gartnera
PR: zeta-chain/node#2881
File: pkg/ticker/ticker.go:83-90
Timestamp: 2024-09-14T03:11:04.109Z
Learning: In the Ticker's panic handling, the panic will not occur in `pkg/ticker`, but in the code called by the task.
zetaclient/zetacore/client_fungible_test.go (10)
Learnt from: gartnera
PR: zeta-chain/node#3105
File: e2e/runner/setup_bitcoin.go:51-69
Timestamp: 2024-11-06T21:10:14.301Z
Learning: In test code (`e2e/runner/setup_bitcoin.go`), adding security measures for private key handling in the `GetBtcAddress` method is not required.
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: kingpinXD
PR: zeta-chain/node#3207
File: x/observer/simulation/operation_reset_chain_nonces.go:26-27
Timestamp: 2024-12-12T15:51:51.144Z
Learning: In `x/observer/simulation/operation_reset_chain_nonces.go`, within the `SimulateResetChainNonces` function, the `authAccount` cannot be nil because it corresponds to the policy account which is set using simulation accounts.
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: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#2870
File: zetaclient/orchestrator/orchestrator.go:413-413
Timestamp: 2024-09-13T22:29:09.747Z
Learning: When a method is renamed (e.g., `IsUTXO()` to `IsBitcoin()`), and existing tests cover the affected code paths, do not request additional tests for the renamed methods.
Learnt from: gartnera
PR: zeta-chain/node#3632
File: zetaclient/chains/solana/signer/signer.go:304-304
Timestamp: 2025-03-04T22:39:58.395Z
Learning: The Solana signer implementation in zetaclient/chains/solana/signer/signer.go has limited test coverage, particularly for the transaction broadcasting logic with fallback scenarios. Adding this coverage has been acknowledged as a potential future improvement outside the scope of immediate fixes.
Learnt from: gartnera
PR: zeta-chain/node#3079
File: e2e/e2etests/test_pause_erc20_custody.go:65-68
Timestamp: 2024-11-01T17:07:59.584Z
Learning: In the file `e2e/e2etests/test_pause_erc20_custody.go`, prefer verbosity and avoid extracting common event handling logic into helper functions to enhance readability and ease future refactoring.
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: gartnera
PR: zeta-chain/node#3071
File: e2e/e2etests/test_stress_eth_withdraw.go:0-0
Timestamp: 2024-10-31T16:21:47.362Z
Learning: In `e2e/e2etests/test_stress_eth_withdraw.go`, when handling the error returned by `stats.Describe` in the `TestStressEtherWithdraw` function, log the error instead of failing the test.
pkg/rpc/clients_test.go (11)
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#2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-09-23T19:51:43.360Z
Learning: In the `mineTxNSetNonceMark` function within `bitcoin/observer/outbound_test.go`, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.
Learnt from: ws4charlie
PR: zeta-chain/node#2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-10-08T15:34:48.217Z
Learning: In the `mineTxNSetNonceMark` function within `bitcoin/observer/outbound_test.go`, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.
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: gartnera
PR: zeta-chain/node#3070
File: cmd/zetae2e/init.go:0-0
Timestamp: 2024-10-30T17:56:16.341Z
Learning: In code reviews for Go files like `cmd/zetae2e/init.go` in the ZetaChain project, avoid suggesting unrelated refactoring. Focus comments on changes relevant to the PR objectives.
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: gartnera
PR: zeta-chain/node#3228
File: zetaclient/orchestrator/orchestrator.go:388-401
Timestamp: 2024-11-27T22:01:49.732Z
Learning: When reviewing code changes in `zetaclient/orchestrator/orchestrator.go`, avoid suggesting refactoring that is unrelated to the current PR.
Learnt from: gartnera
PR: zeta-chain/node#3632
File: zetaclient/chains/solana/signer/signer.go:304-304
Timestamp: 2025-03-04T22:39:58.395Z
Learning: The Solana signer implementation in zetaclient/chains/solana/signer/signer.go has limited test coverage, particularly for the transaction broadcasting logic with fallback scenarios. Adding this coverage has been acknowledged as a potential future improvement outside the scope of immediate fixes.
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#2870
File: zetaclient/orchestrator/orchestrator.go:413-413
Timestamp: 2024-09-13T22:29:09.747Z
Learning: When a method is renamed (e.g., `IsUTXO()` to `IsBitcoin()`), and existing tests cover the affected code paths, do not request additional tests for the renamed methods.
Learnt from: gartnera
PR: zeta-chain/node#3071
File: e2e/e2etests/test_stress_eth_withdraw.go:58-59
Timestamp: 2024-10-31T16:19:26.038Z
Learning: In the Go code within `e2e/utils/zetacore.go`, the function `WaitCctxMinedByInboundHash` does not return an error.
zetaclient/zetacore/client_test.go (11)
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`.
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: gartnera
PR: zeta-chain/node#3632
File: zetaclient/chains/solana/signer/signer.go:304-304
Timestamp: 2025-03-04T22:39:58.395Z
Learning: The Solana signer implementation in zetaclient/chains/solana/signer/signer.go has limited test coverage, particularly for the transaction broadcasting logic with fallback scenarios. Adding this coverage has been acknowledged as a potential future improvement outside the scope of immediate fixes.
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: gartnera
PR: zeta-chain/node#3105
File: e2e/runner/setup_bitcoin.go:51-69
Timestamp: 2024-11-06T21:10:14.301Z
Learning: In test code (`e2e/runner/setup_bitcoin.go`), adding security measures for private key handling in the `GetBtcAddress` method is not required.
Learnt from: ws4charlie
PR: zeta-chain/node#2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-09-23T19:51:43.360Z
Learning: In the `mineTxNSetNonceMark` function within `bitcoin/observer/outbound_test.go`, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.
Learnt from: ws4charlie
PR: zeta-chain/node#2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-10-08T15:34:48.217Z
Learning: In the `mineTxNSetNonceMark` function within `bitcoin/observer/outbound_test.go`, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.
Learnt from: gartnera
PR: zeta-chain/node#3228
File: cmd/zetaclientd/start.go:222-231
Timestamp: 2024-11-27T22:02:48.034Z
Learning: In the `zetaclient` codebase, port numbers like `26657` are hardcoded, and changing them to be configurable via the config struct is considered unrelated refactoring.
Learnt from: gartnera
PR: zeta-chain/node#3079
File: e2e/e2etests/test_pause_erc20_custody.go:65-68
Timestamp: 2024-11-01T17:07:59.584Z
Learning: In the file `e2e/e2etests/test_pause_erc20_custody.go`, prefer verbosity and avoid extracting common event handling logic into helper functions to enhance readability and ease future refactoring.
Learnt from: kingpinXD
PR: zeta-chain/node#3207
File: x/observer/simulation/operation_reset_chain_nonces.go:26-27
Timestamp: 2024-12-12T15:51:51.144Z
Learning: In `x/observer/simulation/operation_reset_chain_nonces.go`, within the `SimulateResetChainNonces` function, the `authAccount` cannot be nil because it corresponds to the policy account which is set using simulation accounts.
zetaclient/chains/interfaces/interfaces.go (6)
Learnt from: ws4charlie
PR: zeta-chain/node#2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-10-08T15:34:48.217Z
Learning: In the `mineTxNSetNonceMark` function within `bitcoin/observer/outbound_test.go`, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.
Learnt from: ws4charlie
PR: zeta-chain/node#2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-09-23T19:51:43.360Z
Learning: In the `mineTxNSetNonceMark` function within `bitcoin/observer/outbound_test.go`, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.
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#3461
File: x/observer/types/chain_params.go:59-62
Timestamp: 2025-02-04T06:12:41.760Z
Learning: The legacy `ConfirmationCount` field in the ChainParams struct is planned to be deprecated after a full upgrade to the new confirmation count system that uses SafeInboundCount, FastInboundCount, SafeOutboundCount, and FastOutboundCount fields.
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: gartnera
PR: zeta-chain/node#3070
File: cmd/zetae2e/init.go:0-0
Timestamp: 2024-10-30T17:56:16.341Z
Learning: In code reviews for Go files like `cmd/zetae2e/init.go` in the ZetaChain project, avoid suggesting unrelated refactoring. Focus comments on changes relevant to the PR objectives.
zetaclient/chains/ton/observer/inbound.go (4)
Learnt from: ws4charlie
PR: zeta-chain/node#2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-10-08T15:34:48.217Z
Learning: In the `mineTxNSetNonceMark` function within `bitcoin/observer/outbound_test.go`, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.
Learnt from: ws4charlie
PR: zeta-chain/node#2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-09-23T19:51:43.360Z
Learning: In the `mineTxNSetNonceMark` function within `bitcoin/observer/outbound_test.go`, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.
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#3306
File: pkg/math/integer_test.go:0-0
Timestamp: 2025-01-14T03:37:18.430Z
Learning: Use big.Int for handling large number calculations and edge cases to avoid integer overflow issues.
pkg/rpc/clients_crosschain.go (6)
Learnt from: ws4charlie
PR: zeta-chain/node#2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-10-08T15:34:48.217Z
Learning: In the `mineTxNSetNonceMark` function within `bitcoin/observer/outbound_test.go`, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.
Learnt from: ws4charlie
PR: zeta-chain/node#2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-09-23T19:51:43.360Z
Learning: In the `mineTxNSetNonceMark` function within `bitcoin/observer/outbound_test.go`, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.
Learnt from: ws4charlie
PR: zeta-chain/node#3461
File: x/observer/types/chain_params.go:59-62
Timestamp: 2025-02-04T06:12:41.760Z
Learning: The legacy `ConfirmationCount` field in the ChainParams struct is planned to be deprecated after a full upgrade to the new confirmation count system that uses SafeInboundCount, FastInboundCount, SafeOutboundCount, and FastOutboundCount fields.
Learnt from: gartnera
PR: zeta-chain/node#3228
File: zetaclient/orchestrator/orchestrator.go:388-401
Timestamp: 2024-11-27T22:01:49.732Z
Learning: When reviewing code changes in `zetaclient/orchestrator/orchestrator.go`, avoid suggesting refactoring that is unrelated to the current PR.
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: gartnera
PR: zeta-chain/node#3395
File: .github/workflows/reusable-sim.yml:29-30
Timestamp: 2025-01-22T22:46:58.820Z
Learning: The zeta-chain/node repository uses Go version >= 1.22. Do not suggest downgrading to earlier versions like 1.21.
zetaclient/zetacore/broadcast_test.go (12)
Learnt from: gartnera
PR: zeta-chain/node#3632
File: zetaclient/chains/solana/signer/signer.go:304-304
Timestamp: 2025-03-04T22:39:58.395Z
Learning: The Solana signer implementation in zetaclient/chains/solana/signer/signer.go has limited test coverage, particularly for the transaction broadcasting logic with fallback scenarios. Adding this coverage has been acknowledged as a potential future improvement outside the scope of immediate fixes.
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`.
Learnt from: gartnera
PR: zeta-chain/node#3079
File: e2e/e2etests/test_pause_erc20_custody.go:65-68
Timestamp: 2024-11-01T17:07:59.584Z
Learning: In the file `e2e/e2etests/test_pause_erc20_custody.go`, prefer verbosity and avoid extracting common event handling logic into helper functions to enhance readability and ease future refactoring.
Learnt from: gartnera
PR: zeta-chain/node#3071
File: e2e/e2etests/test_stress_eth_withdraw.go:0-0
Timestamp: 2024-10-31T16:21:47.362Z
Learning: In `e2e/e2etests/test_stress_eth_withdraw.go`, when handling the error returned by `stats.Describe` in the `TestStressEtherWithdraw` function, log the error instead of failing the test.
Learnt from: gartnera
PR: zeta-chain/node#3105
File: e2e/runner/setup_bitcoin.go:51-69
Timestamp: 2024-11-06T21:10:14.301Z
Learning: In test code (`e2e/runner/setup_bitcoin.go`), adding security measures for private key handling in the `GetBtcAddress` method is not required.
Learnt from: ws4charlie
PR: zeta-chain/node#2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-09-23T19:51:43.360Z
Learning: In the `mineTxNSetNonceMark` function within `bitcoin/observer/outbound_test.go`, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.
Learnt from: ws4charlie
PR: zeta-chain/node#2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-10-08T15:34:48.217Z
Learning: In the `mineTxNSetNonceMark` function within `bitcoin/observer/outbound_test.go`, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.
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: kingpinXD
PR: zeta-chain/node#3207
File: x/observer/simulation/operation_reset_chain_nonces.go:26-27
Timestamp: 2024-12-12T15:51:51.144Z
Learning: In `x/observer/simulation/operation_reset_chain_nonces.go`, within the `SimulateResetChainNonces` function, the `authAccount` cannot be nil because it corresponds to the policy account which is set using simulation accounts.
Learnt from: kingpinXD
PR: zeta-chain/node#2615
File: x/crosschain/keeper/msg_server_vote_outbound_tx_test.go:472-472
Timestamp: 2024-08-01T18:08:13.681Z
Learning: The `SaveFailedOutbound` function in `x/crosschain/keeper/msg_server_vote_outbound_tx.go` requires a string argument for the error message.
zetaclient/zetacore/tx.go (8)
Learnt from: ws4charlie
PR: zeta-chain/node#2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-10-08T15:34:48.217Z
Learning: In the `mineTxNSetNonceMark` function within `bitcoin/observer/outbound_test.go`, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.
Learnt from: ws4charlie
PR: zeta-chain/node#2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-09-23T19:51:43.360Z
Learning: In the `mineTxNSetNonceMark` function within `bitcoin/observer/outbound_test.go`, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.
Learnt from: gartnera
PR: zeta-chain/node#3632
File: zetaclient/chains/solana/signer/signer.go:304-304
Timestamp: 2025-03-04T22:39:58.395Z
Learning: The Solana signer implementation in zetaclient/chains/solana/signer/signer.go has limited test coverage, particularly for the transaction broadcasting logic with fallback scenarios. Adding this coverage has been acknowledged as a potential future improvement outside the scope of immediate fixes.
Learnt from: gartnera
PR: zeta-chain/node#3070
File: cmd/zetae2e/init.go:0-0
Timestamp: 2024-10-30T17:56:16.341Z
Learning: In code reviews for Go files like `cmd/zetae2e/init.go` in the ZetaChain project, avoid suggesting unrelated refactoring. Focus comments on changes relevant to the PR objectives.
Learnt from: gartnera
PR: zeta-chain/node#3071
File: e2e/e2etests/test_stress_eth_withdraw.go:58-59
Timestamp: 2024-10-31T16:19:26.038Z
Learning: In the Go code within `e2e/utils/zetacore.go`, the function `WaitCctxMinedByInboundHash` does not return an error.
Learnt from: gartnera
PR: zeta-chain/node#3228
File: zetaclient/orchestrator/orchestrator.go:388-401
Timestamp: 2024-11-27T22:01:49.732Z
Learning: When reviewing code changes in `zetaclient/orchestrator/orchestrator.go`, avoid suggesting refactoring that is unrelated to the current PR.
Learnt from: kingpinXD
PR: zeta-chain/node#2615
File: x/crosschain/keeper/msg_server_vote_outbound_tx_test.go:472-472
Timestamp: 2024-08-01T18:08:13.681Z
Learning: The `SaveFailedOutbound` function in `x/crosschain/keeper/msg_server_vote_outbound_tx.go` requires a string argument for the error message.
Learnt from: ws4charlie
PR: zeta-chain/node#3306
File: zetaclient/chains/bitcoin/signer/sign.go:155-211
Timestamp: 2025-01-13T23:23:35.853Z
Learning: Bitcoin outbound transaction validation in NewOutboundData includes checks for:
1. Valid CCTX
2. Gas token only
3. Non-negative fee rate
4. Valid receiver address format and type
5. Dust amount threshold
6. Compliance requirements
zetaclient/testutils/mocks/zetacore_client.go (8)
Learnt from: ws4charlie
PR: zeta-chain/node#2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-09-23T19:51:43.360Z
Learning: In the `mineTxNSetNonceMark` function within `bitcoin/observer/outbound_test.go`, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.
Learnt from: ws4charlie
PR: zeta-chain/node#2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-10-08T15:34:48.217Z
Learning: In the `mineTxNSetNonceMark` function within `bitcoin/observer/outbound_test.go`, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.
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/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#3461
File: zetaclient/testutils/mocks/chain_params.go:48-51
Timestamp: 2025-02-04T06:05:00.366Z
Learning: In unit tests for the Zeta Chain project, it's acceptable to disable fast confirmation counts (FastInboundCount and FastOutboundCount) in mock chain parameters when the test doesn't specifically require testing those fields.
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: gartnera
PR: zeta-chain/node#3070
File: cmd/zetae2e/init.go:0-0
Timestamp: 2024-10-30T17:56:16.341Z
Learning: In code reviews for Go files like `cmd/zetae2e/init.go` in the ZetaChain project, avoid suggesting unrelated refactoring. Focus comments on changes relevant to the PR objectives.
Learnt from: gartnera
PR: zeta-chain/node#3071
File: e2e/e2etests/test_stress_eth_withdraw.go:58-59
Timestamp: 2024-10-31T16:19:26.038Z
Learning: In the Go code within `e2e/utils/zetacore.go`, the function `WaitCctxMinedByInboundHash` does not return an error.
zetaclient/zetacore/client.go (6)
Learnt from: gartnera
PR: zeta-chain/node#3228
File: cmd/zetaclientd/start.go:222-231
Timestamp: 2024-11-27T22:02:48.034Z
Learning: In the `zetaclient` codebase, port numbers like `26657` are hardcoded, and changing them to be configurable via the config struct is considered unrelated refactoring.
Learnt from: gartnera
PR: zeta-chain/node#3070
File: cmd/zetae2e/init.go:0-0
Timestamp: 2024-10-30T17:56:16.341Z
Learning: In code reviews for Go files like `cmd/zetae2e/init.go` in the ZetaChain project, avoid suggesting unrelated refactoring. Focus comments on changes relevant to the PR objectives.
Learnt from: gartnera
PR: zeta-chain/node#3228
File: zetaclient/orchestrator/orchestrator.go:388-401
Timestamp: 2024-11-27T22:01:49.732Z
Learning: When reviewing code changes in `zetaclient/orchestrator/orchestrator.go`, avoid suggesting refactoring that is unrelated to the current PR.
Learnt from: gartnera
PR: zeta-chain/node#3277
File: cmd/zetae2e/get_zetaclient_bootstrap.go:62-66
Timestamp: 2024-12-11T17:10:18.193Z
Learning: In the `getZetaclientBootstrap` function within `cmd/zetae2e/get_zetaclient_bootstrap.go`, `Validator` and `Description` in `validatorRes` are not pointers. An error is returned if `validatorRes` is nil (not found), so additional nil checks for these fields are unnecessary.
Learnt from: ws4charlie
PR: zeta-chain/node#2411
File: zetaclient/compliance/compliance.go:18-19
Timestamp: 2024-07-03T23:24:42.906Z
Learning: The `restrictedAddressBook` in `zetaclient/compliance/compliance.go` is currently initialized once and does not change, according to user `ws4charlie`.
Learnt from: kingpinXD
PR: zeta-chain/node#3207
File: x/observer/simulation/operation_reset_chain_nonces.go:26-27
Timestamp: 2024-12-12T15:51:51.144Z
Learning: In `x/observer/simulation/operation_reset_chain_nonces.go`, within the `SimulateResetChainNonces` function, the `authAccount` cannot be nil because it corresponds to the policy account which is set using simulation accounts.
zetaclient/zetacore/tx_test.go (18)
Learnt from: gartnera
PR: zeta-chain/node#3632
File: zetaclient/chains/solana/signer/signer.go:304-304
Timestamp: 2025-03-04T22:39:58.395Z
Learning: The Solana signer implementation in zetaclient/chains/solana/signer/signer.go has limited test coverage, particularly for the transaction broadcasting logic with fallback scenarios. Adding this coverage has been acknowledged as a potential future improvement outside the scope of immediate fixes.
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: gartnera
PR: zeta-chain/node#3079
File: e2e/e2etests/test_pause_erc20_custody.go:65-68
Timestamp: 2024-11-01T17:07:59.584Z
Learning: In the file `e2e/e2etests/test_pause_erc20_custody.go`, prefer verbosity and avoid extracting common event handling logic into helper functions to enhance readability and ease future refactoring.
Learnt from: ws4charlie
PR: zeta-chain/node#2987
File: pkg/memo/codec_abi.go:0-0
Timestamp: 2024-10-10T18:50:23.406Z
Learning: The constant `ABIAlignment` has been moved from `pkg/memo/codec_abi.go` to the `testutil` package.
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: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#2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-10-08T15:34:48.217Z
Learning: In the `mineTxNSetNonceMark` function within `bitcoin/observer/outbound_test.go`, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.
Learnt from: ws4charlie
PR: zeta-chain/node#2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-09-23T19:51:43.360Z
Learning: In the `mineTxNSetNonceMark` function within `bitcoin/observer/outbound_test.go`, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.
Learnt from: gartnera
PR: zeta-chain/node#3277
File: cmd/zetae2e/get_zetaclient_bootstrap.go:62-66
Timestamp: 2024-12-11T17:10:18.193Z
Learning: In the `getZetaclientBootstrap` function within `cmd/zetae2e/get_zetaclient_bootstrap.go`, `Validator` and `Description` in `validatorRes` are not pointers. An error is returned if `validatorRes` is nil (not found), so additional nil checks for these fields are unnecessary.
Learnt from: gartnera
PR: zeta-chain/node#3070
File: cmd/zetae2e/init.go:0-0
Timestamp: 2024-10-30T17:56:16.341Z
Learning: In code reviews for Go files like `cmd/zetae2e/init.go` in the ZetaChain project, avoid suggesting unrelated refactoring. Focus comments on changes relevant to the PR objectives.
Learnt from: gartnera
PR: zeta-chain/node#3395
File: .github/workflows/reusable-sim.yml:29-30
Timestamp: 2025-01-22T22:46:58.820Z
Learning: The zeta-chain/node repository uses Go version >= 1.22. Do not suggest downgrading to earlier versions like 1.21.
Learnt from: gartnera
PR: zeta-chain/node#3252
File: x/observer/keeper/operational_flags.go:16-26
Timestamp: 2024-12-06T16:23:31.240Z
Learning: In the ZetaChain Go codebase, within the `GetOperationalFlags` method in `x/observer/keeper/operational_flags.go`, it's acceptable to use `MustUnmarshal` without additional error handling, as an unmarshalling failure indicates a critical issue that should never occur.
Learnt from: ws4charlie
PR: zeta-chain/node#2987
File: testutil/sample/memo.go:0-0
Timestamp: 2024-10-10T18:54:33.554Z
Learning: In the `testutil/sample/memo.go` file, unchecked type assertions in the `ABIPack` function are acceptable because it is designed for unit tests and simplicity.
Learnt from: kingpinXD
PR: zeta-chain/node#2615
File: x/crosschain/keeper/msg_server_vote_outbound_tx_test.go:472-472
Timestamp: 2024-08-01T18:08:13.681Z
Learning: The `SaveFailedOutbound` function in `x/crosschain/keeper/msg_server_vote_outbound_tx.go` requires a string argument for the error message.
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: kingpinXD
PR: zeta-chain/node#3207
File: x/observer/simulation/operation_reset_chain_nonces.go:26-27
Timestamp: 2024-12-12T15:51:51.144Z
Learning: In `x/observer/simulation/operation_reset_chain_nonces.go`, within the `SimulateResetChainNonces` function, the `authAccount` cannot be nil because it corresponds to the policy account which is set using simulation accounts.
Learnt from: gartnera
PR: zeta-chain/node#3105
File: e2e/runner/setup_bitcoin.go:51-69
Timestamp: 2024-11-06T21:10:14.301Z
Learning: In test code (`e2e/runner/setup_bitcoin.go`), adding security measures for private key handling in the `GetBtcAddress` method is not required.
Learnt from: ws4charlie
PR: zeta-chain/node#2899
File: zetaclient/chains/bitcoin/observer/inbound.go:37-38
Timestamp: 2024-09-19T18:22:35.964Z
Learning: In `BTCInboundEvent`, it's acceptable to use `float64` for monetary values (`Value` and `DepositorFee`) because precision is ensured through conversion to integer when building the vote message.
zetaclient/zetacore/client_monitor.go (14)
Learnt from: gartnera
PR: zeta-chain/node#3406
File: cmd/zetae2e/local/local.go:199-199
Timestamp: 2025-01-23T20:29:05.877Z
Learning: The function `monitorBlockProductionExit` in cmd/zetae2e/local/local.go uses context cancellation to handle errors, not os.Exit, and should ideally be named `monitorBlockProductionCancel` to better reflect its behavior.
Learnt from: gartnera
PR: zeta-chain/node#3228
File: zetaclient/orchestrator/orchestrator.go:388-401
Timestamp: 2024-11-27T22:01:49.732Z
Learning: When reviewing code changes in `zetaclient/orchestrator/orchestrator.go`, avoid suggesting refactoring that is unrelated to the current PR.
Learnt from: gartnera
PR: zeta-chain/node#3079
File: e2e/e2etests/test_pause_erc20_custody.go:65-68
Timestamp: 2024-11-01T17:07:59.584Z
Learning: In the file `e2e/e2etests/test_pause_erc20_custody.go`, prefer verbosity and avoid extracting common event handling logic into helper functions to enhance readability and ease future refactoring.
Learnt from: gartnera
PR: zeta-chain/node#3070
File: cmd/zetae2e/init.go:0-0
Timestamp: 2024-10-30T17:56:16.341Z
Learning: In code reviews for Go files like `cmd/zetae2e/init.go` in the ZetaChain project, avoid suggesting unrelated refactoring. Focus comments on changes relevant to the PR objectives.
Learnt from: gartnera
PR: zeta-chain/node#3228
File: cmd/zetaclientd/start.go:222-231
Timestamp: 2024-11-27T22:02:48.034Z
Learning: In the `zetaclient` codebase, port numbers like `26657` are hardcoded, and changing them to be configurable via the config struct is considered unrelated refactoring.
Learnt from: gartnera
PR: zeta-chain/node#3071
File: e2e/e2etests/test_stress_eth_withdraw.go:0-0
Timestamp: 2024-10-31T16:21:47.362Z
Learning: In `e2e/e2etests/test_stress_eth_withdraw.go`, when handling the error returned by `stats.Describe` in the `TestStressEtherWithdraw` function, log the error instead of failing the test.
Learnt from: gartnera
PR: zeta-chain/node#3071
File: e2e/e2etests/test_stress_eth_withdraw.go:41-42
Timestamp: 2024-10-31T16:23:17.250Z
Learning: In `e2e/e2etests/test_stress_eth_withdraw.go`, since some goroutines may fail, we should use a mutex to append durations of successful withdrawals for accurate latency reporting.
Learnt from: ws4charlie
PR: zeta-chain/node#2899
File: zetaclient/chains/bitcoin/observer/inbound.go:37-38
Timestamp: 2024-09-19T18:22:35.964Z
Learning: In `BTCInboundEvent`, it's acceptable to use `float64` for monetary values (`Value` and `DepositorFee`) because precision is ensured through conversion to integer when building the vote message.
Learnt from: gartnera
PR: zeta-chain/node#3077
File: cmd/zetae2e/local/local.go:188-191
Timestamp: 2024-10-31T21:15:55.881Z
Learning: In Go file `cmd/zetae2e/local/local.go`, the network is fully functional before calling `FundEmissionsPool()`, so retry logic is unnecessary.
Learnt from: kingpinXD
PR: zeta-chain/node#2615
File: x/crosschain/keeper/msg_server_vote_outbound_tx_test.go:472-472
Timestamp: 2024-08-01T18:08:13.681Z
Learning: The `SaveFailedOutbound` function in `x/crosschain/keeper/msg_server_vote_outbound_tx.go` requires a string argument for the error message.
Learnt from: gartnera
PR: zeta-chain/node#3071
File: e2e/e2etests/test_stress_eth_withdraw.go:58-59
Timestamp: 2024-10-31T16:19:26.038Z
Learning: In the Go code within `e2e/utils/zetacore.go`, the function `WaitCctxMinedByInboundHash` does not return an error.
Learnt from: gartnera
PR: zeta-chain/node#3252
File: x/observer/keeper/operational_flags.go:16-26
Timestamp: 2024-12-06T16:23:31.240Z
Learning: In the ZetaChain Go codebase, within the `GetOperationalFlags` method in `x/observer/keeper/operational_flags.go`, it's acceptable to use `MustUnmarshal` without additional error handling, as an unmarshalling failure indicates a critical issue that should never occur.
Learnt from: fbac
PR: zeta-chain/node#2979
File: x/fungible/keeper/zrc20_unlock_token.go:69-72
Timestamp: 2024-10-08T14:57:52.954Z
Learning: In `x/fungible/keeper/zrc20_unlock_token.go`, when performing type assertions like `transferred, ok := ret[0].(bool)`, using the `ok` check is sufficient to prevent panics, and additional type handling is not required.
Learnt from: gartnera
PR: zeta-chain/node#3277
File: cmd/zetae2e/get_zetaclient_bootstrap.go:62-66
Timestamp: 2024-12-11T17:10:18.193Z
Learning: In the `getZetaclientBootstrap` function within `cmd/zetae2e/get_zetaclient_bootstrap.go`, `Validator` and `Description` in `validatorRes` are not pointers. An error is returned if `validatorRes` is nil (not found), so additional nil checks for these fields are unnecessary.
zetaclient/zetacore/broadcast.go (5)
Learnt from: gartnera
PR: zeta-chain/node#3632
File: zetaclient/chains/solana/signer/signer.go:304-304
Timestamp: 2025-03-04T22:39:58.395Z
Learning: The Solana signer implementation in zetaclient/chains/solana/signer/signer.go has limited test coverage, particularly for the transaction broadcasting logic with fallback scenarios. Adding this coverage has been acknowledged as a potential future improvement outside the scope of immediate fixes.
Learnt from: gartnera
PR: zeta-chain/node#3079
File: e2e/e2etests/test_pause_erc20_custody.go:65-68
Timestamp: 2024-11-01T17:07:59.584Z
Learning: In the file `e2e/e2etests/test_pause_erc20_custody.go`, prefer verbosity and avoid extracting common event handling logic into helper functions to enhance readability and ease future refactoring.
Learnt from: gartnera
PR: zeta-chain/node#3228
File: cmd/zetaclientd/start.go:222-231
Timestamp: 2024-11-27T22:02:48.034Z
Learning: In the `zetaclient` codebase, port numbers like `26657` are hardcoded, and changing them to be configurable via the config struct is considered unrelated refactoring.
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`.
cmd/zetatool/cctx/cctx_details.go (10)
Learnt from: ws4charlie
PR: zeta-chain/node#2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-09-23T19:51:43.360Z
Learning: In the `mineTxNSetNonceMark` function within `bitcoin/observer/outbound_test.go`, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.
Learnt from: ws4charlie
PR: zeta-chain/node#2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-10-08T15:34:48.217Z
Learning: In the `mineTxNSetNonceMark` function within `bitcoin/observer/outbound_test.go`, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.
Learnt from: ws4charlie
PR: zeta-chain/node#3461
File: x/observer/types/chain_params.go:59-62
Timestamp: 2025-02-04T06:12:41.760Z
Learning: The legacy `ConfirmationCount` field in the ChainParams struct is planned to be deprecated after a full upgrade to the new confirmation count system that uses SafeInboundCount, FastInboundCount, SafeOutboundCount, and FastOutboundCount fields.
Learnt from: gartnera
PR: zeta-chain/node#3070
File: cmd/zetae2e/init.go:0-0
Timestamp: 2024-10-30T17:56:16.341Z
Learning: In code reviews for Go files like `cmd/zetae2e/init.go` in the ZetaChain project, avoid suggesting unrelated refactoring. Focus comments on changes relevant to the PR objectives.
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: gartnera
PR: zeta-chain/node#3071
File: e2e/e2etests/test_stress_eth_withdraw.go:58-59
Timestamp: 2024-10-31T16:19:26.038Z
Learning: In the Go code within `e2e/utils/zetacore.go`, the function `WaitCctxMinedByInboundHash` does not return an error.
Learnt from: ws4charlie
PR: zeta-chain/node#3461
File: x/observer/migrations/v10/migrate.go:29-34
Timestamp: 2025-02-04T06:03:54.382Z
Learning: In the observer module's ChainParams struct, the Confirmation field is a value type (not a pointer), so nil checks are not needed.
Learnt from: gartnera
PR: zeta-chain/node#3228
File: zetaclient/orchestrator/orchestrator.go:388-401
Timestamp: 2024-11-27T22:01:49.732Z
Learning: When reviewing code changes in `zetaclient/orchestrator/orchestrator.go`, avoid suggesting refactoring that is unrelated to the current PR.
Learnt from: kingpinXD
PR: zeta-chain/node#2568
File: zetaclient/config/types.go:161-163
Timestamp: 2024-07-30T15:14:17.100Z
Learning: Optimize the comparison with an empty `chains.Chain` instance by adding an `IsEmpty` method to the `chains.Chain` struct and using it in the `EVMConfig.Empty` method.
Learnt from: ws4charlie
PR: zeta-chain/node#3306
File: zetaclient/chains/bitcoin/signer/sign.go:155-211
Timestamp: 2025-01-13T23:23:35.853Z
Learning: Bitcoin outbound transaction validation in NewOutboundData includes checks for:
1. Valid CCTX
2. Gas token only
3. Non-negative fee rate
4. Valid receiver address format and type
5. Dust amount threshold
6. Compliance requirements
🧬 Code Graph Analysis (5)
pkg/rpc/clients_crosschain.go (1)
pkg/rpc/clients.go (1)
Clients(29-69)
zetaclient/zetacore/tx.go (1)
zetaclient/zetacore/constant.go (1)
AddOutboundTrackerGasLimit(29-29)
zetaclient/testutils/mocks/zetacore_client.go (2)
zetaclient/chains/interfaces/interfaces.go (1)
ZetacoreClient(58-114)testutil/sample/crosschain.go (1)
OutboundTracker(104-112)
zetaclient/zetacore/client_monitor.go (3)
pkg/constant/constant.go (1)
ZetaBlockTime(8-8)zetaclient/logs/fields.go (1)
FieldZetaTx(16-16)pkg/retry/retry.go (2)
Retry(126-136)DoWithBackoff(62-82)
zetaclient/zetacore/broadcast.go (2)
app/ante/fees.go (1)
GasPriceReductionRate(34-34)zetaclient/zetacore/client.go (1)
Client(34-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: test-sim-fullappsimulation / sim
- GitHub Check: test-sim-nondeterminism / sim
- GitHub Check: test-sim-import-export / sim
- GitHub Check: test-sim-after-import / sim
- GitHub Check: Cursor BugBot
- GitHub Check: lint
- GitHub Check: gosec
- GitHub Check: build-zetanode
- GitHub Check: analyze (go)
- GitHub Check: build-and-test
- GitHub Check: build
🔇 Additional comments (24)
zetaclient/chains/ton/observer/inbound.go (2)
22-22: LGTM: Improved constant namingThe renaming from
maxTransactionsPerTicktopaginationLimitprovides better clarity about the constant's purpose and follows conventional naming patterns for pagination limits.
46-50: LGTM: Consistent usage of the renamed constantThe pagination logic correctly uses the renamed constant throughout the transaction processing flow, maintaining the same behavior while improving code readability.
pkg/bg/bg.go (1)
63-63: LGTM: Appropriate logging level adjustment.The change from Info to Debug level for background task completion messages reduces production log verbosity while maintaining observability during debugging. This aligns with best practices for operational logging.
pkg/rpc/clients_test.go (1)
686-686: LGTM: Test correctly reflects the refactored method signature.The test appropriately passes
chain.ChainIdinstead of the full chain object, aligning with the streamlinedGetOutboundTrackermethod signature. This change reduces coupling and improves API clarity.cmd/zetatool/cctx/cctx_details.go (1)
107-107: LGTM: Consistent with the method signature refactoring.The change to pass
outboundChain.ChainIdinstead of the full chain object aligns with the codebase-wide refactoring effort. This streamlines the API by using the specific data type required (int64) rather than passing the entire chain object.zetaclient/chains/bitcoin/signer/signer.go (1)
90-102: LGTM: Improved error handling and defensive programming.The reordering of operations in the defer block ensures panic recovery and logging occur before the outbound marking cleanup. This defensive approach prevents potential issues where failures in
MarkOutboundcould mask the original panic. The capitalization of the error message also improves consistency.zetaclient/zetacore/client_fungible_test.go (1)
63-63: LGTM: Consistent test setup with proper mocking.Adding the account retriever mock with fixed values ensures test isolation and predictable behavior. This approach eliminates dependencies on external state and maintains consistency with similar test configurations across the codebase.
zetaclient/zetacore/broadcast_test.go (1)
84-84: LGTM: Improved test setup consistency.The addition of
withAccountRetriever(t, 5, 4)to both test cases standardizes the account number and sequence configuration, improving test reliability and consistency across the test suite.Also applies to: 101-101
zetaclient/zetacore/client_test.go (1)
186-186: LGTM: Consistent test setup enhancement.The addition of
withAccountRetriever(t, 5, 4)to all three test functions aligns with the broader effort to standardize test setup and improve reliability by providing consistent account number and sequence mocking.Also applies to: 233-233, 252-252
zetaclient/chains/interfaces/interfaces.go (1)
94-94: AllGetOutboundTrackerimplementations updated to new signatureVerified that every method implementation and call now uses
GetOutboundTracker(ctx, chainID int64, nonce uint64)—no remaining references to the oldchains.Chainparameter. Key locations checked include:• zetaclient/zetacore/tx.go
• zetaclient/testutils/mocks/zetacore_client.go
• pkg/rpc/clients_crosschain.go & clients_test.go
• cmd/zetatool/cctx/cctx_details.go
• x/crosschain/keeper/outbound_tracker.goNo further changes required.
zetaclient/zetacore/tx_test.go (3)
15-15: LGTM: Import alias improves consistency.The addition of the
crosschaintypesalias import unifies the reference style throughout the codebase, making the code more readable and consistent.
265-265: LGTM: Consistent alias usage.The update to use
crosschaintypes.InboundStatus_SUCCESScorrectly applies the newly introduced alias, maintaining consistency with the import change.
278-278: LGTM: Consistent test setup improvement.The addition of
withAccountRetriever(t, 5, 4)to both test setups follows the established pattern across the test suite, providing consistent account number and sequence configuration for improved test reliability.Also applies to: 349-349
pkg/rpc/clients_crosschain.go (1)
130-131: LGTM: Correct implementation of interface change.The method signature and implementation correctly reflect the interface change, using
chainID int64parameter directly in theQueryGetOutboundTrackerRequestconstruction. This eliminates the need to extract the chain ID from achains.Chainobject, simplifying the implementation.zetaclient/testutils/mocks/zetacore_client.go (1)
483-505: Mock signature update looks correct.The parameter type change from
chain chains.ChaintochainID int64properly aligns with the interface update. All type assertions have been correctly adjusted.zetaclient/zetacore/tx.go (1)
92-116: Excellent improvements to PostOutboundTracker.The changes enhance the function in multiple ways:
- Simplified parameter passing by using
chainIDdirectly- Added proper nil check for
trackerto prevent potential panic- Improved error handling with descriptive context wrapping
These modifications align perfectly with the PR's objective of streamlining cross-chain interactions.
zetaclient/zetacore/client.go (3)
299-301: Proper naming convention applied.The function rename from
CometBFTRPCtocometBFTRPCcorrectly follows Go conventions for unexported functions.
152-164: Account initialization consolidation is well-implemented.Moving the account number and sequence retrieval into the constructor eliminates the need for a separate
SetAccountNumbercall, reducing the possibility of initialization errors. The implementation correctly uses theauthz.ZetaClientGranteeKeyfor the grantee account.
133-150: Clean variable naming and initialization flow.The simplified variable names (
cometBFTClientinstead ofcometBFTClientIface) improve readability. The HTTP client creation with proper websocket initialization is well-structured.zetaclient/zetacore/broadcast.go (3)
30-32: Good use of package-level constants and precompiled regex.Moving
reductionRateto package scope and precompiling the regex pattern improves performance by avoiding repeated allocations.
76-85: Clean transaction factory initialization.The inline flag set creation and method chaining provide a clear, self-contained factory setup with appropriate gas adjustment.
101-102: Context propagation properly implemented.Adding the
context.Contextparameter toSignTxfollows Go best practices for cancellation and deadline propagation.Also applies to: 153-160
zetaclient/zetacore/client_monitor.go (2)
18-21: Well-defined monitoring constants.Extracting hardcoded values into named constants improves maintainability. The interval of half the block time (2 seconds) provides responsive monitoring.
76-81: Efficient string handling and comprehensive case coverage.Converting the raw log to lowercase once and adding the "already voted" case prevents unnecessary processing and error logging for benign conditions.
…ode into feat/zetaclient-reliability
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #4014 +/- ##
===========================================
+ Coverage 64.92% 64.95% +0.02%
===========================================
Files 475 475
Lines 35142 35139 -3
===========================================
+ Hits 22817 22823 +6
+ Misses 11255 11243 -12
- Partials 1070 1073 +3
🚀 New features to boost your workflow:
|
|
@ws4charlie can you please review this one when you get some time |
ws4charlie
left a comment
There was a problem hiding this comment.
Overall looks good. A minor comment to confirm.
zetaclient/zetacoreconstructorSummary by CodeRabbit
Refactor
Bug Fixes
Tests
Chores