--- Log opened Thu Mar 04 00:00:46 2021
00:29 -!- michaelfolkson2 [~michaelfo@2a03:b0c0:1:e0::23d:d001] has joined #rust-bitcoin
00:30 -!- junderw_ [sid43070@gateway/web/irccloud.com/x-uqxwulyvlpmwwfhi] has joined #rust-bitcoin
00:43 -!- Netsplit *.net <-> *.split quits: michaelfolkson, junderw
00:43 -!- junderw_ is now known as junderw
02:57 -!- belcher_ is now known as belcher
03:17 -!- michaelfolkson2 is now known as michaelfolkson
03:50 -!- tibo [~tibo@2400:4050:2a83:7000:8128:78ae:f37d:702d] has quit [Remote host closed the connection]
04:18 -!- Maximillia41Wiso [~Maximilli@static.57.1.216.95.clients.your-server.de] has joined #rust-bitcoin
04:27 -!- tibo [~tibo@2400:4050:2a83:7000:8128:78ae:f37d:702d] has joined #rust-bitcoin
04:32 -!- tibo [~tibo@2400:4050:2a83:7000:8128:78ae:f37d:702d] has quit [Ping timeout: 258 seconds]
06:03 -!- jonatack_ [~jon@37.173.203.38] has joined #rust-bitcoin
06:09 -!- jonatack_ [~jon@37.173.203.38] has quit [Quit: jonatack_]
06:38 -!- jonatack [jon@gateway/vpn/airvpn/jonatack] has joined #rust-bitcoin
06:47 -!- Kiminuo [~Kiminuo@141.98.103.76] has joined #rust-bitcoin
06:48 < sgeisler> Does anyone know how well our PSBT merging logic handles malicious input? Is it like "Everything invalid is rejected, if you have enough valid PSBTs for e.g. a multisig spend it'll work after combining all and automatically rejecting the invalid ones" or more like "you have to do a lot of manual filtering for this to work with malicious input"? andytoshi, sanket1729_
06:50 < sgeisler> "Better look yourself" is also a valid answer, but not as useful as potential attacks seem plentiful and it will take quite a while to be sure of the former (the latter otoh only takes a counter example)
06:52 -!- jonatack [jon@gateway/vpn/airvpn/jonatack] has quit [Ping timeout: 256 seconds]
06:55 -!- jonatack [~jon@37.173.203.38] has joined #rust-bitcoin
07:02 < andytoshi> sgeisler: there isn't really any attack vector when merging
07:02 < andytoshi> rust-bitcoin does not have a finalizer, which is where invalid signatures would be dropped
07:02 < andytoshi> but rust-miniscript does have one, and it does indeed check that the signatures are valid
07:04 < sgeisler> Ah, I see that's a kinda annoying API tbh since then one has to find out which peer submitted the faulty sig (I guess the error helps with that), go back, recombine without and do that up to the amount of malicious peers that are allowed :/
07:05 < sgeisler> are there any other gotchas?
07:10 < andytoshi> why do you care who submitted the faulty sig?
07:11 < andytoshi> if you have enough honest peers who are willing to sign, then bad signatures don't matter
07:11 < andytoshi> and if you don't ... bad signatures also don't matter
07:12 < andytoshi> oh, i guess somebody could try to replace one peer's valid sig with an invalid one
07:12 < andytoshi> i think our API will just keep whichever one you saw first
07:18 < andytoshi> oh, no, it does whatever `.extend` does on BTreeMap...which probably replaces
07:18 < andytoshi> yeah we should fix this, rust-bitcoin has enough logic that it can identify bad sigs. i will try to update this as part of my psbt2 work
07:28 -!- Kiminuo [~Kiminuo@141.98.103.76] has quit [Remote host closed the connection]
07:28 -!- Kiminuo [~Kiminuo@141.98.103.76] has joined #rust-bitcoin
08:35 -!- jeremyrubin [~jr@024-176-247-182.res.spectrum.com] has joined #rust-bitcoin
08:56 -!- CubicEarth [~CubicEart@c-67-168-1-172.hsd1.wa.comcast.net] has quit [Ping timeout: 276 seconds]
09:07 -!- CubicEarth [~CubicEart@c-67-168-1-172.hsd1.wa.comcast.net] has joined #rust-bitcoin
09:15 < BlueMatt> ariard: if you get a chance can you review https://github.com/rust-bitcoin/rust-lightning/pull/815 ? Would like to merge it quick-ish. they're nontrivial bugs, but simple fixes.
09:31 < ariard> BlueMatt: yeaah gonna look, is 808 ready to review again?
09:32 < BlueMatt> 808 should be basically ready to go
09:35 -!- Maximillia41Wiso [~Maximilli@static.57.1.216.95.clients.your-server.de] has quit [Ping timeout: 264 seconds]
09:54 -!- jeremyrubin [~jr@024-176-247-182.res.spectrum.com] has quit [Ping timeout: 260 seconds]
10:01 -!- Kimi [~Kiminuo@141.98.103.76] has joined #rust-bitcoin
10:03 -!- Kiminuo [~Kiminuo@141.98.103.76] has quit [Ping timeout: 256 seconds]
12:42 -!- gribble [~gribble@unaffiliated/nanotube/bot/gribble] has quit [Remote host closed the connection]
12:48 -!- gribble [~gribble@unaffiliated/nanotube/bot/gribble] has joined #rust-bitcoin
12:55 -!- dr-orlovsky [~dr-orlovs@31.14.40.19] has quit [Quit: ZNC 1.8.0 - https://znc.in]
12:55 -!- dr-orlovsky [~dr-orlovs@31.14.40.19] has joined #rust-bitcoin
13:06 -!- belcher_ [~belcher@unaffiliated/belcher] has joined #rust-bitcoin
13:06 < valwal_> heyo, question about `rust-lightning-invoice` -- how would i get a payment_secret out of `RawTaggedField::UnknownSemantics`, (or is that even supported?)
13:07 < valwal_> sgeisler: maybe you would know?
13:08 -!- belcher [~belcher@unaffiliated/belcher] has quit [Ping timeout: 260 seconds]
13:09 -!- belcher_ is now known as belcher
13:14 -!- tibo [~tibo@2400:4050:2a83:7000:8128:78ae:f37d:702d] has joined #rust-bitcoin
13:18 -!- tibo [~tibo@2400:4050:2a83:7000:8128:78ae:f37d:702d] has quit [Ping timeout: 240 seconds]
13:26 < sgeisler> valwal_: oh yeah, that's one of the unimplemented fields :( I kinda neglected that crate. To get something out of  `UnknownSemantics(Vec<u5>)` you need to check the first `u5` to be the right tag, can ignore the following 3 (length, already encoded in the `Vec`) and read the remaining `u5`s as your unparsed field.
13:27 -!- Kimi [~Kiminuo@141.98.103.76] has quit [Ping timeout: 245 seconds]
13:27 < valwal_> great! ty, that should work
13:27 < sgeisler> then you probably want to convert the `[u5]` to a `[u8]`
13:28 < valwal_> ah okay... that seems weird
13:28 < sgeisler> That should work via `Vec::<u8>::from_base32`
13:28 < BlueMatt> I do wonder if we should just take the invoice crate stuff into rust-lightning
13:28 < sgeisler> at least that's how it's done for the payment hash
13:28 < BlueMatt> if also because we need to expose features
13:28 < BlueMatt> which we have types for in rust-lightning
13:28 < sgeisler> Feel free to do so :)
13:29 < BlueMatt> and those types really dont belong in another crate (they're used for other purposes too)
13:29 < BlueMatt> I mean its great as a separate crate, but I dunno how we avoid a cricular reference for the features object :(
13:30 < valwal_> i agree @matt, do you think the bindings code would work though?
13:31 < BlueMatt> I mean eventually the bindings logic is going to need to support multiple crates, but that is probably a ways off
13:31 < BlueMatt> so pulling the invoice parsing into rust-lightning's lightning crate directly would avoid needing to worry about it for invoices yet :p
13:31 < valwal_> ahh. yeah, the code's pretty readable/small
13:32 < BlueMatt> as for if the bindings code would work with the current invoice code, I doubt it, but just more stuff to fix anyway
13:32 < sgeisler> valwal_: see https://github.com/rust-bitcoin/rust-lightning-invoice/blob/557b7f6196f681f87ddabeaf42d48bf8bed44838/src/de.rs#L448 for converting u5 arrays to bytes
13:33 < BlueMatt> may want to check with dr-orlovsky before we commit to a change like taking it into rust-lightning.
13:33 < valwal_> sgeisler: 👍
13:33 < valwal_> trying it out now
13:34 < sgeisler> Also, at the risk of stating the obvious, instead of working with unknown fields, implementing them is probably the better long term solution
13:35 < valwal_> lol, i did have like 1/4 of a PR before you responded
13:35 < sgeisler> I think that would be rather straight forward
13:35 < BlueMatt> anyway, step one is pulling out the payment secret, step 2 is exposing features as a vec
13:35 < BlueMatt> step three is moving it into rust-lightning and unifying the types
13:35 < valwal_> sgeisler: ok, i'm down to PR, but do you think we'd be able to get it in by early next week?
13:36 < dr-orlovsky> I assume there is a lot of demand for lightning invoices outside of the rest of LN - like displaying invoice information. I think the separate crate is still needed. However I understand your point about circular dependencies also...
13:37 < sgeisler> I think so? It's not like there is any official release process other than me asking Matt for a second review for more involved PRs xD
13:37 < sgeisler> you'd just define a new type for the secre, impl From/ToBech32 and put it here https://github.com/rust-bitcoin/rust-lightning-invoice/blob/557b7f6196f681f87ddabeaf42d48bf8bed44838/src/de.rs#L432
13:38 < BlueMatt> dr-orlovsky: well, specifically at least (a) want/need a type for payment_hash/payment_secret, which we have newtypes for, (b) need a good type for features, which we have a ton of logic fore
13:39 < BlueMatt> and I dunno how else to solve it. that would kinda make you depend on rust-lightning, though, at least for the features types
13:39 < sgeisler> valwal_: just always ping me if I don't respond swiftly, chances are I overlooked the GH notification or was distracted and forgot about it
13:40 < dr-orlovsky> I see... In this case feel free to kill the crate - and for my purposes I will clone the lib and will include into LNP Core using type system from there
13:40 < valwal_> sgeisler: understood!! thx
13:40 < BlueMatt> dr-orlovsky: hmm, thats obviously kinda a shitty outcome, but I assume LNP is basically forked off of lightning-rfcs anyway and will need additional non-lightning features in the invoice parsing?
13:41 < BlueMatt> (I dont know a ton about LNP, but given you didn't base it on rust-lightning, I assume its....not really lightning but targeting other stuff)
13:41 < dr-orlovsky> nope, it's complete write up from scratch, so I was poorly compatible with lightning invoices already
13:41 < BlueMatt> right, ok, then I suppose thats fine.
13:41 < dr-orlovsky> it targets lightning, but in more generic scope than current BOLTs allow - supporting stuff like multipeer channels, arbitrary commitment transaction structure, RGB etc
13:42 < dr-orlovsky> targeting the lib for more experiments: eltoo, PTLCs, DLCs, channel factories etc
13:42 < sgeisler> BlueMatt: what exactly doesn't work with it as an independent crate? Maybe the API needs fixing instead of pulling it in?
13:42 < BlueMatt> heh, well we're moving towards that in rust-lightning as well, at least cause we want to open rust-lightning up to the DLC stuff. shouldnt be that hard to do while still keeping our state machine, I think, mostly, too.
13:43 < BlueMatt> sgeisler: just the circular dependency of types - specifically features, really.
13:43 < BlueMatt> like, we have features types that are scoped and fancy and mostly used in p2p network communication...but we also need them in invoices
13:43 < sgeisler> The "sanity check invoices" use case seems like one where you don't necessarily want to pull in the whole of rust-ln
13:44 < BlueMatt> so otherwise we have to have 1:1 type mappings and make users move stuff from one to the other manually
13:44 < sgeisler> that's annoying, breaking them out into their own crate doesn't work?
13:44 < sgeisler> so both rust-ln and rust-ln-invoice would depend on rust-ln-features
13:44 < BlueMatt> heh, we could have a rust-lightning-random-sutuff-also-in-invoices, but its not like people cant depend on a larger crate and use a subset
13:45 < BlueMatt> I dont feel *too* strongly, but I weakly prefer bigger crates with good module separation over more crates
13:45 < sgeisler> you could still put it all in one git repo and one cargo workspace to stay sane
13:46 < BlueMatt> thats true too. is there anything we'd lose? does rust-lightning-invoice work without malloc? does it support no-std?
13:46 < sgeisler> No xD
13:46 < BlueMatt> (we'd lose that, though hopefully only temporarily, its not a huge patch to make rust-lightning no-std, but we havent done it)
13:47 < BlueMatt> hmm, right, well then its just a matter of preference. :shrug:
13:47 < BlueMatt> I leave it up to whoever actually opens the pr to move it into rust-lightning, then :)
13:48 < sgeisler> It uses `Vec`s all over the place, but they can probably be replaced by something array-backed since most things have a fixed upper length
13:48 < BlueMatt> well you can still be no_std with vec, just need core/malloc
13:49 < BlueMatt> (which is what rust-lightning eventually wants to target, and almost does today)
13:49 < sgeisler> You won't be able to build something only working with slices since you need to convert between bytes and `u5`s
13:49 < sgeisler> ah, I see, that shouldn't be a problem at all then
13:51 < BlueMatt> right, we'd still be targeting the same compile minimums, then.
13:53 < sgeisler> Oh, I just found an old TODO `// TODO: fix before 2037 (see rust PR #55527)` ^^ we can probably fix this once we bump MSRV again
13:53 < BlueMatt> lol
13:53 < sgeisler> (overflowing time arithmetic etc.)
13:54 < sgeisler> https://github.com/rust-lang/rust/pull/55527
15:03 -!- jonatack [~jon@37.173.203.38] has quit [Ping timeout: 256 seconds]
15:09 -!- tibo [~tibo@2400:4050:2a83:7000:8128:78ae:f37d:702d] has joined #rust-bitcoin
15:15 -!- jonatack [~jon@37.173.203.38] has joined #rust-bitcoin
15:55 < dr-orlovsky> sanket1729_: is there a method which returns a number of signatures required for a given descriptor? As afar as I see there is no: and it is also currently not possible to understand how many signatures are required to satisfy some arbitrary miniscript
15:56 -!- jonatack [~jon@37.173.203.38] has quit [Ping timeout: 260 seconds]
15:59 -!- jonatack [~jon@37.173.203.38] has joined #rust-bitcoin
16:44 -!- gribble [~gribble@unaffiliated/nanotube/bot/gribble] has quit [Remote host closed the connection]
16:52 < ariard> BlueMatt: wdym "force-close doesn't generate a `ChannelMonitor" in https://github.com/rust-bitcoin/rust-lightning/pull/808#discussion_r586538487, the force-closure described did happen during old ChannelManager lifetime?
16:53 < ariard> if force-closure source is anything (e.g faultive HTLC, weird p2p messages, ...), it should have generate a ChannelMonitorUpdate
16:53 -!- jonatack_ [~jon@37.171.225.50] has joined #rust-bitcoin
16:54 -!- gribble [~gribble@unaffiliated/nanotube/bot/gribble] has joined #rust-bitcoin
16:56 -!- jonatack [~jon@37.173.203.38] has quit [Ping timeout: 264 seconds]
16:59 < BlueMatt> ariard: force closer during deserialization doesn't
16:59 < BlueMatt> ariard: when we're deserializing, if we have channelmonitors we dont know about, we already have a reference to the monitor itself
16:59 < BlueMatt> so we just force-close by calling the method on the monitor - it doesn't go through the CHannelMonitorUpdated update_channel calls
16:59 < BlueMatt> thus it doesn't get a new persistence call
17:04 -!- jonatack_ [~jon@37.171.225.50] has quit [Ping timeout: 246 seconds]
17:05 -!- jonatack_ [~jon@37.172.247.108] has joined #rust-bitcoin
17:07 < ariard> BlueMatt: yeah but isn't this assuming that you hit force-closure during deser of old ChannelManager but not *again* at new ChannelManager deser
17:07 < ariard> like force-closure should be some schelling point once reach, no reason to ressuscite chans
17:08 < BlueMatt> right, but there's no guarantee the channelmonitor was stored in between
17:08 < BlueMatt> ie the comment is descirbing the case where you deserialized everything with a stale manager, got some force-closes, broadcasted transactions, then crashed/reloaded *without* any serialization
17:08 -!- jonatack_ [~jon@37.172.247.108] has quit [Read error: Connection reset by peer]
17:09 < BlueMatt> which is totally allowed by the api
17:10 < ariard> wait ChannelManager don't broadcast transaction, it's up to the ChannelMonitor to do this
17:10 -!- tibo_ [~tibo@2400:4050:2a83:7000:9123:9c4:bec3:f6bf] has joined #rust-bitcoin
17:10 < BlueMatt> but it calls that during the deserialization call
17:11 < BlueMatt> during deserialization the ChannelManager has a direct pointer/reference to the ChannelMonitor
17:12 < ariard> hmmmmm, you mean `monitor.broadcast_latest_holder_commitment_txns` L4160
17:13 < BlueMatt> yes
17:13 < ariard> I think the issue is more switching between old/new *ChannelMonitor*
17:13 < BlueMatt> right, how can I rephrase the comment to make that more clear?
17:13 < ariard> like the older one won't consider the channel has closed and as such authorize a new update
17:14 < BlueMatt> yes
17:14 -!- tibo [~tibo@2400:4050:2a83:7000:8128:78ae:f37d:702d] has quit [Ping timeout: 260 seconds]
17:16 -!- jeremyrubin [~jr@024-176-247-182.res.spectrum.com] has joined #rust-bitcoin
17:19 < ariard> "If you deserialize a newer ChannelMonitor with an older ChannelManager, force-closing transactions will be broadcasted. If you deserialize an older ChannelMonior with the same ChannelManager, channel won't be see as closed and the state of broadcast transactions might be revoked" ?
17:21 < ariard> BlueMatt: in fact detecting your ChannelMonitor and ChannelManager aren't maching anymore is a pathological case, could we succeed the deser but block any further channel update for this ChannelManager?
17:22 < BlueMatt> no, that phrasing is incorrect - if you later deserialize the same montior and manager pair, that's ok, it'll do the same thing
17:22 < BlueMatt> the issue only applies if you deserialize with one channelmanager, and then the same monitor with a *different* (ie newer) channelmanager
17:23 < BlueMatt> there isn't, I believe, any bug in the code, just a noteworthy case.
17:26 < ariard> BlueMatt: I don't think in current code but can't we turn some flag in `broadcast_latest_holder_commitment_txn` to prevent accepting update from the different, newer channelamanger?
17:27 < BlueMatt> no? Because you cant persist that flag anywere?
17:27 < BlueMatt> i believe we do have a flag in the channelmonitor, so the issue only really exists if the channelmanager isnt persisted
17:27 < ariard> aaaaah I see, it's a reference to your monitor and not the final one?
17:27 < BlueMatt> hmm? no, always the final monitor
17:27 < BlueMatt> monitors arent allowed to be stale, per api guarantees
17:28 < BlueMatt> managers are allowed to be stale, you just cant use two different ones
17:36 < ariard> BlueMatt: oky I think comment should say if you "later deserialize the ChannelManager version corresponding to this ChannelMonitor, channels will be considered alive and you may end up revoking a state..."
17:37 < ariard> IIUC this noteworthy case only applies when commitment numbers are matching at second deser
17:38 -!- tibo [~tibo@2400:4050:2a83:7000:990e:ed7e:a28f:fabe] has joined #rust-bitcoin
17:39 < ariard> saying ChannelManager XYZ or ChannelMonitor 123 would help here, older/newer is a bit confusing :p
17:42 -!- tibo_ [~tibo@2400:4050:2a83:7000:9123:9c4:bec3:f6bf] has quit [Ping timeout: 260 seconds]
17:45 -!- Franck[m] [franckroye@gateway/shell/matrix.org/x-tytnmgipozsqofqo] has quit [Ping timeout: 265 seconds]
17:48 -!- zkao [zkaomatrix@gateway/shell/matrix.org/x-rjnrdfshscajnjjm] has quit [Ping timeout: 244 seconds]
17:49 < BlueMatt> ariard: hmm, I cant parse your suggested wording there lol
17:50 < BlueMatt> ariard: but, notably, the channelmonitors involved isnt the issue
17:50 < BlueMatt> its really just about two channelmanager copies, one newer, one older
17:50 < BlueMatt> you cant use both.
17:53 -!- Franck[m] [franckroye@gateway/shell/matrix.org/x-vxzwasqykdclslym] has joined #rust-bitcoin
18:27 -!- tibo_ [~tibo@2400:4050:2a83:7000:7c55:3f3a:3a85:d0ab] has joined #rust-bitcoin
18:30 -!- tibo [~tibo@2400:4050:2a83:7000:990e:ed7e:a28f:fabe] has quit [Ping timeout: 264 seconds]
18:40 -!- zkao [zkaomatrix@gateway/shell/matrix.org/x-bcfuhmmpyqeficpa] has joined #rust-bitcoin
20:41 -!- jeremyrubin [~jr@024-176-247-182.res.spectrum.com] has quit [Ping timeout: 260 seconds]
20:46 -!- shesek [~shesek@unaffiliated/shesek] has quit [Remote host closed the connection]
20:46 -!- shesek [~shesek@164.90.217.137] has joined #rust-bitcoin
20:46 -!- shesek [~shesek@164.90.217.137] has quit [Changing host]
20:46 -!- shesek [~shesek@unaffiliated/shesek] has joined #rust-bitcoin
20:48 -!- gribble [~gribble@unaffiliated/nanotube/bot/gribble] has quit [Remote host closed the connection]
20:57 -!- gribble [~gribble@unaffiliated/nanotube/bot/gribble] has joined #rust-bitcoin
21:02 -!- jeremyrubin [~jr@024-176-247-182.res.spectrum.com] has joined #rust-bitcoin
23:32 -!- Kiminuo [~Kiminuo@141.98.103.76] has joined #rust-bitcoin
23:54 -!- Kimi [~Kiminuo@141.98.103.196] has joined #rust-bitcoin
23:58 -!- Kiminuo [~Kiminuo@141.98.103.76] has quit [Ping timeout: 260 seconds]
--- Log closed Fri Mar 05 00:00:47 2021