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

Various bug fixes and improvements for the market #3502

Merged
merged 11 commits into from
Mar 16, 2018

Conversation

devos50
Copy link
Contributor

@devos50 devos50 commented Mar 13, 2018

In this PR, I fixed some bugs in the market community. I also removed a log statement in the tunnel community that slowed anonymous downloads. I decided to have the improvement as suggested by @qstokkink in a separate PR since it did not work out of the box and this PR was growing quite a lot with fixes/improvements for the market community.

I also upgraded the Electrum wallet to 2.9.4 and tested some basic trading functionality between two different machines, with Testnet Bitcoin wallets.

Working market experiment with this PR: https://jenkins.tribler.org/job/pers/job/market_experiment_devos50/745/

Fixes #3494

It was slowing down the anonymous downloads.
@devos50 devos50 requested a review from qstokkink March 13, 2018 15:25
@devos50 devos50 changed the title Removed log statement READY: Removed log statement Mar 13, 2018
@qstokkink
Copy link
Contributor

Please also include this in this PR:

Make the session dispatching more efficient here:
https://github.com/devos50/tribler/blob/777f59a08c09131fc94760f4b03bb060395799a7/Tribler/community/triblertunnel/dispatcher.py#L44
Through a map of the circuit_id to udp_connection.socksconnection (which is removed on circuit_dead) here:
https://github.com/devos50/tribler/blob/777f59a08c09131fc94760f4b03bb060395799a7/Tribler/community/triblertunnel/dispatcher.py#L57

@qstokkink
Copy link
Contributor

ERROR   1521022296  test_as_server:TestTunnelCommunity:124  The reactor was dirty during tearDown:
ERROR   1521022296  test_as_server:TestTunnelCommunity:126  >     <DelayedCall 0x7f065a166758 [7.30270910263s] called=0 cancelled=0 RequestCache._on_timeout(<Tribler.pyipv8.ipv8.messaging.anonymization.caches.CreatedRequestCache object at 0x7f065a154f90>)>

@devos50 devos50 force-pushed the removed_log branch 3 times, most recently from 1c92591 to 93f1d08 Compare March 15, 2018 12:28
@devos50 devos50 force-pushed the removed_log branch 3 times, most recently from d201629 to 0cbb5b4 Compare March 15, 2018 15:50
@devos50 devos50 changed the title READY: Removed log statement WIP: Various bug fixes and improvements for the market Mar 15, 2018
@devos50 devos50 changed the title WIP: Various bug fixes and improvements for the market READY: Various bug fixes and improvements for the market Mar 15, 2018
@@ -240,7 +240,7 @@ def render_POST(self, request):

.. sourcecode:: none

curl -X GET http://localhost:8085/wallets/BTC/transfer
curl -X POST http://localhost:8085/wallets/BTC/transfer
Copy link
Contributor

Choose a reason for hiding this comment

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

Please update the docstring to replace get request with post

@@ -0,0 +1,36 @@
import json
Copy link
Contributor

Choose a reason for hiding this comment

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

@qstokkink
Copy link
Contributor

qstokkink commented Mar 16, 2018

Apparently the test is failing on this call, which is leaking a DelayedCall:
https://github.com/Tribler/py-ipv8/blob/master/ipv8/requestcache.py#L96

self.register_task(cache, reactor.callLater(cache.timeout_delay, self._on_timeout, cache))

This might have to be a deferLater instead.

@qstokkink qstokkink changed the title READY: Various bug fixes and improvements for the market Various bug fixes and improvements for the market Mar 16, 2018
@devos50 devos50 merged commit 219b490 into Tribler:devel Mar 16, 2018
@devos50 devos50 deleted the removed_log branch March 16, 2018 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Wallet management page must show the address of a specific wallet
3 participants