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

Integrate new netlayer library #987

Closed
wants to merge 14 commits into from

Conversation

mrosseel
Copy link
Contributor

@mrosseel mrosseel commented Oct 30, 2017

This is the first part of the integration of the new netlayer lib. (see issue #971)
These are the 3 parts:

  • integrate netlayer, adapt TorNetworkNode
  • add bridges UI
  • clean up

Note: feel free to ignore this pull request, it's just open in case someone wants to test the new library.

Conflicts:
	network/pom.xml
	network/src/main/java/io/bisq/network/p2p/network/TorNetworkNode.java
@ManfredKarrer
Copy link
Contributor

Just tested but localhost mode does not work. The relevant code in LocalhostNetworkNode is commented out.
With the tor version Bitcoin nodes are not discovered.
There are also a few data which seems not set: "hiddenservicedir_changeme", streamID ("Foo) - I assume that should be a random string.
Please clean up also unused code, and make sure there are no complaints from code inspection (nullable problems,...)

@mrosseel
Copy link
Contributor Author

I clearly stated that cleanup was not yet done.
Do you have any idea why the seednodes do work and the bitcoin nodes not?

@ManfredKarrer
Copy link
Contributor

Ah, sorry did not read the comment above... No idea yet but can look into it.

Had to merge by hand because of the many changes in the new bisq
structure. Also removed the 'default bridges' concept because it's
discouraged to do that.
Open issues: UI screen is still WIP, and bitcoinj SOCKS proxy error
which makes that we can't connect to bitcoin nodes anymore.

Signed-off-by: Mike Rosseel <[email protected]>
Replaced by the new netlayer library

Signed-off-by: Mike Rosseel <[email protected]>
so far i understand your version is working bc another bug in the
walletsetup where the Socks5MultiDiscovery is not used. so u use default
dnsdiscover which leaks dns. that works only with SafeSocks 0.
if using Socks5MultiDiscovery as is it should i only get 4 nodes form dns seeds.

Signed-off-by: Mike Rosseel <[email protected]>
@ManfredKarrer
Copy link
Contributor

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.

2 participants