discovery: fix panic in DNS fallback SRV lookup

[erickcestari]

The fallback SRV lookup type-asserted each DNS Answer record to *dns.SRV
unconditionally. If the response contains a non-SRV record (e.g. an A or
CNAME), the type assertion panics and crashes the daemon. Use the
comma-ok form to skip non-SRV records instead.

Also guard against an empty LookupHost result for the shim, which would
otherwise panic on an out-of-bounds index into addrs.

This is safe to discuss and fix in public. The bug is very unlikely to be
exploitable: triggering it requires either a DNS seeder to serve a
malformed response, or an on-path MITM injecting one (the fallback
response is unauthenticated). A malicious seeder already has far more
direct ways to disrupt a node, and a MITM attack is hard to mount, so the
panic does not meaningfully widen the attack surface.

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! 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 addresses potential stability issues in the DNS fallback SRV lookup mechanism. By replacing unconditional type assertions with safe checks and adding validation for empty lookup results, the changes prevent the daemon from panicking when receiving malformed or unexpected DNS responses.

Highlights

• Panic Prevention: Updated the DNS fallback SRV lookup to use comma-ok type assertions, preventing crashes when encountering non-SRV records in DNS responses.
• Bounds Checking: Added a check for empty results in the DNS shim LookupHost to prevent out-of-bounds index panics.
• Testing: Introduced a new test stub and test cases to verify that non-SRV records are filtered correctly and that empty shim results are handled gracefully.

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 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 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 .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 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

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. ↩

[gemini-code-assist]

Code Review

This pull request fixes potential panics in the DNS fallback SRV lookup by handling empty address results and safely skipping non-SRV records using a comma-ok type assertion. It also adds corresponding unit tests and updates the release notes. The reviewer recommended improving the unit tests by verifying that the type assertions on the bootstrapper interface succeed using require.True instead of ignoring the boolean result, which could lead to nil pointer dereferences.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[github-actions]

PR Severity: HIGH (severity-high)

Automated classification | 2 files (excl. tests) | 21 lines changed (excl. tests)

High (1 file):

• discovery/bootstrapper.go - Gossip protocol / peer discovery bootstrapping logic

Low (1 file):

• docs/release-notes/release-notes-0.22.0.md - Release notes documentation

Analysis

The primary change is in discovery/bootstrapper.go, which belongs to the discovery/* package responsible for the gossip protocol and peer bootstrapping. This falls into the HIGH severity tier and requires review from a knowledgeable engineer familiar with peer discovery and gossip mechanics.

The test file (discovery/bootstrapper_test.go) adds 128 lines of new test coverage but is excluded from severity classification per policy. The release notes entry is documentation-only (LOW).

No severity bump was triggered: only 2 non-test files changed (threshold: >20) and 21 non-test lines changed (threshold: >500).

---

To override, add a severity-override-{critical,high,medium,low} label.
<!-- pr-severity-bot -->

[gijswijs]

Only two things I could find, one pre-existing. I made inline-comments that you can address individually.

One other pre-existing thing that caught my eye is the return nil, err inside SampleNodeAddrs. One attacker-supplied malformed target would kill the whole round. It happens at 4 sites:

• bootstrapper.go:514: LookupHost(bechNodeHost) failure
• bootstrapper.go:539: bech32.Decode failure
• bootstrapper.go:550: bech32.ConvertBits failure
• bootstrapper.go:554: btcec.ParsePubKey failure

This fix could be applied at each of those four sites, e.g. line 537-540:

_, nodeBytes5Bits, err := bech32.Decode(bechNode[0])
if err != nil {
        log.Tracef("Skipping node %v: %v", bechNodeHost, err)
        continue
}

But you could also make the case that failing at those point is the right call, especially at LookupHost. So it's more of a judgement call left to the creator.

I'm confident @erickcestari will fix the issues, so I'm approving upfront.

[yyforyongyu]

Looking good, just a few nits

[erickcestari]

Thanks for the review @yyforyongyu @gijswijs . Your comments should be addressed!

[yyforyongyu]

LGTM🛡️

[github-actions]

Created backport PR for v0.21.x-branch:

• [v0.21.x-branch] Backport #10914: discovery: fix panic in DNS fallback SRV lookup #10922 with remaining conflicts!

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-10914-to-v0.21.x-branch
git worktree add --checkout .worktree/backport-10914-to-v0.21.x-branch backport-10914-to-v0.21.x-branch
cd .worktree/backport-10914-to-v0.21.x-branch
git reset --hard HEAD^
git cherry-pick -x 2ee49698afa74373fa51acc61edd5620cb623f61
git push --force-with-lease