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

autorelay: define RelayFinder as an interface to support custom RelayFinder #2814

Closed
wants to merge 1 commit into from

Conversation

wlynxg
Copy link
Contributor

@wlynxg wlynxg commented May 28, 2024

In the previous autoRelay, relayFinder was defined as a structure wrapped in autoRelay

The logic of autoRelay itself is separated from relayFinder. Here we can abstract relayFinder into an interface for decoupling autoRelay and relayFinder.

At the same time, Options are added here for developers to transfer their own implementation of relayFinder.

@MarcoPolo
Copy link
Contributor

Thanks @wlynxg, can you give me some context on why you want this change? What's your use case, and if you can share code that would be great.

I'm hesitant about this because it would limit our ability to refactor this in the future. relayAddrs is very much an internal function and not a great interface point.

@wlynxg
Copy link
Contributor Author

wlynxg commented May 29, 2024

Thanks @wlynxg, can you give me some context on why you want this change? What's your use case, and if you can share code that would be great.

I'm hesitant about this because it would limit our ability to refactor this in the future. relayAddrs is very much an internal function and not a great interface point.

My program will use libp2p as the network layer, and relay is necessary for user experience. The relay connection will consider some complex business logic, and the relay connection management that libp2p has implemented cannot meet my needs. When I implement this logic, I don’t want to implement these logics directly inside libp2p.

Therefore, I hope to separate the logic of relay connection management here, so that users can implement their own connection management program and register it inside libp2p.

Regarding future expansion issues, isn't the current method of directly using the relayFinder structure more difficult to expand?

@MarcoPolo
Copy link
Contributor

My program will use libp2p as the network layer, and relay is necessary for user experience. The relay connection will consider some complex business logic, and the relay connection management that libp2p has implemented cannot meet my needs. When I implement this logic, I don’t want to implement these logics directly inside libp2p.

That sounds reasonable, but I don't think this change will help you a lot. You should just make your own version of AutoRelay. AutoRelay itself is just something that wraps a host and starts the relay finder. There's not that much reuse to gain by exposing the internals of AutoRelay as a configuration. It would make maintenance harder for the benefit of saving you maybe 100LOC? Please try making your own AutoRelay first. If you run into issues doing that I'd be keen on solving those :)

Regarding future expansion issues, isn't the current method of directly using the relayFinder structure more difficult to expand?

No. We can do whatever changes we want to the relay finder right now and consumers wouldn't see a difference. If we provided this interface we'd be limited to only changes that keep that interface. Very simple example let's say I want to rename .relayAddrs to .myPrivAddrsWithRelays we wouldn't be able to without breaking users.

@wlynxg
Copy link
Contributor Author

wlynxg commented May 30, 2024

My program will use libp2p as the network layer, and relay is necessary for user experience. The relay connection will consider some complex business logic, and the relay connection management that libp2p has implemented cannot meet my needs. When I implement this logic, I don’t want to implement these logics directly inside libp2p.

That sounds reasonable, but I don't think this change will help you a lot. You should just make your own version of AutoRelay. AutoRelay itself is just something that wraps a host and starts the relay finder. There's not that much reuse to gain by exposing the internals of AutoRelay as a configuration. It would make maintenance harder for the benefit of saving you maybe 100LOC? Please try making your own AutoRelay first. If you run into issues doing that I'd be keen on solving those :)

Regarding future expansion issues, isn't the current method of directly using the relayFinder structure more difficult to expand?

No. We can do whatever changes we want to the relay finder right now and consumers wouldn't see a difference. If we provided this interface we'd be limited to only changes that keep that interface. Very simple example let's say I want to rename .relayAddrs to .myPrivAddrsWithRelays we wouldn't be able to without breaking users.

Thank you for your patience. I have implemented my own relayFinder. I just found that this place seems to be an optional configuration for libp2p during the implementation process, so I opened this PR.

@MarcoPolo
Copy link
Contributor

Closing this PR. Report back if you have trouble making your own AutoRelay, and if there are changes in go-libp2p that would unblock you.

I don't want to add this configuration to AutoRelay since it'll make future maintenance harder.

@MarcoPolo MarcoPolo closed this Jun 3, 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.

None yet

2 participants