Skip to content

Conversation

@agaffney
Copy link
Contributor

@agaffney agaffney commented Dec 9, 2025

Fixes #467


Summary by cubic

Correct the formatting of generated SYNTH4 and SYNTH6 record names so they use base32-hex without padding and are lowercase. This aligns with the Handshake spec and fixes lookup failures noted in #467.

Written for commit 591beb4. Summary will update automatically on new commits.

Summary by CodeRabbit

  • Refactor
    • NS names generated for synthesized IPv4/IPv6 domain records now use a lowercase, no‑padding base32-hex encoding, changing the canonical form of those names.
    • No changes to record synthesis logic or public interfaces.

✏️ Tip: You can customize this high-level summary in your review settings.

@agaffney agaffney requested a review from a team as a code owner December 9, 2025 21:38
@coderabbitai
Copy link

coderabbitai bot commented Dec 9, 2025

📝 Walkthrough

Walkthrough

The change updates synthesized NS name generation in internal/indexer/handshake.go for both Synth4DomainRecord and Synth6DomainRecord cases: base32 hex encoding is now performed without padding and the encoded string is converted to lowercase. The strings package import was added. All other synthesis and record construction logic is unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Single file, small localized change
  • Verify correct use of base32 hex encoding without padding and lowercase conversion
  • Check both Synth4 and Synth6 branches produce expected names (no padding, lowercase)
  • Confirm new import is used and builds cleanly

Pre-merge checks and finishing touches

✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: fixing the formatting of SYNTH records to use base32-hex without padding and lowercase encoding.
Linked Issues check ✅ Passed The PR successfully implements all coding requirements from issue #467: stripping base32 padding and converting encoded names to lowercase for SYNTH4 and SYNTH6 records.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing SYNTH record formatting as specified in issue #467; no unrelated modifications are present.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/handshake-synth-record

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 1 file

wolf31o2
wolf31o2 previously approved these changes Dec 9, 2025
@agaffney agaffney force-pushed the fix/handshake-synth-record branch from 5f17e4b to 591beb4 Compare December 9, 2025 22:11
@agaffney agaffney merged commit a4f8765 into main Dec 9, 2025
11 checks passed
@agaffney agaffney deleted the fix/handshake-synth-record branch December 9, 2025 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Formatting of SYNTH records is incorrect

3 participants