-
Notifications
You must be signed in to change notification settings - Fork 109
Conversation
…n/additions to sendpayment, taker, wallet code to accommodate
… id reporting bug
…ing transaction abort correctly in GUI
…le help text, used this for donation help text, set donation off by default
…tart, which is a little inelegant workflow but a much easier code patch
…in section GUI, and persist config to joinmarket.cfg on update, a couple more minor tidy up edits
…ents to log.debug
…transaction agreement dialog.
…into gui Merging v0.1.3
|
I'm going to pull and try to test now. Is anyone else testing at all? @lacksfish what about you, could you give info on any tests you've done? Note we now have a test_blockr.py; but I only put in a test for testnet there, still, in principle, it would be desirable for PR commiters to deliver tests with the code. I do appreciate that this is a particularly difficult one, but still. |
|
First test, OK - can run wallet-test against real wallets and get correct info, although I noted a difference of 19s for bc.i vs 4s for blockr.io, don't know how repeatable that is. I will try a sendpayment. Second test: tried wallet-tool again with a big wallet (my main yg wallet), results not so good: The timing delta is partly due to timeouts as the bc.i version inserted several 60 second waits, so less concerned about that. |
|
Took a bit of burrowing, but I think I found the problem. The sync_addresses code implicitly assumes that the list of data returned is in the same order as the address list passed to the API, but it isn't for bci it seems. See here: https://github.com/JoinMarket-Org/joinmarket/pull/481/files#diff-3b408ed85f23411e81db7c0ae451edfdR188 (it's the same in the blockr code of course). Continuing to do tests; if I got that right, it'll be fiddly but it can be fixed. |
|
Yes, that solved it. @lacksfish you can add insert here this code: and then the next line obviously becomes: With this edit the balance is reported correctly. Without it you will get something unpredictable because the ending value of Edit: of course some extra sanity check might make sense there, also there might be a cleverer way to re-sort the list without looping, I just couldn't think of one offhand. |
Conflicts: joinmarket/__init__.py joinmarket/blockchaininterface.py joinmarket/taker.py wallet-tool.py
|
Sorry for the late replay, am quite busy at the moment. I will look into it in the coming two weeks. |
Extra line, looks like a merge error
Fix NotifyThread
fbd7b95 to
9cff309
Compare
|
AdamISZ, added your solution and also updated my repository. |
|
Seems to have picked up a bunch of old stuff, something's gone wrong. By the way I notice a lot of it is joinmarket-qt stuff which shouldn't be here (it's supposed to be only in the gui branch). |
|
I'd be very thankful for someone testing this updated version of the API Interface, I implemented AdamISZ's fix for properly sorting the data array in sync_addresses() |
|
@lacksfish did you see my earlier comment? This needs to be rebased somehow. |
97d2603 to
47479d5
Compare
|
I've added this (although had to fix up the bugfix above, didn't seem to have been done properly) in fa61dbd ; of course it would be much better to give proper attribution but in the absence of cleaning up this PR I just went ahead. Apart from that attribution issue, the other issue is that it's dreadfully slow (even not on Tor, but of course especially on Tor as you regularly get endpoint limit reached and waits). I see it as something to use in extremis when nothing else is available. Closing; @lacksfish let me know if you want to fix this up somehow so you can get attribution. |
Fully functional blockchain.info API
How to use
Change the source in joinmarket.cfg to
Then, tumble as usual
Notes
The pushtx() function in the BlockchaininfoInterface class is functional but could need a quick review. I left my findings and questions in the comments.
The interface got an extra make_bci_request() function for directing requests to the API. Includes API error handling for common errors.