Reviewing files that changed from the base of the PR and between 80ca921 and 22d8a51.

• cmd/zetaclientd/inbound.go (1 hunks)
• cmd/zetaclientd/start.go (2 hunks)
• e2e/e2etests/test_bitcoin_deposit_and_call_revert.go (1 hunks)
• e2e/e2etests/test_bitcoin_deposit_and_call_revert_with_dust.go (1 hunks)
• e2e/e2etests/test_bitcoin_deposit_call.go (3 hunks)
• e2e/e2etests/test_bitcoin_donation.go (1 hunks)
• e2e/e2etests/test_bitcoin_std_deposit.go (2 hunks)
• e2e/e2etests/test_bitcoin_std_deposit_and_call.go (1 hunks)
• e2e/e2etests/test_bitcoin_std_memo_inscribed_deposit_and_call.go (2 hunks)
• e2e/runner/bitcoin.go (6 hunks)
• go.mod (2 hunks)
• pkg/chains/chain.go (2 hunks)
• pkg/scheduler/opts.go (1 hunks)
• pkg/scheduler/scheduler.go (3 hunks)
• pkg/ticker/ticker.go (3 hunks)
• zetaclient/chains/base/observer.go (1 hunks)
• zetaclient/chains/bitcoin/bitcoin.go (1 hunks)
• zetaclient/chains/bitcoin/common/errors.go (1 hunks)
• zetaclient/chains/bitcoin/common/fee.go (1 hunks)
• zetaclient/chains/bitcoin/common/fee_test.go (1 hunks)
• zetaclient/chains/bitcoin/common/tx_script.go (1 hunks)
• zetaclient/chains/bitcoin/common/tx_script_test.go (31 hunks)
• zetaclient/chains/bitcoin/common/utils.go (1 hunks)
• zetaclient/chains/bitcoin/observer/inbound.go (11 hunks)
• zetaclient/chains/bitcoin/observer/inbound_test.go (9 hunks)
• zetaclient/chains/bitcoin/observer/observer.go (6 hunks)
• zetaclient/chains/bitcoin/observer/outbound.go (4 hunks)
• zetaclient/chains/bitcoin/observer/rpc_status.go (1 hunks)
• zetaclient/chains/bitcoin/observer/witness.go (5 hunks)
• zetaclient/chains/bitcoin/observer/witness_test.go (2 hunks)
• zetaclient/chains/bitcoin/rpc/rpc_live_test.go (4 hunks)
• zetaclient/chains/bitcoin/signer/signer.go (10 hunks)
• zetaclient/context/chain.go (2 hunks)
• zetaclient/context/context.go (1 hunks)
• zetaclient/maintenance/shutdown_listener_test.go (1 hunks)
• zetaclient/orchestrator/bootstrap.go (2 hunks)
• zetaclient/orchestrator/bootstrap_test.go (8 hunks)
• zetaclient/orchestrator/contextupdater.go (0 hunks)
• zetaclient/orchestrator/orchestrator.go (2 hunks)
• zetaclient/orchestrator/v2_bootstrap.go (1 hunks)
• zetaclient/orchestrator/v2_bootstrap_test.go (1 hunks)
• zetaclient/orchestrator/v2_orchestrator.go (1 hunks)
• zetaclient/orchestrator/v2_orchestrator_test.go (1 hunks)
• zetaclient/testutils/mocks/chain_params.go (1 hunks)
• zetaclient/testutils/testlog/log.go (1 hunks)

• zetaclient/orchestrator/contextupdater.go

• zetaclient/chains/bitcoin/common/errors.go
• zetaclient/chains/bitcoin/common/fee.go
• zetaclient/chains/bitcoin/common/fee_test.go
• zetaclient/chains/bitcoin/common/tx_script.go

• zetaclient/testutils/mocks/chain_params.go
• go.mod

zetaclient/orchestrator/v2_bootstrap.go (3)

16-20: Validate chain type before proceeding

This check is good for ensuring the method only boots a Bitcoin chain. Ensure no other code paths rely on the assumption that chain.IsBitcoin() is always true, since an unexpected chain type would not boot properly.

---

42-43: File naming caution for database creation

Using the chain name to build the database filename is sensible. Ensure collisions cannot occur with similarly named test networks or future expansions.

---

53-65: Verify observer initialization consistency

When you create a new Bitcoin observer, ensure that its critical fields align with the requirement for multi-network or future concurrency expansions (e.g., ensuring safe usage of DB and concurrency).

zetaclient/chains/bitcoin/bitcoin.go (2)

44-46: Favor using chain's identity from the aggregator

While returning b.observer.Chain() is straightforward, confirm that it consistently reflects the same chain data across observer, signer, and scheduler. Desyncs can occur if chain references are updated incorrectly.

---

112-113: Confirm early exit is correct upon Start success

The method returns nil if no immediate errors occurred. Make sure any asynchronous errors triggered by the tasks get handled or logged to prevent silent failures.

zetaclient/orchestrator/v2_orchestrator.go (3)

47-52: Confirm telemetry usage is always safe

Telemetry references might operate asynchronously. Ensure they are thread-safe under heavy concurrency or repeated orchestrator lifecycles.

---

91-96: Stop call usage

oc.scheduler.Stop() stops all tasks. If partially used by other orchestrators or older code, confirm that stopping one orchestrator does not unexpectedly interrupt tasks not associated with V2.

---

126-201: Improve chain detection logic for new or removed chains

Your loop over app.ListChains() carefully decides to add or remove chains. Incorporate robust logging and tests to confirm you do not inadvertently skip a chain that is partially configured or in transitional states (like a chain upgrade).

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

17-17: Confirm correct usage of moved common package

Imports from github.com/zeta-chain/node/zetaclient/chains/bitcoin/common appear to replace older bitcoin references. Verify that the functions (like fee calculators, decoders) match the required functionality to avoid subtle differences in logic.

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

21-21: Check alignment of new common usage with legacy references

Replacing old references to bitcoin with common is a good step. Verify that any leftover references to bitcoin do not break functionalities or lead to partial code references.

---

241-243: Check the TSS address retrieval for test networks

