Skip to content

Conversation

@gregdhill
Copy link
Contributor

@gregdhill gregdhill commented Oct 31, 2025

Summary by CodeRabbit

  • New Features

    • Support for connecting to specific Bitcoin wallets via RPC endpoint configuration.
  • Documentation

    • Fixed spelling and comment typos in deployment and code documentation.
  • Chores

    • Improved Bitcoin network detection to derive network from genesis data rather than URL patterns.
    • Cleaned up crate attributes and code-generation header notes.

Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
@vercel
Copy link

vercel bot commented Oct 31, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
bob-docs Ready Ready Preview Comment Nov 5, 2025 0:00am

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 31, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Changes include minor documentation fixes, removal of a crate-level lint allowance, addition of a wallet-aware BitcoinClient constructor (duplicated in the patch), and refactoring EsploraClient network detection to use the genesis block hash with new unit tests.

Changes

Cohort / File(s) Summary
Documentation and Comment Corrections
contracts/src/swap/Marketplace.sol, deploy-all.md
Fixed typos: "cant have struct as key, nor tupple" → "can't have struct as key, nor tuple"; "Miscelaneous deployments" → "Miscellaneous deployments". No behavioral changes.
Rust Lint Configuration
crates/bindings/src/lib.rs
Removed crate-level dead_code allowance; retained unused_imports, clippy::all, rustdoc::all. Added autogenerated-code maintenance header line.
Bitcoin RPC Client Enhancement
crates/utils/src/bitcoin_client.rs
Added pub fn new_with_wallet(url: &str, wallet_name: &str, rpc_user: impl Into<String>, rpc_pass: impl Into<String>) -> Self which appends /wallet/{wallet_name} to base URL and delegates to existing new(); signature appears duplicated in the patch.
Network Detection Refactoring
crates/utils/src/esplora_client.rs
Replaced URL-based network detection with genesis-block-hash-based detection: fetch block 0, derive ChainHash via constants::ChainHash::from_genesis_block_hash(), then map to Network. Added test_get_network() for Bitcoin, Testnet, and Signet.

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant EsploraClient
    participant EsploraAPI

    Note over Caller,EsploraClient: New flow — genesis-hash network detection
    Caller->>EsploraClient: get_bitcoin_network()
    EsploraClient->>EsploraAPI: GET /block/0/hash
    EsploraAPI-->>EsploraClient: genesis_hash
    EsploraClient->>EsploraClient: ChainHash::from_genesis_block_hash(genesis_hash)
    EsploraClient->>EsploraClient: Network::try_from(chain_hash)
    EsploraClient-->>Caller: Network
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Inspect duplicated new_with_wallet() entries in crates/utils/src/bitcoin_client.rs and confirm/remove redundancy.
  • Verify error handling and mapping logic in crates/utils/src/esplora_client.rs for block fetch failures and invalid genesis hashes; review new unit test coverage.
  • Confirm lint change in crates/bindings/src/lib.rs does not surface expected warnings.

Possibly related PRs

Suggested reviewers

  • nakul1010

Poem

🐰 Hops of change across the code,

Genesis hashes gently showed,
Wallet paths now find their way,
Spelling straightened for the day,
I nibble bytes and celebrate!

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title mentions 'typos, bindings, helpers' which aligns with the actual changes: typo fixes in comments and documentation, modifications to bindings configuration, and a new helper method for BitcoinClient.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/things

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c7763d1 and 95b16a2.

📒 Files selected for processing (1)
  • crates/utils/src/esplora_client.rs (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
crates/utils/src/esplora_client.rs (1)
crates/utils/src/mempool_client.rs (1)
  • new (28-41)
⏰ 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). (1)
  • GitHub Check: test
🔇 Additional comments (3)
crates/utils/src/esplora_client.rs (3)

2-3: LGTM! Required import for genesis hash-based network detection.

The addition of constants::ChainHash is necessary for the new implementation of get_bitcoin_network and is correctly placed in the import list.


259-262: Excellent refactor to genesis hash-based detection.

