--- Day changed Wed Aug 08 2018
02:37 -!- undeath [~undeath@unaffiliated/undeath] has joined #joinmarket     
02:38 < undeath> waxwing: the bug might be in displayall, not in the sync code. that seems more likely
03:09 <@waxwing> ah. good thought. just got back, am going to start testing without qt (on regtest) to isolate that factor.
03:10 <@waxwing> grubles, re wasabi: i asked nopara about that ,he told me he doesn't want to put the time publically, something to do with DOS. or i might have misunderstood.
03:10 <@waxwing> afaik you just have to leave it running and wait. i did the first 2 or 3 rounds, but stopped after that. seemed to be working fine, apart from the point you mention
03:11 <@waxwing> (that you don't know when it's going to happen. i think it's supposed to be 24 hours if not 100, but not sure if that's actually enforced.
03:22 < undeath> I'm pretty sure when you do displayall more than once things will go wrong. That was a possibility I didn't consider when writing the code. The cli only calls it once and then exits. Everything else can result in weird behaviour
03:29 <@waxwing> yeah it's not that, though
03:30 < undeath> you see the same thing on cli?
03:30 <@waxwing> i've isolated it to : doing a direct send (although i bet it's the same with sendpayment) *in the Qt* ... so that's good.
03:30 <@waxwing> it's a pretty weird error. it's basically choosing a change address a gap limit away, something like that. still tinkering.
03:33 <@waxwing> when i say "it's not that" i meant it's not about displayall. the fast vs not fast is only important in as much as, the fast sync is kind of more robust, it can still find the coins whereas the not fast doesn't as it only scans up to the gap limit, whereas the "fast" (badly named) is just a different algo and is more insistent on finding all coins (which is why i kind of wanted to shift entirely to that algorithm, whether it's actually faster or 
03:33 <@waxwing> so that's a side issue here.
03:35 < undeath> I see. this is very weird
03:36 <@waxwing> yep. after another run, same pattern. coins were in mixdepth 4, internal, index 8.
03:36 <@waxwing> after spend, change is created in index 15.
03:37 <@waxwing> this is all me testing on regtest but i saw the same pattern on the (Qt) sendpayment i did on mainnet.
03:37 <@waxwing> ok i'll actually look at the code now since i've got some idea what's actually happening.
03:38 <@waxwing> hmm before that .. i guess, the wallet.index must be set forward 6 (7?) somehow in the syncing before it starts the coin sending. somehow. why would that be Qt? maybe something happens in scripts version, outside the syncing code, in the calling script.
03:38 <@waxwing> and isn't happening here. that could explain it.
03:39 < undeath> maybe it's generating addresses somehow somewhere?
03:39 < undeath> does qt do an implicit display[all] before the sendpayment?
03:41 <@waxwing> it calls sync_wallet and then it calls get_wallet_printout to get the addresses.
03:42 <@waxwing> i don't think this is about display tho' . it's not displaying addresses wrongly, it's setting the index wrong before it gets a change address.
03:42 < undeath> get_wallet_printout calls wallet_display which I think is the culprit
03:44 <@waxwing> so is it like, wallet display is going to set the index forward gaplimit , whatever the index is now?
03:44 < undeath> yes, I think so
03:45 < undeath> there is probably a reason its first LOC does wallet.close()
03:45 < undeath> because it does things it does not reverse and didn't care about what happens to the wallet later on
03:45 <@waxwing> undeath, side point, you asked in your FIXME, why does the xpub if/else exist; the answer is because you're only going to "hand out"/export external address xpubs, not internal ones.
03:46 < undeath> I'm sure that may depend on your use case, but point taken
03:46 <@waxwing> probably? isn't that wallet.close() line part of your stuff?
03:46 < undeath> it is
03:47 < undeath> I was just a little lazy when implementing that because I did not consider the possibility of the method being called more than once and not right before program exit
03:47 <@waxwing> ok, np, i know the feeling of not remembering details of what i wrote :) anyway, yeah ... don't we want display to be stateless? i guess that's what you're trying to achieve with the .close()
03:48 < undeath> yes, I will fix that
03:49 < undeath> there are probably some more wallet.close() in wallet_utils that should be removed with that insight
03:50 <@waxwing> can you explain the relevance of .close() to this? i don't get it
03:50 <@waxwing> how does it affect the index?
03:51 < undeath> the close does not affect the index
03:51 < undeath> but the stuff happening later on
03:52 < undeath> as a fix, can you try to add after line 375 (right after the for block)
03:52 < undeath> wallet.get_next_unused_index(m, forchange, unused_index)
03:52 < undeath> err     
03:52 < undeath> wallet.set_next_index(m, forchange, unused_index)
03:52 < undeath> that, and removing the wallet.close() should do the trick
03:53 <@waxwing> thx will try it
03:57 <@waxwing> yeah that fixed it. moves forward from 15 to 16, as expected.
03:57 <@waxwing> now i will actually read code a bit :)
04:03 < undeath> ok, that was actually the only place I relied on the wallet.close() hack
04:08 <@waxwing> ok i lied i did more tests. that fix seems to result in correct behaviour. gonna run more sendpayment and tumbler tests with it.
04:08 <@waxwing> is that the fix you think we should go with? something seems a bit off with updating the index in that function, the sync is already supposed to have happened there, no?
04:11 < undeath> that's the correct fix
04:11 < undeath> in order to generate the "new" addresses we need to advance the index
04:13 <@waxwing> oh generating new addresses
04:14 <@waxwing> afair the old version of the code had a "raw" get address that didn't automatically increment the index. so you call one version of the function to simply get the address, another to *take* an address (use it up, hence increment index)
04:14 <@waxwing> i mean presumably it's the same here. anyway perhaps academic.
04:15 < undeath> I'm more a fan of "explicit is better than implicit" here. It's too easy to accidentally use the wrong method
04:16 <@waxwing> sure. it's fine. perhaps more explicit naming helps; imagine "read_address" vs "consume_address"
04:17 < undeath> the problem is that the wallet has some asumptions about the scripts it knows and the ones it hands out
04:17 < undeath> and that might end up being in a very weird state if you could query arbitrary addresses
04:18 < undeath> namely it assumes that if you query address at index i, it already knows all up to (i-1)
04:19 < undeath> pushed a fix
04:20 <@waxwing> thanks
06:00 <@waxwing> ok i've got a patch for Qt, it's probably about 15 lines all told (trivial now the above is fixed). it seems to be working with that. will apply after merge.
06:01 <@waxwing> will take a bit of a look at electruminterface. anyone else tried anything with the PR?
06:42 <@waxwing> i think electruminterface probably isn't going to work with big wallets. but there is a strong argument not to use them. there's a strong argument to not support it.
06:42 <@waxwing> oh sorry repetition was unintentional :)
07:00 -!- lukedashjr [~luke-jr@unaffiliated/luke-jr] has joined #joinmarket     
07:00 <@waxwing> undeath, 'script' fields are coming out unhexlified, don't know if you noticed
07:01 -!- luke-jr [~luke-jr@unaffiliated/luke-jr] has quit [Ping timeout: 248 seconds]     
07:01 -!- lukedashjr is now known as luke-jr     
07:02 <@waxwing> also in the report on no available podle commitment you get stuff like: 
07:02 <@waxwing> For reference, here are the utxos in your wallet:
07:02 <@waxwing> defaultdict(<type 'dict'>, {0: {u'**txid:n**': {u'path': ('\xd1\xb6=', 2147483697, 2147483648, 2147483648, 0, 0), u'address': '**address**', u'value': **amt**, u'script': '\xa9\x14\x...script in binary ...x87'}}})
07:04 <@waxwing> i'm now "happy" that electruminterface is kinda only basically working taker side, much as it was before (was never workable for Maker), although i'll do a couple more tests.
07:05 <@waxwing> so i'll just leave this as is for a couple of days, then merge, then add a patch for Qt to make sure that works, then have it in master for a while before there's a release.
07:05 <@waxwing> also you may want to add a bit in the README and docs/ folder instructions documents undeath, especially for wallet conversion
07:06 -!- Giszmo [~leo@pc-72-54-46-190.cm.vtr.net] has joined #joinmarket     
07:31 < undeath> yes, will write some documentation for the wallet conversion
07:31 < undeath> anything else that needs documenting?
07:33 < undeath> user feedback mostly lacks any kind of abstraction layer in jm, that's why you (now) sometimes see data that is not really suitable for humans to read
07:59 <@waxwing> yes. at least print hex not binary. it's in the log printouts for transactions and also that ^ , before it was like at the very end of this page: https://github.com/JoinMarket-Org/joinmarket/wiki/Sourcing-commitments-for-joins
07:59 <@waxwing> yes i know it's crap but no need to make it worse :)
08:04 -!- MaxSan [~user@185.9.19.107] has quit [Ping timeout: 248 seconds]     
08:04 < grubles> waxwing, ah ok. thx. re: wasabi.
08:05 < undeath> ah, the podle debug output even has user output abstraction, it just needs to be adopted
08:06 < undeath> I'll have a look at that and other instances where the data is logged
08:09 <@waxwing> it's there and when the scripts are printed out as part of the transaction inputs (the deserialized transaction printed in debug). iirc.
09:53 < arubi> waxwing, I'm curious, are you manually testing these things?  I mean, run makers\takers locally on miniircd and do a bunch of operations from the user-executed scripts?
10:07 < arubi> reason I'm asking, something like that sounds a lot like core's functional tests.  if you have an established flow that you're going over, maybe it can be automated
10:09 < undeath> ygrunner automates taker/yg
10:14 < arubi> oh cool, yea I remembered there was some way to run a local network of takers\makers and even interact with it
10:23 < undeath> not taker, but multiple ygs
10:26 < arubi> ah okay, so you'd be running the taker yourself
10:27 < undeath> exactly
10:27 < undeath> but it already creates a wallet for the taker
10:28 -!- rdymac [uid31665@gateway/web/irccloud.com/x-sezswkmhcfxnozqx] has quit [Quit: Connection closed for inactivity]     
10:28 <@waxwing> it's nowhere near good enough to rely on unit tests, especially with such a broad set of functionality as we have here. i'd be a lot more casual about it with things that don't touch the main body of code (signing, wallets), but here, great care has to be taken. manual testing has already thrown up several issues.
10:30 < arubi> yea I bet.  I think that's also the point of core's functional tests.  run the stuff as if a user does it
11:14 <@waxwing> maybe i should clarify, when i asked other people to "test" i really just meant run with the PR instead of release/master. i've been doing it since yesterday. i don't think a yieldgen will find any issues (except they need to run the conversion script on the wallet)
11:18 < arubi> I'm ashamed to say that I'm still not running jm myself..  coins are in hsm, I don't trust my computers\network setup enough to leave coins on a pc :(
11:20 < arubi> next vacation I'm setting up a full fledged local joinmarket setup.  it's decided :)
11:31 <@waxwing> arubi, well, i wasn't really asking you, don't worry about it :) just anyone who's running at the moment.
11:40 < arubi> yea I realize, but still I wanna explore jm-cs so I can at least feel comfortable with the interface when I finally have a proper setup :)
11:43 -!- belcher [~user@unaffiliated/belcher] has quit [Ping timeout: 240 seconds]     
12:09 -!- belcher [~user@unaffiliated/belcher] has joined #joinmarket     
12:33 -!- rdymac [uid31665@gateway/web/irccloud.com/x-jmypwnhhbfsvqzht] has joined #joinmarket     
12:46 < undeath> waxwing: pushed a commit to fix the output formating
12:47 <@waxwing> gotcha, cheers
12:49 <@waxwing> i'm ready to add just one commit for Qt, but will wait 1-2 days. oh, and you'll do that extra one for a bit more docs right.
12:49 < undeath> yep, sure
12:50 < undeath> anything besides the wallet conversion that needs documenting?
12:52 < undeath> and do you still prefer to have this included in the release that features the new wallet implementation? https://github.com/JoinMarket-Org/joinmarket-clientserver/issues/62#issuecomment-318890399
12:52 <@waxwing> i don't think so re:doc
12:52 < undeath> I could work on that next
12:52 <@waxwing> undeath, i think so.
12:53 < undeath> it would kinda make sense since ppl have to convert their wallet anyway I think
12:54 < undeath> so they can in the same process also move their files to ~/.joinmarket
12:55 <@waxwing> this seemed to work ok: https://github.com/AdamISZ/CoinSwapCS/blob/master/coinswap/configure.py#L253-L262
12:55 <@waxwing> well, wrong highlighting, the whole function. but then again not sure i tested it off-linux.
12:56 < undeath> ah, cool
12:58 -!- Giszmo [~leo@pc-72-54-46-190.cm.vtr.net] has quit [Ping timeout: 260 seconds]     
12:59 -!- davex__ [~user@97-119-117-177.omah.qwest.net] has joined #joinmarket     
13:15 -!- Giszmo [~leo@pc-72-54-46-190.cm.vtr.net] has joined #joinmarket     
14:59 -!- arubi [~ese168@gateway/tor-sasl/ese168] has quit [Remote host closed the connection]     
15:00 -!- arubi [~ese168@gateway/tor-sasl/ese168] has joined #joinmarket     
16:26 -!- undeath [~undeath@unaffiliated/undeath] has quit [Quit: WeeChat 2.1]     
22:03 -!- arubi [~ese168@gateway/tor-sasl/ese168] has quit [Ping timeout: 250 seconds]     
22:03 -!- arubi [~ese168@gateway/tor-sasl/ese168] has joined #joinmarket