You retrieve the TSS address from the chain ID. For multi-environment setups or ephemeral test networks, confirm that TSS addresses are aligned with chain parameters to avoid mismatch or invalid addresses.

---

381-381: Account for varied fee rates in testnet

common.GetRecentFeeRate is a helpful approach, but testnet volatility can differ from mainnet. Ensure you have fallback or bounding logic if returned rates fluctuate or appear unreasonably large.

zetaclient/orchestrator/orchestrator.go (1)

202-202: Check for nil chain parameters before setting

Invoking observer.SetChainParams(*chain.Params()) directly dereferences the chain parameters. Consider a prior nil check or graceful fallback if chain.Params() is unexpectedly nil.

zetaclient/chains/bitcoin/observer/outbound.go (11)

17-17: Introduction of the common package

Using common fosters shared utilities across Bitcoin modules, reducing code duplication. This approach enhances code reusability and clarity.

---

24-25: New function signature ObserveOutbound

Replaces prior polling-based approach, simplifying logic to a single call. Validate invocation frequency elsewhere to ensure timely processing.

---

27-27: Asynchronous retrieval of all outbound trackers

Grabbing all trackers in ascending order seems correct. Confirm that no partial or large-batch side effects occur for massive tracker lists.

---

29-29: Error wrapping for clarity

return errors.Wrap(err, "unable to get all outbound trackers") helps track the origin of failures. This is a good pattern for debugging.

---

32-38: Iterating through outbound trackers

The loop fetches CCTX by nonce and checks consistency. This procedure is straightforward and robust, ensure concurrency concerns are addressed if multiple calls can overlap.

---

40-43: Nonce mismatch check

Excellent safeguard ensuring tracker and CCTX remain consistent. This prevents incorrect or stale data from propagating.

---

66-67: No functional change

These lines appear to be closing braces. No actionable feedback needed.

---

68-72: Handling multiple or zero included transactions

Robust approach to either set or remove included transactions. Consider logging each scenario for debugging if multiple collisions happen frequently.

---

75-76: Return on completion

The function now returns a single error or nil. This is a clean, straightforward exit point.

---

382-382: Use of common.GetSatoshis

Leverages the shared logic. Handle and log errors carefully if conversion fails for unexpected amounts.

---

626-626: Decoding TSS output with common.DecodeTSSVout

Applying a common function for decoding ensures consistency across the Bitcoin observer code. This fosters maintainability and shared improvements.

zetaclient/chains/bitcoin/observer/rpc_status.go (6)

5-5: Blank line modification

No functional changes here.

---

6-6: Importing github.com/pkg/errors

Adopts standard error wrapping conventions. This is consistent with the rest of the codebase.

---

11-12: Renamed function with context parameter

CheckRPCStatus uses context.Context, enabling better cancellation and timeouts. Ensures best practices for external calls.

---

15-15: Wrapping TSS address error

Proactive approach to provide context about address fetching failures. A well-structured error message is beneficial for debugging.

---

20-20: Wrapping RPC status error

Similar approach as above. Maintains consistency throughout the observer’s error handling.

---

24-25: Block latency reporting and return

Reporting metrics is helpful to track node health. The function’s concluding return nil is semantically clean.

zetaclient/context/context.go (1)

11-11: Improved error message for missing AppContext

Switching to “unable to get AppContext from context.Context” clarifies the root cause. This fosters better debugging and explicit problem statements.

zetaclient/testutils/testlog/log.go (2)

22-32: Well-structured concurrency approach for test logging.

The approach of combining zerolog.NewTestWriter(t) with a thread-safe buffer ensures minimal data races when capturing logs in parallel test scenarios. This is a neat and purposeful solution.

---

38-43: Efficient lock usage.

Locking at write scope is appropriate for this scenario, ensuring log entry atomicity without overcomplicating concurrency. This design is suitable for capturing test logs.

zetaclient/chains/bitcoin/common/utils.go (1)

1-1: Reorganized to a common package.

This refactor promotes clearer responsibility separation. Ensure that references to bitcoin are updated accordingly across the entire codebase.

e2e/e2etests/test_bitcoin_donation.go (1)

12-12: Import path aligns with refactored structure.

This change consistently follows the adoption of the common package for Bitcoin utilities. Good job keeping references precise.

pkg/scheduler/opts.go (2)

32-32: Clear documentation enhancement.

Clarifying that returning true will skip the task prevents confusion and ensures correct usage.

---

39-42: Immediate interval assignment.

Applying opts.interval = intervalUpdater() before setting the updater function ensures the first task iteration reflects the latest interval. This is a practical approach to sync the schedule from the outset.

e2e/e2etests/test_bitcoin_deposit_and_call_revert.go (1)

9-9: Confirm the new import path aligns with the refactoring.
Bringing in the common package is consistent with the broader refactoring. Ensure that any references to Bitcoin-specific constants or methods still function as intended and do not break any existing tests.

e2e/e2etests/test_bitcoin_deposit_call.go (1)

23-23: Validate fee usage in test scenarios.
Adding the default depositor fee is correct for typical deposit flows. However, confirm that all test cases handle edge scenarios (e.g., dust inputs or minimum transaction fees) to maintain coverage.

e2e/e2etests/test_bitcoin_std_memo_inscribed_deposit_and_call.go (1)

56-56: Review deposit fee logic for large memos.
When inscribing large memos, the depositor fee calculation could interact with variable on-chain fees. Confirm that the deposit fee covers all overhead scenarios so no partial or failed transactions occur in certain corner cases.

e2e/e2etests/test_bitcoin_std_deposit.go (2)

13-13: Renamed import path aligns with refactoring.

Switching from bitcoin to common indicates a reorganization of shared Bitcoin-related utilities. This change strengthens modularity and prevents import cycles.

---

57-57: Direct usage of common.GetSatoshis is valid.

No functional issues found. The rename appears consistent with the broader refactoring, and the usage in the deposit test remains correct.

e2e/e2etests/test_bitcoin_deposit_and_call_revert_with_dust.go (1)

13-13: New import path promotes code reusability.

Adopting common for Bitcoin utilities removes potential duplication across packages. This update is in line with the ongoing reorganization.

