feat(config): support custom signet block time#10864
Conversation
Summary of ChangesHello, 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 the ability to configure a custom block interval for signet networks in lnd. By adding the Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe 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
Customization To customize the 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 Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. 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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new configuration option, bitcoin.signetblocktime, allowing users to override the expected block interval for custom signet networks. It includes validation logic, configuration updates, and corresponding unit tests. The reviewer suggested adding a nil check for the params pointer in applySigNetBlockTime to prevent potential nil pointer dereferences.
|
PR Severity: MEDIUM Automated classification | 3 files (excl. tests) | 51 lines changed Medium files (3):
Analysis: No bump conditions triggered: 3 non-test files changed (threshold: 20), 51 non-test lines changed (threshold: 500), no critical packages touched. To override: add a severity-override-{critical,high,medium,low} label. |
721aaa3 to
b7d4e69
Compare
|
LGTM |
| params.TargetTimespan) | ||
| } | ||
|
|
||
| params.TargetTimePerBlock = blockTime |
There was a problem hiding this comment.
There are other fields of type time.Duration in chaincfg.Params
// TargetTimespan is the desired amount of time that should elapse
// before the block difficulty requirement is examined to determine how
// it should be changed in order to maintain the desired block
// generation rate.
TargetTimespan time.DurationTargetTimespan should be changed so that difficulty is re-targeted every 2016 blocks.
Also I see MinDiffReductionTime, but it seems to be disabled in signet.
There was a problem hiding this comment.
Agreed, TargetTimespan is another consensus-critical option we could expose. For this PR, I think lnd should mirror the Mutinynet fork directly and only override TargetTimePerBlock:
Supporting custom TargetTimespan values seems like a separate option that we should add when there is a live custom signet that needs it.
But LND fails in So LND options Can you change LND so it fails with more user-friendly message if someone tries to pass Also could you adjust the docs to write it down there, please?
|
c0d9590 to
3266a74
Compare
starius
left a comment
There was a problem hiding this comment.
LGTM pending the comments 🚀
The code is good and it was tested manually to work with both neutrino and MutinyNet's bitcoind.
|
sweet, I think I covered those last comments, but please let me know if there is anything else |
Change Description
Adds support for configuring the expected block interval for custom signet
networks via
bitcoin.signetblocktime.This allows
lndto validate headers correctly when connected to a customsignet whose backing
bitcoindnodes were started with-signetblocktime.The configured value is applied to the custom signet chain params and is
validated to be at least one second and no greater than the target timespan.
This simplifies running
lndnodes on custom signet networks such asMutinynet.
Steps to Test
Run the new unit test:
Optionally test manually with a Mutinynet Bitcoin Core backend configured
with 30-second custom signet blocks and compact filter support:
Start
lndwith matching Mutinynet signet settings and neutrino pointed atthat Bitcoin Core node:
If testing through a local port-forward, forward the Bitcoin Core P2P port
to
127.0.0.1:29333. If public P2P is reachable, setneutrino.connectdirectly to that host instead.Verify that
lndstarts and syncs headers for that custom signet.Verify invalid values fail config validation:
This should fail with an
invalid signet block timeerror.Pull Request Checklist
Testing
this is a new feature.
Code Style and Documentation
insubstantial.
Typo fixes are not accepted to fight bot spam.
Code Documentation and Commenting
guidelines, and lines wrap at 80.
Ideal Git Commit Structure.
level. N/A, no logging statements added.
in the proto file. N/A, no lncli commands added.
or
[skip ci]in the commit message for small changes.