--- Log opened Wed Apr 14 00:00:25 2021 01:13 -!- shesek [~shesek@unaffiliated/shesek] has quit [Remote host closed the connection] 01:14 -!- shesek [~shesek@164.90.217.137] has joined #rust-bitcoin 01:14 -!- shesek [~shesek@164.90.217.137] has quit [Changing host] 01:14 -!- shesek [~shesek@unaffiliated/shesek] has joined #rust-bitcoin 03:16 -!- __gotcha [~Thunderbi@plone/gotcha] has joined #rust-bitcoin 03:25 -!- Jamarcus16Jaskol [~Jamarcus1@static.57.1.216.95.clients.your-server.de] has joined #rust-bitcoin 05:21 -!- __gotcha [~Thunderbi@plone/gotcha] has quit [Read error: Connection reset by peer] 05:22 -!- __gotcha [~Thunderbi@plone/gotcha] has joined #rust-bitcoin 05:53 -!- tibo_ [~tibo@2400:4050:2a83:7000:4b9:9503:79a1:9e28] has quit [] 06:29 -!- belcher_ is now known as belcher 07:09 -!- Kiminuo [~Kiminuo@141.98.103.140] has joined #rust-bitcoin 07:22 < jkczyz> ariard: fyi, looks like I need to rebase 07:44 -!- Jamarcus16Jaskol [~Jamarcus1@static.57.1.216.95.clients.your-server.de] has quit [Ping timeout: 260 seconds] 07:50 -!- Kiminuo [~Kiminuo@141.98.103.140] has quit [Ping timeout: 246 seconds] 09:36 -!- Kiminuo [~Kiminuo@109.183.7.106] has joined #rust-bitcoin 09:37 -!- Kimi [~Kiminuo@141.98.103.236] has joined #rust-bitcoin 09:40 -!- Kiminuo [~Kiminuo@109.183.7.106] has quit [Ping timeout: 240 seconds] 11:21 < ariard> hmmm investigating this : https://github.com/rust-bitcoin/rust-lightning/pull/858#discussion_r612614801, quite surprised by this behavior on #858 11:43 < BlueMatt> ariard: I dont believe 858 introduced/changed that behavior? 11:48 < ariard> BlueMatt: I agree, I wonder if it's not an issue in the TestChainSource impl 11:48 < ariard> like there shouldn't be any child tx returned from an in-flight commitment_tx 11:48 < BlueMatt> ariard: no? its just a height-bump which generates an event which includes a new watch 11:49 < BlueMatt> ariard: hmmm, no the issue jeff pointed out was *not* a new child tx returned 11:49 < BlueMatt> only a new registration 11:49 < ariard> ah the new registration is legit 11:49 < BlueMatt> ariard: I'd suggested asserting that there are no *registrations* of outputs to monitor, which is broken (hence jeff's comment) 11:50 < ariard> aaaaah gotcha not asserting that txdata is always empty in update-best-block 11:50 < BlueMatt> then I suggested that if we can't do that, I feel uneasy with an assertion on the return-side, cause it feels like something we could break without noticing, and its not exactly complicated code that we think is wrong 11:50 < ariard> i mean i think we can get ride of the `transaction_confirmed` call in `update_best_block 11:51 < ariard> break the might-return-a-registration? 11:51 < BlueMatt> hmm? no, break the may-return-a-new-tx-on-registration 11:52 < ariard> but this registration of the commitment tx output should never return a newer tx, at least one considered in the same block 11:53 < BlueMatt> jkczyz: ugh, looks like it needs rebase now :( 11:53 < BlueMatt> ariard: agreed, I'm saying I could see some future protocol or feature change implying a new registration that may screw us here. maybe eg an accidental duplicate registration 11:53 < BlueMatt> and in that case, meh, its not a bug, but we shouldnt assert 11:55 < BlueMatt> ariard: but, I honestly dont feel that strongly 11:55 < BlueMatt> ariard: if you do, we can change it 11:55 < ariard> BlueMatt: trying to imagine what this protocol change could be, ... 11:55 < BlueMatt> ariard: I couldnt imagine it being a protocol change, more likely a duplicate registration 11:56 < BlueMatt> ariard: though I guess that could lead to an infinite loop? 11:56 < BlueMatt> hmmm 11:56 < ariard> yeah a duplicate registration which would be a hint of a buggy chain::Filter impl? 11:57 < BlueMatt> no, I mean *us* generating a duplicate registration 11:57 < BlueMatt> which I guess shouldnt lead to a callback, but users are users and usually try to break things 11:58 < ariard> BlueMatt: right but this duplicate registration still shouldn't lead to a new transaction returned? 11:59 < BlueMatt> ariard: I would have to look at the api docs again, but I could imagine a user returning it 12:00 < ariard> i see but that's a buggy behavior from user impl imho 12:00 < ariard> BlueMatt: assuming this, yeah i agree you might have user breaking things 12:00 < ariard> but if this happen we should assert and not recurse again with `transaction_confirmed` ? 12:00 < BlueMatt> ariard: but, anyway, if you feel strongly that you want an assert, we can do that 12:01 < BlueMatt> ariard: I just figured the code is readable so not worth it, but maybe we can do a debug_assert and not assert in prod? 12:01 < ariard> ah a debug_assert sounds good to me, let me see if it breaks our test coverage 12:04 < ariard> BlueMatt: okay doesn't break anything which match my intuition that this behavior doesn't exist with current protocol 12:07 < BlueMatt> ariard: anyway, I'm headed to meetup, see ya. 12:08 < ariard> BlueMatt: oky see you :) 12:08 < ariard> jkczyz: if you're around ^^ 12:09 < jkczyz> ariard: but need to catch up on messages... one sec 12:13 < jkczyz> ariard: so remove recursion from `ChainMonitor::update_best_block` and add a debug_assert? 12:15 < ariard> jkczyz: yeah see comment just added: https://github.com/rust-bitcoin/rust-lightning/pull/858#discussion_r613522301 12:16 < ariard> well still need to register new outputs from `update_best_block` ofc 12:18 < jkczyz> ah, thanks.. registration is part of `process_transactions` (or whatever I'll rename it) so that should just work 12:24 < ariard> yeah still calling `process_transactions` in `update_best_block` just the recurse branch at the end should never be called whatever scenario considered 12:26 < ariard> otherwise i'm ack with the lastest push :) 12:41 -!- Kimi [~Kiminuo@141.98.103.236] has quit [Read error: Connection reset by peer] 12:54 < jkczyz> ariard: I pushed the fixups will squash and rebase in the meanwhile 15:46 -!- sanketcell [~sanketcel@ec2-100-24-255-95.compute-1.amazonaws.com] has quit [Ping timeout: 240 seconds] 15:46 -!- sanket1729 [~sanket172@ec2-100-24-255-95.compute-1.amazonaws.com] has quit [Ping timeout: 268 seconds] 16:34 -!- __gotcha [~Thunderbi@plone/gotcha] has quit [Remote host closed the connection] 16:35 -!- __gotcha [~Thunderbi@plone/gotcha] has joined #rust-bitcoin 17:49 < ariard> BlueMatt: just acked #858 ! 19:55 < BlueMatt> ariard: huh? you can route anywhere? 19:55 < BlueMatt> ariard: yayyyyy, 858 here we gooooo 20:26 -!- belcher [~belcher@unaffiliated/belcher] has quit [Ping timeout: 252 seconds] 20:40 -!- belcher [~belcher@unaffiliated/belcher] has joined #rust-bitcoin 23:50 -!- jeremyrubin [~jr@024-176-247-182.res.spectrum.com] has quit [Ping timeout: 268 seconds] --- Log closed Thu Apr 15 00:00:26 2021