pkg/scheduler/scheduler.go (3)

147-151: Logging enhancements look good.

Including the number of tasks and group being stopped is helpful for debugging and monitoring.

---

181-183: Method Group() is straightforward and adequately encapsulated.

---

185-187: Method Name() is clear and consistent with the rest of the task structure.

zetaclient/context/chain.go (1)

148-150: LogFields() method nicely extends the Chain struct’s logging capabilities.

Exposing log fields via LogFields promotes structured logging and consistent log formats.

pkg/ticker/ticker.go (2)

217-219: Avoid potential nil-pointer by checking t.ticker != nil before stopping.

This check ensures safe invocation of t.ticker.Stop(). The current approach is correct and prevents runtime panics.

---

224-229: isStopped method is a concise helper for checking the ticker’s state.

This approach cleanly handles concurrency by locking while reading the stopped flag.

zetaclient/chains/bitcoin/observer/witness.go (4)

13-13: Reorganized import for extensibility

Importing common from the new bitcoin/common package aligns with the broader refactoring effort and consolidates Bitcoin utilities under a single package. This change fosters maintainability and clearer boundaries between modules.

---

27-27: Updated parameter type for deposit fee calculation

Switching from bitcoin.DepositorFeeCalculator to common.DepositorFeeCalculator is consistent with the reorganization in the common package. This unifies the code structure for fee calculations across Bitcoin-related modules.

---

140-140: Preserved logic while updating OP_RETURN decoder

The invocation of common.DecodeOpReturnMemo preserves OP_RETURN parsing and ensures that any future improvements in the shared common package are automatically leveraged.

---

190-190: Unified approach for P2WPKH script decoding

Using common.DecodeScriptP2WPKH centralizes the decoding logic, preventing discrepancies across different Bitcoin modules. This also guards against future code duplication.

cmd/zetaclientd/start.go (1)

17-17: New import for scheduled orchestration

Introducing the scheduler package paves the way for more structured, time-based tasks and periodic triggers in the orchestrator, improving modularity and maintainability.

pkg/chains/chain.go (2)

11-12: Importing logs package for structured logging

Introducing logs from zetaclient/logs suggests structured and contextual logging support for chain-related activities, which enhances debugging and traceability.

---

113-119: New LogFields method for improved observability

Providing a LogFields method promotes structured logging across components. This is beneficial for debugging multi-chain environments where chain identification is critical. Ensure that potential sensitive data (if any) is never logged.

zetaclient/orchestrator/v2_orchestrator_test.go (6)

1-2: New orchestrator V2 test file initialization

Declaring the package and initiating the test file structure sets the foundation for comprehensive testing of orchestrator V2. This organized approach is commendable.

---

29-74: Test coverage for orchestrator V2’s dynamic context updates

The TestOrchestratorV2 sub-test structure effectively verifies that updated chain parameters propagate to the application context in real time. This is an excellent pattern for validating runtime adjustments and ensuring concurrency safety.

---

75-92: Centralized test suite design

Using a testSuite struct to bundle test dependencies (logger, appContext, chains, etc.) facilitates reusability and readability, especially for complex test scenarios.

---

106-169: Comprehensive test suite setup

newTestSuite neatly composes all mock services and orchestrator dependencies. This approach encourages clarity, as all major test objects are created in one place, preventing hidden side effects.

---

171-179: Dynamic chain params mocking

MockChainParams method provides a swift mechanism to inject test scenarios on-the-fly. This approach is well-suited for verifying orchestrator responsiveness to rapid chain parameter changes.

---

Line range hint 254-269: Utility for chain membership verification

chainsContain is a clean helper function that grants quick validation of chain presence within the application context. The use of sets simplifies membership checks and fosters clarity.

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

13-13: Consistent package import.
Switching from the bitcoin package to the common package is consistent with the new modular structure.

zetaclient/orchestrator/bootstrap_test.go (9)

9-9: No issues with new import.
Adding the mock library import is standard practice.

---

68-69: Graceful exclusion of BTC signer from V1 orchestrator.
Updating the assertion to 1 signer aligns with the updated logic that moves BTC to V2.

---

75-75: Explicit inclusion of Polygon chain.
Ensures that Polygon is considered at runtime.

---

109-109: Re-enable Ethereum chain test scenario.
This test name and scenario properly document the reintroduction of Ethereum as an active chain.

---

154-155: Mock client call for Zetacore.
Utilizing mockZetacore(client) ensures consistent tests and avoids unmocked calls.

---

193-194: Clarity in chain usage comments.
Lines now emphasize that Polygon and Solana are intentionally excluded here and BTC is handled in orchestrator V2.

Also applies to: 198-198

---

209-209: Observer count updated for BTC shift.
Asserted observer count of 2 and explicitly excluding Bitcoin aligns with the orchestrator V2 approach.

Also applies to: 212-212

---

417-418: Helper function annotation.
Marking the function as a test helper ensures clearer stack traces if assertions fail.

---

429-437: Comprehensive mock setup for Zetacore.
Provides stable test scenarios by defining flexible expectations for PostVoteGasPrice.

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

27-27: Separation of common utilities.
Importing common clarifies that shared Bitcoin functionality is decoupled from chain-specific logic.

---

102-102: Use of common.GetSatoshis.
Properly leverages the new utility method for unit conversion, improving code cohesion.

---

109-109: Calculation of remaining satoshis.
Uses common.GetSatoshis() for consistency and clarity.

---

169-169: Fee estimation with OutboundBytesMax.
Making fees dependent on maximum outbound bytes is consistent with bridging constraints.

---

208-208: Outbound size estimation.
Utilizes common.EstimateOutboundSize, reflecting the new modular approach for BTC logic.

---

212-212: Size limit check against BtcOutboundBytesWithdrawer.
Ensures a separate fee structure for ZRC20 'withdraw' calls.

---

216-219: Enforce minimum size if under OutboundBytesMin.
Correctly prevents underestimation of transaction size.

---

221-224: Enforce maximum size if above OutboundBytesMax.
Safeguards against overly large transactions.

---

245-245: Conversion of multi-UTXO amounts.
Maintains consistency in satoshi conversion for each input.

