--- Day changed Thu Sep 06 2018
00:33 < BlueMatt> ariard: running out, but am looking forward to seeing what you come up with for https://github.com/rust-bitcoin/rust-lightning/issues/137 tomorrow. I'm not sure its gonna be as simple to pull off as the ChannelManager/Channel thing, but maybe I'll be pleasantly surprised :)
00:35 < ariard> yeah, I'm finishing funding_created and continue on this one!
00:36 < ariard> will review #152 before to go
13:37 < Blackwolfsa> @BlueMatt, I made a patch for the tmep_channel_id copy change over form deserialize. So that, that code can go in while we continue the discussion regarding the errors.
13:46 < BlueMatt> Blackwolfsa: thanks!
13:48 < BlueMatt> oops, had a rebase error in it, though
13:50 -!- elichai2 [uid212594@gateway/web/irccloud.com/x-bfffddgntzlxpgmm] has joined #rust-bitcoin
13:52 < Blackwolfsa> haha, yip :/ happens if you do edits in the browser, while sitting in a bus.
13:56 -!- itaseski [~itaseski@213.135.177.14] has joined #rust-bitcoin
14:06 < BlueMatt> Blackwolfsa: heh, while you're here, re: panics: I dunno I mean at least in this particular case the error is "memory corruption"
14:07 < BlueMatt> which, in my experience with bitcoin core (which is mostly cause its super super good at pegging your cpu), implies you're gonna have *more* memory corruption going forward
14:07 < BlueMatt> this is especially true on shitty arm processors, eg phones/RPis/etc, which are at least a decent part of the target client here, I think
14:08 < BlueMatt> maybe a better example for discussion is https://github.com/rust-bitcoin/rust-lightning/pull/152/files#diff-d152f468921ba1eb6da8dac49a90a97aR1156
14:09 < BlueMatt> it panics in case there is some error in the implementation in Channel which results in a largely-unhandle-able error
14:10 < BlueMatt> its possible we could force-close the channel at that point, but that is a case that will never ever be tested, so who knows if it'll work
14:21 < Blackwolfsa> I am literarily talking about cosmic rays. If you run code enough you will bits that are swopped around where it should not happen.
14:22 < Blackwolfsa> This does not happen rarely but it does, if you have enough devices running it, you will get it
14:22 < BlueMatt> sure, but like 95% of the time when it does, assuming it hits actually-used memory, the app will crash no matter how its written
14:23 < Blackwolfsa> not necessarily, as when it happens its just one bit. So you get like a null pointer exception for example
14:25 < BlueMatt> an NPE will crash any C/Rust program :p
14:26 < BlueMatt> or an integer overflow
14:29 < Blackwolfsa> true but only that thread.
14:34 < Blackwolfsa> Looking at #152, if that happens I would close down the  library , clean up memory as if the whole program suddenly lost connection. As it failed to send through an error message. We then force the user app to reinitialise the library from the start as if he is starting up his app
14:35 < Blackwolfsa> @BlueMatt that patch fix is in, all tests passed
14:40 < BlueMatt> Blackwolfsa: can you squash the commits/change the commit title?
14:40 < BlueMatt> I dunno if github can do that
14:41 < BlueMatt> I can if not
14:44 < Blackwolfsa> it should be possible
14:44 < Blackwolfsa> what do you want the title to be?\
14:46 < Blackwolfsa> I think you need merge rights to squash merge
14:48 < BlueMatt> squash+merge doesnt generate a merge commit, though
14:48 < BlueMatt> I'll just do it, give me a sec
14:48 < BlueMatt> I just did it for #151 anyway
14:49 < Blackwolfsa> cwl, I dont see any option to squash the commits, but perhaps I am missing it.
14:49 < BlueMatt> there probably isnt from the web interface
14:50 < BlueMatt> the web interface is kinda meh
14:50 < Blackwolfsa> I am used to work from a windows environment with git-extensions, but that doesnt work on mac
14:54 < BlueMatt> should be able to work on windows on rust stuff, no?
14:55 < Blackwolfsa> yes, should be, but work issued me a mac, which I work on.
14:57 < BlueMatt> Blackwolfsa: anyway, as for the earlier discussion, I'd hope panic!() is a way to "shut down the library and make the user re-init"
14:57 < BlueMatt> thats kinda my goal with any error condition like that
14:58 < BlueMatt> but I dont know how to better do it, and I *really* dont want to end up with half the code in the library be "handle unreachable condition in a safe-ish way"
14:58 < BlueMatt> cause that's just unreadable
14:59 < Blackwolfsa> Yes, I agree that should be the goal, and I also agree that we should not have a insane amount of error handling code just because .
15:01 < BlueMatt> I think panic!() is a reasonable tradeoff in that area - the client can chose to isolate the process if they want to have some way to restart without the whole app crashing
15:01 < BlueMatt> either way, everything *must* be consistent on-disk, so crashing vs clean-teardown is no different
15:04 < Blackwolfsa> But I am cautious about letting the user handle that and not the library. I dont think we should cause his app to crash or pass the error onto him. Preferable we should pass some error to him notifying what happend
15:05 < Blackwolfsa> But I agree on the consistant
15:05 < BlueMatt> I mean we'd need a way to a) have a second error type everywhere in the codebase without a ton of text blowup, b) have a way to have the result of that force the user to re-create all the library's objects
15:40 < Blackwolfsa> That's behind the reason I was asking about the reason for using macro's with error handling and conversion.  You might be able to reduce the code quite a bit by creating a separate code handler, and we use the error trait and from conversion to map between those. I saw you guys added a action to some of the errors which I think is quite cool, This can them be used to ship/ escalate an action as the error gets
15:40 < Blackwolfsa> converted along a standard path. We can then at the top end pass this error to an action handler to do some action like kill the node, kill the channel, send error message etc.
15:41 < Blackwolfsa> From a user perspective, he doesnt need to know that field x failed on checking ( he might want to know if it exceeded a limit), but generally he only wants to know, node failed, channel to node failed etc.
16:30 -!- Fabian_ [~Mutter@x59cc8bec.dyn.telefonica.de] has joined #rust-bitcoin
16:32 -!- Fabian_ [~Mutter@x59cc8bec.dyn.telefonica.de] has quit [Client Quit]
18:13 -!- itaseski [~itaseski@213.135.177.14] has quit [Quit: Leaving]
19:00 -!- elichai2 [uid212594@gateway/web/irccloud.com/x-bfffddgntzlxpgmm] has quit [Quit: Connection closed for inactivity]
19:55 < BlueMatt> well that doesn't solve (b) which seems pretty nontrivial to solve
19:56 < BlueMatt> like, I dont really see a way to have an Err type which, when returned, results in the whole object self-destructing and the user having to re-init it (and what does that mean for library usability that the client has to implement doing so to handle cases we believe to only be possible in case of hardware failure or other random-but-absurdly-rare corruption)