--- Log opened Tue May 04 00:00:45 2021
01:13 < sgeisler> BlueMatt: thx for the heads up, sure, go ahead, just checked you are an owner already
03:21 -!- Judson13Terry [~Judson13T@static.57.1.216.95.clients.your-server.de] has joined #rust-bitcoin
05:54 < andytoshi> heh, technically removing the fuzztarget feature was a breaking change in bitcoin_hashes
05:54 < andytoshi> but i think removing something with an all caps "do not use this" warning is ok
06:33 -!- ghost43 [~daer@gateway/tor-sasl/daer] has quit [Remote host closed the connection]
06:33 -!- ghost43 [~daer@gateway/tor-sasl/daer] has joined #rust-bitcoin
07:22 < BlueMatt> ariard: if you get a chance, can you squash the fixup commits on packagetemplate?
07:23 < BlueMatt> its a bit hard to review with the fixup commits not in order of what they fix up...
07:23 < BlueMatt> andytoshi: well we should just drop the fuzztarget feature in rust-bitcoin too
08:03 < ariard> BlueMatt: yep will fix, thanks for reviewing
08:05 < BlueMatt> ariard: I dont mind doing it in a followup, but I do feel like the enums could be tightened a lot - there's a bunch of "in this context, enum must be of variants A, B, but cannot be variant C"-type stuff, which just feels icky
08:06 -!- Judson13Terry [~Judson13T@static.57.1.216.95.clients.your-server.de] has quit [Ping timeout: 240 seconds]
08:08 < BlueMatt> if nothing else, it really needs to be carefully documented.
08:33 < ariard> BlueMatt: you mean interdependency between PackageMalleability and type of PackageSolvingData? i guess i can dry-up malleability in PackageSolvingData
08:59 < andytoshi> 14:23 < BlueMatt> andytoshi: well we should just drop the fuzztarget feature in rust-bitcoin too
08:59 < andytoshi> agreed
09:03 < BlueMatt> andytoshi: sgeisler disagrees
09:04 < sgeisler> I mean we certainly could do that, but it's not really clean :(
09:04 < BlueMatt> andytoshi: see https://github.com/rust-bitcoin/rust-bitcoin/pull/602#issuecomment-832038974
09:04 < sgeisler> This all seems a bit rushed
09:04 < BlueMatt> sgeisler: I agree, I mean I wish we weren't here and rushing it. That said, now that we are, at least the fuzztarget feature specifically I see no reason to stress about
09:05 < BlueMatt> I'd put good money on the only use being in some of the fuzz subcrates in other rust-bitcoin crates
09:05 < BlueMatt> which, like, its just testing stuff, not released stuff
09:05 < BlueMatt> shit the rust-ligtning fuzz crate already breaks every time there is a minor release of secp
09:05 < sgeisler> let's at least check bdk, I'll take a look
09:05 < sgeisler> breaking downstream tests is not nice
09:06 < BlueMatt> right, agreed, just depends on how "downstream" it is :)
09:07 < sgeisler> ok, they don't seem to do fuzzing
09:08 < sgeisler> for me it's mostly about respecting downstream's time, we fucked up we ideally fix it internally
09:08 < BlueMatt> yea, agreed, but also applies much less for fuzztarget
09:08 < BlueMatt> the actual macro issues is a much bigger deal
09:08 < sgeisler> I agree that fuzztaget is probably ok, but so we thought about removing pub core xD
09:10 < sgeisler> so, if we remove fuzztaget, what are we fixing: only our tests or downstream builds?
09:11 < sgeisler> because old rust-bitcoin versions will stay out ther
09:11 < sgeisler> they better not be broken
09:11 < BlueMatt> right, its definitely a statement/assumption that we don't care about breaking fuzztarget - it was technically never supported anyway and had a comment in cargo.toml telling downstream not to use it.
09:15 < sgeisler> Ok, I played around a bit and it appears we have to do nothing really to make old rust-bitcoin avoid the newer version, cargo is intelligent enough not to choose one that lacks the necessary features
09:16 < BlueMatt> huh, cool
09:17 < sgeisler> I tried to force it with `update -p` and it complained: the package `bitcoin` depends on `bitcoin_hashes`, with features: `fuzztarget` but `bitcoin_hashes` does not have these features.
09:17 < sgeisler> so it stayed 0.9.4
09:17 < BlueMatt> cool
09:18 < sgeisler> So we can just do whatever in that case xD still feels a bit dirty, but go ahead with that PR if you want
09:19 < BlueMatt> certainly agree about the original mess, though fuzztarget I dont care about :)
09:38 < andytoshi> sgeisler: we explicitly don't care about fuzztarget
09:38 < andytoshi> and publicly exporting `core` we fucked up because we missed that we were exposing use of the symbol through macros
09:39 < andytoshi> which is not a process failure, that's just life, macros in rust are shit and they have caused this exact problem multiple times and will cause this problem again in the future
09:40 < andytoshi> i'm very surprised by this cargo behavior and i don't think we should rely on it
09:40 < andytoshi> but whatever, if anyone was using the fuzztarget feature then their code was badly broken in a security-compromising way that affected not only their crate but every codependency
09:44 < andytoshi> lol regarding macros being shit, i see rcassata complaining that even our fix has hygeine issues
09:44 < andytoshi> so there we go, we made it exactly 0 time before again hitting this Rust design flaw
09:45 < BlueMatt> yea, for all the "look, rust's macros are 10000x better than C/C++" rust people say, they're still shit
09:45 < BlueMatt> its one of those zero times anything is still zero
09:45 < andytoshi> lol
09:47 < andytoshi> we can do another release (and we should yank the old ones)
09:47 < sgeisler> abdytoshi: thinking about it a bit more I agree by now that the proposed PR is probably a reasonable step forward, I just wanted to avoid piling on bad decisions and instead take the time to evaluate
09:48 < sgeisler> Knowing how cargo works now idc about 0.9.6 being out there (0.9.5 should be yanked)
09:49 < sgeisler> We should remember re-applying the core PR once we know we want to do a breaking version soon (it doesn't seem too urgent in and of itself to justify a 0.10 I think)
09:50 < andytoshi> ok lemme yank 0.9.5
09:50 < andytoshi> we don't know how to apply the core PR
09:50 < andytoshi> like, rewrite the macros to not use ::core when std is on?
09:51 < andytoshi> yanked 0.9.5
09:51 < BlueMatt> I dont think that fixes it - it depends on if the downstream crate has no_std on or not
09:52 < andytoshi> we could have two version of the macro lol, one for no_std and one not
09:52 < BlueMatt> lolol
09:52 < andytoshi> but i don't think it's worthwhile, i think if anything we could add a commnet to the re-export-core nonsense explaining that we need to do it because of the macros
09:52 < andytoshi> and just leave it be
09:55 < BlueMatt> right, we also doc[hide] it, and prefix it with a _
09:56 < BlueMatt> soooo...whatever
10:00 < BlueMatt> ariard: also eg the weight function in packagesolvingdata sometimes returning 0? or RecokedOutput sometimes coningina a InputDescriptors which is non-revoked?
10:00 < BlueMatt> ariard: its just confusing when there's multiple enums with different type constraints its unclear to me what preconditions are where
16:14 < ariard> BlueMatt: right weight function returning 0 it should be changed in the following PRs of the patchset, iirc i could panic on it but going to change just after
16:15 < BlueMatt> right, ok, a comment to that effect would be nice :)
16:16 < ariard> though a RevokedOutput with a non-revoked InputDescriptors shouldn't happen iirc?
16:17 < ariard> right i could list all the preconditions for wrapped enums when we have different levels
16:18 < BlueMatt> right, its really confusing that its possible in the datastructure, but isn't allowed, and its not commented anywhere
16:20 < ariard> right, i should panic on those cases, InputDescriptors is a legacy datastruct compare to this patchset, didn't think to incorporate new restrictions :)
17:02 -!- belcher_ [~belcher@unaffiliated/belcher] has joined #rust-bitcoin
17:06 -!- belcher [~belcher@unaffiliated/belcher] has quit [Ping timeout: 268 seconds]
17:09 -!- belcher_ is now known as belcher
17:47 -!- jeremyrubin [~jr@024-176-247-182.res.spectrum.com] has quit [Ping timeout: 265 seconds]
17:48 -!- jeremyrubin [~jr@024-176-247-182.res.spectrum.com] has joined #rust-bitcoin
--- Log closed Wed May 05 00:00:46 2021