Skip to content
This repository has been archived by the owner on May 13, 2022. It is now read-only.

Switch blockr to electrum as default blockchain interface #704

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

AlexCato
Copy link
Contributor

Rationale:
blockr as default does not work for TAILS and other TOR users any more. This will reduce potential confusion, while at the same time not reducing privacy (blockr potentially has worse privacy implications, electrum is at least somewhat decentralized).

Rationale:
blockr as default does not work for TAILS and other TOR users any more. This will reduce potential confusion, while at the same time not reducing privacy (blockr potentially has worse privacy implications, electrum is at least somewhat decentralized).
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 77.178% when pulling f8e00b9 on AlexCato:patch-5 into 5c5b6b9 on JoinMarket-Org:develop.

@chris-belcher
Copy link
Collaborator

chris-belcher commented Jan 28, 2017

Yes but we should test the electrum blockchain interface more. Run a sendpayment and tumbler run with it.

I just found this bug in the pushtx() function while reading it https://github.com/JoinMarket-Org/joinmarket/blob/develop/joinmarket/electruminterface.py#L246

@AdamISZ
Copy link
Member

AdamISZ commented Jan 28, 2017

On being default, I had two reasons not to want to do that: first, it's slow without some improvements, two, it's only had minimal testing.

Thanks for that reviewing/bug find @chris-belcher . I have only tested this a few times, on main net (no other way sadly), and I only tested it from Taker side. As I mentioned on reddit, this is intended as a help for Takers, not Makers (because I think it will be unworkable with big wallets until some improvements are made). So that's why I didn't encounter that particular bug .. (edit, well, seems the point is only that it would not detect a push failure, since (False, None) is not False; looks like a simple fix anyway).

Re: testing strategy. It's not going to be easy, but I guess a 'mock' style approach will work OK, it'll just be a bit of a pain making sure of the correct return values for the various electrum server calls, and then setting up some dummy data to return.

Overall it needs work ... just seems better to have it there as a fallback right now, than nothing ... I thin k bc.i is very nearly unusable.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants