--- Day changed Fri Sep 21 2018 04:08 < waxwing> any further thoughts on this? https://github.com/JoinMarket-Org/joinmarket-clientserver/issues/191#issuecomment-422822131 04:17 < belcher> in theory the -m should be stored in the seed phrase too, because ideally the seed phrase contains all the information required to restore the money back... but thats not very practical because the software doesnt know which mixdepths you'll be using when it creates the seed 04:18 < belcher> tumbler might use mixdepths up to -m 10 or something, then if you want to display the balance of that wallet you would need to set -m to a higher value 04:19 < belcher> so its definitely a bad thing that you cant increase -m i think 04:20 < belcher> if we see it as a property of the wallet then it should be encoded into the seed phrase too, which isnt possible 04:25 < belcher> so i'd say the wallet-tool should allow setting -m 04:25 < waxwing> well one thing to consider is, as undeath put it, the m value shouldn't be *downgradeable*. i'm not sure but that seems logical. upgradeable (increase) is different of course. 04:26 < waxwing> yeah i think i agree it's probably best to just have it always be settable. 04:26 < waxwing> maybe some warning if you reduce it. 04:26 < belcher> yes thats a fair point, reducing could make it seem theres less money in the wallet 04:27 < waxwing> yeah the linked discussion in #104 was talking about that a bit 04:28 < belcher> yes 05:49 -!- undeath [~undeath@hashcat/team/undeath] has joined #joinmarket 06:13 -!- Sentineo [~Undefined@unaffiliated/sentineo] has joined #joinmarket 06:24 -!- grubles_ [~grubles@gateway/tor-sasl/grubles] has joined #joinmarket 06:27 -!- grubles [~grubles@gateway/tor-sasl/grubles] has quit [Ping timeout: 256 seconds] 06:31 < waxwing> install script worked first time on a fresh machine (Ubuntu 1804). on master anyway. just a datapoint. 07:38 -!- grubles_ [~grubles@gateway/tor-sasl/grubles] has quit [Quit: Leaving] 07:49 -!- grubles [~grubles@gateway/tor-sasl/grubles] has joined #joinmarket 08:18 -!- grubles [~grubles@gateway/tor-sasl/grubles] has quit [Remote host closed the connection] 08:18 -!- grubles [~grubles@gateway/tor-sasl/grubles] has joined #joinmarket 10:34 < waxwing> undeath, ping 11:08 < waxwing> damn, fixing this mixdepth thing is really a pain in the new version. i have to overwrite storage, and make it temporarily non-read only. yuck. 11:13 < waxwing> thing is, although it'll be logical to restrict changes to increase, otoh there will be cases where it's a bit of a pain to *always* have to use say 7 mixdepths when you've gone back to only using the first 1 or 2. 11:14 < waxwing> but i guess that's a lesser concern. 11:56 < undeath> waxwing: ping 11:56 < waxwing> ah hi 11:56 < waxwing> so i was looking for a quick rough patch so people can access coins in mixdepth higher than 4. 11:56 < undeath> i'm not arguing we shouldn't allow to change the mixdepth at all 11:56 < waxwing> but a naive change just doesn't work here, i'm pinging you because i know you'll be able to do it better than I 11:57 < undeath> but adding a method to wallet tools to change the wallet's default mixdepth is quickly achieved 11:57 < undeath> and everything else would just be messy and I can't see any use case for it 11:57 < waxwing> the issue is specifically, as i said on github, that people do have coins in higher mixdepths, now. 11:58 < waxwing> it's not about arguing the right way for things to work so much. also note that tumbler's setup makes it quite common that people will want to use higher mixdepths, because it moves through N mixdepths starting from M. 11:58 < undeath> I get that. all that needs to be done is open the storage, change the mixdepth, save the storage 11:59 < waxwing> i've just been looking at it and found problems, due to index_cache 11:59 < undeath> well, at least in theory 11:59 < waxwing> it borks in the sync. 11:59 < undeath> really? 11:59 < waxwing> anyway whatever, i can fix it, but you will def fix it faster. 11:59 < undeath> did you save and re-open the wallet after changing the mixdepth? 12:00 < waxwing> it's complicated to explain, i first tried basically that: make storage readable, reset wallet mixdepth, call wallet.save, set back to read only 12:01 < waxwing> not enough; sync borks if i then try with a smaller maxmixdepth, seems you have to change at least a couple of things related to the process of populating the script map from the index cache 12:02 < waxwing> do you have time to get that done soon-ish undeath ? if not i'll continue trying to figure it out 12:02 < undeath> i'll check quickly if it work as I imagine but I don't really have time at all atm 12:02 < undeath> :/ 12:03 < waxwing> i consider it urgent only because i now know at least one person finds themself locked out of coins; of course in theory they can just use the old version, but that's a pretty annoying thing to have to do, and we shouldn't make them. 12:03 < waxwing> yeah, i get it. quick look would be great, thanks. 12:04 < waxwing> perhaps the fact that it failed is restricted to the case i tried: a *smaller* maxmixdepth. in theory we may only need to allow a larger one. i'll try that now. 12:05 < waxwing> nope 12:05 < waxwing> anyway let's see what you say 12:08 < undeath> https://gist.github.com/undeath/e2e3db81c9dcfdfd367b7dac2b9e272d 12:08 < undeath> in theory this should work 12:09 < undeath> ofc it's missing error checking and everything 12:10 < undeath> gtg, will check back later 12:11 < waxwing> that's exactly what i did :( 12:11 < undeath> :/ 12:11 < waxwing> except you have to set it to read_only false otherwise it won't update 12:11 < undeath> don't open it in any read-only mode 12:11 < waxwing> wait, i may have got the LN wrong, in which function is that? 12:12 < waxwing> oh wait no, i didn't read carefully, give me a minute 12:12 < undeath> it is a new wallet_utils method, no function 12:12 < waxwing> ah cool 12:12 < waxwing> a totally different/independent function, hmm, well i'll give it a go, thanks 12:13 < undeath> what I was talking about all along :P 12:14 < waxwing> just got to go out, will be back shortly and give it a whirl 12:15 < undeath> whoops, that should have been added to noscan_methods, not noseed_methods 12:15 < undeath> but otherwise that's how I imagine it should look like 12:15 < undeath> really away now 12:26 < undeath> ok, manages irc access while being afk 12:26 < undeath> i see the problem you're talking about 12:29 < undeath> in wallets.py after line 1076 adding "if md >= self.max_mixdepth: continue" should fix that 12:30 < undeath> err, ">" not ">=" 12:33 < undeath> i'm not sure why it would cause problems though, it shouldn't 12:35 < undeath> fast sync will probably fail, but that's expected 12:36 < undeath> when downgrading 12:49 < undeath> i dont think any change in wallet.py is necessary 13:13 < waxwing> undeath, yeah, you mean blockchaininterface.py, i did that, but i wasn't doing it your way 13:14 < waxwing> now i'm trying your code and encountering difficulties again ... when running with method 'changemixdepth' i find that it loads the wallet, but seems to trip up in get_path, 'mixdepth outside of wallet's range' 13:14 < waxwing> i'm trying to change an existing 5 mixdepth wallet to 6. 13:14 < waxwing> well more accurately, it doesn't load the wallet; the error comes up in calling wallet_cls in open_wallet 13:15 < waxwing> i think it's path-dependent, so to speak. 13:19 < undeath> ah, i see 13:20 < undeath> but does that happen when increasing, to 13:21 < undeath> no, i didn't mean blockchaininterface.py 13:22 < undeath> the path exception is probably fixed with the wallet.py change 13:22 < waxwing> that was a typo sorry 13:22 < waxwing> it seems to basically work now 13:23 < undeath> the gist patch has a off-by-one error btw and the mwallet._storage access is not needed 13:24 < waxwing> at least going up. as we've discussed, going down is generally not a good thing, so not sure how much I'd want to get into that. the difficulty'll be, should we just print a big warning on that, or better, just allow it, but keep the index cache at the larger size. 13:24 < waxwing> 'the mwallet. storage access'? sorry didn't get it. 13:24 < undeath> ignore the m 13:25 < waxwing> still don't get it :) btw = between or by the way? 13:25 < undeath> by the way 13:25 < waxwing> re: off by one are you talking about options.mixdepths vs options.mixdepths -1 ? 13:26 < undeath> exactly 13:26 < waxwing> storage access not needed meaning, just reset wallet.max_mixdepth? 13:27 < undeath> yep 13:27 < waxwing> yes i think that's what i saw too 13:27 < undeath> for some reason i was (mis)remembering wallet not updating that in storage 13:27 < waxwing> ok, so i'll PR it and add the changemixdepth method to the help, only thing left is, what advice do we give users re: reducing it. 13:28 < undeath> what does actually go wrong when refucing? 13:28 < waxwing> tbh if it's coded smartly it should always be fine. 13:28 < undeath> other than inaccessoble funds and broken fast sync 13:28 < waxwing> here smartly just means, remember higher mixdepth index_caches. at least it hink so. 13:29 < undeath> it does,at least it shoud, looking at thecode 13:29 < waxwing> fast sync broke: i didn't think of that. well if at least one sync method works for this edge case, i *guess* that's OK 13:30 < undeath> it 13:30 < undeath> it'snotlike that wouldn't havehappend withthe old implementation anyway 13:30 < waxwing> btw (got lost in the discussion mix), i got the impression that options.mixdepth is right, not options.mixdepth - 1 (so patch is right). see: wallet_generate_recover 13:31 < waxwing> (but this particular off-by-one is *always* confusing...) 13:31 < undeath> options.mixdepths - 1 is correct 13:31 < undeath> if recover does something diffferemt it's a bug 13:32 < waxwing> huh, i really don't think so. i've just tried it a few times. 13:33 < undeath> line 507 has the -1 13:34 < undeath> the naming is all fixed now 13:35 < undeath> since we start with md 0 our max_mixdepth is always onebelow theactual number of mixdepths in the wsllet 13:38 < waxwing> right, i see 13:39 < waxwing> so i'm trying now, basically everything is working when going up, but i get the get_path exception when going down. i'll try adding the patch you mention in wallet.py now to fix that. 13:44 -!- mr_paz_ [~mr_paz@c-71-57-73-68.hsd1.il.comcast.net] has joined #joinmarket 13:48 < waxwing> seemed to work fine going down, but syncing got really strange when i bumped it back up 13:49 < waxwing> but as we discussed, some weirdness in that case is not unexpected. for some reason, when bumping it back up to the normal 5, normal sync didn't work without a big gap limit sync, but --fast worked fine. shrug. 13:51 < undeath> mhm, that sounds like the index_cache is not actually retained as expected 13:52 < undeath> otoh I'm not sure if anyone would every need/want to downgrade a wallet's mixdepth 13:52 < undeath> but we should update the wallet conversion script to detect the wallet's mixdepth 13:59 < undeath> no idea why it wouldn'tbe retained though. code looks correct 14:00 < undeath> (talking about index cache) 14:04 < undeath> convert_old_wallet.py already detects the mixdepth, may be a bug there 14:05 < waxwing> undeath, i checked, the index_cache was kept 14:05 < waxwing> PR opened btw (not sure what gh bot status is) 14:07 < undeath> we're still +r, unfortunately still needed 14:08 < undeath> the if in wallet.py shoud be> 14:09 < undeath> not >= 14:09 < undeath> that might explain your sync troubles, too 14:12 < waxwing> oh! yeah, it was pretty weird otherwise! 14:13 < undeath> and I'd recommend renaming the change_wallet_mixdepth arg to max_mixdepth to avoid confusion 14:13 < waxwing> agreed 14:14 < waxwing> ah perfect, now sync behaves normally when going down then up 14:16 < undeath> don't yoi needto updaye the 'methods' variable? 14:18 < undeath> mh. 14:18 < waxwing> huh, i'd forgotten about that. it appears to be unused (perhaps didn't used to be) 14:18 < undeath> that one's unused 14:18 < waxwing> prob should be a if method in methods somewhere 14:18 < waxwing> or not in, rather 14:20 < undeath> yeah 14:20 < waxwing> can add that check separately 14:21 < waxwing> thanks for the help. 14:21 < undeath> the usage help is incorrect too with regard to method position 14:22 < undeath> it says method is expected last 14:22 < undeath> thanks for un-messing my patch! 14:25 < waxwing> oh? message is last, e.g. here python wallet-tool.py -m 6 walletname.jmdat changemixdepth 14:27 < undeath> oh. 14:27 < undeath> right 14:27 < undeath> nvm me 14:30 < undeath> decreasing the wallet mixdepth does not depend on the default but on the wallet's current md 14:31 < undeath> the help text may be a bit confusing there 14:32 < undeath> other than that the pr looks good to me 14:33 < waxwing> right 14:36 < undeath> sorry for not doing the pr myself, i'm quite limited in proper pc access currently 14:36 < waxwing> np 15:30 -!- undeath [~undeath@hashcat/team/undeath] has quit [Quit: WeeChat 2.2] 15:47 -!- lnostdal [~lnostdal@77.70.119.51] has quit [Read error: Connection reset by peer] 19:49 -!- azizLIGHT [~azizLIGHT@unaffiliated/azizlight] has quit [Ping timeout: 252 seconds] 20:40 -!- azizLIGHT [~azizLIGHT@unaffiliated/azizlight] has joined #joinmarket 21:18 -!- mr_paz_ [~mr_paz@c-71-57-73-68.hsd1.il.comcast.net] has quit [Remote host closed the connection] 23:31 -!- d3spwn is now known as Guest83042 23:32 -!- Guest83042 [~Jimmy@a83-161-157-200.adsl.xs4all.nl] has quit [Ping timeout: 260 seconds] 23:33 -!- d3spwn [~Jimmy@a83-161-157-200.adsl.xs4all.nl] has joined #joinmarket