initial draft allowing takers to optionally complete transactions wit… #647
Conversation
|
Tried on main net, chose 2 known malicious out of 6 using -P, got a 4 party tx completed. |
| # to complete, accounting for the fact that some makers might not be | ||
| # responsive. Should be an integer >=2 for privacy, or set to 0 if you | ||
| # want to disallow any reduction from your chosen number of makers. | ||
| minimum_makers = 2 |
There was a problem hiding this comment.
we could just default this to 0 so the new behavior is opt-in
There was a problem hiding this comment.
We could. But cognitive load; as of right now the experience of many new users is "try, then fail" because they very often get even 1 malicious/nonresponsive maker. I think if you canvassed the users they would be strongly expressive of a principal desire not to have their transactions stuck, privileged over having larger anonymity sets, although obviously they prefer both.
There was a problem hiding this comment.
I also believe this should be opt-out. "First user" experience is important, also default-settings should just work. Which they currently dont, due to these unresponsive bots.
There was a problem hiding this comment.
Recommendation for user UI though: at the point where they confirm that they will pay 0.xx % of cjfees in total, there should be a note if minimum_makers > 0. Something like:
If some makers do not respond, we will still create a coinjoin with at least (minimum_makers) makers. If you do not desire this behavior, you can set "minimum_makers = 0" in joinmarket.cfg.
There was a problem hiding this comment.
@AlexCato I agree. Will push and squash that edit shortly.
|
Additional notes as of 4fe8e25 (will squash before merge, don't want to bork comments for now) This commit does 3 things:
I did a bunch of tests of cases: (1) minmakers 0, no malicious bots, (2) minmakers 0, randomly malicious bots, (3) minmakers 2, no malicious bots, (4) minmakers 2, malicious bots and (5) sweeps with malicious bots (minmaker setting not relevant). The tests seem to behave as expected. I'll run a couple more tumbler tests today, this time including malicious behaviour in bots to see if it handles it OK. |
|
Tumbler tests with inserted maker maliciousness seem to be working fine. I'd anticipate merging this in the next few days - and making a new release - unless anyone objects; additional tests by others hugely appreciated. |
joinmarket/taker.py
Outdated
| self.msgchan.fill_orders(new_orders, self.cj_amount, | ||
| self.kp.hex_pk(), self.commitment) | ||
| else: | ||
| log.info("Two few makers responded to complete, trying again.") |
There was a problem hiding this comment.
Thanks! damn, i caught that one and then forgot it again :)
…h less than the initially requested number of makers; sweep function is unchanged Insert message to console advising user if minmakers!=0 disallow sendpayment -N choice < minimum_makers, fail before sync
AdamISZ
force-pushed
the
completewithsubset
branch
from
c18f3af to
c89347a
Compare
…sactions wit… c89347a initial draft allowing takers to optionally complete transactions with less than the initially requested number of makers; sweep function is unchanged Insert message to console advising user if minmakers!=0 disallow sendpayment -N choice < minimum_makers, fail before sync (Adam Gibson)
4 participants
…h less than the initially requested number of makers; sweep function is unchanged
It occurred to me that this is a much simpler, faster, and possibly more robust way of dealing with unresponsive makers who don't even provide utxos or signature up to !ioauth.
This is not really intended to supercede the idea of more intelligent selection, something like outlined here but is a more basic idea that is simpler.
If the number of full respondants in the
!fill-!ioauthphase is less than that specified (by -N or similar), but is at least equal tojm_single().config.getint("POLICY", "minimum_makers"), then continue anyway - the change will be calculated correctly, higher than originally anticipated, and the txfee will be less than originally anticipated but it is already re-calculated at this point.The timeout code in
recover_from_nonrespondantsthus checks the above arithmetic, if sufficient, it callsrecv_txiowith dummy values, which flags the code to skip the first step and pass straight into calculation of our change output and construction of!txmessages (nothing for the makers changes; their outputs are already set by their own fees).Nothing changes for sweep; there was previously no way to handle any change in the input set of orders, and there still isn't (at least I haven't thought of one yet).
I've done various tests with randomly malicious makers (using
test/ygrunner.pyand inserted random failure-to-ioauth intomaker.py) and it seems to work as intended.User can flag not to use this feature by setting
POLICY.minimum_makersto 0.Review greatly welcomed. This needs careful testing, even if it is a fairly simple change (at least in comparison to other ways of dealing with the problem!)