--- Log opened Sun Dec 02 00:00:43 2018 02:32 -!- TamasBlummer1 [~Thunderbi@p200300DD673DE906C09D22D3048757B8.dip0.t-ipconnect.de] has joined #rust-bitcoin 02:34 -!- TamasBlummer [~Thunderbi@p200300DD673DE957C09D22D3048757B8.dip0.t-ipconnect.de] has quit [Ping timeout: 264 seconds] 02:34 -!- TamasBlummer1 is now known as TamasBlummer 03:06 -!- belcher [~belcher@unaffiliated/belcher] has joined #rust-bitcoin 10:42 < BlueMatt> ariard: ugh, sorry, was nursing a hangover most of yesterday so didnt get to it, doing another review now 10:45 < BlueMatt> ariard: I'm still pretty unsure about the api for passing back preimages into other ChannelMonitors 10:45 < BlueMatt> ariard: I really feel like I want to keep that centralized to ChannelManager 10:50 < BlueMatt> ariard: ie if there is a monitor for a channel that has been closed, if we get a preimage from the upstream channel it will come in via Channel/ChannelManager and we'll need to fail backwards then 10:50 < BlueMatt> if we get it via on-chain txn, why not use the same codepath via ChannelManager, given its already implemented there? 12:39 < BlueMatt> ariard: so see comment at https://github.com/rust-bitcoin/rust-lightning/pull/198#pullrequestreview-180574398 and https://github.com/rust-bitcoin/rust-lightning/pull/198#discussion_r238112582 I think by writing update-htlc-on-off-chain-channel in ManyChannelMonitor right now we're gonna end up with dup code in the future - since the update-from-on-chain-to-offchain case is gonna be an event generated by Channel/ChannelManager anyway 12:40 < BlueMatt> ariard: for the sake of etting 198 merged, it may make sense to just rip out the fail/fulfill-to-off-chain-channel stuff until both bits are implemented and just comment out the tests? 18:52 -!- _cryptodesktop_i [~John@91.245.78.70] has joined #rust-bitcoin 19:26 -!- _cryptodesktop_i [~John@91.245.78.70] has quit [Ping timeout: 250 seconds] 20:03 < ariard> BlueMatt: well in fact you were right that is duplicated code to have preimage passing backward in block_connected, I've cut it 20:03 < ariard> because it's already done in get_update_fulfill_htlc 20:04 < ariard> modified my onchain_to_onchain_claim test because need to call block_connected twice, you can't anymore have monitor updated in one block scanning event 20:05 < ariard> will finish to modify and harden it tomorrow 20:06 < ariard> upstream just need to be aware that in case of update-from-on-chain-to-offchain event it may be needed to rescan block again because you can have a HTLC-Success tx and a remote commitment tx from different channels in same block 20:07 < ariard> and first scan backward monitor didn't get preimage from ChannelManager 20:08 < ariard> BlueMatt: oh and I'm pretty sure we don't need to add anymore short_channel_id in Storage which means a good part of #264 can go so don't merge it yet --- Log closed Mon Dec 03 00:00:41 2018