Skip to content

Conversation

@mohamedawnallah
Copy link
Collaborator

Change Description

Description of change / link to associated issue.

Steps to Test

Steps for reviewers to follow to test the change.

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.

Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

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

LGTM 💥

Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

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

Changes look good, but the unit tests look to be failing.

FWIW, we've typically relied on the lnd lnwallet integration tests, but I do support updating these. I think we might be using some older commands/configs that have been deprecated.

Copy link
Collaborator

@yyforyongyu yyforyongyu left a comment

Choose a reason for hiding this comment

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

Don't do it yet - let's finalize our CI pipeline and run tests against multiple versions of bitcoins first.

@mohamedawnallah mohamedawnallah added the P2 Medium Priority label Oct 16, 2025
@mohamedawnallah mohamedawnallah self-assigned this Oct 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Medium Priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants