Skip to content

Conversation

@mohamedawnallah
Copy link
Contributor

Objectives

The goal of this PR is to make sure there are no unintended consequences of applying that btcwallet patch. This can be explicitly verified by making sure there are no relevant tests failing in the CI.

Motivation and Context

Follow-up on @yyforyongyu's comment btcsuite/btcwallet#1114 (review).

Triggering Points

This is the only place in LND codebase where Confirmations field from GetTransaction API is conditionally utilized:

// GetTransactionDetails returns details of a transaction given its
// transaction hash.
func (b *BtcWallet) GetTransactionDetails(
txHash *chainhash.Hash) (*lnwallet.TransactionDetail, error) {
// Grab the best block the wallet knows of, we'll use this to calculate
// # of confirmations shortly below.
bestBlock := b.wallet.SyncedTo()
currentHeight := bestBlock.Height
tx, err := b.wallet.GetTransaction(*txHash)
if err != nil {
return nil, err
}
// For both confirmed and unconfirmed transactions, create a
// TransactionDetail which re-packages the data returned by the base
// wallet.
if tx.Confirmations > 0 {
txDetails, err := minedTransactionsToDetails(
currentHeight,
base.Block{
Transactions: []base.TransactionSummary{
tx.Summary,
},
Hash: tx.BlockHash,
Height: tx.Height,
Timestamp: tx.Summary.Timestamp},
b.netParams,
)
if err != nil {
return nil, err
}
return txDetails[0], nil
}
return unminedTransactionsToDetail(tx.Summary, b.netParams)
}

Pull Request Checklist

Testing

  • Your PR passes all CI checks.
  • Tests covering the positive and negative (error paths) are included.
  • Bug fixes contain tests triggering the bug to prevent regressions.

Code Style and Documentation

📝 Please see our Contribution Guidelines for further guidance.

@gemini-code-assist
Copy link

Summary of Changes

Hello @mohamedawnallah, 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 validating a specific patch to the btcwallet's GetTransaction API confirmations. To achieve this, the PR updates a wide array of Go module dependencies to their most recent versions and includes a temporary replace directive for btcwallet. The core objective is to confirm that these updates and the btcwallet patch do not introduce any regressions or unintended side effects, primarily verified by the successful execution of CI tests.

Highlights

  • Dependency Updates: Numerous Go module dependencies have been updated to their latest versions, including core btcsuite components, golang.org/x libraries, and google.golang.org/grpc.
  • Temporary btcwallet Replacement: A replace directive has been added in go.mod to temporarily point github.com/btcsuite/btcwallet to a specific fork by the author, github.com/mohamedawnallah/btcwallet.
  • Patch Evaluation: The primary goal of these changes is to evaluate the properness of a btcwallet confirmations patch for the GetTransaction API, ensuring that the updated dependencies do not introduce unintended consequences or regressions.
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

@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 updates numerous dependencies and, most notably, adds a replace directive in go.mod to use a personal fork of btcwallet. This is presumably to test a patch for btcwallet via CI. My review contains a critical comment regarding this replace directive. Using a personal fork for a core dependency introduces significant security and maintainability risks and should not be merged into a main branch. The patch should be merged upstream into the official repository instead.

@yyforyongyu yyforyongyu added this to the v0.21.0 milestone Nov 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants