ssbc / ssb-config

standard configuration for ssb
MIT License
23 stars 18 forks source link

Create default incoming based on opts.host and opts.port #21

Closed regular closed 5 years ago

regular commented 5 years ago

Fixes a bug that ignores host and port given in the config file. Variable "overrides" only includes command-line overrides, not the contents of the config file. Therfore the default incoming connection alsways uses the default port, not the port given in the config file.

(This also removes dupicate key "party")

regular commented 5 years ago

@christianbundy yes, it does. What is hardcoded is the default port, the cli overrides are merge in. This is no different than with all the other settings. The code I removed was a failing attempt to make a default incoming connection that would respect user preferences for host and port. Instead it did only respect command line overrides and did ignore the user's config file.

It now creates the default incoming connection after RC has combined defaults, config file settings and cli overrides, so all of them are respected when creating the incoming connection.

This fixes a bug where {port: xxx, host: xxx} in .ssb/config simply is ignored, which breaks client applications that don't use the default port.

There is now an integration-level test that will hopefully catch this in the future: https://github.com/regular/ssb-integration-tests

regular commented 5 years ago

It would be good if we can move fast on this, and also to get this into scuttlebot-release, because until we do, sbot is broken, and sbot is critical infrastructure. cc @dominictarr @arj03 @cryptix

dominictarr commented 5 years ago

@regular awesome! merged into 2.3.5

dominictarr commented 5 years ago

will be in scuttlebot-release@13.0.0

regular commented 5 years ago

@dominictarr Great, thanks!

regular commented 5 years ago

@dominictarr in sbot@13 ws.getAddress does not exist anymore?

dominictarr commented 5 years ago

@regular if you do sbot.getAddress it will give you an multiaddress containing a ws address. Ideally, you should just use that straight with ssb-client in the browser and it will just ignore the parts it doesn't understand. (there may be some bugs to fix about that though)

regular commented 5 years ago

@dominictarr we could easily keep ws.getAddress around to not break the client API though.

See also

regular commented 5 years ago

@dominictarr in scuttlebot@13 getAddress returns one address only, a net address and ssb-client in the browser even tries to use it, resulting in net.connect is not a function. Now every browser client relying on the stock sbot binary is completely broken. :(