Make tumbler idempotent [$1,000 bounty]

[eduard6]

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:

$ python tumbler.py -n mysession [options] wallet.json addr1 addr2 addrN $ sleep 3600; kill $ python tumbler.py -n mysession [options] wallet.json addr1 addr2 addrN

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:

• When session name is declared (-n), tumbler will create a sessions/<session name>.json file (or .txt or anything).
• tumbler will store all options passed on command line including wallet and destination addresses.
• tumbler will store "tumble plan".
• When tumbler is run with session name (-n) and sessions/<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 using python tumbler -n mysession and either command will work the same.
• tumbler continues "tumble plan" from where it left off. Obviously when one of addr1...addrN has been sent to it should not be used again.
• [Optional] tumbler stores PID in session file and uses it to lock session.
• [Optional] session file should be set to 0600/only owner can read.

Bounty:

• I will transfer bounty in advance to any core developer for them to pay to who ever finishes the feature. Please provide address here or by message.
• I will leave it to core developers to decide who deserves the bounty.
• If not finished in 30 days I may cancel bounty and ask for refund, or I may extend deadline.
• $1,000 in BTC when transferred to core developer (BTC may be worth more or less $ when finished of course).
• To qualify for bounty, code should be merged into main JoinMarket project (or be good code that was rejected by core developers for some reason unrelated to code quality)

[eduard6]

Previous similar issues: #464 #467

[AdamISZ]

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.

[jamesphillipturpin]

I don't fully understand the control flow in JoinMarket, so I probably can't solve this but I had to do something similar for my exchange trading bot. I use the Threading module in Python. The first line in my main loop is creates a time delayed thread that restarts my main loop if control isn't returned in a reasonable amount of time. If control is returned then that thread is stopped and the main loop repeats. I save two copies of a file that documents current state of progress. The first file is closed before the second file is opened to write. In case one copy gets corrupted from the process or thread terminating, the other should be preserved. In all it looks something like this:

minutes = # Some generous amount of time to wait for a cycle in the loop to complete,
          # depending on your application.

def main(recursion_level):
  print "Control is at recursion level %s." % recursion_level
  main_thread = threading.Timer(minutes*60, main, recursion_level+1)
  main_thread.start()
  # Load progress file
  for i in [1,2]:
    f = open("Path\\Filename%s.txt"%i, 'r')
    # Read stuff from file
    f.close  
    if (File_Is_Not_Corrupted()): break
  ############
  # Do stuff #
  ############
  for i in [1,2]:
    f =  open("Path\\Filename%s.txt"%i, 'w')
    # Write stuff to file
    f.close
  if recursion_level == 0:
    main_thread.cancel()
  return main_thread

if __name__ == '__main__':
  while(True):
    main_thread = main(0)
    print "Will start main loop again in %s minutes." % minutes
    time.sleep(minutes*60)

I also set up shortcuts in system startup folder so that my trading bot and join market maker bot restart automatically on the regular operating system boot. All I have to do is enter the password, the rest is automated. For Join Market it looks something like this:

[weex]

Is this bounty it still available? Would you be willing to place the funds in a semi-public multisig escrow until this is done?

[eduard6]

@weex Yes the bounty is still available. I will add extra $500 if done in 2 weeks or less from now. If you start working on this post a comment and I will send the funds to @AdamISZ or another core developer for acting as escrow.

[AdamISZ]

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.

[weex]

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.

[piratelinux]

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.

[weex]

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.👍

[eduard6]

@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.

[AdamISZ]

@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.

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----- Hash: SHA1

Address for escrow for eduard6 issue 634: 1EqstsQxpKU4bNJQkVu43FFp7jNEgYRopq -----BEGIN PGP SIGNATURE----- Version: GnuPG v1

iQEcBAEBAgAGBQJYiujQAAoJELOuCfHpoxl6mHkIAIZvt5voGsrrIIAl/DZ9GVbh n/nKaLpUay+30EYK/SAjlnYPqQkTEIOJ/59QF6HHoBwpF14MHydjvcu6uMqqJNbc IfvO08AOGYJIezdkHyh+LxMVqBzE78M5n25UgQwKdFxCjPtFeQY24wG0XjaMYen5 ISE4VOcOsnrzsQI35dK2u/gEi2QK9g7+iLp8HNRnbGvH18LW8408VOUAKX+GwV13 +yp5TRfsRmIy48Z1JBB+I2x2CIac/rWQZdLAsh3GrhdLIuK0dqd+MQygUSVkQcOV WC66gyDZnRztfEn0kQDlULko84QnZh0BW444RcZGiZVf9WAHjtnVg8ZTHNhgBDU= =Hn2+ -----END PGP SIGNATURE-----

(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.

[weex]

1. Specifying the job description and what you want at the end is extremely important. If you want to minimize the amount of testing and evaluating, then require tests. Probably applies to future gigs more than this one since @eduard6 was really specific yet didn't ask for tests. Another way to make life easier in this respect is to improve testing to include everything you do to validate that something is ready for merge/release.

2. This is very cool because once you start thinking about specific gigs and cultivating some expertise, progress can be made much more quickly....if someone will put up some funds. That means making a case for the value of the feature. What is the clientserver branch, how much work is left, and what new doors do you see it opening for JM?

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).

[AdamISZ]

What is the clientserver branch, how much work is left, and what new doors do you see it opening for JM?

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!

[eduard6]

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).

[piratelinux]

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

[piratelinux]

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

[eduard6]

@piratelinux It is just the one commit, correct? (https://github.com/piratelinux/joinmarket/commit/55399547e67ab7113c1273a93c2c15d6d6027919). I see save_session_to_file and update_session_tx_confirmed, but I do not see any code that calls them.

[piratelinux]

Ya it is just https://github.com/piratelinux/joinmarket/commit/55399547e67ab7113c1273a93c2c15d6d6027919 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.

[piratelinux]

@eduard6 , here's save_session_to_file for example: https://github.com/piratelinux/joinmarket/commit/55399547e67ab7113c1273a93c2c15d6d6027919#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: https://github.com/JoinMarket-Org/joinmarket/compare/develop...piratelinux:develop

[piratelinux]

@eduard6: Just let me know if you approve of a partial payment and how much (0.2 BTC is ok). Then @AdamISZ can send it to this address: 1BP1UR4DFjwVTPep2mMoc2WXno7AhjwTke

[eduard6]

@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

[AdamISZ]

@eduard6 @piratelinux done.

[piratelinux]

Thanks! 😂

[piratelinux]

@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

[piratelinux]

@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.

[piratelinux]

@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

[piratelinux]

@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.

[weex]

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?

[piratelinux]

I think it is ready for merging. Once you're ready @AdamISZ, you can send me the funds to 1K4z5ikWRBzj2UEyy4DiDYQ4yBdwA7yFBF.

Thanks

[AdamISZ]

@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.)