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

WIP Updated netlayer, jtorctl and tor-binary #2499

Merged
merged 2 commits into from
Mar 6, 2019

Conversation

freimair
Copy link
Contributor

@freimair freimair commented Mar 5, 2019

Changes

Before merging

We need a tester for each platform. I did my linux part and it works nicely.

Missing tests:

  • Windows
  • OSX

The testing effort needed is simply building bisq and starting it up. If that works (or at least no other than it did before), the branch can be merged.

@freimair freimair requested a review from cbeams as a code owner March 5, 2019 14:50
@ManfredKarrer ManfredKarrer changed the title Updated netlayer, jtorctl and tor-binary WIP Updated netlayer, jtorctl and tor-binary Mar 5, 2019
@ManfredKarrer
Copy link
Contributor

@freimair If a PR is not ready for merge/review please mark them with prefix WIP and use the draft feature (new in GH - when you create the PR you can select it as draft)

@ManfredKarrer ManfredKarrer requested review from ManfredKarrer and removed request for cbeams March 5, 2019 14:53
@ManfredKarrer ManfredKarrer self-assigned this Mar 5, 2019
@freimair
Copy link
Contributor Author

freimair commented Mar 5, 2019

@ManfredKarrer the PR is ready for merging and review. That is exaclty the point. I cannot test on OSX neither can I test on Windows. The reviewing process is meant to make it unlikely that a merge does break master. So please review and make it unlikely that the merge is going to break master by testing on OSX for example.
Since we still do not have integration testing on different platforms, we have to do it that way...

@devinbileck
Copy link
Member

I gave this a test on my primary Windows machine and it ran without issue. However, on my Windows VM I still encounter the auth cookie issue. But this time the behavior is slightly different. It is raising an IOException wrapped in a TorCtlException. So instead of performing onSetupFailed, it is performing restartTor.
image

@freimair
Copy link
Contributor Author

freimair commented Mar 6, 2019

ah yes, and although the Tor spec says that with this new version the default HS addresses are of the new format, a short chat with a tor dev resulted in https://trac.torproject.org/projects/tor/ticket/29669. so no worries. we are still with the old address format

@ManfredKarrer
Copy link
Contributor

@freimair Did a quick test and all looks ok on OSX

Copy link
Contributor

@ManfredKarrer ManfredKarrer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

@ManfredKarrer ManfredKarrer merged commit ab921b5 into bisq-network:master Mar 6, 2019
@freimair freimair deleted the netlayer_tweaks branch March 7, 2019 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants