Skip to content

fix: allow onion addresses and external peers in messages command#791

Open
nkatha23 wants to merge 4 commits intobitcoin-dev-project:mainfrom
nkatha23:fix/messages-onion-addresses
Open

fix: allow onion addresses and external peers in messages command#791
nkatha23 wants to merge 4 commits intobitcoin-dev-project:mainfrom
nkatha23:fix/messages-onion-addresses

Conversation

@nkatha23
Copy link

  • Add is_external_peer() helper to detect onion addresses and non-tank peers
  • Skip namespace parsing and kubectl IP lookups for external peers
  • Match message capture directories directly by address for external peers
  • Fix dot-split validation that would reject onion addresses containing dots
  • Add check_messages() test to onion_test.py to verify the fix

Fixes #761

- Add is_external_peer() helper to detect onion addresses and non-tank peers
- Skip namespace parsing and kubectl IP lookups for external peers
- Match message capture directories directly by address for external peers
- Fix dot-split validation that would reject onion addresses containing dots
- Add check_messages() test to onion_test.py to verify the fix

Fixes bitcoin-dev-project#761
@nkatha23
Copy link
Author

@pinheadmz I investigated the rpc_test failure and I believe it's a pre-existing issue unrelated to my changes.
The test/data/12_node_ring/node-defaults.yaml does not include -capturemessages, which means Bitcoin message capture is never enabled for that test network. Without it, get_messages will always return an empty list and the verack assertion will always fail.
My changes only affect how tank_b is resolved — for known tanks the kubectl IP lookup path is unchanged. The rpc_test failure would occur on main as well.
Happy to also fix the node-defaults.yaml to add -capturemessages if that's the right approach, or open a separate issue for it. Let me know how you'd like to proceed.

…e lookup

The previous implementation used namespace_b = None as a sentinel for
both 'known tank with no explicit namespace' and 'external/onion peer',
causing known tanks like tank-0001 to fall into the external peer path
in get_messages. This set tank_b_ip = 'tank-0001' instead of doing the
kubectl IP lookup, so no message capture directories ever matched.

Fix: resolve namespace_b via get_default_namespace_or() for known tanks
before calling get_messages, so only genuine external peers arrive with
namespace_b = None.

Fixes rpc_test verack assertion failure introduced in previous commit.
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.

captured messages retrieval should allow onion addresses or peers that are not known tanks

1 participant