Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add AutoNAT behavior #632

Merged
merged 12 commits into from
May 17, 2024
Merged

feat: Add AutoNAT behavior #632

merged 12 commits into from
May 17, 2024

Conversation

bgins
Copy link
Contributor

@bgins bgins commented Apr 2, 2024

Description

This PR makes the following changes:

  • Add AutoNAT behavior to probe for public listen addresses
  • Update external_addresses with reported public addresses
  • Add StatusChangedAutonat notification
  • Add NatStatus extension trait with to_tuple helper
  • Confirm addresses observed on identify events with an AutoNAT probe
  • Minor refactoring in settings

When the NatStatus changes to public, the reported address is added to external_addresses. If a previously reported address transitions from public to private or unknown, the address is removed from external_addresses.

Note that announce_addresses are always kept in external_addresses and will not be removed by changes to NatStatus.

Link to issue

Implements #398

Type of change

  • New feature (non-breaking change that adds functionality)
  • Refactor (non-breaking change that updates existing functionality)
  • This change requires a documentation update

Test plan (required)

This PR adds the following tests:

  • AutoNAT confirms a reachable address
  • StatusAutoNat notification to and from IPLD bytes and JSON tests
  • Test NatStatus to_tuple returns expected tuples

Copy link

codecov bot commented Apr 2, 2024

Codecov Report

Attention: Patch coverage is 65.78947% with 65 lines in your changes are missing coverage. Please review.

Project coverage is 69.81%. Comparing base (2d0cfe9) to head (8e39d28).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #632      +/-   ##
==========================================
- Coverage   69.85%   69.81%   -0.05%     
==========================================
  Files          98      100       +2     
  Lines       13394    13577     +183     
==========================================
+ Hits         9357     9479     +122     
- Misses       4037     4098      +61     
Files Coverage Δ
homestar-runtime/src/event_handler/notification.rs 65.21% <ø> (ø)
homestar-runtime/src/libp2p/nat_status.rs 100.00% <100.00%> (ø)
homestar-runtime/src/network/swarm.rs 72.34% <100.00%> (+1.61%) ⬆️
homestar-runtime/src/settings.rs 98.45% <ø> (ø)
homestar-runtime/src/settings/libp2p_config.rs 100.00% <100.00%> (ø)
...-runtime/src/event_handler/notification/network.rs 95.41% <96.15%> (+0.03%) ⬆️
.../src/event_handler/notification/network/autonat.rs 93.87% <93.87%> (ø)
homestar-runtime/src/event_handler/swarm_event.rs 22.07% <8.95%> (-0.99%) ⬇️

... and 1 file with indirect coverage changes

@bgins bgins marked this pull request as ready for review April 3, 2024 18:03
@bgins bgins requested a review from a team as a code owner April 3, 2024 18:03
Copy link
Contributor

@zeeshanlakhani zeeshanlakhani left a comment

Choose a reason for hiding this comment

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

Damn good stuff @bgins.

autonat::Event::StatusChanged { old, new } => {
match &new {
NatStatus::Public(address) => {
event_handler.swarm.add_external_address(address.clone());
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏽

@bgins bgins merged commit dc4f7a4 into main May 17, 2024
33 checks passed
@bgins bgins deleted the bgins/autonat branch May 17, 2024 16:41
@release-plz-ipvm-wg release-plz-ipvm-wg bot mentioned this pull request May 17, 2024
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