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

Iroh Connection cannot adjust TransportConfig for outgoing connections #2872

Open
CGamesPlay opened this issue Oct 31, 2024 · 1 comment
Open
Labels
bug Something isn't working c-iroh-net

Comments

@CGamesPlay
Copy link

CGamesPlay commented Oct 31, 2024

I was advised to create an issue from #2860 (reply in thread). Iroh-net's Endpoint::connect is hard-coded to use a TransportConfig of every-second keepalives with 30-second timeouts (latter default from Quinn), regardless of any TransportConfig that was passed to Builder::transport_config (which is only used for incoming connections). This effectively means that you can only increase the keepalive interval and reduce the timeout from these defaults.

I can't think of a great reason that the incoming and outgoing TransportConfig values should ever be different, so I think the default should be to use the Endpoint's configured values for outgoing connections. However, if there is a use case for this (perhaps different ALPNs should have different timeouts), a method like connect_with(node_addr, alpn, transport_config) could work well.

@flub flub added bug Something isn't working c-iroh-net labels Oct 31, 2024
@flub
Copy link
Contributor

flub commented Nov 5, 2024

My proposal would be to:

  • Add an Endpoint::connect_with_transport_config(&self, node_addr: impl Into<NodeAddr>, alpn: &[u8], transport_config: TransportConfig) (modulo some bikeshedding).
  • Modify connect_quinn to take the TransportConfig as well, so that the above can work. But in the first instance keep the current default TransportConfig but moved into connect, things work now for many folks and we need to be careful changing those values. I'd like to give folks a chance to try things out first before changing the defaults.
  • If Builder::transport_config is used use that as the default in connect as well as for accepting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working c-iroh-net
Projects
Status: No status
Development

No branches or pull requests

2 participants