This implementation is more robust than URL pattern matching since it verifies the actual network by fetching and validating the genesis block hash. The error handling is appropriate with the ? operators propagating any network or conversion errors.


348-360: Test assertions now correct, but note external dependencies.

The test properly uses assert_eq! instead of the previous unchecked matches! macro, addressing the concerns raised in prior reviews. The assertions will now correctly fail if the wrong network is returned.

However, this test relies on external Esplora servers (mainnet, testnet, signet) being available and responsive, which could cause flakiness in CI/CD environments or when services are temporarily unavailable. Consider whether this integration test should be marked as requiring network access or moved to a separate test suite.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @gregdhill, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request focuses on a series of small but impactful improvements across the codebase. It addresses minor textual errors in comments and documentation, refines Rust code generation attributes, and enhances the utility clients. A key improvement is the more robust Bitcoin network detection logic in the Esplora client, ensuring greater accuracy and reliability when interacting with different Bitcoin networks.

Highlights

  • Typo Corrections: Several minor typos were corrected across a Solidity contract comment and a Markdown documentation file, improving clarity and professionalism.
  • Rust Bindings Refinement: The dead_code allowance was removed from the crates/bindings/src/lib.rs file, indicating improved code generation or cleaner usage of the generated bindings.
  • Bitcoin Client Helper: A new helper function, new_with_wallet, was added to the BitcoinClient in crates/utils/src/bitcoin_client.rs to simplify connecting to a Bitcoin RPC with a specific wallet context.
  • Robust Bitcoin Network Detection: The EsploraClient in crates/utils/src/esplora_client.rs now determines the Bitcoin network by querying the genesis block hash, a more reliable method than string matching URLs. This change is accompanied by new unit tests.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces several small but useful changes, including typo corrections, code cleanup in bindings, and a new helper function for the Bitcoin client. The most significant improvement is in esplora_client.rs, where network detection is now more robustly implemented by checking the genesis block hash. While this is a great change, the accompanying new test has a flaw: it doesn't actually assert the correctness of the returned network. I've left a comment with a suggestion to fix this.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b605eff and c7763d1.

📒 Files selected for processing (5)
  • contracts/src/swap/Marketplace.sol (1 hunks)
  • crates/bindings/src/lib.rs (1 hunks)
  • crates/utils/src/bitcoin_client.rs (1 hunks)
  • crates/utils/src/esplora_client.rs (3 hunks)
  • deploy-all.md (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
crates/utils/src/esplora_client.rs (1)
crates/utils/src/mempool_client.rs (1)
  • new (28-41)
⏰ 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). (1)
  • GitHub Check: test
🔇 Additional comments (6)
contracts/src/swap/Marketplace.sol (1)

11-11: LGTM! Comment typo corrected.

The comment has been properly corrected from "cant have struct as key, nor tupple" to "can't have struct as key, nor tuple".

deploy-all.md (1)

40-40: LGTM! Heading spelling corrected.

The section heading has been corrected from "Miscelaneous" to "Miscellaneous".

crates/utils/src/esplora_client.rs (2)

2-3: LGTM! Imports added to support genesis-hash-based network detection.

The additions of consensus and constants::ChainHash are necessary for the refactored get_bitcoin_network method.


259-261: Excellent refactor! Genesis-hash-based network detection is more robust.

The new implementation derives the network from the genesis block hash rather than URL patterns, which is more reliable and canonical. The logic is clean and leverages the standard ChainHash::from_genesis_block_hash conversion.

crates/utils/src/bitcoin_client.rs (1)

262-270: LGTM! Wallet-specific constructor is well-implemented.

The new_with_wallet constructor correctly builds a wallet-scoped RPC URL and delegates to the existing new method. Verification confirms no duplicate definitions exist in the file—only one definition at line 262.

crates/bindings/src/lib.rs (1)

1-1: Removing dead_code from lint allowance is safe.

Verification confirmed no dead code warnings are triggered in the bindings crate. The change is approved.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants