fix(zetaclient): respect DisableTssBlockScan in Bitcoin observer#4597
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis pull request introduces a conditional disable mechanism for inbound Bitcoin block scanning. When the ChangesBitcoin inbound TSS block scanning disable
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
zetaclient/chains/bitcoin/observer/inbound_test.go (1)
146-160: ⚡ Quick winHarden the guard tests with explicit “no processing” assertions.
require.NoErroralone is broad. Add explicit assertions that no inbound-scan/tracker-processing side effect occurred (for example, unchanged scan state and/or zero receipt/vote path invocations) so regressions are caught deterministically.Also applies to: 162-181
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@zetaclient/chains/bitcoin/observer/inbound_test.go` around lines 146 - 160, Update Test_ObserveInbound_DisableTssBlockScan to assert no processing occurred beyond returning no error: after setting DisableTssBlockScan on ob.ChainParams() and calling ob.ObserveInbound(ob.ctx), assert the scan state remained unchanged (e.g., compare previous and current scan height/marker from ob.ScanState or equivalent) and assert the mock Bitcoin client received zero calls to GetBlockHash/GetBlockVerbose and that tracker/receipt/vote handlers were not invoked (use the mock call counters or expectations). Apply the same explicit “no processing” assertions to the sibling test (the one at 162-181) so both tests deterministically fail on regressions.zetaclient/chains/bitcoin/observer/inbound.go (1)
29-34: ⚡ Quick winEmit an explicit log when inbound scanning is disabled.
This branch is operationally significant; add a single info log before returning so pause/resume behavior is visible in production diagnostics.
Proposed patch
if ob.ChainParams().DisableTssBlockScan { + logger.Info(). + Bool("disable_tss_block_scan", true). + Uint64("last_block_scanned", ob.LastBlockScanned()). + Msg("skipping bitcoin inbound block scan by chain params") return nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@zetaclient/chains/bitcoin/observer/inbound.go` around lines 29 - 34, Add an info log before returning when inbound scanning is disabled: in the block that checks ob.ChainParams().DisableTssBlockScan, emit a single info-level message (e.g. using the observer's logger such as ob.logger or ob.Log.Infof/Info) stating that TSS block scanning is disabled and inbound scanning is paused, then return nil as before; use the existing ob receiver to find the correct logger instance and keep the message concise for operational visibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@zetaclient/chains/bitcoin/observer/inbound_test.go`:
- Around line 146-160: Update Test_ObserveInbound_DisableTssBlockScan to assert
no processing occurred beyond returning no error: after setting
DisableTssBlockScan on ob.ChainParams() and calling ob.ObserveInbound(ob.ctx),
assert the scan state remained unchanged (e.g., compare previous and current
scan height/marker from ob.ScanState or equivalent) and assert the mock Bitcoin
client received zero calls to GetBlockHash/GetBlockVerbose and that
tracker/receipt/vote handlers were not invoked (use the mock call counters or
expectations). Apply the same explicit “no processing” assertions to the sibling
test (the one at 162-181) so both tests deterministically fail on regressions.
In `@zetaclient/chains/bitcoin/observer/inbound.go`:
- Around line 29-34: Add an info log before returning when inbound scanning is
disabled: in the block that checks ob.ChainParams().DisableTssBlockScan, emit a
single info-level message (e.g. using the observer's logger such as ob.logger or
ob.Log.Infof/Info) stating that TSS block scanning is disabled and inbound
scanning is paused, then return nil as before; use the existing ob receiver to
find the correct logger instance and keep the message concise for operational
visibility.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 23aab3cb-c115-4ef1-8248-af40671cc665
📒 Files selected for processing (3)
zetaclient/chains/bitcoin/observer/inbound.gozetaclient/chains/bitcoin/observer/inbound_test.gozetaclient/chains/bitcoin/observer/inbound_tracker.go
ws4charlie
approved these changes
May 19, 2026
Contributor
|
missing change log |
Member
Author
|
@greptile |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
kingpinXD
approved these changes
May 19, 2026
This was referenced May 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Align the Bitcoin observer with the EVM observer (zetaclient/chains/evm/observer/inbound.go:190), which already honors ChainParams.DisableTssBlockScan to skip TSS-address block scanning.
Bitcoin's only inbound mechanism is direct transfers to the TSS address, so this flag effectively provides operators a per-chain inbound observation pause for Bitcoin, with no effect on the outbound/withdrawal flow.
Notes:
How Has This Been Tested?
Note
Medium Risk
Changes Bitcoin inbound processing behavior by short-circuiting both block scanning and tracker-based voting when
DisableTssBlockScanis enabled; misconfiguration could pause deposit observation and delay inbound finalization.Overview
Bitcoin observer now respects
ChainParams.DisableTssBlockScanby returning early fromObserveInbound(afterupdateLastBlock) and fromobserveInboundTrackers, effectively pausing all inbound deposit detection/voting while leaving outbound processing untouched.Adds unit tests asserting that enabling the flag results in no RPC scanning/processing calls for both block-range scanning and inbound-tracker handling.
Reviewed by Cursor Bugbot for commit f011b7a. Configure here.
Greptile Summary
This PR aligns the Bitcoin observer with the EVM observer by honoring
ChainParams.DisableTssBlockScan, short-circuitingObserveInbound(afterupdateLastBlock) andobserveInboundTrackerswhen the flag is set. Since Bitcoin's only inbound mechanism is direct TSS transfers, the flag acts as a full per-chain inbound pause without touching outbound/withdrawal processing.inbound.go: Guard is placed afterupdateLastBlockso chain-tip health monitoring continues;LastBlockScanneddoes not advance while paused, enabling clean resume-from-last-scanned when the flag is cleared.inbound_tracker.go: Both external (ProcessInboundTrackers) and internal (ProcessInternalTrackers) tracker paths respect the flag, preventing ballot submission for any tracker discovered during the pause window.inbound_test.go: Two negative-pattern unit tests confirm no RPC scanning or voting occurs when the flag is enabled; the testify mock client would panic on any unexpected call, making the guard genuinely enforced by the test.Confidence Score: 5/5
Safe to merge — the guard is correctly placed after health-monitoring, logging is in place, and tests enforce the early-return path.
The change is a targeted, well-scoped guard that mirrors existing EVM observer behavior. The flag position (after updateLastBlock, before any scanning) is correct, the sampled log makes the skip observable in production, and the unit tests use testify mock panics to enforce that no RPC calls are made when the flag is active. No existing code paths are modified; the only risk is the operational concern already documented in the PR description about coordinated rollout.
No files require special attention.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[ObserveInbound called] --> B[updateLastBlock\nhealth monitoring continues] B --> C{DisableTssBlockScan?} C -- true --> D[Log sampled info\nreturn nil] C -- false --> E[Fetch fee rate\nGet scan ranges] E --> F[observeInboundInBlockRange\nSafe blocks] F --> G[SaveLastBlockScanned] E --> H[observeInboundInBlockRange\nFast blocks] I[ProcessInboundTrackers called] --> J[GetInboundTrackers from ZetaRepo] J --> K[observeInboundTrackers\nisInternal=false] K --> L{DisableTssBlockScan?} L -- true --> M[Log sampled info\nreturn nil] L -- false --> N[CheckReceiptAndPostVoteForBtcTxHash\nfor each tracker] O[ProcessInternalTrackers called] --> P[GetInboundInternalTrackers] P --> Q[observeInboundTrackers\nisInternal=true] Q --> LReviews (2): Last reviewed commit: "changelog" | Re-trigger Greptile