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

sendpayment.py chooses counterparties, then stalls and fails #154

Closed
kristapsk opened this issue Jun 26, 2018 · 9 comments
Closed

sendpayment.py chooses counterparties, then stalls and fails #154

kristapsk opened this issue Jun 26, 2018 · 9 comments

Comments

@kristapsk
Copy link
Member

Tried multiple times today.

$ time python2 sendpayment.py -N 2 -m 4 wallet.json nnnnnn XXX
Enter wallet decryption passphrase:
2018-06-26 13:35:58,794 [MainThread  ] [WARNI]  Cannot listen on port 27183, trying next port
2018-06-26 13:35:58,797 [MainThread  ] [INFO ]  Listening on port 27184
connection was made, starting client
2018-06-26 13:36:04,636 [MainThread  ] [INFO ]  JM daemon setup complete
2018-06-26 13:36:34,663 [MainThread  ] [INFO ]  Chose these orders: {u'J55MnqmqcqVGYqQs': {u'cjfee': u'0.001',
                       u'counterparty': u'J55MnqmqcqVGYqQs',
                       u'maxsize': 749206132,
                       u'minsize': 200000,
                       u'oid': 8,
                       u'ordertype': u'swreloffer',
                       u'txfee': 1000},
 u'J5BKSrM2zFwRmbLm': {u'cjfee': u'0.001',
                       u'counterparty': u'J5BKSrM2zFwRmbLm',
                       u'maxsize': 749206132,
                       u'minsize': 200000,
                       u'oid': 8,
                       u'ordertype': u'swreloffer',
                       u'txfee': 1000}}
2018-06-26 13:36:34,664 [MainThread  ] [INFO ]  total cj fee = -nnnn
2018-06-26 13:36:34,664 [MainThread  ] [INFO ]  total coinjoin fee = -0.nnn%
send with these orders? (y/n):y
2018-06-26 13:36:40,473 [MainThread  ] [INFO ]  total estimated amount spent = nnnnnn
2018-06-26 13:37:11,152 [MainThread  ] [INFO ]  Makers didnt respond: [u'J55MnqmqcqVGYqQs', u'J5BKSrM2zFwRmbLm']
2018-06-26 13:44:48,715 [MainThread  ] [WARNI]  Cannot reconnect to dropped nick, no connections available.
2018-06-26 13:45:58,835 [MainThread  ] [INFO ]  STALL MONITOR:
2018-06-26 13:45:58,835 [MainThread  ] [INFO ]  Stall detected. Regenerating transactions and retrying.
done

real    10m22.210s
user    0m18.357s
sys     0m0.247s
@AdamISZ
Copy link
Member

AdamISZ commented Jun 26, 2018

Yes, it unfortunately seems we have a large number of effectively Sybil bots that are disrupting; specifically, they participate in setup, then block at the last step by not sending a signature.
Currently we simply start again from the beginning in this case.
Note that if they block at the first step (the tx negotiation), they get dropped and we continue and complete the coinjoin.
If they are a small-ish fraction, this doesn't matter. But if they are larger and occupying all the lowest fees, then the probability of at least one being included, and blocking completion is high.

1/ Use the option -P to sendpayment, and choose counterparties which are not part of the set which can be obviously seen on the orderbook, or just choose those with higher fees at random.
2/ The best solution is for me (or someone) to add code and make sure that the ignored_makers parameter gets updated between the multiple rounds/attempts above. You will then filter out sybils, but it still may be quite annoying if they are really swamping the orderbook.
3/ A minor point but something I've been saying for literally years: the default algo for choosing should be "random under a maximum" not "heavily weighted towards cheapest" random which makes it all the easier to have a particular sybil group swamp your offers.

@chris-belcher
Copy link
Contributor

Other possibly-useful discussion here about 3/ JoinMarket-Org/joinmarket#14 (comment)

@kristapsk
Copy link
Member Author

Ok, 1/ worked, often I'm too lazy to RTFM, so I even didn't know about -P option and how it works. :)

2/ Probably I could look into this. Guess it should have some file where to store all bad IRC nicknames together with expiration timestamps (now + 24h by default, configurable?). That banfile should be per-wallet, as it should be possible to run multiple parallel sendpayment's with different wallets on the same machine IMHO and shared banlist file will complicate things, as all instances would like to have write access to it. And, as per per-wallet thing, seems to me little bit related to #62 and #85 too and could be done together.

@AdamISZ
Copy link
Member

AdamISZ commented Jun 26, 2018

Yes, but your not reading TFM is only part of the laziness here - it's a complete pain to do that as I'm sure you observed!

