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

fix(comms): only set final forward address if configured to port 0 #5406

Merged

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented May 24, 2023

Description

Sets the onion service forward address to the correct port only if configured to port 0.

Motivation and Context

Fixes #5405

How Has This Been Tested?

Manually, default settings were also tested.

What process can a PR reviewer use to test or verify this change?

# This doesnt work (no incoming connections as expected)
#tor.forward_address = "/dns4/localhost/tcp/12345"
# This does work
tor.forward_address = "/ip4/10.71.1.141/tcp/12345"
tor.listener_address_override = "/ip4/10.71.1.141/tcp/12345"

Breaking Changes

  • None
  • Requires data directory on base node to be deleted
  • Requires hard fork
  • Other - Please specify

@github-actions
Copy link

github-actions bot commented May 24, 2023

Test Results (CI)

1 147 tests  ±0   1 147 ✔️ ±0   10m 3s ⏱️ +3s
     37 suites ±0          0 💤 ±0 
       1 files   ±0          0 ±0 

Results for commit 94f3e6e. ± Comparison against base commit c704890.

♻️ This comment has been updated with latest results.

@ghpbot-tari-project ghpbot-tari-project added P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged labels May 24, 2023
@github-actions
Copy link

github-actions bot commented May 24, 2023

Test Results (Integration tests)

  2 files  +  2  12 suites  +12   25m 14s ⏱️ + 25m 14s
29 tests +29  28 ✔️ +28  0 💤 ±0  1 +1 
30 runs  +30  29 ✔️ +29  0 💤 ±0  1 +1 

For more details on these failures, see this check.

Results for commit 94f3e6e. ± Comparison against base commit c704890.

♻️ This comment has been updated with latest results.

@ghpbot-tari-project ghpbot-tari-project removed the P-reviews_required Process - Requires a review from a lead maintainer to be merged label May 24, 2023
@stringhandler stringhandler enabled auto-merge May 24, 2023 12:27
Copy link
Collaborator

@SWvheerden SWvheerden left a comment

Choose a reason for hiding this comment

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

Ack

@SWvheerden SWvheerden disabled auto-merge May 25, 2023 08:17
@SWvheerden SWvheerden merged commit ff7fb6d into tari-project:development May 25, 2023
@sdbondi sdbondi deleted the comms-custom-forward-address branch May 25, 2023 08:18
SWvheerden added a commit that referenced this pull request May 25, 2023
…5406)

Description
---
Sets the onion service forward address to the correct port only if
configured to port 0.

Motivation and Context
---
Fixes #5405 

How Has This Been Tested?
---
Manually, default settings were also tested.

What process can a PR reviewer use to test or verify this change?
---
```toml
# This doesnt work (no incoming connections as expected)
#tor.forward_address = "/dns4/localhost/tcp/12345"
# This does work
tor.forward_address = "/ip4/10.71.1.141/tcp/12345"
tor.listener_address_override = "/ip4/10.71.1.141/tcp/12345"
```

Breaking Changes
---

- [x] None
- [ ] Requires data directory on base node to be deleted
- [ ] Requires hard fork
- [ ] Other - Please specify

<!-- Does this include a breaking change? If so, include this line as a
footer -->
<!-- BREAKING CHANGE: Description what the user should do, e.g. delete a
database, resync the chain -->

Co-authored-by: SW van Heerden <swvheerden@gmail.com>
SWvheerden added a commit that referenced this pull request May 29, 2023
##
[0.50.0-pre.2](v0.50.0-pre.1...v0.50.0-pre.2)
(2023-05-29)

### ⚠ BREAKING CHANGES

* add optional range proof types (5372)

### Features

* add metadata signature check
([5411](#5411))
([9c2bf41](9c2bf41))
* add optional range proof types
([5372](#5372))
([f24784f](f24784f))
* added burn feature to the console wallet
([5322](#5322))
([45685b9](45685b9))
* improved base node monitoring
([5390](#5390))
([c704890](c704890))


### Bug Fixes

* **comms:** only set final forward address if configured to port 0
([5406](#5406))
([ff7fb6d](ff7fb6d))
* deeplink to rfc spec
([5342](#5342))
([806d3b8](806d3b8))
* don't use in memory datastores for chat client dht in integration
tests ([#5399](#5399))
([cbdca6f](cbdca6f))
* fix panic when no public addresses
([5367](#5367))
([49be2a2](49be2a2))
* loop on mismatched passphrase entry
([5396](#5396))
([ed120b2](ed120b2))
* use domain separation for wallet message signing
([5400](#5400))
([7d71f8b](7d71f8b))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-acks_required Process - Requires more ACKs or utACKs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[comms(p2p)]: remote tor incoming connection not working
4 participants