feat: enable inscription parsing on Bitcoin mainnet #3425
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a significant enhancement to Bitcoin mainnet functionality by enabling inscription parsing. The changes primarily involve modifying the Bitcoin observer's event handling mechanism in the ZetaChain node, specifically removing the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
lumtis
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
changelog.md (1)
11-11: Include version context for clarity.
Adding “enable inscription parsing on Bitcoin mainnet” is informative. Consider appending a reference to the upcoming release version or date for heightened transparency.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
changelog.md(1 hunks)zetaclient/chains/bitcoin/observer/inbound.go(1 hunks)zetaclient/chains/bitcoin/observer/inbound_test.go(16 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
zetaclient/chains/bitcoin/observer/inbound_test.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
zetaclient/chains/bitcoin/observer/inbound.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
🔇 Additional comments (17)
zetaclient/chains/bitcoin/observer/inbound.go (1)
288-288: Consider a fallback for non-witness transactions.
The updatedGetBtcEventfunction exclusively callsGetBtcEventWithWitness, removing any handling for non-witness data. Confirm whether legacy transactions or unupgraded nodes would fail under this approach. If necessary, implement a graceful fallback or detection mechanism for older transaction types.zetaclient/chains/bitcoin/observer/inbound_test.go (16)
277-277: Test renaming is consistent.
Renaming the test fromTestGetBtcEventWithoutWitnesstoTestGetBtcEventaptly aligns with the consolidated approach in production code.
318-318: Clear function call alignment.
Switching toGetBtcEventensures test parity with the updated observer logic. The scenario coverage is adequately maintained.
351-351: Good insufficient fee scenario coverage.
Validating the depositor fee edge case withGetBtcEventhelps maintain confidence in the updated event extraction logic.
376-376: Proper handling of P2TR address.
UsingGetBtcEventfor Taproot inputs demonstrates that the codebase now comprehensively covers witness-based transactions.
401-401: Thorough P2WSH test coverage.
Ensuring test coverage for P2WSH inputs inGetBtcEventboosts confidence in the correctness of the new consolidated function.
426-426: P2SH sending scenario validated.
The test confirms that sending from a P2SH address remains supported under the unified event parsing flow.
451-451: P2PKH coverage is consistent.
This test demonstrates that legacy P2PKH inputs are accurately handled, even with the consolidated witness-based extraction approach.
472-472: Minimum output count check.
Skipping transactions with fewer than two outputs remains an important safeguard, ensuring only relevant inbound transactions are processed.
493-493: Rejecting non-P2WPKH outputs at vout[0].
Maintaining this check ensures transactions intended for other addresses are gracefully ignored, preventing extraneous event generation.
508-508: Extended script length tested.
Appending a byte to the script and confirming that it is rejected highlights careful boundary handling with the new function approach.
529-529: Rejecting non-TSS addresses.
Confirming that the inbound flow halts if the target address is not TSS is essential to the inbound filtering logic.
549-549: Depositor fee calculation failure scenario.
The test covers fee calculation RPC error handling, ensuring failures do not disrupt the rest of the inbound flow.
570-570: Guarding against non-OP_RETURN second outputs.
Skipping transactions lacking the second output memo logic remains a cornerstone of reliable inbound event detection.
591-591: Memo decoding failure check.
Ensuring the process discards transactions with broken memo scripts prevents potential misinterpretations or invalid event creation.
623-623: Empty sender address scenario.
The test verifies that unknown or malformed sender scripts result in a nil event, strengthening input validation.
662-662: Proper error handling for RPC failures.
Verifying that the function surfaces errors fromGetRawTransactionpreserves reliability for fault-tolerant inbound processing.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3425 +/- ##
===========================================
- Coverage 62.89% 62.86% -0.04%
===========================================
Files 436 436
Lines 30796 30726 -70
===========================================
- Hits 19369 19315 -54
+ Misses 10615 10603 -12
+ Partials 812 808 -4
|
Description
Closes #3424
Summary by CodeRabbit
New Features
Refactor
Tests