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.

Sign up for GitHub

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: Addresses.AppendAnnounce #8177

Merged
merged 7 commits into from
Nov 30, 2021
Merged

feat: Addresses.AppendAnnounce #8177

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3aafda4
feat: append annouces
Jorropo Jul 19, 2021
aec3c20
refactor: go-ipfs-config
lidel Nov 15, 2021
8d6a5e5
docs: Addresses.AppendAnnounce
lidel Nov 15, 2021
aacf2fc
test: Addresses.AppendAnnounce
lidel Nov 15, 2021
23c028b
chore: go-ipfs-config v0.18.0
lidel Nov 23, 2021
81aeec2
Merge branch 'master' into feat/7791
lidel Nov 30, 2021
9673571
fix: deduplicate Swarm.Announce and AppendAnnounce
lidel Nov 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/node/groups.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ func LibP2P(bcfg *BuildCfg, cfg *config.Config) fx.Option {
BaseLibP2P,

fx.Provide(libp2p.AddrFilters(cfg.Swarm.AddrFilters)),
fx.Provide(libp2p.AddrsFactory(cfg.Addresses.Announce, cfg.Addresses.NoAnnounce)),
fx.Provide(libp2p.AddrsFactory(cfg.Addresses.Announce, cfg.Addresses.AppendAnnounce, cfg.Addresses.NoAnnounce)),
fx.Provide(libp2p.SmuxTransport(cfg.Swarm.Transports)),
fx.Provide(libp2p.RelayTransport(enableRelayTransport)),
fx.Provide(libp2p.RelayService(cfg.Swarm.RelayService.Enabled.WithDefault(true), cfg.Swarm.RelayService)),
Expand Down
24 changes: 17 additions & 7 deletions core/node/libp2p/addrs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,23 @@ func AddrFilters(filters []string) func() (*ma.Filters, Libp2pOpts, error) {
}
}

func makeAddrsFactory(announce []string, noAnnounce []string) (p2pbhost.AddrsFactory, error) {
var annAddrs []ma.Multiaddr
for _, addr := range announce {
maddr, err := ma.NewMultiaddr(addr)
func makeAddrsFactory(announce []string, appendAnnouce []string, noAnnounce []string) (p2pbhost.AddrsFactory, error) {
var err error // To assign to the slice in the for loop

annAddrs := make([]ma.Multiaddr, len(announce))
for i, addr := range announce {
annAddrs[i], err = ma.NewMultiaddr(addr)
if err != nil {
return nil, err
}
}

appendAnnAddrs := make([]ma.Multiaddr, len(appendAnnouce))
for i, addr := range appendAnnouce {
appendAnnAddrs[i], err = ma.NewMultiaddr(addr)
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the intended behavior if both announce and appendannounce are set? Do we just need to quietly ensure that we don't duplicate addresses, but that otherwise it's fine to set both fields?

Copy link
Member

@lidel lidel Nov 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I believe deduplicating should be enough (if libp2p does not do that already?)

My mental model is:

  • Announce is "ignore infered ones, use these"
  • AppendAnnounce is "add these to the final list, no matter if static or inferred"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the same thing as @lidel

Copy link
Member

@lidel lidel Nov 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Added deduplication in 9673571

if err != nil {
return nil, err
}
annAddrs = append(annAddrs, maddr)
}

filters := ma.NewFilters()
Expand All @@ -57,6 +66,7 @@ func makeAddrsFactory(announce []string, noAnnounce []string) (p2pbhost.AddrsFac
} else {
addrs = allAddrs
}
addrs = append(addrs, appendAnnAddrs...)

var out []ma.Multiaddr
for _, maddr := range addrs {
Expand All @@ -71,9 +81,9 @@ func makeAddrsFactory(announce []string, noAnnounce []string) (p2pbhost.AddrsFac
}, nil
}

func AddrsFactory(announce []string, noAnnounce []string) func() (opts Libp2pOpts, err error) {
func AddrsFactory(announce []string, appendAnnouce []string, noAnnounce []string) func() (opts Libp2pOpts, err error) {
return func() (opts Libp2pOpts, err error) {
addrsFactory, err := makeAddrsFactory(announce, noAnnounce)
addrsFactory, err := makeAddrsFactory(announce, appendAnnouce, noAnnounce)
if err != nil {
return opts, err
}
Expand Down
12 changes: 12 additions & 0 deletions docs/config.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ config file at runtime.
- [`Addresses.Gateway`](#addressesgateway)
- [`Addresses.Swarm`](#addressesswarm)
- [`Addresses.Announce`](#addressesannounce)
- [`Addresses.AppendAnnounce`](#addressesappendannounce)
- [`Addresses.NoAnnounce`](#addressesnoannounce)
- [`API`](#api)
- [`API.HTTPHeaders`](#apihttpheaders)
Expand Down Expand Up @@ -336,8 +337,19 @@ Default: `[]`

Type: `array[string]` (multiaddrs)

### `Addresses.AppendAnnounce`

Similar to [`Addresses.Announce`](#addressesannounce) except this doesn't
override inferred swarm addresses if non-empty.

Default: `[]`

Type: `array[string]` (multiaddrs)

### `Addresses.NoAnnounce`

An array of swarm addresses not to announce to the network.
Takes precedence over `Addresses.Announce` and `Addresses.AppendAnnounce`.

Default: `[]`

Expand Down
26 changes: 24 additions & 2 deletions test/sharness/t0140-swarm.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,18 +65,40 @@ test_expect_success 'Addresses.Announce affects addresses' '

test_kill_ipfs_daemon

announceCfg='["/dnsaddr/dynamic.example.com", "/ip4/10.20.30.40/tcp/4321"]'
test_expect_success "test_config_set AppendAnnounce succeeds" "
ipfs config --json Addresses.AppendAnnounce '$announceCfg'
"

test_launch_ipfs_daemon

test_expect_success 'Addresses.AppendAnnounce is applied on top of Announce' '
ipfs swarm addrs local >actual &&
grep "/ip4/1.2.3.4/tcp/1234" actual &&
grep "/dnsaddr/dynamic.example.com" actual &&
grep "/ip4/10.20.30.40/tcp/4321" actual &&
ipfs id -f"<addrs>" | xargs -n1 echo | tee actual &&
grep "/ip4/1.2.3.4/tcp/1234/p2p" actual &&
grep "/dnsaddr/dynamic.example.com/p2p/" actual &&
grep "/ip4/10.20.30.40/tcp/4321/p2p/" actual
'

test_kill_ipfs_daemon

noAnnounceCfg='["/ip4/1.2.3.4/tcp/1234"]'
test_expect_success "test_config_set succeeds" "
ipfs config --json Addresses.NoAnnounce '$noAnnounceCfg'
"

test_launch_ipfs_daemon

test_expect_success "Addresses.NoAnnounce affects addresses" '
test_expect_success "Addresses.NoAnnounce affects addresses from Announce and AppendAnnounce" '
ipfs swarm addrs local >actual &&
grep -v "/ip4/1.2.3.4/tcp/1234" actual &&
grep -v "/ip4/10.20.30.40/tcp/4321" actual &&
ipfs id -f"<addrs>" | xargs -n1 echo >actual &&
grep -v "/ip4/1.2.3.4/tcp/1234" actual
grep -v "/ip4/1.2.3.4/tcp/1234" actual &&
grep -v "//ip4/10.20.30.40/tcp/4321" actual
'

test_kill_ipfs_daemon
Expand Down