---

338-338: Explicit chain retrieval.
Extracting observer.Chain() clarifies which chain is being processed.

---

373-373: Fee rate conversion using FeeRateToSatPerByte.
Provides clearer bridging between network relay fees and local gas price.

---

402-402: Observer parameter alignment.
Ensures direct access to the relevant Bitcoin observer methods.

---

449-449: Tracking broadcasted transaction.
observer.SaveBroadcastedTx properly records successful BTC outbound transactions.

e2e/runner/bitcoin.go (6)

26-26: Rename to zetabtc import.
The rename clarifies that these helpers belong to the updated Bitcoin common package.

---

103-103: Default depositor fee usage.
Explicitly adding zetabtc.DefaultDepositorFee ensures consistent deposit fees.

---

151-151: BTC deposit plus default fee.
Aligns with centralized handling of depositor fees, improving code clarity.

---

244-244: Integer conversion with GetSatoshis.
Makes deposit operations less error-prone by using shared conversion logic.

---

354-354: Consistent satoshi conversion for inscriptions.
Reuses GetSatoshis to unify how amounts are processed in reveal transactions.

---

415-415: Use DecodeScriptP2WPKH.
Decodes the script with the new zetabtc method for consistent script parsing.

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

169-175: Avoid redundant parameter assignments and logs.
This newly introduced check prevents unnecessary reassignments when the chainParams haven't changed, enhancing efficiency and reducing log noise. Implementation appears correct, with proper locking to avoid data races.

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

21-21: Well-aligned package restructuring.
Importing the common package for Bitcoin-related logic consolidates functionality, improving maintainability.

---

366-366: Refactor for consistency.
Switching to btc.CalcBlockAvgFeeRate in the common package aligns with the new code structure, maintaining consistency throughout the codebase.

---

418-418: Enhanced modular approach.
The usage of btc.GetRecentFeeRate from the common package is coherent with the broader refactoring effort.

---

599-601: Test coverage for default depositor fee.
The test accurately enforces and verifies the expected default depositor fee behavior.

---

605-611: Concise calculation check.
Calculating and comparing the depositor fee with the expected dynamic fee rate ensures correctness. The test is well-structured to catch regressions.

zetaclient/chains/bitcoin/observer/inbound_test.go (4)

19-19: Seamless shift to the common package.
Switching imports to reference common maintains consistency in the refactored architecture.

---

35-35: Appropriate mock creation.
Defining mockDepositFeeCalculator in terms of common.DepositorFeeCalculator keeps tests aligned with the new package.

---

Line range hint 58-147: Unified usage of common.CalcBlockAvgFeeRate.
All references to CalcBlockAvgFeeRate have been updated to use the common package. The tests remain clear and thorough, verifying correct handling of various edge cases (e.g., empty blocks, coinbase issues, invalid heights).

---

286-286: Consistent depositor fee calculation.
Replacing bitcoin.DepositorFee with common.DepositorFee ensures uniformity in fee computations across the codebase.

zetaclient/chains/bitcoin/common/tx_script_test.go (12)

Line range hint 1-20: Well-structured rename to common_test.
Renaming the package and updating the import paths clarifies the scope. Updating TestDataDir ensures correct file paths after the reorganization.

---

Line range hint 33-72: Spot-on P2TR decoding tests.
Decoding logic for P2TR is now hosted in common, with comprehensive tests for invalid and valid scenarios. This shift is well justified.

---

Line range hint 88-127: Robust P2WSH decoding coverage.
Both success cases and failure modes (e.g., wrong hash length, invalid OP codes) are adequately tested. Refactoring to common introduces no regressions.

---

145-155: P2WPKH decoding verification.
These additions ensure that inbound/outbound transactions relying on P2WPKH scripts are validated properly under the new structure.

---

174-189: Reliability for P2WPKH error handling.
Handling various script anomalies ensures the decode pipeline is robust. All negative paths appear well tested.

---

221-250: Distinct P2SH decode logic.
Shifting to common.DecodeScriptP2SH centralizes script handling, with thorough coverage of known script pitfalls.

---

Line range hint 266-328: P2PKH decoding validated.
Confirming that typical pay-to-pubkey-hash scripts decode consistently matches the overall approach in the other script decodes.

---

Line range hint 372-417: Comprehensive OP_RETURN memo decoding checks.
Tests highlight multiple edge cases, including pushdata size mismatches and invalid OP codes. These checks significantly reduce the risk of parsing errors.

---

495-495: DecodeSenderFromScript function usage.
This test thoroughly evaluates script-based address decoding under various script types, ensuring correctness for all common use cases.

---

Line range hint 513-561: Expanded TSS vout decoding.
Incorporating all major script types (P2TR, P2WSH, P2WPKH, P2SH, P2PKH) in the TSS decode path underscores the new package’s versatility.

---

Line range hint 580-613: Solid negative testing for TSS vout decode.
Covers invalid amounts, unsupported chain IDs, incorrect address decodes, and broken scripts. The variety of test cases is beneficial for reliability.

---

Line range hint 626-695: Detailed multi-path script decoding coverage.
Complex scripts, large pushdata sequences, truncated data, and missing opcodes—each scenario is explicitly tested. This broad coverage instills confidence in the decoding routine.

Reviewing files that changed from the base of the PR and between 80ca921 and 22d8a51.

