--- Day changed Mon Dec 03 2018 01:18 -!- undeath [~undeath@hashcat/team/undeath] has joined #joinmarket 04:13 < waxwing> undeath, this one's quite remarkable/strange: https://github.com/JoinMarket-Org/joinmarket-clientserver/pull/241#issuecomment-443689484 04:14 < waxwing> i guess the secret just needs to be converted to bytes from str, or similar 04:18 < undeath> the password must be converted to a bytes type 04:18 < undeath> so probably .encode('utf-8') before passing it to the storage 04:20 < undeath> yeah, needs to be encoded at these two places 04:21 < undeath> https://github.com/JoinMarket-Org/joinmarket-clientserver/blob/master/jmclient/jmclient/wallet_utils.py#L467 04:21 < undeath> https://github.com/JoinMarket-Org/joinmarket-clientserver/blob/master/jmclient/jmclient/wallet_utils.py#L505 04:39 < waxwing> undeath, i guess .encode('utf-8') is guaranteed to give the same output on both python versions? 04:39 < waxwing> ? 04:39 < undeath> yes 04:40 < undeath> oh, well 04:41 < undeath> it seems py2 does this automatically actually 04:41 < undeath> so it must only be encode'd on py3 04:42 < undeath> ah, I see 04:42 < undeath> it should work 04:43 < undeath> assuming the password is read as u'' 04:43 < waxwing> yeah my understanding of this is not complete. and we need to get this right; one thing we have to pay attention to is wallets created with one version must work with the other :) 04:43 < undeath> py2 04:43 < undeath> >>> u"Ä".encode('utf-8') 04:44 < undeath> '\xc3\x84' 04:44 < undeath> py3 04:44 < undeath> >>> "Ä".encode('utf-8') 04:44 < undeath> b'\xc3\x84' 04:44 < undeath> so simply tacking the .encode() there should do the job 04:47 < waxwing> so getpassword will handle a unicode password then? 04:47 < undeath> it should 04:47 < waxwing> i guess i'm just asking on py2 there 04:47 < undeath> with the future import 04:49 < undeath> actually, good question, since py2 should complain as well if you pass in unicode 04:54 < undeath> it's a bit annoying the python documentation does not mention return types at all 04:57 < undeath> this is weird 04:58 < undeath> when testing in the interpreter getpass() returns unicode (with builtins import) and I get the same error on py2 that you got on py3 04:58 < undeath> but you said py2 worked for you 05:29 < waxwing> undeath, yeah this is just generate; it worked with py2 but not py3 05:30 < waxwing> the password i used just ascii, like 'xyzblah', same in both cases 05:33 < undeath> for me getpass reliably returns unicode on py2 05:36 < undeath> what py2 version are you using? maybe it's a problem with older versions? 05:37 < undeath> i'm on 2.7.15 05:37 < waxwing> me too. 05:37 < waxwing> it's whatever came with 1804 from a couple months back. 15rc1. 05:38 < waxwing> but undeath , wait, can you reproduce the error in Python2, from wallet-tool generate? 05:38 < undeath> ok, without the builtins import I get a str in py3 05:38 < undeath> I'm only testing in the interpreter right now 05:38 < undeath> *I get a str in py2 05:40 < waxwing> ah, we have "from getpass import getpass" 05:40 < waxwing> it's not "getpassword" 05:49 < waxwing> yeah as of now i am also observing that both py2 and py3, with `from builtins import *`, both return type 'str' from getpass() 05:49 < waxwing> but under the hood it's not exactly the same right 05:50 < waxwing> python2: >>> x = getpass() 05:50 < waxwing> Password: 05:50 < waxwing> >>> x 05:50 < waxwing> 'abcd' 05:50 < waxwing> >>> type(x) 05:50 < waxwing> 05:50 < waxwing> >>> isinstance(x, str) 05:50 < waxwing> False 05:50 < waxwing> python3: 05:50 < waxwing> >>> x = getpass() 05:50 < waxwing> Password: 05:50 < waxwing> >>> x 05:50 < waxwing> 'abcd' 05:50 < waxwing> >>> type(x) 05:50 < waxwing> 05:50 < waxwing> >>> isinstance(x, str) 05:50 < waxwing> True 05:52 < undeath> on the interpreter, with the builtins import, getpass returns unicode 05:52 < undeath> but inside joinmarket it returns str 05:52 < undeath> on py2 05:52 < undeath> i don't understand it 05:52 < waxwing> the above is with the builtins import on both sides 05:52 < waxwing> oh, i see 05:53 < waxwing> right so our experience is different, on the interpret for me (see above), i'm getting a returned from getpass, in python2 05:53 < waxwing> interpet*er* 05:53 < undeath> did you do the builtins import on the interpreter, too? 05:54 < undeath> the order of imports might matter here 05:59 < undeath> ok, I found the reason we're seeing different behaviour 05:59 < undeath> py2 isn't entirely sure what it's doing itself :/ 06:00 < undeath> getpass is a different function depending on the shell you're on and its features 06:00 < undeath> getpass 06:00 < undeath> 06:00 < undeath> this one returns unicode 06:00 < undeath> >>> getpass 06:00 < undeath> 06:00 < undeath> this one however returns str 06:01 < undeath> looks like we need a isinstance check, at least in py2 06:02 < waxwing> yes, the above log is from the interpreter and as i said, it's with the builtins import on both sides 06:02 < waxwing> yeah i saw in the getpass code it has several different versions, makes this tricky 06:03 < waxwing> re: needing is instance check in py2, ok but don't forget in py3 it actually crashes :) 06:04 < waxwing> (which doesn't mean it's entirely correct in py2 ofc) 06:04 < undeath> py3 reliably returns str 06:04 < undeath> for both of those getpass variants 06:05 < undeath> so in py3 we can _probably_ safely use .encode('utf-8') 06:05 < waxwing> ah ofc, yes makes sense 06:07 < undeath> password encoding in py2 may be particularly messy 06:08 < undeath> I assume getpass returns an encoded str in whatever charset the terminal is configured 06:08 < undeath> or did in the past at least in joinmarket 06:08 < undeath> so people that have a non-utf8 terminal may run into trouble 06:09 < undeath> that's just because python's encoding handling wasn't very portable at all 06:09 < undeath> but that does probably deserve a note in the release notes 06:19 -!- deafboy [quasselcor@cicolina.org] has quit [Remote host closed the connection] 06:19 -!- deafboy [quasselcor@cicolina.org] has joined #joinmarket 06:30 < waxwing> undeath, i'm a bit lost though on what to do. using password.encode('utf-8') works on py2 and py3. but is the problem that it might not be compatible? 06:30 < waxwing> i mean obv i can check. 06:30 < waxwing> might there be a case for using ascii encoding, i.e. forcing users not to use unicode, just to be conservative? 06:31 < undeath> well, when creating the files after #241 they will always be using utf-8 06:31 < undeath> trouble is with older wallets 06:32 < undeath> in py2 you should only do .encode() if the password is unicode 06:32 < undeath> otherwise this will raise an exception for non-ascii characters 06:33 < undeath> maybe the most failsafe check would be if not isinstance(password, bytes): password = password.encode('utf-8') 06:34 < undeath> that should work for all cases in py2 and py3 06:36 < waxwing> that sounds like it makes sense, but i want to understand your previous statement, and i don't quite: you're saying, in Py2, if a user enters a password which is unicode, which Py2 treats as a separate type, then .encode('utf-8') needs to be called to convert to bytes. But if the password is not unicode and you do .encode('utf-8') an exception will be thrown? 06:36 < waxwing> so the troublesome part of that is 'if a user enters a password which is unicode' - what are the precise circumstances that lead to that? 06:36 < undeath> yes 06:37 < waxwing> something to do with the terminal? like, if i just enter ascii characters for my password here, it accepts .encode('utf-8') ok. 06:37 < undeath> well, we saw above, fallback_getpass will return unicode 06:37 < waxwing> hmm, "here" happens to be ubuntun 1604 06:37 < waxwing> i see. so it uses fallback for my linux terminal? 06:37 < undeath> no, usually it will use unix_getpass 06:38 < undeath> it was using fallback_getpass for me in the pycharm shell 06:40 < waxwing> right, so it just depends, i see, interesting. 06:40 < waxwing> so hopefully your isinstance check should cover all cases. 06:41 < waxwing> undeath, could you write your suggestion in the PR? 06:41 < undeath> ok 06:44 < waxwing> undeath, oh also perhaps put it in the support.py getpass call not at the call sites? 06:44 < undeath> yes 06:52 -!- puddinpop [~puddinpop@unaffiliated/puddinpop] has quit [Ping timeout: 268 seconds] 07:56 -!- achow101 [~achow101@unaffiliated/achow101] has quit [Ping timeout: 250 seconds] 07:57 -!- achow101 [~achow101@unaffiliated/achow101] has joined #joinmarket 10:13 -!- mr_paz [~mr_paz@84.39.112.86] has joined #joinmarket 10:58 -!- luke-jr [~luke-jr@unaffiliated/luke-jr] has quit [Excess Flood] 10:58 < Lightsword> waxwing, pushed up some changes 10:58 -!- luke-jr [~luke-jr@unaffiliated/luke-jr] has joined #joinmarket 11:00 < Lightsword> let me know if that fixes the password stuff 11:00 < Lightsword> and the configparser issue 11:07 < undeath> configparser should be part of the standard library 11:07 < undeath> it's just spelt differently 11:25 < waxwing> undeath, configparser isn't recognized for some reason in py3 there, this is after ./install.sh --develop -p python3 11:25 < waxwing> or in python2, i forget which way round. 11:25 < waxwing> yeah py2 11:25 < undeath> yes, it was renamed 11:25 < waxwing> Lightsword, thanks, i know it's a lot of little details - the one right at the end interests me the most right now though. 11:26 < undeath> py2: ConfigParser py3: configparser 11:26 < waxwing> yeah i know. point was it didn't work in py2 as was. 11:26 < undeath> but handling that would not require an extra package 11:26 < waxwing> Lightsword, i'm interested to know specifically about that last one, can we catch builtins.BrokenPipeError and specifically which of the except blocks it should go in. 11:27 < undeath> oh, this is actually what the futures lib recommends 11:27 < undeath> that seems weird 11:45 < Lightsword> yeah, you need it for py2 11:45 < Lightsword> apparently 11:46 < Lightsword> undeath, configparser has different behavior from the py2 builtin 11:46 < Lightsword> since it’s a backport of the py3 version 11:46 < undeath> I see 11:47 < Lightsword> waxwing, I’m not entirely sure what should be done with brokenpipe 11:53 < Lightsword> I’m going to make it so configparser package is only installed for py2 11:57 < waxwing> Lightsword, i found a way that seems to solve it: put the builtin import at the top, and then catch BrokenPipeError before the socket.error catch, and do a conn close and open, as in that socket.error catch. 11:57 < waxwing> Not only do I suspect that's not the best way, but i doubt it's even a good way. anyway, managed to do a coinjoin with that patch, so it's something. 11:57 < Lightsword> waxwing, yeah researching that to see if catching it is safe 11:58 < waxwing> btw anecdotal and all, but over the last few weeks having tried quite a few coinjoins, i think the reliability is *much* higher now after the random-under-max change, so there's some good news. 11:58 < waxwing> i mean it was relatively reliable before, but only in the sense it'd usually finish, but usually with a few "holdouts" so to speak, so N reduced. 11:59 < waxwing> Lightsword, cool. do you think running multiwallet might affect it? i just started doing that. 11:59 < waxwing> i have two different bots (in different directories, and different wallets). 11:59 < Lightsword> not sure on multiwallet 11:59 < waxwing> just occurred to me. i guess i could try it with the other one switched off. 12:01 < Lightsword> waxwing, can you see what bitcoin core logs show when you get that error? 12:01 < Lightsword> the broken pipe error 12:02 < waxwing> you mean the debug.log file? 12:02 < Lightsword> yeah 12:03 < waxwing> i cant see anything at all. does it usually log rpc requests? 12:03 < waxwing> i always get the addtowallet stuff, and the blocks and the p2p stuff. but i don't think rpc? 12:03 < Lightsword> waxwing, put it in debug mode 12:04 < waxwing> ok; so, restart bitcoind with -debug, is that the way? 12:06 < waxwing> -debug=rpc seems to be 12:07 < Lightsword> yeah 12:07 < Lightsword> waxwing, I’m going to push a potential fix for automatic reconnect on pipe errors 12:08 < waxwing> it seems to just be showing "ThreadRPCServer method=listunspent user=bitcoinrpc" for each call, no other info. but i'll try to prompt the error again. 12:11 < Lightsword> https://github.com/JoinMarket-Org/joinmarket-clientserver/pull/241/commits/67befa7f094c51ca8c053dd27122def1faf0ecfa 12:11 < waxwing> ah, k, that makes sense 12:12 < waxwing> it is weird that it was happening most of the time, rather than occasionally 12:12 < Lightsword> waxwing, well on osx you will see tons of ECONNRESET errors 12:12 < waxwing> interesting, just triggered it again and it doesn't show up in the rpc log 12:13 < waxwing> i guess that makes sense, if the connection is broken 12:13 < undeath> are the brokenpipe errors really the problem? since they did not appear before it seems like they are only a symptom 12:13 < Lightsword> there’s a number of brokenpipe error reports with bitcoin core 12:14 < undeath> but they did not happen before, did they? 12:14 < waxwing> going to test with the other bot switched off, see if the problem disappears. my suspicion is i'll still get it. 12:14 < undeath> i mean with joinmarket 12:14 < waxwing> after that i'll apply your patch Lightsword , that should fix it since my half-cocked version already seemed to. 12:16 < waxwing> yeah as suspected, other bot was not causing, still get it. seems to be large majority of time (without any patch) 12:17 < Lightsword> that patch shouldn’t reduce the amount of errors seen, it will just trigger a reconnect automaticallly 12:21 < waxwing> sure, ofc 12:22 < waxwing> btw seems to have worked on first try. i expect that'll be enough, although i certainly share concerns that it's a bit weird. if it's an idiosyncracy of core then so be it. 12:23 < undeath> is there some pattern to when the broken pipe fires? 12:23 < undeath> once after every rpc call or so? 12:24 < waxwing> i think the pattern i saw was it failed twice in a row, specifically on the importaddress for the changeaddress, then i think the third time, it failed a little later. oh i just remember i reported this in the PR. 12:25 < waxwing> and btw from today's set, this one is still not addressed, i'm guessing either Lightsword or arubi may have an idea about it: https://github.com/JoinMarket-Org/joinmarket-clientserver/pull/241#issuecomment-443685259 12:25 < waxwing> but clearly no rush on that. 12:25 < waxwing> i don't really know shell script so can't help. 12:31 < Lightsword> yeah, I think I have a potential fix for that 13:15 -!- mr_paz [~mr_paz@84.39.112.86] has quit [Ping timeout: 272 seconds] 13:16 < Lightsword> waxwing, are there any remaining issues that I need to look at? 13:16 < waxwing> Not right now, at least from me 13:18 < Lightsword> waxwing, there still things needing testing? 13:20 < waxwing> still going to do a bit more regtest testing of tumbler with malicious makers 13:20 < waxwing> but already started doing testing on mainnet 13:20 < waxwing> i think it's mostly done. maybe undeath still has some comments/queries? 13:48 < undeath> what about the defaultconfig encoding (see unresolved question) and the obviously incorrect check in secp256k1_transaction.py (see other unresolved question)? 13:51 -!- mr_paz [~mr_paz@84.39.112.86] has joined #joinmarket 13:51 -!- belcher_ [~user@unaffiliated/belcher] has joined #joinmarket 13:53 -!- beIcher [~user@unaffiliated/belcher] has quit [Ping timeout: 246 seconds] 13:59 < Lightsword> undeath, which defaultconfig issue? 14:00 < undeath> the encoding thing 14:00 -!- beIcher [~user@unaffiliated/belcher] has joined #joinmarket 14:01 < Lightsword> can you link to it? I’m not sure exactly what you’re referring to 14:02 < undeath> https://github.com/JoinMarket-Org/joinmarket-clientserver/pull/241#discussion_r237639216 14:02 -!- belcher_ [~user@unaffiliated/belcher] has quit [Ping timeout: 246 seconds] 14:02 < Lightsword> undeath, I think I already changed that, is it correct now? 14:03 < undeath> oh yes, indeed 14:05 < Lightsword> undeath, secp256k1_transaction.py is fixed as well right? 14:06 < undeath> no 14:07 < undeath> there can be no valid hex-encoded tx hash that has a len other than 64 14:08 < undeath> if there is a test that does otherwise the test is broken 14:35 < Lightsword> kk, testing with it removed 14:36 -!- puddinpop [~puddinpop@unaffiliated/puddinpop] has joined #joinmarket 14:45 < Lightsword> undeath, this look right? https://github.com/JoinMarket-Org/joinmarket-clientserver/pull/241/commits/4a71c791eeedc26bc0f7d7f6477b6951948d298a 14:49 < undeath> address_to_script() returns hex data, hence the tx-hashes should be hex as well 14:50 < undeath> "hash":x*32 14:50 < undeath> changing this to 64 instead should fix the test 14:50 < Lightsword> ah 14:50 < undeath> but the tests looks a little scarry in general 14:51 < undeath> i'm not even sure what exactly it's supposed to test :/ 14:54 < Lightsword> undeath, like that? https://github.com/JoinMarket-Org/joinmarket-clientserver/pull/241/commits/d8f548bad7a22215d35749249d15495a33ca2526 14:55 < undeath> yes, exactly 15:05 -!- undeath [~undeath@hashcat/team/undeath] has quit [Quit: WeeChat 2.3] 16:09 -!- mr_paz [~mr_paz@84.39.112.86] has quit [Ping timeout: 250 seconds] 16:30 < waxwing> my best effort (i can't remember ofc) at interpreting is to say that that test was attempting to get better code coverage of the mktx function, just by triggering an invalid format. 16:31 < waxwing> however, as i said the other day, the very idiosyncratic way that mktx parses its arguments is completely unnecessary, we should at the least change it to parse its arguments as (list of ins, list of outs) rather than the whacky auto-detction of ins and outs. 16:31 < waxwing> but, nbd, ofc, i agree we can insist on the hash length :) 16:34 < Lightsword> waxwing, yeah there’s a lot of refactoring that can be done there, but best to wait on that until after things stabailize with all these py3 changes 16:35 < Lightsword> since most of my conversions were attempting to leave the logic as is, at least as much as makes sense 17:37 -!- takamatsu [~takamatsu@unaffiliated/takamatsu] has quit [Ping timeout: 250 seconds] 17:56 -!- AgoraRelay [~jmrelayfn@p5DE4A76B.dip0.t-ipconnect.de] has quit [Ping timeout: 250 seconds] 18:08 -!- AgoraRelay [~jmrelayfn@p5DE4AAB9.dip0.t-ipconnect.de] has joined #joinmarket 22:45 -!- midnightmagic [~midnightm@unaffiliated/midnightmagic] has quit [Ping timeout: 264 seconds] 22:53 -!- midnightmagic [~midnightm@unaffiliated/midnightmagic] has joined #joinmarket 22:53 -!- takamatsu [~takamatsu@unaffiliated/takamatsu] has joined #joinmarket 23:05 -!- Cory [~Cory@unaffiliated/cory] has quit [Ping timeout: 240 seconds] 23:13 -!- Cory [~Cory@unaffiliated/cory] has joined #joinmarket 23:15 -!- lnostdal [~lnostdal@77.70.119.51] has quit [Quit: https://www.Quanto.ga/]