--- Log opened Fri Sep 17 00:00:11 2021 00:44 < dr-orlovsky> andytoshi: you are right, sorry. Rebased. CC sebx2a:, sanket1729: for re-ACK 00:47 -!- belcher_ is now known as belcher 05:21 -!- tibo_ [~tibo@2400:4050:2a82:1000:52c:661e:76cb:bbcb] has quit [] 06:07 < sebx2a> dr-orlovsky: can you take a look at https://github.com/rust-bitcoin/rust-bitcoin/pull/563#discussion_r710472043 please? With fresh eyes it looks more relevant now. Otherwise the PR looks good and I'd like to re-ACK. 06:30 -!- Guest6876 is now known as h4sh3d 06:31 -!- h4sh3d is now known as Guest5915 06:32 -!- Guest5915 [~h4sh3d@109.202.212.237] has quit [Changing host] 06:32 -!- Guest5915 [~h4sh3d@user/h4sh3d] has joined #bitcoin-rust 08:37 < andytoshi> cool, thanks 09:42 -!- Guest5915 is now known as h4sh3d 13:29 < BlueMatt> andytoshi: stevenroose any thoughts on https://github.com/rust-bitcoin/rust-bitcoincore-rpc/pull/201 ? 13:30 < BlueMatt> I'm a fan of replacing fancy http client with our own that doesn't bother implementing a ton of crap that we dont need or being blazing fast ause we're querying bitcoin core, which is slow as balls anyway 13:48 < andytoshi> what does it do to the MSRV? 13:48 < andytoshi> i support this as well, though jsonrpc also has a simple HTTP client in it now 13:49 < BlueMatt> I believe up to the msrv for futures, sadly, cause we dont have a good way to make a function both async and sync based on features 13:49 < BlueMatt> but even futures/async isnt that high of an msrv anymore 13:50 < andytoshi> looks like 1.41, from the website 13:50 < andytoshi> i think this is fine tbh, rust-bitcoincore-rpc has never really had the same MSRV requiremetns as the "core" 0libraries 13:51 < BlueMatt> it does take one dependency, fwiw, just to parse the http chunked encoding crap. we had code for it, but its just annoying enough that taking a zero-dep crate as a dep seemed worth it 13:52 < BlueMatt> so it supports fetching both the json-rpc stuff and bitcoin core rest 13:52 < BlueMatt> tho i assume the rest support will be only barely publicly callable if it goes in 13:53 < BlueMatt> tho it would be nice to get support for that in rust-bitcoincore-rpc (or maybe a second crate in the repo) 13:56 < BlueMatt> but, yea, the real goal is to get async support there, with a custom http client so we dont have to take any big non-optional deps 13:56 < BlueMatt> this way you can flip a flag and use tokio for the underlying tcp stream (and adding async-std or whatever would be like a 5-line patch, so also trivial) 13:56 < BlueMatt> as long as you can squint and make it look like TcpStream its trivial 15:05 < ariard> BlueMatt: oh that's right didn't push yesterday, fixing the latest tests 15:05 < ariard> if you have eyes for it now, will spend my evening to get it done 15:06 < ariard> though need rebase first 15:09 < BlueMatt> ariard: yea, I'm around to review 15:09 < BlueMatt> would be nice to get it rebased first, though, yea. 15:11 < ariard> 2min 15:12 < BlueMatt> :+1: 15:36 < BlueMatt> ariard: uh huh. 15:50 < ariard> BlueMatt: yeah as usual i failed my rebase, pushed! 15:50 < BlueMatt> thanks! 15:50 < BlueMatt> ariard: lol, you keep pushing new branches upstream, btw 15:50 < BlueMatt> * [new branch] 2021-07-add-chan-closed -> upstream/2021-07-add-chan-closed 15:51 < BlueMatt> github is stupid and doesn't have an ability to control access to that 15:51 < ariard> wait let me check 15:51 < BlueMatt> i just deleted it 15:51 < BlueMatt> you did also update your own branch 15:51 < ariard> ah yeah i think sometimes never now 15:53 < BlueMatt> ariard: do you want to go ahead and squash the -fs? 15:53 < BlueMatt> or are they in the right order there? 15:53 < ariard> BlueMatt: what do you prefer one big commit? 15:53 < BlueMatt> i know sometimes you dont order them which is confusing 15:54 < ariard> or at least logically organized with changes/tests 15:54 < ariard> well i still have 19 tests to fix, then i was thinking to squash 15:54 < BlueMatt> i prefer multiple commits generally, if its practical, but for fixup commits them ordered along with the commits that they are to be squashed into 15:54 < BlueMatt> ah, if you intend it to be one commit then ok 15:54 < BlueMatt> its already several commits, though? 15:55 < ariard> yeah noted, i'm leaning reviewing full diff most of times 15:55 < ariard> yeah several commits 15:55 < ariard> do you prefer squash now or finishing fixing the tests :) ? 15:55 < BlueMatt> up to you, i was mostly just gonna review the channelmanager stuff and let you focus on the tests until they all pass :p 15:55 < BlueMatt> but i can just review the whole-pr channelmanager changes 15:56 < ariard> yeah let me finis tests, then i clean the fixes commit 15:56 < ariard> yes, it's mostly checking if the ClosureReason make sense 15:56 < ariard> and if they're other locations I should emplace some 15:57 < BlueMatt> yea, ok 16:56 < BlueMatt> ariard: found three more cases for you :) 17:00 < ariard> BlueMatt: lol just pushed a branch with all tests fixed :p 17:00 < BlueMatt> nice 17:02 < ariard> BlueMatt: okay noted well I'll address them this we 17:02 < ariard> going to dinner/bed 17:02 < ariard> if you have PRs I should have a quick eye on? 17:03 < BlueMatt> ariard: I'm torn on what to do on https://github.com/rust-bitcoin/rust-lightning/pull/1065 17:03 < BlueMatt> I kinda wanna just land it and re-add the test later 17:04 < BlueMatt> cause I dont wanna delay shipping it and then end up not being able to open channels to other impls 17:04 < BlueMatt> we already cant open channels with eclair 17:04 < BlueMatt> but other than that, ariard, there's nothing that's super in the security domain aside from your prs. 17:21 -!- belcher [~belcher@user/belcher] has quit [Ping timeout: 268 seconds] 17:34 -!- belcher [~belcher@user/belcher] has joined #bitcoin-rust --- Log closed Sat Sep 18 00:00:11 2021