• cmd/zetaclientd/inbound.go (1 hunks)
• cmd/zetaclientd/start.go (2 hunks)
• e2e/e2etests/test_bitcoin_deposit_and_call_revert.go (1 hunks)
• e2e/e2etests/test_bitcoin_deposit_and_call_revert_with_dust.go (1 hunks)
• e2e/e2etests/test_bitcoin_deposit_call.go (3 hunks)
• e2e/e2etests/test_bitcoin_donation.go (1 hunks)
• e2e/e2etests/test_bitcoin_std_deposit.go (2 hunks)
• e2e/e2etests/test_bitcoin_std_deposit_and_call.go (1 hunks)
• e2e/e2etests/test_bitcoin_std_memo_inscribed_deposit_and_call.go (2 hunks)
• e2e/runner/bitcoin.go (6 hunks)
• go.mod (2 hunks)
• pkg/chains/chain.go (2 hunks)
• pkg/scheduler/opts.go (1 hunks)
• pkg/scheduler/scheduler.go (3 hunks)
• pkg/ticker/ticker.go (3 hunks)
• zetaclient/chains/base/observer.go (1 hunks)
• zetaclient/chains/bitcoin/bitcoin.go (1 hunks)
• zetaclient/chains/bitcoin/common/errors.go (1 hunks)
• zetaclient/chains/bitcoin/common/fee.go (1 hunks)
• zetaclient/chains/bitcoin/common/fee_test.go (1 hunks)
• zetaclient/chains/bitcoin/common/tx_script.go (1 hunks)
• zetaclient/chains/bitcoin/common/tx_script_test.go (31 hunks)
• zetaclient/chains/bitcoin/common/utils.go (1 hunks)
• zetaclient/chains/bitcoin/observer/inbound.go (11 hunks)
• zetaclient/chains/bitcoin/observer/inbound_test.go (9 hunks)
• zetaclient/chains/bitcoin/observer/observer.go (6 hunks)
• zetaclient/chains/bitcoin/observer/outbound.go (4 hunks)
• zetaclient/chains/bitcoin/observer/rpc_status.go (1 hunks)
• zetaclient/chains/bitcoin/observer/witness.go (5 hunks)
• zetaclient/chains/bitcoin/observer/witness_test.go (2 hunks)
• zetaclient/chains/bitcoin/rpc/rpc_live_test.go (4 hunks)
• zetaclient/chains/bitcoin/signer/signer.go (10 hunks)
• zetaclient/context/chain.go (2 hunks)
• zetaclient/context/context.go (1 hunks)
• zetaclient/maintenance/shutdown_listener_test.go (1 hunks)
• zetaclient/orchestrator/bootstrap.go (2 hunks)
• zetaclient/orchestrator/bootstrap_test.go (8 hunks)
• zetaclient/orchestrator/contextupdater.go (0 hunks)
• zetaclient/orchestrator/orchestrator.go (2 hunks)
• zetaclient/orchestrator/v2_bootstrap.go (1 hunks)
• zetaclient/orchestrator/v2_bootstrap_test.go (1 hunks)
• zetaclient/orchestrator/v2_orchestrator.go (1 hunks)
• zetaclient/orchestrator/v2_orchestrator_test.go (1 hunks)
• zetaclient/testutils/mocks/chain_params.go (1 hunks)
• zetaclient/testutils/testlog/log.go (1 hunks)

• zetaclient/orchestrator/contextupdater.go

• zetaclient/chains/bitcoin/common/errors.go
• zetaclient/chains/bitcoin/common/fee.go
• e2e/e2etests/test_bitcoin_deposit_and_call_revert.go
• zetaclient/chains/bitcoin/common/tx_script.go
• zetaclient/chains/bitcoin/common/fee_test.go

zetaclient/context/chain.go (2)

67-69: Ensure unlock is called in all code paths for robust concurrency.

The use of a mutex here is appropriate. Confirm that any potential error paths or early returns do not bypass cr.mu.Unlock().

---

148-150: Pass-through logging fields implemented.

This convenience method properly delegates to the underlying raw chain’s logging fields. Looks good and keeps the code DRY.

pkg/chains/chain.go (1)

11-12: Import statements properly structured.

Importing the logging definitions from zetaclient/logs is consistent with the rest of the codebase.

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

11-12: Public function name and doc comment alignment look good.
The exported function now has an appropriate doc comment that starts with the method name and clearly describes its purpose. No issues found here.

---

15-15: Error wrapping is clear and consistent.
Using errors.Wrap adds valuable context. Keep the approach consistent across the codebase.

---

20-20: Solid error handling.
Likewise, continuing to wrap errors here ensures the origin of failures remains transparent.

zetaclient/orchestrator/v2_bootstrap.go (3)

16-20: Validate early chain checks.
If chain.IsBitcoin() is false, returning an error is correct. This is a suitable guard clause that prevents confusion and misconfiguration.

---

27-31: Revisit error type in errSkipChain.
Using errSkipChain is fine, but ensure the broader orchestration logic also properly handles chain skipping without leaving orphaned resources.

---

53-65: Check concurrency approach for the observer.
Review if the observer’s internal logic spawns routines or uses any concurrency that might require careful orchestration with the new orchestrator.

cmd/zetaclientd/start.go (2)

157-162: Validate constructor parameters for Dependencies.
If any required fields (like Zetacore) are missing or misconfigured, consider a pre-check, returning a descriptive error. This will aid debugging.

---

172-173: Evaluate orchestrator V2 lifecycle.
Adding the orchestrator V2 to graceful.AddService is correct. Confirm that any externally spawned goroutines and resources within V2 are also properly shut down.

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

48-52: Double-check concurrency safety.
The function calls b.observer.Observer.Start() but does not evidently manage concurrency around shared states. Confirm that internal observer structures are thread-safe.

---

53-63: Check subscription error handling.
The code logs errors when creating the new block subscriber fails. Ensure the app’s top-level error handling can gracefully degrade if the subscription fails.

---

64-91: Keep schedulers well-defined.
Registering multiple intervals with specialized skip conditions is good practice. Confirm that overlapping intervals (inbound vs. outbound) do not create race conditions on shared data.

---

109-113: Scrutinize block-based scheduling.
Using scheduler.BlockTicker(newBlockChan) is an effective approach, but confirm that blocking or slow tasks do not lead to queue buildup.

---

126-130: Enhance documentation.
The scheduleCCTX constraints are well-described in comments. This clarity is helpful for new devs.

---

153-160: Validate chain ID usage.
In the loop, line 161 logs a mismatch if params.ReceiverChainId != chainID. Ensure inbound chain ID checks are consistent across the codebase.

---

208-224: Validate chain param updates.
updateChainParams refreshes chain parameters from the updated context. Confirm that concurrency is not overwriting old data structures in observer.

zetaclient/orchestrator/v2_orchestrator.go (5)

