-
Notifications
You must be signed in to change notification settings - Fork 119
Make tumbler idempotent [$1,000 bounty] #634
Comments
Not addressing this proposal right now, but coincidentally, and relevant-ly: Earlier in the week I was debugging a problem with a tumbler run with a user, and it slowly dawned on me that a problem had been introduced in the new sync code such that tumbler restarts can crash on a mismatch with max_mix_depth and index_cache. I was going to fix this up today. Assuming that's all figured out correctly, I'll link it here for reference when done. |
Is this bounty it still available? Would you be willing to place the funds in a semi-public multisig escrow until this is done? |
Re: escrow, that's fine with me. Re: the goal of this issue, I am working on something similar in https://github.com/AdamISZ/joinmarket-clientserver around now actually. The first part is the use of "schedules" which can be read from files (see the scripts/README.md for a bit more info). The second is generation of tumble schedules. This has been done for a while now. The third part is, using the twisted model it's a bit less problematic to monitor state, and detect a failure condition such as waiting in a loop, or the tx failing due to no liquidity or maker rejection. I'm working on that bit at the moment. Two more steps I haven't done, but are fairly minor, are: re-generating the tumble schedule on such a failure (because the failure may have been connected with the exact transaction parameters, so it makes sense to re-generate from that point in the flow), and specific handling for the case of inability to create podle commitment. The latter problem should hopefully be less frequent as we avoid the current scenario of tx fails, then retry again and again (which was an excellent feature until retrying had a commitment cost). I think the main thing there is to slightly polish/improve what we already have; wait for sufficient confirmations if there are utxos which are too new, else definitively stop. I think I still have 3 makers running on testnet on #joinmarket-pit(-test) on freenode if anyone wants to test out. I've run the tumbler.py on there a couple of times, but as you can see, there's still a bit to do. There is also sendpayment of course.These functions are also available in the joinmarket-qt.py if you have PyQt4 installed (there are binaries too but it's all a WIP). As for bounties, @weex had the idea of generalising bounties a bit, having goals at multisig addresses and so on. Seems it could be a good idea. As for this specific goal, I'm trying to address it in spirit if not in letter, but it's part of a broader attempt to improve taker functionality in that new repo, which has been ongoing work for the last couple of months. If someone wants to create something specifically giving this function, and in this codebase, I'll certainly not complain. The important thing is to set up a working test framework that tests error conditions. If anyone needs help setting up regtest for that, feel free to ask. |
I was actually not planning on doing this myself but as @AdamISZ mentioned I would like to see more bounties like this work. It's a challenge to understand the JM code enough to complete this quickly but the bounty certainly helps motivate. I'll ping this thread again if I've found someone willing to tackle. |
Hi, I'd be interested in trying this out. I did some work for Rein, and if you want you can post this on Rein and I can bid on it. I used joinmarket once, but never developed on it, so it may take me like a week to first get used to the environment. |
I pointed @piratelinux at this thread since they've done some great stuff on my decentralized freelancing project that was mentioned. That was my reference to multisig escrows. I would love to see this done via that tool/protocol but to the point of why I started that project, the main thing is that we can accelerate dev on important projects like this with solidly escrowed bitcoin. Feel free to ping me on freenode irc if you decide to test Rein with this and need help. I've used it to do a handful of jobs so am confident it can work. Either way.👍 |
@AdamISZ Is there an issue on Github for your clientserver work? I would like to discuss that with you separate from this issue. Also please post a BTC address where I can send this issue's bounty. @piratelinux Ok, please do get started. If you need clarification on the requirements let me know. If it is alright with you I will use @AdamISZ as escrow because I don't have time to set up Rein just now. @weex Rein looks interesting. I could see myself using that for future projects. I don't have time just now to set it up though. |
Not one specific issue on this repo, no, as I recall, I talked about refactoring in a few contexts (gui, plugins, testing); the motivation is in the main README of that repo. I've talked with @chris-belcher and @adlai and others on IRC about it. Re: address for escrow. OK, I'll just use one under my exclusive control, it sounds like you're fine with that: -----BEGIN PGP SIGNED MESSAGE----- Address for escrow for eduard6 issue 634: iQEcBAEBAgAGBQJYiujQAAoJELOuCfHpoxl6mHkIAIZvt5voGsrrIIAl/DZ9GVbh (from my key 4668 9728 A9F6 4B39 1FA8 71B7 B3AE 09F1 E9A3 197A which i hope is what you have, if not let me know) We can look into 2 of 3s or more perhaps if we start looking into bounties more generally. Pinging @chris-belcher and @adlai at least, maybe we can chat on IRC when convenient. I have a couple of concerns about this: (1) am I going to have to do a lot of testing and evaluating? I don't mind per se, but it may end up a big time sink (2) I'd be much happier if I could convince people to start helping me on that *clientserver repo because as I've explained, it's addressing this issue along with a bunch of other things connected with the experience of users/Takers. I'm kind of badly in need of help, too - it's very largely functional, but there's many things where another pair of eyes really matters. Anyway, if any proposed patch is put in a branch and has been tested, then I guess @eduard6 can report on whether it meets his needs. That would minimize workload for current devs, although I'm sure we'd want to review it, just a question of how much. As I might have mentioned before, this kind of work is particularly demanding testing-wise given the nature of how the tumble algo works. |
My theory is that lots and lots of this stuff makes sense for the community to fund but they just have to get over some challenges like trust (multisig escrows), privacy (joinmarket) and funding (a little salesmanship). |
It's not a branch, it's a new repo (as it's a complete restructure of the codebase, although big chunks of it is the same code, so it's a refactoring but a very substantial one), I linked the repo above AdamISZ/joinmarket-clientserver. What it provides, again, I discussed above, read the main README for the motivation. Re: tests yes I last year created #496 specifically about tests and new features, see point (2). It hasn't been stuck to of course, but at least efforts have been made. But this particular request has a special flavour to it: it requires a testing setup with an entire, fairly large simulated pit (I'd say 6 makers minimum), all of which fail and or reject transactions at different times in different ways. I'm about to do such a test on the clientserver repo, but note how it's kind of intrinsically about end-to-end testing, not really unit testing I think. Although if significant extra code is written, it's quite likely that extra unit tests are helpful as well as that end to end testing. Well, however you look at it, it's certainly tricky! |
The bounty of 1.08932462 BTC ($1000 @ $918/BTC) will soon arrive in the escrow address. I will discuss the clientserver code with @AdamISZ in another issue in that repo. Focusing on the clientserver code instead of this issue might make sense. But no matter the result of that discussion I am still happy to fund the development of this issue on the current code (develop branch). |
Great will start now. My estimate is it will take 3 weeks to complete, but I would put a maximum deadline of 4 weeks to be safe. I will let you know eduard6, if I need more clarification. Thanks |
Hi @eduard6. I made a first draft of the feature you request at my fork https://github.com/piratelinux/joinmarket. I think it does what you want except for the -D and -y flags that I still have to add. (Mostly just the changes to tumbler.py and support.py are relevant). I did a lot of testing in general to better understand the functionality of the tumbler, and to have a working testing environment set up. Now I just need to polish this up, make a file for unit tests, and it should be done. I am very low on basic funds now (until another client's payment comes next week). So I am asking if you are able to release a partial payment (like 0.2 BTC) for the work I did up till now, and then release the rest when it's finalized? Please let me know today or tomorrow. You can also talk to @AdamISZ for more confirmation. Thanks |
@piratelinux It is just the one commit, correct? (piratelinux@5539954). I see |
Ya it is just piratelinux@5539954 on the master branch. I am now making the changes to the develop branch, and actually I realized there's one subtlety I should take care of (saving the txid in case it crashes before it gets confirmed). But I am planning to have this done within a week and maybe I will spend more time if Adam wants to add some other features like parameter tweaking for reruns. |
@eduard6 , here's save_session_to_file for example: piratelinux@5539954#diff-7dca4cc17829f93a2517b5a8dd3ef018R718 There's a lot of messy changes in there also because I was testing, so I will clean that up. Edit: Here's the develop branch with a more clean view for comparison: develop...piratelinux:develop |
@piratelinux Ok, that develop diff is showing more code than the commit diff, so that is good. I will test it soon. @AdamISZ Please send 0.2 BTC of the bounty to @piratelinux at 1BP1UR4DFjwVTPep2mMoc2WXno7AhjwTke |
@eduard6 @piratelinux done. |
Thanks! 😂 |
@eduard6 Can you check the latest commit of my develop branch now? It should have all the features you want, just need to complete the formal testing to get it to merge in the official repo. For finding automatically the lowest depth with a balance, it only activates when you put the -D flag, it doesn't do this even when -m is unspecified because I didn't want to change the default behavior, but I can easily change that. The main difference is that now you put your wallet password first (so it can scan for the lowest mix depth), and then you say y/n to the tumbler plan. I also included other options (mixdepthlimit and gaplimit) to control how deep through the wallet the tumbler searches for unspent utxos. And since it seems you want to automate everything, I included a password option where you can put the wallet password directly into the command. Let me know if it is what you want, while I work to fix my testing environment. Thanks |
@eduard6 Seems to pass the basic tests, just one thing to work out with the wallet password feature, and I think it needs just more testing of the tumbler.py script. Please let me know when you will be testing this, thanks. |
@eduard6 Before I do more tests I need to know if it is what you want. Also would be good to wrap this up soon, so please let me know when you will test this. Thanks |
@eduard6 It's been 8 days and I still didn't get a reply. Are you still there? Sorry to bother with the messages, but I need to get my bills paid, so would be good if I could get your approval on this. |
Hey, it seems like @piratelinux may be done so would be good for @eduard6 to provide some guidance on what else is needed or accept the PR as sufficient. @AdamISZ, do you have a way to contact @eduard6 or evaluate if the requirements seem to have been satisfied? |
I think it is ready for merging. Once you're ready @AdamISZ, you can send me the funds to 1K4z5ikWRBzj2UEyy4DiDYQ4yBdwA7yFBF. Thanks |
@eduard6 last chance to review (same goes for anyone else). If not I will complete the payment tomorrow on the basis of the features being provided. However it's not mergeable as is (this is not so much criticism of the work, albeit I have several nits still, as a reflection of how complex and difficult a piece of code this is to work on ). (I'd also like to cross-reference again the fact that I have provided this functionality here, including implementation of same into the Qt GUI version there.) |
Majorly sorry for going away guys. @piratelinux This looks very good, I will be using it all soon. Thank you for the work. @AdamISZ Please release the rest of the bounty to @piratelinux |
@piratelinux was pinging earlier on IRC, wanted to confirm address via another channel, pls respond there, ready to pay. thanks. |
Thanks received. @eduard6 if you want the password input automated, you can wait until the other pull request for passwordless tumbling is merged: #679, or you can see how I did it in my previous commits (they didn't want it for security purposes) If someone wants more work done on joinmarket, I can put 50% of my time on this for 1 BTC a month. I have some free time starting mid April, but you have to tell me ahead of time, or I may be busy with something else. But some kind of time limits need to be agreed upon beforehand (for the client to respond and for the mediator/escrow to decide whether the requirements were satisfied). And it should be called just a "paid task", not a bounty. I wouldn't risk that much time of working if some random person could come up with the solution and claim the prize. update: I squashed my commits into one, so not sure if you can see my commit history anymore... |
Because the tumbler can crash or stall for many reasons being able to restart it and have it pick up where it left off is very useful.
Example:
Options:
-n <session name>
: Declare session name.-m <depth>
: If declared, first tumbler run starts from this mixdepth.-D
: If declared, or if no -m is declared, first tumbler run starts from lowest mixdepth that has balance.-y
: Answer yes to all questions. If not declared, user has to approve "tumble plan" on first run. If user does not approve "tumble plan" on first run then session file (see below) should not be created.Details:
-n
), tumbler will create asessions/<session name>.json
file (or.txt
or anything).-n
) andsessions/<session name>.json
file exists, ignore all command line options/wallet/addresses and load from saved options. tumbler should be able to resume using either the exact same command line as the first run or just usingpython tumbler -n mysession
and either command will work the same.addr1
...addrN
has been sent to it should not be used again.Bounty:
The text was updated successfully, but these errors were encountered: