network flag to support testnet4 #1120
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @bitcoin-coder-bob, 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 introduces comprehensive support for testnet4 as a network option. It enables the application's UI and components to operate seamlessly on testnet4 by allowing it to be specified via the network flag and ensuring the underlying LND Bitcoin configuration is correctly applied for this network.
Highlights
- Network Configuration Expansion: I've expanded the supported options for the
networkcommand-line flag to includetestnet4. This allows users to specifytestnet4directly when launching the application. - LND Testnet4 Support: I've updated the
setNetworkfunction to correctly configure the LND Bitcoin settings fortestnet4. Whentestnet4is selected,cfg.Lnd.Bitcoin.TestNet4will now be set totrue.
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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| 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 issue 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 or fill out our survey to provide feedback.
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
-
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. ↩
bitcoin-coder-bob
force-pushed
the
testnet4-network-option
branch
from
b7a79ec to
4d506da
Compare
There was a problem hiding this comment.
Thanks a for the contribution @bitcoin-coder-bob 🙏!
There are some small additional changes that you'd would be need to add to this PR prior to us being able to merge it:
-
We would need to add a release note entry for this change, similar to how one was added for the
signetflag indocs/release-notes/release-notes-0.14.0.md. However, since there haven't been anydocs/release-notes/release-notes-0.15.2.mdfile added to themasterbranch yet, you would need to add that file and add an entry for this change.
If you are unsure of how to add that file, I can create a PR that just adds the file and merge it prior to merging this, whereby you'd rebase this PR on that merged change and just add an entry for this PR to that file. Let me know if you'd prefer that, or if you feel comfortable adding the full file to this PR! -
In
itest/litd_node.goon line 292, you'd need to add a case fortestnet4.
bitcoin-coder-bob
force-pushed
the
testnet4-network-option
branch
from
4d506da to
5047e2d
Compare
Please let me know if I added the new release notes file correctly @ViktorTigerstrom |
| * Add support for connecting LiT to [testnet4](https://github.com/lightninglabs/lightning-terminal/pull/1120). | ||
| This can be done using the `--network=testnet4` config option. |
There was a problem hiding this comment.
nit:
| * Add support for connecting LiT to [testnet4](https://github.com/lightninglabs/lightning-terminal/pull/1120). | |
| This can be done using the `--network=testnet4` config option. | |
| * Add support for connecting LiT to the [testnet4 | |
| network](https://github.com/lightninglabs/lightning-terminal/pull/1120). This | |
| can be done using the `--network=testnet4` config option. |
The reasoning behind this suggested change is that we try to keep lines to 80 characters if possible, even in the release-notes.
bitcoin-coder-bob
force-pushed
the
testnet4-network-option
branch
2 times, most recently
from
5bfa75d to
0eb9294
Compare
There was a problem hiding this comment.
Nice, thanks for the change @bitcoin-coder-bob!
Appologies though, I noticed that my previous suggestion lead to that the docs in the config.go file now exceeds 80chars, which we have maintained in that file so far.
Would you have the possibility to just fix that and I'll proceed to merge this 🚀?
Feel no need to re-request a review after that, just push the change and I'll proceed to merge as soon as CI passes :).
config.go
Outdated
| // `lnd.bitcoin.mainnet|testnet|testnet4|regtest|signet` and also for the other | ||
| // daemons. That way only one global network flag is needed. |
There was a problem hiding this comment.
nit:
| // `lnd.bitcoin.mainnet|testnet|testnet4|regtest|signet` and also for the other | |
| // daemons. That way only one global network flag is needed. | |
| // `lnd.bitcoin.mainnet|testnet|testnet4|regtest|signet` and also for | |
| // the other daemons. That way only one global network flag is needed. |
bitcoin-coder-bob
force-pushed
the
testnet4-network-option
branch
from
0eb9294 to
2685477
Compare
testnet4was not a supported option for thenetworkflag. This PR addstestnet4as an option and supports its usage insetNetwork.I ran this and the node now connects to testnet4.
thank you @jharveyb who discovered this during the PlebFi hackathon