124-125: Confirm usage of errSkipChain.
This sentinel error merges seamlessly with orchestrator logic. Validate usage across other chain bootstraps to avoid partial states.

---

203-209: Safeguard hasChain concurrency.
Currently, hasChain obtains an RLock for read access. Confirm no chain updates occur outside the orchestrator that might need synchronization or atomic references.

---

233-247: Encapsulate chain stopping logic.
removeChain calls Stop() on the observer signer. If the chain implementer changes the stop signature, ensure no partial states remain unclosed.

---

249-271: Optimize chain removal.
removeMissingChains uses a straightforward approach. For large-scale environments, a more efficient set difference approach may be beneficial, but this is acceptable for typical chain counts.

---

273-290: Fail fast on missing dependencies.
validateConstructor ensures the orchestrator cannot be misconfigured. This is an excellent pattern to prevent runtime errors.

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

17-17: Use the common package consistently.
Importing common for Bitcoin-specific logic looks structured. Ensure references to common are consistently used throughout the observer codebase to avoid confusion.

---

183-183: Confirm depositor fee assumptions.
common.CalcDepositorFee is invoked here. Validate that fee logic aligns with chain updates (e.g., changes in dust threshold or transaction priorities).

---

223-223: Confirm block re-scan fallback behavior.
If GetBtcEvent fails for a single transaction, the code returns an error, prompting a re-scan. Ensure this re-scan occurs so one failing transaction does not block the entire block’s inbound processing.

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

21-21: Keep imports aligned with the new common package structure
By importing common, you're adhering to the recent restructuring. This promotes clarity and centralizes shared utilities.

---

136-136: Good use of error wrapping
This statement provides valuable context to help diagnose issues quickly.

---

141-141: Error wrapping is consistent
Continuing the consistent approach of wrapping errors helps with deeper debugging contexts.

---

197-197: Fee conversion logic appears correct
This line demonstrates proper adaptation to the common.FeeRateToSatPerByte utility, ensuring consistent fee conversions.

---

381-381: Transparent fee rate retrieval
This line properly delegates fee rate acquisition to a centralized method in the common package, improving maintainability.

zetaclient/chains/bitcoin/observer/outbound.go (4)

17-17: Consistent import usage
Importing the common package aligns with the refactored structure, maintaining uniform references.

---

382-382: Validate return type
common.GetSatoshis processes floating amounts into integer satoshis. Ensure subsequent operations handle potential rounding issues or remainders gracefully.

---

572-572: Maintain flexible decoding
common.DecodeTSSVout is well-named; confirm that any future expansions to script types do not break backward compatibility.

---

626-626: Ensure canceled vout consistency
Decoding with common.DecodeTSSVout for a canceled transaction is correct. Confirm that no additional checks on dust or edge cases are necessary if partial refunds are canceled.

zetaclient/context/context.go (1)

11-11: Improved clarity in error message
Changing the text to “unable to get AppContext” is more direct, helping developers quickly identify missing context usage.

zetaclient/testutils/testlog/log.go (8)

1-2: Structured package declaration
The package name testlog clearly indicates its intended role in providing logging utilities for tests.

---

3-10: Selective imports
All imported packages appear relevant to the logging buffer and test environment. No concerns at this stage.

---

12-15: Central logger struct
Embedding zerolog.Logger provides flexible and concise custom logging for tests. Storing a buffer pointer allows introspection.

---

17-20: Concurrent buffer struct
Maintaining a dedicated bytes.Buffer protected by sync.RWMutex is a neat approach to thread-safe log capture.

---

22-32: Constructor method
Instantiating a multiwriter with both a test writer and the buffer ensures logs appear in standard test outputs and are concurrently captured. This helps in advanced test analysis.

---

34-36: String retrieval
Providing a String() method to the main Log struct simplifies log content retrieval for assertions or debugging.

---

38-43: Thread-safe writes
Locking for writes protects the buffer from concurrent modifications, ensuring log integrity.

---

45-50: Thread-safe read
Locking for reads prevents race conditions. This design choice is well-suited for test logs, even under parallel test execution.

zetaclient/chains/bitcoin/common/utils.go (1)

1-1: Refactored package name
Switching from bitcoin to common clarifies the scope of shared functionality and aligns with the multi-chain structure.

e2e/e2etests/test_bitcoin_donation.go (1)

12-12: Import path update appears consistent.

The updated import path reflects a centralized consolidation of Bitcoin functionalities under the common package, promoting clearer code organization.

pkg/scheduler/opts.go (2)

32-32: Enhanced clarity in the skipper function comment.

Documenting the skipper behavior (that true skips the task) is helpful for maintainers and clarifies task flow.

---

39-42: Immediate interval update ensures consistency.

Calling intervalUpdater() to set opts.interval right away aligns the initial interval setting with the updated logic. Ensure thread-safety if multiple goroutines invoke this option concurrently.

e2e/e2etests/test_bitcoin_std_deposit_and_call.go (1)

13-13: Aligned import path with refactored Bitcoin code.

Switching to the common package unifies Bitcoin utilities and aligns with ongoing modularization efforts.

e2e/e2etests/test_bitcoin_deposit_call.go (2)

12-12: Refactored import path.

Moving to common fosters more maintainable Bitcoin-related code.

---

48-48: Replacing GetSatoshis reference for clarity.

Adopting common.GetSatoshis(amount) improves readability, reflecting the refactoring choices of the new common package. Ensure all references to this function are consistently updated across tests.

zetaclient/testutils/mocks/chain_params.go (1)

35-35: Reassess default ticker value.

Previously set to zero, the WatchUtxoTicker is now set to 1. Ensure this increment does not negatively affect workflow or system resources. Consider verifying if a non-zero default value causes unintended overhead in test environments.

e2e/e2etests/test_bitcoin_std_memo_inscribed_deposit_and_call.go (2)

13-13: Modular import path transition acknowledged.

Switching from the main bitcoin package to the common sub-package is consistent with the ongoing refactoring. This enhances maintainability and keeps common logic centralized.

---

56-56: Validate depositor fee usage and error handling.

Here, the common.DefaultDepositorFee is converted to satoshis. Ensure this default aligns with operational requirements and that any errors in retrieval are handled gracefully beyond merely logging.