Problem with banning (using ignored_makers list) is, as @chris-belcher observed earlier today on IRC, of course they are ephemeral, people can change them as they like. Still may be worth doing, but meh. More radical alterations are possible longer term (Coinshuffle for example) but they only make for better (symmetrical) privacy; without persistent identities non-honest participants can still keep trying to block. Ultimately some kind of solution to JoinMarket-Org/joinmarket#693 would be needed if we want this to work at scale against serious attackers.

@kristapsk
Copy link
Member Author

I had some thoughts about this. Bad actor could spawn thousands of malicious bots and they could join IRC channel every second. So, keeping permantent banlist with nicknames between script runs will not be enough in such attack scenario (and will not actually help anyhow). In my example above, even if we ban all selected counterparties in case of stall detected, while you re-run the script, hundreths of more bad counterpaties could have joined the pit.

Only workaround I see is to somehow define time period when we start sendpayment and then never choose from counterparties that did not exist prior that point in time. Probably instead of terminating after "Stall detected. Regenerating transactions and retrying." script should really retry everything, by banning all involved counterparties that didn't respond and not choosing between any new ones. Then, eventually, you could just run sendpayment which sooner or later will either do the coinjoin or run out of counterparties to choose from (which should not happen if there are enough honest peers). But, in this case, no point to implement banlist files between script runs at all.

This is just an idea, haven't looked at the code, how easy or hard it would be to implement this.

Unfortunatelly, in case of sybil attack, sendpayment anyway will not be reliable to be used to pay to merchants directly, as they usually give you limited time window for a payment, but still can be used to send them to some wallet you control, from where you can then make payment with simple bitcoin tx.

Or probably some script could be written that tries to do sendpayment and, if that does not succeed in given amount of time, just use normal TX from same mixdepth. Input(s) for tx will be anyway most likely already went through some coinjoins and change will go back to JM wallet.

Also, 3/ would really help against such attacks, as with randomization there is more chance that honest counterparty will be choosen.

I believe we need both other privacy solutions for Bitcoin (lie Coinshuffle) as well JoinMarket should be improved with protocol changes, like JoinMarket-Org/joinmarket#693, but I don't see it coming soon enough.

@kristapsk kristapsk changed the title sendpayment.py chooses counterpaties, then stalls and fails sendpayment.py chooses counterparties, then stalls and fails Jun 26, 2018
@AdamISZ
Copy link
Member

AdamISZ commented Jun 27, 2018

Re: the first paragraph, yes, I thought that was implicit but what I said, but yes what you describe explicitly is exactly the reason for "meh".

Only workaround I see is to somehow define time period when we start sendpayment and then never choose from counterparties that did not exist prior that point in time. Probably instead of terminating after "Stall detected. Regenerating transactions and retrying." script should really retry everything, by banning all involved counterparties that didn't respond and not choosing between any new ones. Then, eventually, you could just run sendpayment which sooner or later will either do the coinjoin or run out of counterparties to choose from (which should not happen if there are enough honest peers). But, in this case, no point to implement banlist files between script runs at all.

Yes, this is sound thinking, and is basically the same as Coinshuffle's blame protocol (except they have to do some extra steps due to the crypto). The code currently does "complete_with_subset" after non-response in the first round of interaction, this is basically the same thing (except "restart with subset" more than "complete with subset") after the second round of interaction (tx-> , <-sig); i.e. start again from the beginning with the honest subset. (Another somewhat related idea is @adlai 's "Taker sophistication" concept).

Re: we also need Coinshuffle; do note that (a) there's nothing stopping Coinshuffle being added to this (b) Coinshuffle doesn't really stop the Sybil problem (I have seen this misunderstanding everywhere), and it only "guarantees" completion in the presence of at least 1 honest counterparty, which is no different from here, if we use that "restart with subset" approach.

Re: 693 not coming soon enough, couldn't agree more but the problem is, I don't even know of any good solution.

@AdamISZ
Copy link
Member

AdamISZ commented Jul 8, 2018

Yes, this is sound thinking, and is basically the same as Coinshuffle's blame protocol (except they have to do some extra steps due to the crypto). The code currently does "complete_with_subset" after non-response in the first round of interaction, this is basically the same thing (except "restart with subset" more than "complete with subset") after the second round of interaction (tx-> , <-sig); i.e. start again from the beginning with the honest subset. (Another somewhat related idea is @adlai 's "Taker sophistication" concept).

FWIW I'm now starting to look at this. It doesn't solve 693 but I still think it's a big unconditional improvement, assuming nothing makes it super-difficult.

@AdamISZ
Copy link
Member

AdamISZ commented Jul 10, 2018

See #159

@AdamISZ
Copy link
Member

AdamISZ commented Jul 17, 2018

Should be partially addressed by the merge of 159, but feel free to re-open if you want to discuss further.

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

No branches or pull requests

3 participants