--- Day changed Tue Nov 20 2018
01:28 -!- belcher [~belcher@unaffiliated/belcher] has quit [Ping timeout: 252 seconds]     
01:41 -!- belcher [~belcher@unaffiliated/belcher] has joined #joinmarket     
07:01 < waxwing> this is my new gpg key btw (old one is expiring early next month), still in the process of updating links. new is signed with old. http://keys.gnupg.net/pks/lookup?op=vindex&fingerprint=on&search=0x141001A1AF77F20B
07:40 -!- mr_paz [~mr_paz@84.39.112.84] has joined #joinmarket     
07:48 < waxwing> i found this the other day (shortly before we had that twisted conversation) https://lwn.net/Articles/692254/ it's pretty high level/discursive but may be interesting
07:49 < waxwing> Lightsword, i just realised, i never checked, were you aware i'd done some porting in the py3 branch on the repo? in particular, although it was a long time ago, i think i did complete jmbitcoin. not to say it should be used, but if you're going through the same process it might be helpful, i don't know.
07:58 -!- GitHub160 [GitHub160@gateway/service/github.com/x-xdzfcnrbvpsepwmr] has joined #joinmarket     
07:58 < GitHub160> [joinmarket-clientserver] AdamISZ pushed 1 new commit to master: https://git.io/fplgH
07:58 < GitHub160> joinmarket-clientserver/master 55c9483 AdamISZ: update to new code signing key for AdamISZ
07:58 -!- GitHub160 [GitHub160@gateway/service/github.com/x-xdzfcnrbvpsepwmr] has left #joinmarket []     
08:13 < Lightsword> waxwing, yeah, I’ve looked at that
08:13 < waxwing> Lightsword, ok, phew, it'd be annoying if you didn't know :)
08:14 < Lightsword> it’s a little bit different since it was in py3 style rather than universal
08:15 < waxwing> ah yeah. btw when i try to run master I get 'no module named builtins'
08:15 < waxwing> is this a case of i'll have to install a couple of packages that are new in the install process?
08:15 < waxwing> i could just run install from scratch but lazy :)
08:16 < waxwing> messing around with gpg is as ... fun ... as it always is
08:16 < waxwing> maybe i just need to install 'future'
08:17 < waxwing> yeah that seems fine, cool
08:21 < Lightsword> waxwing, yeah builtins is part of future
08:22 < Lightsword> btw what’s the point of gpg if we are validating sha256 checksums?
08:22 < waxwing> that's one of those old debates :)
08:23 < waxwing> a hash doesn't tie to an identity, it just checks integrity between two points right
08:23 < waxwing> i guess you can say, diff between trusting github's key and trusting the coder's key?
08:35 < Lightsword> waxwing, well if you already trust the joinmarket code…not too much more to trust a hash within it
08:38 < waxwing> yeah def much less interesting for non-binaries. still there are people using the code that will refuse if the latest commit isn't signed. it's not like everyone can make a judgement purely on reading code.
08:58 < Lightsword> waxwing, yeah, I don’t really understand why we need to gpg verify stuff like openssl if we are doing sha256 hash checks
09:00 < Lightsword> waxwing, btw have you looked into reworking the test suite to use tox?
09:01 < Lightsword> https://tox.readthedocs.io/en/latest/
09:02 < Lightsword> might be better than the test/run_tests.sh script
09:03 < arubi> the hash check for external deps is used for deciding whether we need to download the file or not.  I agree it's excessive to gpg verify afterwards in this case
09:07 < Lightsword> waxwing, btw do you think you can do the conversion for jmbitcoin yourself? I’ve made a few attempts at it but I don’t really have a good enough understanding of that part of the codebase to know which parts should be bytes and which should be unicode
09:09 < Lightsword> you basically just stick these lines at the top of each file in jmbitcoin and rework as needed until everything works normally and tests pass https://github.com/JoinMarket-Org/joinmarket-clientserver/blob/master/jmdaemon/jmdaemon/irc.py#L1-L3
09:18 < waxwing> Lightsword, ok, i'll give it a shot. it'll take me a while though.
09:18 < waxwing> well, i shouldn't say that, i don't actually know :) we'll see. i'll ask you if i'm unsure.
09:20 < Lightsword> waxwing, the big issue I think is that there aren’t obvious boundaries between bytes and unicode, the code assumes they are interchangable
09:22 < Lightsword> stuff like this for instance should be bytes I think https://github.com/JoinMarket-Org/joinmarket-clientserver/blob/master/jmbitcoin/jmbitcoin/secp256k1_main.py#L275 but a lot of the tests seem to call functions with incompatible types
09:24 < Lightsword> there’s also quite a few areas where it looks like you would be working with what would be unicode(after adding the future imports) but should be bytes since you otherwise get bytes that can’t be encoded to unicode strings properly
09:27 < Lightsword> I would generally suggest trying to make most of the internals operate on bytes and just ensure that when ingesting/outputting unicode data that it’s appropriately converted, bytes types are generally much easier to work with for binary formats
09:27 < Lightsword> waxwing, btw have you used struct before? https://docs.python.org/3/library/struct.html
09:30 < Lightsword> I think using it for some of the parsers might make the code cleaner
09:31 < waxwing> yes i've seen it
09:35 < Lightsword> waxwing, it’s especially useful for conversion between integers and bytes, I’d recommend eliminating all usage of chr() and ord() and replacing with struct
09:40 < Lightsword> a big issue with jmbitcoin is that it uses integer conversions that in python3 are designed for unicode strings and not bytes, it only works in python2 because python2 strings are interchangeable with bytes, the future library imports however force what is essentially the backported python3 bytes type to be used
10:08 -!- undeath [~undeath@hashcat/team/undeath] has joined #joinmarket     
11:54 -!- mr_paz [~mr_paz@84.39.112.84] has quit [Ping timeout: 268 seconds]     
11:57 < Lightsword> waxwing, I’m making another attempt at jmbitcoin but by refactoring rather than trying to do the drop in style I was before, what’s the purpose of this exactly? https://github.com/JoinMarket-Org/joinmarket-clientserver/blob/aeb45412317be8ba5082db47164a9666dec9c2fb/jmbitcoin/jmbitcoin/secp256k1_main.py#L179
11:58 < Lightsword> is there a reason leading zeros can’t just be stripped?
11:58 < undeath> because the checksum would not match?
11:59 < undeath> and because you would drop the version byte and use the next one if it was 0x00
12:01 < undeath> the real question is, isn't something wrong with changebase() that it doesn't retain leading zero bytes?
12:01 < Lightsword> undeath, I’m trying to get rid of changebase since it’s a source of lots of problems
12:02 < waxwing> Lightsword, undeath in other news the latest update breaks coinjoins :(
12:02 < waxwing> what's the right way to revert?
12:02 < undeath> :(     
12:02 < Lightsword> waxwing, what specifically is broken with it?
12:02 < waxwing> basically i'm observing the latest commit prevents joins from completing. i'm willing to bet it's an incompatibility with the new commands stuff and the old.
12:03 < undeath> the coincurve or the py3 one?
12:03 < waxwing> prob using Unicode() does it but just a guess. for now need to revert.
12:03 < Lightsword> give me a few minutes to see if I can find a fix
12:03 < undeath> oh yeah, having the same
12:03 < waxwing> seen this kind of thing happen before. need test suite to test the delta with bots of old type. but, for another time.
12:03 < Lightsword> how do you reproduce exactly?
12:03 < waxwing> guys before you go off, please tell me how to revert correctly :)
12:03 < waxwing> need to minimise the time that's up on master
12:04 < undeath> jmclient.podle.PoDLEError: Failed to deserialize, wrong format
12:04 < undeath> git revert [hash] ?
12:04 < waxwing> reproduce: try to do joins on master
12:04 < waxwing> undeath, just that? then push master?
12:04 < undeath> probably with git amend -S
12:05 < undeath> probably with git commit --amend -S
12:05 < undeath> so it gets sighed
12:05 < undeath> *signed
12:05 < waxwing> right. and i have two commits, the code and the merge. and i have one after. can i still use `revert`?
12:05 < undeath> I think so
12:05 < waxwing> looks like it from help
12:06 < Lightsword> does changing separator='|' to separator=u’|’ do anything on this line? https://github.com/JoinMarket-Org/joinmarket-clientserver/blob/aeb45412317be8ba5082db47164a9666dec9c2fb/jmclient/jmclient/podle.py#L165
12:07 < waxwing> reverting a merge apparently causes some trickiness
12:07 < undeath> Lightsword: https://gist.github.com/undeath/81ab81bd8b4c4632a0cd35695358d362
12:08 < undeath> waxwing: did you revert the merged commit or the merge-commit?
12:08 < undeath> i think you need to revert the merged commit
12:08 < waxwing> i didn't revert anything yet. i tried, but it complained that one of the two commits is a merge.
12:09 < undeath> what command did you run?
12:09 < undeath> git revert c08b69ba7558f705614510b22949827626aa92ff
12:09 < undeath> that should do the job i think
12:11 < waxwing> undeath, yes seems to do it, thanks. well, i'll just sanity check.
12:12 < Lightsword> undeath, add a print(repr(ser_rev)) above this line https://github.com/JoinMarket-Org/joinmarket-clientserver/blob/aeb45412317be8ba5082db47164a9666dec9c2fb/jmclient/jmclient/podle.py#L166
12:12 < Lightsword> and see what happens
12:13 < undeath> k     
12:15 < undeath> waxwing: do you want me to revert it or did it work?
12:15 < waxwing> works, i'm just running test locally. just a minute or so.
12:15 < undeath> ah, alright
12:15 < waxwing> thanks
12:15 < Lightsword> so how exactly do you reproduce this locally?
12:16 < undeath> waxwing: did you test this with the ygrunner.py?
12:17 -!- GitHub3 [GitHub3@gateway/service/github.com/x-ahwsdjcgpzvzkgwt] has joined #joinmarket     
12:17 < GitHub3> [joinmarket-clientserver] AdamISZ pushed 1 new commit to master: https://git.io/fplpJ
12:17 < GitHub3> joinmarket-clientserver/master 2eb3a87 AdamISZ: Revert "Python 3 style conversion"...
12:17 -!- GitHub3 [GitHub3@gateway/service/github.com/x-ahwsdjcgpzvzkgwt] has left #joinmarket []     
12:18 < waxwing> Lightsword, well it's more difficult than it might be. i blame myself for this, since i've seen such problems occur before (admittedly, a long time ago). any change that impacts inter-bot communication at all has to be tested on old vs new versions.
12:19 < Lightsword> waxwing, so it only comes up when mixing bot types?
12:19 < waxwing> yes, def, i tried several times with the new commit to be sure. all cases all counterparties refused to respond to !auth.
12:19 < waxwing> whereas not the case pre-that commit.
12:20 < undeath> isn't the data send over irc some custom stuff not tied to twisted?
12:20 < waxwing> yes nothing to do with twisted. it's just a text protocol over IRC. and yes the podle message has a specific format, so i agree it's probably that. sorry haven't actually tried to test anything yet, was just focused on removing it for now.
12:21 < waxwing> the problem is such a failure is somewhat cascading; if you had a mix of both types of bots in meaningful fractions, it would not only cause failed txs but also use up podle commitments so, super important to get rid of it asap.
12:21 < undeath> but it seems weird because that should not make the data sent between the local twisted daemons invalid
12:22 < undeath> are you sure it's not caused by the coincurve pr?
12:22 < undeath> otoh, that shouldn't affect that specific exception
12:22 < waxwing> yeah was just using master
12:23 < waxwing> i'm not ready to run coincurve live yet, have barely tested it
12:23 < waxwing> (although it's really far less likely to cause an issue)
12:23 < waxwing> but since it involves, like, bitcoin keys, it has a higher bar :)
12:23 < Lightsword> so how do I actually reproduce this?
12:23 < Lightsword> without running on live network
12:24 < waxwing> yeah i get it Lightsword , was getting to responding :) problem is it's not an easy answer.
12:24 < Lightsword> oh, so you’ve only seen it on live?
12:24 < undeath> you need to run a taker + maker locally with mixed versions
12:24 < waxwing> right. because the test suite does not test different versions in the same pit, as i said.
12:24 < waxwing> i think the simplest way is: run test/ygrunner.py -s to get makers on one version.
12:24 < waxwing> then do a git command to shift versions before running the taker.
12:25 < waxwing> well and vice versa also. yeah that's pretty easy, not as bad as i thought. i can try it now.
12:28 < Lightsword> “python test/ygrunner.py -s” doesn’t seem to actually do anything
12:30 < waxwing> pytest --btcconf=/home/adam/bitcoin.conf --btcroot=/home/adam/code/bitcoin-0.17.0/bin/ --btcpwd=123456abcdef --nirc=2 test/ygrunner.py -s or similar :)
12:31 < waxwing> there's some details but i think you already saw them in TESTING.md . hmm not sure it's up to date but i guess basically
12:33 -!- arubi [~ese168@gateway/tor-sasl/ese168] has quit [Remote host closed the connection]     
12:34 -!- arubi [~ese168@gateway/tor-sasl/ese168] has joined #joinmarket     
12:40 < Lightsword> waxwing, so I run one instance like that right? what do I run the other as?
12:40 < Lightsword> same?
12:41 < undeath> ygrunner will set up irc + maker
12:41 < undeath> then you run a taker manually
12:41 < Lightsword> how do I do taker?
12:41 < undeath> eg using joinmarket-qt or sendpayment.py
12:41 < waxwing> on a separate prompt, yeah
12:41 < waxwing> confirmed the error  raise PoDLEError("Failed to deserialize, wrong format")
12:42 < waxwing> so i can check it now. this is with new-style on taker side. with new-style on maker i think it actually crashes in a really weird way, but get back to that later.
12:42 < waxwing> (feel free to disregard that, it was something confusing)
12:42 < Lightsword> how do I setup a testing wallet for this?
12:43 < undeath> ygrunner will set up a wallet for you
12:43 < undeath> it prints the hex seed for that
12:43 < waxwing> here's a stack trace of that error https://0bin.net/paste/vq+eHfEnB2emIKTW#we10wEppOoi9nq-KYqraNFsdylm7R3CNXUIy0OFk5cf
12:43 < waxwing> Lightsword, yeah there's a note about it at the end of TESTING.md
12:43 < waxwing> by default it populates a wallet for a hex seed with some coins in first two mixdepths
12:44 < undeath> you can run joinmarket with that hex seed in place of a wallet name
12:44 < waxwing> yeah it could be that separator is not '|', indeed, i can try that now
12:45 < undeath> insert the print Lightsword mentioned above and check what it looks like
12:47 < waxwing> ok .. should i add that print for both sides though? this is where it gets tricky with these kinds of problems :)
12:49 < undeath> well, it won't break anything :)
12:49 < undeath> but the problem happens on the decoding side
12:49 < undeath> we can go from there
12:50 < Lightsword> did you get a print?
12:51 < waxwing> sec     
12:52 < waxwing> https://0bin.net/paste/MvvLYy25YivlZlhi#TZj-DEeHnjG/F6t7bZmRJxg2PAHiAZmgkwiPHXdRRcl
12:52 < undeath> heh     
12:53 < Lightsword> so I’m trying “python sendpayment.py 97605babde471818998e4d395240612a 100000 2NEWtGgyNpJYNLE5WXyTbDjsrvbjKve8Ezt -N 2” but I get “ERROR not enough liquidity in the orderbook n=2 suitable-counterparties=0 amount=100000 totalorders=0”
12:53 < Lightsword> but not tracebacks
12:54 < undeath> are you running sendpayment with the regtest config?
12:54 < Lightsword> yeah, what’s correct way to call sendpayment.py with that?
12:55 < undeath> make sure it's called joinmarket.cfg
12:55 < waxwing> Lightsword, -N 2 has to come before the wallet seed
12:55 < undeath> shouldn't matter
12:55 < waxwing> it's python wallet-tool (options) walletname amount address
12:55 < waxwing> oh, didn't know
12:55 < waxwing> Lightsword, oh it's too small
12:56 < waxwing> their minsize is rather large iirc
12:58 < waxwing> yeah it's 1.5 million
13:01 < Lightsword> so this crashes the old makers it seems right?
13:02 < undeath> new makers
13:03 < Lightsword> when you run multiple instances of different version out of the same tree by using git revert are they sharing an AMP server?
13:04 < undeath> i guess that's possible. you probaby need different daemon ports
13:05 < waxwing> undeath, you set it up to tick the port forwards
13:05 < waxwing> here's (for reference) what it looks like with both sides on the old code: https://0bin.net/paste/5CuJpznh0lmn+mw4#-dLnDLLWYdYvo9CA9QINDvcPvk9fGE9cXG4ASPRVaCj
13:05 < undeath> but the client will still connect to the first port?
13:07 < waxwing> undeath, no, in the same process, it uses the same port variable
13:07 < undeath> ok, nice
13:07 < waxwing> client_protocol line 502 or so
13:07 < waxwing> yeah but it's a damn good point if we used separate processes
13:09 < waxwing> been a long day, i was actually going to give it a rest after fiddling with gpg for hours, but after i started looking jmbitcoin py3 stuff i became suspicious about whether what we'd already done would be OK and then this happened :) still no huge disaster, wasn't up for too long.
13:11 < Lightsword> yeah, testing is def tricky
13:53 < Lightsword> ok, I found where the bug is I think, it’s in the message decryption routine
14:26 < undeath> how would message decryption be influenced by changes to the protocol?
14:39 < Lightsword> undeath, pushing up fix, one sec
14:43 -!- GitHub71 [GitHub71@gateway/service/github.com/x-sacbmrlsegbtzjjy] has joined #joinmarket     
14:43 < GitHub71> [joinmarket-clientserver] jameshilliard opened pull request #224: Python 3 style conversion (master...future-fix) https://git.io/fp8q3
14:43 -!- GitHub71 [GitHub71@gateway/service/github.com/x-sacbmrlsegbtzjjy] has left #joinmarket []     
14:44 < undeath> I see, isinstance was the problem
14:45 < undeath> that part of the code always seemed particularly messy to me
14:46 < undeath> "autodetecting" hex/non-hex encoding of input
14:46 < undeath> I'd really love to have that removed
14:46 < Lightsword> undeath, there were basically 4 issues
14:47 < Lightsword> and yeah, the autodetection in general is super messy and a source of a number of issues it seems
14:47 < Lightsword> I commented all the changes here https://github.com/JoinMarket-Org/joinmarket-clientserver/pull/224
14:48 < Lightsword> the encrypt and decrypt routines don’t really work properly unless operating on bytes types
14:48 < undeath> (which is fair)
14:49 < undeath> so with those changes you were able to complete a mixed regtest cj?
14:49 < Lightsword> undeath, testing again now, can you also check it?
14:50 < undeath> wonder why it wasn't causing problems with non-mixed participants
14:50 < Lightsword> it was
14:51 < undeath> oh, just nobody had tested it?
14:51 < Lightsword> yeah…I had been testing with test/run_tests.sh
14:51 < Lightsword> which wasn’t picking that up
14:51 < undeath> we have a test for cj but that (on purpose) bypasses the whole communication part
14:52 < Lightsword> yeah, we really probably should have an automated test that doesn’t bypass the communications
14:57 < Lightsword> undeath, I had done the bytes encoding in the encryption tests but had forgotten to check the actual messaging stuff to ensure it also was encoded to bytes
14:57 < Lightsword> the other 2 issues were only found after
15:13 < Lightsword> anyways I’ve tested this now with both combinations of maker and taker on new and old versions
15:14 < undeath> cool
15:14 < undeath> I can't imagine how different versions could cause problems unless the client is running a different version than the daemon
15:20 < Lightsword> undeath, yeah I was somewhat skeptical that was the issue, tricky part with these conversions is making sure all the codepaths were tested
16:16 -!- GitHub149 [GitHub149@gateway/service/github.com/x-vbfmobgxbnxhvczp] has joined #joinmarket     
16:16 < GitHub149> [joinmarket-clientserver] undeath opened pull request #226: fix crash on bad podle revelation (master...fix-podle-crash) https://git.io/fp8Zl
16:16 -!- GitHub149 [GitHub149@gateway/service/github.com/x-vbfmobgxbnxhvczp] has left #joinmarket []     
16:27 < waxwing> was debating whether to try to be surreptitious about that :) (but almost certainly wasn't; it's an awful mistake to have not been fixed for so long)
16:31 < undeath> it's a DoS :/
16:45 < waxwing> yes that's what i meant. we've had similar things in the past. but this is fully public now. forgive me if i don't respond promptly but it's very late here.
17:12 -!- AgoraRelay [~jmrelayfn@p5DE4AD4F.dip0.t-ipconnect.de] has quit [Ping timeout: 244 seconds]     
17:14 -!- undeath [~undeath@hashcat/team/undeath] has quit [Quit: WeeChat 2.3]     
17:27 -!- AgoraRelay [~jmrelayfn@p5DE4AA4D.dip0.t-ipconnect.de] has joined #joinmarket     
18:11 -!- d3spwn_ [~Jimmy@a83-161-157-200.adsl.xs4all.nl] has quit [Read error: Connection reset by peer]     
18:11 -!- d3spwn [~Jimmy@83.161.157.200] has joined #joinmarket     
18:37 -!- arubi [~ese168@gateway/tor-sasl/ese168] has quit [Remote host closed the connection]     
18:38 -!- arubi [~ese168@gateway/tor-sasl/ese168] has joined #joinmarket