e2e/e2etests/test_bitcoin_std_deposit.go (2)

13-13: Unified Bitcoin logic import recognized.

Importing from the common package centralizes Bitcoin utility functions, promoting code reuse. This is a good step toward a cleaner architecture.

---

57-57: Check numeric conversions and ensure consistent usage.

Converting floats to satoshis can cause rounding discrepancies. Confirm that the GetSatoshis function correctly handles edge cases (e.g., extremely small amounts) to avoid rounding errors.

e2e/e2etests/test_bitcoin_deposit_and_call_revert_with_dust.go (1)

13-13: Refactoring import path for improved consistency.

Switching to the common package matches the broader shift across the codebase. This maintains consistency and simplifies future changes in Bitcoin processing logic.

zetaclient/orchestrator/v2_bootstrap_test.go (4)

19-23: Smooth test initialization.

The initialization of the test suite and orchestrator setup is well-structured. The approach of creating a new test suite (ts := newTestSuite(t)) is clear and fosters maintainable testing patterns.

---

28-30: Configuration update usage looks solid.

Assigning the btcConfig to cfg.BTCChainConfigs[chains.BitcoinMainnet.ChainId] is straightforward and keeps the Bitcoin-specific logic isolated.

---

57-67: Helper function for task grouping is concise.

The tasksHaveGroup function efficiently checks for group membership, contributing to clear test readability.

---

69-78: Mock setup for Bitcoin calls is clean.

The mockBitcoinCalls function properly configures the mocked environment, making the test deterministic and easier to maintain.

pkg/scheduler/scheduler.go (2)

147-151: Informative logging for group stop operation.

Logging the count of tasks being stopped in a group gives clear operational insights and is beneficial for troubleshooting.

---

181-187: Getter methods for Task struct are straightforward.

The Group() and Name() methods improve code clarity by exposing task properties succinctly.

pkg/ticker/ticker.go (2)

142-142: Validate concurrency safety for task invocation.

Inside the loop, if multiple ticks occur sequentially in quick succession, ensure the method enforces concurrency controls on task usage (if the task is not inherently thread-safe).

---

217-219: Use consistent checks for t.ticker != nil before stopping.

Since t.ticker is conditionally created, ensure code paths always guard calls that reference t.ticker. This helps prevent inadvertent nil-pointer dereferences if the ticker was not successfully started.

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

13-13: Confirm dependencies in common package exist and are tested.

With the new import reference to common, ensure that all dependencies (e.g., common.DepositorFeeCalculator) are properly tested and documented, preventing regressions in deposit fee logic.

---

140-140: Avoid potential nil index on tx.Vout[1].

Although the code checks len(tx.Vout) < 2 before referencing tx.Vout[1], ensure there are no parallel changes that might skip this early check.

---

162-162: Guard script usage in tryExtractInscription.

In the loop, script can be nil if decoding the final witness element fails. You have a check to continue, which is good. Just confirm no panics arise in subsequent logic.

zetaclient/orchestrator/v2_orchestrator_test.go (3)

1-28: Keep test imports minimal and cohesive.

The import set is clean. Just ensure no redundant test dependencies are introduced to maintain lean test coverage.

---

94-105: Maintain consistent chain usage in defaultChainsWithParams.

defaultChainsWithParams includes both EVM and non-EVM chains. Ensure the test suite does not inadvertently test unsupported chains or create confusion in chain-specific logic.

---

208-252: Handle partial/empty chain lists in newAppContext.

If chainList is empty, Update(...) might skip certain validations. Consider logging or returning an error for an empty chain set.

zetaclient/orchestrator/bootstrap_test.go (4)

9-9: Check for redundant or missing imports.

The updated import block is straightforward. Confirm no extraneous imports remain and that new mocks or testing libraries remain necessary.

---

109-109: Maintain consistent runtime chain toggling coverage.

Re-enabling Ethereum after prior removal is good coverage. Also consider verifying chain-dependent resources remain consistent, e.g., signers or config references.

---

154-154: Evaluate mockZetacore usage across tests.

Centralizing Zetacore mocks in one helper function is beneficial. Ensure each test scenario that requires PostVoteGasPrice either sets a proper expectation or clarifies if the call is not expected.

---

417-418: Split out helper logic from test assertion code.

The hasObserver function calls t.Helper(), which is excellent for clarity. If it grows more complex, consider a dedicated test utility package.

zetaclient/orchestrator/bootstrap.go (1)

139-140: Ensure graceful fallback for residual Bitcoin flows.
By continuing here, you skip Bitcoin signers for orchestrator V1. Ensure that any residual or legacy Bitcoin flows gracefully transition to orchestrator V2, or provide a clear error path if orchestrator V2 is unavailable.

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

27-27: Modular usage of common package acknowledged.
Switching to common fosters improved clarity and reusability across Bitcoin-related functionality.

---

102-109: Check for integer overflow on large amounts.
While common.GetSatoshis() is helpful, consider verifying that large floats do not overflow when converted to satoshis.

---

169-169: Fee calculation is clear.
Multiplying gasPrice.Uint64() by common.OutboundBytesMax is straightforward and aligns well with prior logic. No immediate concerns.

---

245-245: Re-check the prevOuts[ix].Amount for edge cases.
Defensive checks against zero or negative amounts can prevent potential underflows in corner cases.

---

309-309: Cross-module type usage is consistent.
Injecting observer *observer.Observer is a reasonable pattern to keep chain details together for BTC sign operations.

---

338-338: Minimal overhead in retrieving chain object.
Acquiring the chain from observer ensures alignment with the local chain context.

---

373-373: Synergy with FeeRateToSatPerByte.
Good to see a consistent method for relay fee handling.

---

402-402: Consistent function signature extension.
The extended function signature with observer parameter properly centralizes chain data.

---

449-449: Broadcast record introduced.
Storing broadcasted TX in observer helps with concurrency and indexing in the new orchestrator design.

e2e/runner/bitcoin.go (6)

26-26: Clarified alias zetabtc.
Renaming the import alias to distinctly represent the common package fosters better readability.

