Open aarshkshah1992 opened 1 month ago
@aarshkshah1992 in the specific case you raised can't you get around this by doing an interface check for GetAutoNat rather than a type check?
Maybe the underlying type should be exported, but it seems more likely that it's just a symptom of the more important need of getting access to some of the optional (if common) components of the Host.
@aschmahmann Yeah, you are right. We don't need this change for Lotus anymore. Please feel free to close the issue.
I'm definitely open to exporting this type, but I'd like to see a better use case. If other folks have use cases or things that broke, please let me know.
It's not as simple as just exporting the type and changing what this function returns because it can now return either a closableBasicHost
or a closableRoutedHost
.
And generally I want to move away from BasicHost actually being a KitchenSinkHost. I want to move towards services being more pluggable (https://github.com/libp2p/go-libp2p/issues/1993).
After chatting with @sukunrt for a bit, we think (if needed) we could flip the current style and have basic host call app.Close when it closes. That way we wouldn't need to export a new type and could keep exporting basic host.
Writing this for posterity, but it seems like it may not be needed.
libp2p.New
function was changed to replace the exportedBasicHost
type with an un-exportedclosableBasicHost
type that wraps aBasicHost
https://github.com/libp2p/go-libp2p/blob/04b2096cb952142073df125a47087fb91eb6b8de/config/config.go#L438
BasicHost
implementation from a libp2p Host and that breaks now.Please can we export the
closableBasicHost
type ?For reference, the Lotus code that breaks is at https://github.com/filecoin-project/lotus/blob/967524aa83852123206715d0f54fd552c7546d5b/node/impl/net/net.go#L138.