Skip to content

fix: default CLI to IPv6 dual-stack, add --ipv4-only flag#43

Merged
grumbach merged 3 commits intomainfrom
fix/cli-ipv6
Apr 22, 2026
Merged

fix: default CLI to IPv6 dual-stack, add --ipv4-only flag#43
grumbach merged 3 commits intomainfrom
fix/cli-ipv6

Conversation

@grumbach
Copy link
Copy Markdown
Contributor

@grumbach grumbach commented Apr 17, 2026

Summary

  • ant-cli was building CoreNodeConfig with ipv6(false), so the client was IPv4-only regardless of host capability — inconsistent with ant-core (ipv6(true)) and ant-node (dual-stack by default).
  • Default ant-cli to dual-stack and expose --ipv4-only for hosts without working IPv6, mirroring ant-node's flag of the same name.
  • Refresh the stale doc comment in ant-core/src/data/network.rs that claimed ant-cli built the same config.
  • Extend the IPv6 knob to the library: ant-core::Network::new now takes an ipv6: bool and ClientConfig has an ipv6 field (default true), so embedders of ant-core can pick IPv4-only without touching CoreNodeConfig directly.

Behavior change (please read before upgrading)

This PR changes the CLI's default network binding from IPv4-only to IPv6 dual-stack. On hosts with a working IPv6 stack this is a straight improvement. On hosts with no IPv6 interface, or IPv6 present but broken, the client will advertise unreachable v6 addresses to the DHT, which causes slow connects and junk address records. The mitigation is the new --ipv4-only flag — pass it and the client behaves exactly as it did before this PR. The same knob is available to library callers as ClientConfig { ipv6: false, .. }.

Test plan

  • cargo fmt --all
  • cargo clippy --all-targets --all-features -- -D warnings (clean)
  • ant --help shows --ipv4-only with dual-stack as the default
  • Smoke test against a dual-stack testnet / IPv4-only host — cargo test -p ant-core --test e2e_chunk test_chunk_put_get_round_trip passes on macOS against a local loopback testnet (IPv4-only path, exercised via MiniTestnet). Unit test suite (cargo test -p ant-core --lib) also passes, 135/135.

ant-cli built its CoreNodeConfig with ipv6(false), making the client
IPv4-only even though ant-core and ant-node default to dual-stack.
Align ant-cli with the rest of the stack and expose --ipv4-only for
hosts without working IPv6, matching the ant-node flag of the same name.
@Nic-dorman
Copy link
Copy Markdown
Contributor

Small, well-scoped fix. Flips CLI default from IPv4-only to dual-stack (matching ant-core and ant-node), adds --ipv4-only escape hatch, updates the stale doc comment. CI is all green.

What I'd want before merging

  1. Behavior change — worth calling out in release notes. Users on IPv4-only hosts (no v6 interface, or v6 present but broken) will now advertise v6 addresses to the DHT and may see slower connects or junk address records. The --ipv4-only flag is the out, but it's opt-in — someone hitting the regression has to know it exists. A line in CHANGELOG or a paragraph in the PR description pointing at the flag would save someone a debugging session.

  2. Unchecked test plan item. The PR checklist still has [ ] Smoke test against a dual-stack testnet / IPv4-only host unchecked. That's the only end-to-end validation of the change's reason to exist. Worth doing at least one of the two paths before merging.

P2 — polish

  • ant-core::Network::new still hardcodes .ipv6(true) (ant-core/src/data/network.rs:37). Anyone embedding ant-core directly has no way to run IPv4-only. Not required for this PR, but if you want true parity between CLI and library, a follow-up could take the v6 flag as an arg here too.
  • tests/support/mod.rs:205 still has .ipv6(false). Fine for loopback tests, but worth a comment noting why (localhost only) so it doesn't look like drift from the new default.
  • Doc wording nit in network.rs: "builds an equivalent CoreNodeConfig" is only equivalent when --ipv4-only is not passed. Maybe "builds a similar CoreNodeConfig, with ipv6 toggled by --ipv4-only".
  • Telescoping argscreate_client_node(bootstrap, allow_loopback, ipv4_only) is at 3 bool/bool-ish params. Not worth fixing for this PR, but the next flag added here should probably be the point where these collapse into a small options struct.

Low risk otherwise. LGTM after (1) + (2).

- CHANGELOG: note the IPv4-only to IPv6 dual-stack default change and
  the --ipv4-only mitigation for hosts without a working IPv6 stack.
- ant-core::Network::new now takes an `ipv6: bool` arg so library callers
  (ant-gui, future embedders) have the same knob as the CLI. Defaults to
  `true` via ClientConfig::default(), matching the CLI's new default.
- ClientConfig grows an `ipv6` field; Client::connect threads it through
  to Network::new.
- tests/support/mod.rs: add a comment explaining the intentional
  .ipv6(false) for loopback-only testnets.
- network.rs: tighten the doc comment so it no longer claims the direct
  CoreNodeConfig built in ant-cli is "equivalent" to Network::new when
  --ipv4-only flips the ipv6 flag.

Signed-off-by: grumbach <anselmega@gmail.com>
@grumbach
Copy link
Copy Markdown
Contributor Author

Thanks @Nic-dorman. Addressed the review in b12095d:

  • CHANGELOG — added an [Unreleased] section noting the IPv4-only to IPv6 dual-stack default change and pointing at --ipv4-only as the escape hatch.
  • ant-core::Network::new parity — now takes an ipv6: bool; ClientConfig grows an ipv6 field (default true). Client::connect threads it through. Library embedders (ant-gui, future consumers) no longer need to go around ant-core to pick IPv4-only.
  • tests/support/mod.rs:205 — added a one-paragraph comment on the .ipv6(false) line explaining it's intentional for the loopback-only MiniTestnet.
  • Doc wording in network.rs — changed "builds an equivalent CoreNodeConfig" to "builds a similar CoreNodeConfig directly, with ipv6 toggled by the --ipv4-only flag" so the doc stays accurate when --ipv4-only is passed.
  • PR description — checked the smoke test box and added a "Behavior change" paragraph pointing at --ipv4-only for folks on IPv4-only hosts, per your release-notes suggestion.
  • Smoke test — ran cargo test -p ant-core --test e2e_chunk test_chunk_put_get_round_trip (passes) and the full unit suite cargo test -p ant-core --lib (135/135). The e2e test uses a loopback testnet (MiniTestnet at 127.0.0.1) so it exercises the IPv4-only code path via the explicit .ipv6(false) in tests/support/mod.rs. The production dual-stack path is identical apart from the ipv6 bool, which flows through CoreNodeConfig::builder().ipv6(..) in both callers.

Skipped the create_client_node(bootstrap, allow_loopback, ipv4_only) opts-struct refactor as you suggested. The next flag added there should be the trigger to collapse them.

Copy link
Copy Markdown
Contributor

@Nic-dorman Nic-dorman left a comment

Choose a reason for hiding this comment

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

macOS CI failure is due to failing to get enough quotes, not related to the PR

@grumbach grumbach merged commit b8adf42 into main Apr 22, 2026
22 of 24 checks passed
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.

2 participants