---

103-103: Ensure deposit fee is consistently updated.
Appending zetabtc.DefaultDepositorFee ensures deposit amounts remain correct. Keep the default fee synchronized with any global parameters.

---

151-151: Inline arithmetic.
Combining 1.15 + zetabtc.DefaultDepositorFee is straightforward. Consider verifying that these default additions remain aligned with the expected test flows.

---

244-244: Satoshis conversion logic.
Double-check for potential floating-point boundary conditions. Overall, the call to zetabtc.GetSatoshis is consistent with prior logic.

---

354-354: Uniform usage of GetSatoshis.
This confirms a unified approach for inscriptions as well. Maintain consistent usage across all deposit and inscription flows.

---

415-415: Decoding script is well-rationalized.
Decoupling script decoding to zetabtc.DecodeScriptP2WPKH standardizes the approach for P2WPKH addresses.

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

169-175: Early return avoids redundant updates.
The addition of a parameter-equality check eliminates needless churn. This ensures improved stability and cleaner logs.

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

21-21: Migration to btc alias looks consistent.
No concerns here; referencing the common package under the btc alias is in line with the ongoing refactor.

---

366-366: Confirm edge-case handling.
The switch to btc.CalcBlockAvgFeeRate is valid. Ensure that large or small block sizes and edge cases (e.g., zero transactions) are addressed in the function’s implementation.

---

599-611: Testing different network parameters.
Lines 599-611 demonstrate correct usage of btc.CalcDepositorFee under different Bitcoin network parameters. Validating deposit fees across MainNet, RegressionNet, etc., is a good practice.

zetaclient/orchestrator/orchestrator.go (1)

202-202: Chain parameter assignment is clear.
Straightforward parameter injection for the observer. Check whether any additional safety checks are needed if chain.Params() returns unexpected values.

zetaclient/chains/bitcoin/observer/inbound_test.go (5)

19-19: New import path is aligned with consolidation efforts.
Switching from bitcoin to bitcoin/common refines the structure and improves maintainability.

---

35-35: Mocking deposit fee calculator.
The updated return type, common.DepositorFeeCalculator, aligns with the new package structure. The mock approach looks straightforward and test-friendly.

---

Line range hint 58-122: Refactored fee rate calculations.
These lines consistently use common.CalcBlockAvgFeeRate. Ensure that any potential integer overflow or type conversion issues are handled inside this function, especially for large blocks or unusual transaction fees.

Also applies to: 147-147

---

286-286: Inline deposit fee assignment.
Switch to common.DepositorFee with clientcommon.BTCOutboundGasPriceMultiplier is noted. Continue verifying that the multiplier is up-to-date with any dynamic fee adjustments.

---

Line range hint 372-417: Consistent usage of common.DecodeOpReturnMemo.
The new function calls for decoding OP_RETURN data appear logical and consistent. Verify that long or malformed inputs are handled without panics.

zetaclient/chains/bitcoin/common/tx_script_test.go (7)

Line range hint 1-20: Rename to common_test and updated imports.
The package rename better reflects the shared nature of this code. The reference to the test data directory is also properly maintained.

---

Line range hint 33-128: Decoding various P2TR/P2WSH scripts.
The test coverage for decoding logic is quite comprehensive. Be sure to keep an eye out for new script types or any consensus rule changes that could break assumptions here.

---

Line range hint 145-328: Expanded tests for P2WPKH, P2SH, P2PKH decodes.
These updates consistently call common.DecodeScript* functions. The guard clauses and error checks appear robust.

---

Line range hint 372-417: OP_RETURN memo decoding.
Decoding logic is thorough and accounts for multiple OP_PUSHDATA scenarios. This helps ensure that complex memos or on-chain data are properly parsed without corruption.

---

Line range hint 495-561: Decode TSS vout approach.
These lines ensure that different script types (P2TR, P2WSH, P2WPKH, P2SH, P2PKH) can still decode TSS vout data. The approach is clean and consistent.

---

Line range hint 580-613: Error handling within DecodeTSSVout.
Negative amounts, invalid chain IDs, or mismatched addresses now produce readable error messages. This is good for debugging.

---

Line range hint 626-695: General script decoding.
The extended coverage for unusual or truncated scripts is commendable. This robust handling will prevent silent failures when new script patterns or malformed data are encountered.

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

13-13: LGTM! Import path update aligns with package restructuring.

The change to use the new bitcoin/common package is consistent with the PR's objective of consolidating Bitcoin-related functionalities.

go.mod (1)

166-166: LGTM! Dependencies now directly used.

The transition of these dependencies from indirect to direct usage aligns with the PR's integration of Prometheus metrics. The versions are maintained, ensuring stability.

Run the following script to verify the latest stable versions of the dependencies:

Also applies to: 252-253

✅ Verification successful

Dependencies are correctly transitioned and actively used in the codebase

The verification confirms that:

• github.com/google/uuid is actively used in zetaclient/orchestrator/v2_bootstrap_test.go and pkg/scheduler/scheduler.go
• Prometheus-related packages are extensively used across multiple files in the zetaclient directory
• The transition from indirect to direct dependencies is justified by their actual usage in the codebase
• The version v1.6.0 of uuid package was already present in the previous go.mod, maintaining stability

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for latest versions of the direct dependencies

# Check latest versions on pkg.go.dev
for pkg in "github.com/google/uuid" "github.com/prometheus/client_model" "github.com/prometheus/common"; do
  echo "Checking $pkg..."
  curl -s "https://pkg.go.dev/$pkg" | grep -A 1 "Latest version"
done

Length of output: 851

---

Script:

#!/bin/bash
# Let's verify if these dependencies are actually used in the codebase
# and check their transition from indirect to direct dependencies

# First, let's check the old go.mod state to confirm they were indirect
git show HEAD^:go.mod | grep -A 1 "google/uuid"

# Then verify actual usage of these packages in the code
echo "Checking usage of github.com/google/uuid:"
rg "github.com/google/uuid" --type go

# Check for prometheus dependencies usage
echo -e "\nChecking usage of prometheus packages:"
rg "github.com/prometheus" --type go

Length of output: 1363