--- Day changed Fri Nov 23 2018
01:16 -!- GitHub168 [GitHub168@gateway/service/github.com/x-siaxhkcmnuzftesp] has joined #joinmarket     
01:16 < GitHub168> [joinmarket-clientserver] jameshilliard opened pull request #229: WIP: Convert jmbitcoin to py3 style (master...future) https://git.io/fpR8l
01:16 -!- GitHub168 [GitHub168@gateway/service/github.com/x-siaxhkcmnuzftesp] has left #joinmarket []     
01:31 < waxwing> Lightsword, so does 229 include the 223 (coincurve) change, so we no longer need 223? i was going to test the latter today. hmm maybe i should still just do that, obv the 229 thing is going to take a while.
01:36 < Lightsword> waxwing, well I would merge 223 before 229 since 229 is a massive change
01:36 < Lightsword> 229 is also kinda broken
01:38 < Lightsword> I’ve refactored some of the decoder functions but the codebase really seems to rely on the automatic type conversions a lot
01:38 < waxwing> Lightsword, yes it was more a git question than a code question, i was confused how 223 was already in 229, i guess it doesn't matter right, if you merge 223 then 229 merge still works. just confused.
01:38 < Lightsword> waxwing, yeah I just based 229 off of 223
01:39 < Lightsword> you can merge 223 whenever, I’ll just rebase 229 off of master whenever you do
01:40 < Lightsword> I’m thinking I’m probably just going to have to convert jmclient at the same time as jmbitcoin, I’ve been attempting to keep them separate but that’s proving to be difficult
01:40 < waxwing> afk a few hrs.
01:40 < Lightsword> yeah, I’m going to bed for now
01:41 < Lightsword> will look at it again in the morning
05:04 -!- undeath [~undeath@hashcat/team/undeath] has joined #joinmarket     
05:23 -!- GitHub189 [GitHub189@gateway/service/github.com/x-zewqjzqiepyxplax] has joined #joinmarket     
05:23 < GitHub189> [joinmarket-clientserver] AdamISZ pushed 1 new commit to master: https://git.io/fpRKs
05:23 < GitHub189> joinmarket-clientserver/master 361cdc2 AdamISZ: Merge #227: add full coinjoin test...
05:23 -!- GitHub189 [GitHub189@gateway/service/github.com/x-zewqjzqiepyxplax] has left #joinmarket []     
05:23 -!- GitHub39 [GitHub39@gateway/service/github.com/x-gmlzipcjrdqnfwbt] has joined #joinmarket     
05:23 < GitHub39> [joinmarket-clientserver] AdamISZ closed pull request #227: add full coinjoin test (master...full-coinjoin-test) https://git.io/fp4l1
05:23 -!- GitHub39 [GitHub39@gateway/service/github.com/x-gmlzipcjrdqnfwbt] has left #joinmarket []     
05:24 < waxwing> As noted in the commit message, that's just the test script itself, it's not added to travis; since i want to use it to test stuff on command line i added it now, can look into how to make it detect all possible errors later.
05:41 -!- mode/#joinmarket [+o waxwing] by ChanServ     
05:41 -!- waxwing changed the topic of #joinmarket to: Welcome to JoinMarket: Increase Fungibility and Subsidise Your Fees | https://github.com/Joinmarket-Org/joinmarket-clientserver | @joinmarket r/joinmarket | Live View: https://joinmarket.me/ob | Latest release 0.4.2 | Also on ssl 6dvj6v5imhny3anf.onion:6697 #joinmarket
05:42 -!- mode/#joinmarket [-o waxwing] by ChanServ     
05:42 < waxwing> belcher, can you update the topic on cgan? cheers
06:22 < belcher> done
06:24 -!- belcher_ [~user@unaffiliated/belcher] has quit [Ping timeout: 268 seconds]     
06:26 < waxwing> cheers
07:09 -!- belcher_ [~user@unaffiliated/belcher] has joined #joinmarket     
08:04 < waxwing> undeath, so it doesn't build libgmp then? that's what was on my mind, i remember building it taking ages
08:05 < waxwing> if not, yeah who cares
08:05 < undeath> no, coincurve only needs the headers to compile
08:05 < undeath> for running it, it will use the precompiled binaries that are installed alongside the dev package
08:06 < waxwing> ok good. so about your other Q, i saw it, but it's going to take me a while to even remember what i was investigating. i remember i had a specific test scenario i was looking at and seeing it creating multiple irc nicks in a situation i believe it didn't need to. sorry, can't answer immediately.
08:07 < undeath> no heat, I just noticed it while browsing through the open issues
08:07 < waxwing> maybe it was the electrum plugin. hmm. it's weird though, it doesn't do that for tumbler which is kind of a similar scenario, a series of joins with one wallet/instance.
08:17 -!- GitHub155 [GitHub155@gateway/service/github.com/x-cxzqlrvmvkwrabuy] has joined #joinmarket     
08:17 < GitHub155> [joinmarket-clientserver] AdamISZ pushed 2 new commits to master: https://git.io/fpRdn
08:17 < GitHub155> joinmarket-clientserver/master abad597 James Hilliard: Replace secp256k1-py with coincurve
08:17 < GitHub155> joinmarket-clientserver/master 974b598 AdamISZ: Merge #223: Replace secp256k1-py with coincurve...
08:17 -!- GitHub155 [GitHub155@gateway/service/github.com/x-cxzqlrvmvkwrabuy] has left #joinmarket []     
08:17 -!- GitHub51 [GitHub51@gateway/service/github.com/x-vlniglxztlhmzvhb] has joined #joinmarket     
08:17 < GitHub51> [joinmarket-clientserver] AdamISZ closed pull request #223: Replace secp256k1-py with coincurve (master...coincurve) https://git.io/fpcdl
08:17 -!- GitHub51 [GitHub51@gateway/service/github.com/x-vlniglxztlhmzvhb] has left #joinmarket []     
09:38 -!- arubi_ [~ese168@gateway/tor-sasl/ese168] has joined #joinmarket     
09:38 -!- arubi [~ese168@gateway/tor-sasl/ese168] has quit [Ping timeout: 256 seconds]     
10:26 < undeath> Lightsword: welcome to another episode of: "we're doing magic"!
10:26 < Lightsword> would it potentially make sense to use an external library such as https://github.com/petertodd/python-bitcoinlib for handling serialization?
10:27 < undeath> tbh, it would make sense using a dedicated class for creating transactions instead of a python dict
10:28 < undeath> but lets not get there now
10:28 < Lightsword> yeah, guess probably shouldn’t do any of that until after conversion
10:30 < Lightsword> undeath, what should I do to fix that deserialize function exactly?
10:31 < undeath> if hex-encoded data is input, the output should only contain hex-encoded str, if input data is bytes output data should only contain bytes, no hex stuff
10:31 < undeath> that's how it currently behaves
10:32 < undeath> if you can de-mangle the hex-encoding stuff into a different function I can make sure every place in the existing code calls the correct function
10:32 < Lightsword> undeath, so I guess I need to add json_changebase back
10:32 < undeath> well, something like that I guess :/
10:33 < undeath> the transaction dict is pretty fixed, so you could just check for the specific keys that need re-encoding
10:33 < waxwing> good observation. we're not handling arbitrary dicts
10:34 < undeath> also there is somewhere a byte order problem, according to the tests
10:34 < waxwing> (albeit many of the fields are optional)
10:35 < Lightsword> one issue I was hitting is that when passing data between jmclient and jmbitcoin is that the non-binary string types essentially come in as bytes types I think
10:35 < Lightsword> yeah, not sure all the changes I made with struct are correct
10:35 < Lightsword> was mostly just easier to read
10:37 < undeath> well, jmclient uses str for everything
10:37 < undeath> so yes, you will only get str types probably
10:38 < Lightsword> I thought there was some unicode mixed in with jmclient
10:38 < undeath> not that I know of, other than that you recently added
10:40 < Lightsword> hmm, thought I hadn’t really touched jmclient much
10:41 < undeath> correct, therefore the amount of unicode used in jmclient is approaching zero
10:42 < Lightsword> well some stuff has unicode_literals imports like https://github.com/JoinMarket-Org/joinmarket-clientserver/blob/master/jmclient/test/test_wallet.py
10:42 < Lightsword> and https://github.com/JoinMarket-Org/joinmarket-clientserver/blob/master/jmclient/jmclient/wallet.py
10:43 < undeath> wallet has clean python3 ready code :)
10:43 < undeath> everything else doesn't
10:45 < Lightsword> ah     
10:45 < undeath> and the wallet isn't using unicode for obvious reasons :)
10:46 < Lightsword> shouldn’t it be using unicode for non-binary data types?
10:47 < undeath> it's operating on raw data unless in the legacy methods inherited from the old implementation
10:48 < undeath> but indeed, I didn't use any unicode, I only differentiated between str and bytes
10:48 < Lightsword> well anything without a “b” prefix is unicode in that file I think
10:49 < undeath> b"" should be the same as ""
10:49 < Lightsword> since you have the unicode_literals import
10:49 -!- arubi_ is now known as arubi     
10:49 < Lightsword> b"" is only the same when not using unicode_literals AFAIK
10:49 < undeath> no, it should be the other way round I think
10:49 < Lightsword> otherwise "" is actually u""
10:50 < undeath> "" is u"" only with unicode_literals
10:50 < Lightsword> yeah, but wallet.py has unicode_literals
10:50 < undeath> yes     
10:50 < undeath> so "" == u""
10:50 < undeath> ?     
10:50 < Lightsword> yeah
10:50 < Lightsword> think so
10:50 < undeath> so that's alright
10:51 < undeath> where bytes are used it has b""
10:51 < Lightsword> yeah, so that’s fine, I think there may be some weird conversion boundary behavior going on, I’m thinking I should convert the rest of jmclient to use the full backport imports
10:52 < Lightsword> from builtins import * makes it try and emulate essentially as much py3 behavior as possible
10:52 < undeath> for wallet that should be a no-op drop-in
10:53 < Lightsword> yeah, probably will be some differences though
10:53 < undeath> does that also work for dict methods btw?
10:53 < undeath> like dict.iteritems()
10:55 < Lightsword> it does do some stuff with dicts
10:55 < Lightsword> http://python-future.org/compatible_idioms.html#dictionaries
10:56 < undeath> ok, so it requires shim code
10:56 < Lightsword> may require from future.utils import iteritems though
10:56 < undeath> not only import, also using it
10:56 < Lightsword> yeah, probably
10:57 < Lightsword> for key in heights.values(): should work though
10:57 < undeath> but is inefficient
10:57 < Lightsword> as long as the builtins import * is there
10:57 < undeath> on larger data sets
10:57 < Lightsword> it says “# efficient on Py2 and Py3”
10:58 < undeath> oh, but only for values()
10:58 < undeath> not iteritems()
10:58 < undeath> i think that's more commonly used throught the codebase
10:59 < Lightsword> yeah, have to do for (key, value) in iteritems(heights):
10:59 < Lightsword> with from future.utils import iteritems
10:59 < Lightsword> I think
10:59 < undeath> that's what the doc says
10:59 < undeath> would be to easy to have things just work :(
11:00 < Lightsword> we could convert to for (key, value) in heights.items(): if we don’t care about the efficiency that much
11:01 < Lightsword> since that will work find on py3
11:01 < undeath> there are some fairly big dicts I think
11:01 < undeath> and anyway, it's not a no-op
11:02 < Lightsword> yeah, I’ll probably just use from future.utils import iteritems
11:02 < undeath> yeah
11:57 < waxwing> so I'll try to start contributing on this tomorrow probably; so far I've only got as far as the first test breakage in test_tx_serialization (some kind of endian business).
11:58 < waxwing> btw coincurve seems all good here, if anyone is of a mind, you can try running master and you'll need to install the coincurve package (pip install coincurve, or just re-run the installation script).
11:58 < waxwing> oh that's a thought, did anyone actually run the installation from scratch with coincurve? doh, i forgot
12:04 -!- lnostdal [~lnostdal@77.70.119.51] has quit [Ping timeout: 272 seconds]     
12:05 -!- lnostdal [~lnostdal@77.70.119.51] has joined #joinmarket     
12:06 < Lightsword> waxwing, I’ve done fresh installs with install.sh which seemed to work fine
12:08 < arubi> technically, it's being run on a fresh machine by travis
12:09 < arubi> but I don't dismiss running checking it manually on your own host, without cache
12:11 < arubi> actually, might be intersting to add a job flavor for travis with the cache disabled.  also we can set it to "allow fail" so it doesn't immediately paints the PR red if it's just some network issue
12:19 < undeath> Lightsword: don't switch from lists to str concatenations in serialize(). Since strings are immutable in python that's grossly inefficient
12:26 < Lightsword> undeath, is it actually worse with how we use it? the list of bytes would still need to be combined at the end of the function
12:26 < undeath> yes     
12:26 < undeath> because str.join only once creates a new str obj
12:26 < undeath> while every o += "something" creates a new str obj as well
12:27 < Lightsword> we would be using reduce(lambda x, y: x + y, o, bytes()) since ''.join(o) is py2 style
12:28 < undeath> doesn't py3 have a join equivalent?
12:28 < undeath> b''.join
12:28 < undeath> that exists
12:29 < Lightsword> I think it’s problematic when working with bytes and not strings
12:30 < undeath> why would it?
12:30 < undeath> it's exactly the same thing with bytes instead of str
12:30 < Lightsword> https://bugs.python.org/issue8662
12:31 < Lightsword> seems bytes gets interpreted as integers in some circumstances
12:31 < undeath> that's only the case if you pass a bytes object to join
12:31 < undeath> but join expects an iterable
12:31 < undeath> so the bug reporter is just doing something wrong
12:32 < Lightsword> the other main reason for that change was readability
12:33 < undeath> please don't.
12:33 < undeath> there is really no good reason not to use a list in that case
12:33 < Lightsword> looking at other options
12:34 < undeath> what do you have against join?
12:34 < undeath> I don't really understand
12:34 < undeath> it's how you do such things in python
12:34 < Lightsword> it’s annoying mostly when adding debug prints
12:35 < undeath> use a debugger? :P
12:35 < Lightsword> ok, so I think what I actually should be using is BytesIO
12:36 < undeath> and that's going to be less intrusive than o.append() ?
12:37 < Lightsword> I think so, that’s what this uses https://github.com/petertodd/python-bitcoinlib/blob/master/bitcoin/core/serialize.py
12:37 < Lightsword> it’s probably more efficient as well
12:39 < undeath> based on what?
12:39 < undeath> gut feeling?
12:39 < Lightsword> the semantics look cleaner
12:40 < Lightsword> and I think that’s what bytes IO is designed for
12:41 < undeath> idk, I've never seen BytesIO being used for this purpose. But it's surely better than ordinary concatenation
12:41 < Lightsword> I think it will work better for deserialize() as well
12:44 < Lightsword> should be able to use it to eliminate this hack https://github.com/JoinMarket-Org/joinmarket-clientserver/blob/master/jmbitcoin/jmbitcoin/secp256k1_transaction.py#L53-L56
12:44 < Lightsword> since io streams have built in seeking essentially
12:45 < undeath> ah, that does look handy indeed
12:46 < undeath> that is, if BytesIO actually has "insert" and not just "overwrite"
12:47 < Lightsword> you do https://docs.python.org/3/library/io.html#io.BufferedIOBase.write I think
12:49 < undeath> i'd expect that to overwrite, based on my experience how files work
12:49 < Lightsword> eh? files are typically append
12:49 < undeath> writing to a random position in a file is not append
12:50 < Lightsword> you just do something like this f.write(struct.pack(b'<H', i)) with f being the BytesIO object
12:51 < Lightsword> so each append is just a write essentially
12:52 < undeath> then no tracking of a seek position is needed?
12:53 < Lightsword> right
12:53 < undeath> that means the variable pos isn't needed at all?
12:53 < Lightsword> same with reads, you read a certain length and your seek position is updated automatically
12:54 < Lightsword> right
12:54 < undeath> I mean in the current code
12:55 < Lightsword> yeah, instead of https://github.com/JoinMarket-Org/joinmarket-clientserver/blob/master/jmbitcoin/jmbitcoin/secp256k1_transaction.py#L56 you just put tx into a bytesio object
12:55 < Lightsword> and operate on that
12:57 < undeath> bytesio.write is overwrite as expected
12:59 < Lightsword> how are you using it exactly?
12:59 < undeath> seeking to a position and then writing
13:00 < Lightsword> oh, yeah I meant it’s append if your position is at the end of the stream already
13:00 < Lightsword> it’s not an insert
13:00 < undeath> ah, yes
13:00 < Lightsword> but we are appending so that’s fine
13:16 < Lightsword> undeath, how’s that look? https://github.com/JoinMarket-Org/joinmarket-clientserver/pull/229/files#diff-590754ac316dfffc53883e4a7606680fR175
13:17 < undeath> yes, much better than the concat :)
13:18 < undeath> btw, line 178 can just be '"txinwitness" in x'
13:18 < undeath> no need to do anything to x
13:20 < Lightsword> undeath, like “if "txinwitness" in x:”?
13:20 < Lightsword> that throws TypeError: argument of type 'int' is not iterable
13:21 < Lightsword> oh, woops
13:21 < undeath> if any("txinwitness" in x for x in txobj["ins"]):
13:22 < Lightsword> pushed
13:22 < undeath> :)     
13:25 < Lightsword> reafactoring deserialize() now
13:31 < Lightsword> undeath, how’s that look? https://github.com/JoinMarket-Org/joinmarket-clientserver/pull/229/files#diff-590754ac316dfffc53883e4a7606680fR56
13:34 < undeath> looks good
14:52 -!- puddinpop [~puddinpop@unaffiliated/puddinpop] has quit [Remote host closed the connection]     
14:58 -!- midnightmagic [~midnightm@unaffiliated/midnightmagic] has quit [Ping timeout: 264 seconds]     
15:09 < undeath> where is this message coming from? https://travis-ci.org/JoinMarket-Org/joinmarket-clientserver/jobs/458975537#L5610
15:11 < Lightsword> undeath, not exactly sure, I’m currently refactoring deserialize_script()
15:12 < undeath> I don't think it's a problem with your pr, I think it might be a problem with the test setup as a whole
15:15 < undeath> hah, yes
15:19 < Lightsword> undeath, not sure I have everything here correct https://github.com/JoinMarket-Org/joinmarket-clientserver/pull/229/files#diff-590754ac316dfffc53883e4a7606680fR446
15:20 < undeath> well, it's still unconditionally outputting hex
15:20 < Lightsword> ah, right
15:24 < Lightsword> undeath, how’s that look now?
15:26 < undeath> ugly :/
15:26 < Lightsword> heh, yeah
15:26 < undeath> can you do something about the code duplication?
15:27 < Lightsword> I’m somewhat avoiding trying to deduplicate too much, eventually the autodetection should be able to be removed, but probably that will be after we drop py2 support
15:27 < undeath> well, it can still be removed after deduplicating the code
15:29 < undeath> why do you think it would be a bad idea to deduplicate it?
15:31 < Lightsword> undeath, one issue with the codebase overall is that there’s a lot of custom helper functions doing weird type conversions, I’m mostly trying to move away from those and to built in functions
15:31 < undeath> you can simply declare a local helper function, that wouldn't clutter the overall code
15:32 < undeath> and it would probably make it easier to later remove the whole thing
15:36 -!- rdymac [uid31665@gateway/web/irccloud.com/x-vjqckaubtfrljexw] has quit [Quit: Connection closed for inactivity]     
15:49 -!- midnightmagic [~midnightm@unaffiliated/midnightmagic] has joined #joinmarket     
16:20 -!- raedah [~x@184.75.221.179] has joined #joinmarket     
17:09 -!- AgoraRelay [~jmrelayfn@p5DE4AB50.dip0.t-ipconnect.de] has quit [Ping timeout: 244 seconds]     
17:22 -!- GitHub36 [GitHub36@gateway/service/github.com/x-wugjfwwkzhwcrqav] has joined #joinmarket     
17:22 < GitHub36> [joinmarket-clientserver] undeath opened pull request #230: add undeath's pgp key (master...gpg-key) https://git.io/fp0OQ
17:22 -!- GitHub36 [GitHub36@gateway/service/github.com/x-wugjfwwkzhwcrqav] has left #joinmarket []     
17:23 -!- AgoraRelay [~jmrelayfn@p548667C2.dip0.t-ipconnect.de] has joined #joinmarket     
17:25 -!- GitHub57 [GitHub57@gateway/service/github.com/x-zuppdkzyrhysxxab] has joined #joinmarket     
17:25 < GitHub57> [joinmarket-clientserver] undeath opened pull request #231: clean up and re-enable test_configure (master...test-configure) https://git.io/fp0OA
17:25 -!- GitHub57 [GitHub57@gateway/service/github.com/x-zuppdkzyrhysxxab] has left #joinmarket []     
17:37 -!- undeath [~undeath@hashcat/team/undeath] has quit [Quit: WeeChat 2.3]     
17:41 -!- arubi [~ese168@gateway/tor-sasl/ese168] has quit [Remote host closed the connection]     
17:42 -!- arubi [~ese168@gateway/tor-sasl/ese168] has joined #joinmarket     
19:16 -!- arubi [~ese168@gateway/tor-sasl/ese168] has quit [Remote host closed the connection]     
19:16 -!- arubi [~ese168@gateway/tor-sasl/ese168] has joined #joinmarket     
20:01 -!- arubi [~ese168@gateway/tor-sasl/ese168] has quit [Remote host closed the connection]     
20:01 -!- arubi [~ese168@gateway/tor-sasl/ese168] has joined #joinmarket     
22:04 -!- arubi [~ese168@gateway/tor-sasl/ese168] has quit [Remote host closed the connection]     
22:04 -!- arubi [~ese168@gateway/tor-sasl/ese168] has joined #joinmarket     
23:25 -!- viasil [~viasil@185.212.171.210] has joined #joinmarket     
23:25 -!- viasil_ [~viasil@185.212.171.210] has quit [Read error: Connection reset by peer]