nat: use libp2p UserAgent for the UPnP comment string

[master255]

This is probably a minor bug.
When creating a host, there is a libp2p.UserAgent() field
It should name the port forwarded upnp with this name, not the name of the library (libp2p)

[marten-seemann]

This is where it comes from:

go-libp2p/p2p/net/nat/nat.go

Lines 221 to 227 in 305282b

	func (nat *NAT) establishMapping(protocol string, internalPort int) (externalPort int) {
	log.Debugf("Attempting port map: %s/%d", protocol, internalPort)
	const comment = "libp2p"
	nat.natmu.Lock()
	var err error
	externalPort, err = nat.nat.AddPortMapping(protocol, internalPort, comment, MappingDuration)

It's a comment / description, so it doesn't really matter what we put there. I agree though that it would be nice to use the user agent. Want to submit a patch?

[master255]

@marten-seemann Yes. Of course :-) I'll give it a try.

[master255]

@marten-seemann #2310

[master255]

@marten-seemann It's been over two and a half months now. You promised me it would be added. Keep your promise. The task is ready.

[master255]

@marten-seemann @MarcoPolo I'm starting to think you're spamming me on purpose to avoid adding my code.

[p-shahi]

@marten-seemann @MarcoPolo I'm starting to think you're spamming me on purpose to avoid adding my code.

Hi @master255 , the maintainers are not spamming you. I don't know why you make such a claim.
Looking at #2310 it seems like there's some back and forth between you and the maintainers. I agree with Marco's point about not changing the interface of the public DiscoverNAT method.
If you are having difficulty in designing the new function, then I suggest you join the maintainers call and we can discuss it a bit in person. Here's the community calendar

Apologies resolving this has taken a bit longer, the team has been dealing with other issues at higher priority (see recent patch release and fixes) - I can bring this up for a discussion the maintainer call so we can bring it to a close

[master255]

@p-shahi I think such issues are better handled in writing. As it can take up to an hour of time to respond. And the conference does not have that much time.

What is the purpose of this public method? I think sometimes it is necessary to change public methods. This is the case when it is necessary to change it.

[master255]

My last few responses there. Marco hasn't responded in days. Even very busy - I always find 5 minutes to reply. There's no way he doesn't have time. He probably just doesn't care.

[master255]

Finally, developers themselves can correct the code if they see a bug.

[p-shahi]

@master255 It is a matter of priority - as you have pointed out this is a minor issue. Please don't attribute it to not caring but understand that maintainers are tasked with doing a dozen things at once. I agree that our response times need to be better as a team - that's fair.
However, the back and forth between you and maintainers in the PR shows that we need to be more hands on and guide/mentor you through the change - unfortunately, that's not a time commitment that can be made all the time.
Again you're more than welcome to join the open maintainers call where we can discuss this face to face and resolve it more quickly.
I won't be responding back anymore other than to provide an update on the path forward to this bugfix (no later than 2023/08/24)

[ameya-deshmukh]

Hey @p-shahi can I pick this up if the team is busy?