--- Day changed Mon Aug 20 2018
07:53 -!- itaseski [~itaseski@213.135.176.244] has joined #rust-bitcoin
08:20 -!- elichai2 [uid212594@gateway/web/irccloud.com/x-bzfnoksqguxoizcv] has joined #rust-bitcoin
14:07 < andytoshi> BlueMatt: can you take a look at https://github.com/rust-bitcoin/rust-bitcoin/pull/113 today? i think this is the main thing blocking me moving forward on rust-bitcoin 0.14
14:08 < andytoshi> (well, i have to review the serde stuff and shit, but i think i can do that without you)
14:08 < BlueMatt> yep, k
14:09 < BlueMatt> its in need of rebase
14:09 < BlueMatt> but will take a look
14:10 < andytoshi> oh kk, i'll rebase it
14:10 < andytoshi> let me just merge #95 and #138 (both very minor) first
14:22 < andytoshi> done. rebased
14:55 < BlueMatt> andytoshi: why does PubKey::from_secret_key require a Signing context?
15:00 < andytoshi> because it is doing a constant-time multiply by the generator
15:00 < andytoshi> it's the same operation as deriving a public nonce from a secret nonce (which is what signing requires a Signing context for)
15:00 < BlueMatt> ah, yea, ok, fair
15:26 < BlueMatt> andytoshi: k, reviewed!
15:26 < BlueMatt> andytoshi: fwiw I'm really not a fan of #136
15:29 < BlueMatt> requiring minimal pushes for iterator seems like it could break someone using the iterator to check if a script is spendable by them and someone sends a weirdly-encoded script, breaking something?
15:29 < andytoshi> BlueMatt: yeah, i'm redoing #136 right now to make it optional
15:29 < BlueMatt> oh, was just gonna recommend that
15:29 < andytoshi> do you think the default behaviour should be to enforce minimality?
15:29 < BlueMatt> given that I can equally see someone being broken by *not* enforcing minimal push
15:29 < andytoshi> like have `iter()` and `iter_sloppy()`
15:29 < BlueMatt> I think there should be no default
15:30 < BlueMatt> iter(bool)
15:30 < andytoshi> oh, sure, i can do that
15:30 < BlueMatt> force people to be aware of the potential issue
15:30 < andytoshi> how about the impl on &'a Script ... sholud i remove that?
15:30 < BlueMatt> you mean impl Iterator? yea, just remove, I'd say
15:30 < andytoshi> (i also think that's a bit surprising because i'd expect that to be a byte iterator anyway)
15:30 < BlueMatt> its a weird behavior
15:30 < andytoshi> cool
15:31 < andytoshi> agreed .. you do `for x in &myscript { .. }` your `x`s are some weird associated type which the script is not actually made of
15:31 < andytoshi> also, there's a bug in the script iterator where it will return an infinite stream of Errors if anything goes wrong
15:32 < andytoshi> so i'll fix that in this PR, to kill itself after the first error
15:34 < BlueMatt> #139 looks good to me, but it also kinda seems like needless api churn?
15:34 < BlueMatt> but its the right nomenclature
15:34 < andytoshi> yeah, on the one hand it feels needless
15:34 < andytoshi> and on the other hand, every time i have to type `prev_index` and `prev_hash` instead of txid/vout i have to look it up
15:34 < andytoshi> i have no clue why i chose those names
15:34 < BlueMatt> heh, I mean whatever, up to you
15:34 < BlueMatt> I dont care either way
15:34 < andytoshi> kk
15:35 < andytoshi> i'm going to accept it because the existing API is so irritating :P
15:39 < BlueMatt> andytoshi: re: panic=abort in bitcoin_hashes, I think that forces all dependant projects to be completely panic=abort
15:39 < BlueMatt> which sucks?
15:39 < BlueMatt> or maybe thats only true on old rust?
15:40 < BlueMatt> not 100% sure but I was annoyed by that previously when I was using it
15:41 < andytoshi> i googled around and i _think_ that's no longer true
15:41 < andytoshi> but i was confused by the answers
15:41 < andytoshi> i can remove it
15:41 < andytoshi> because i'm uncertain
15:41 < BlueMatt> yea, I think it was only true on older rustc
15:41 < BlueMatt> but not-that-older
15:41 < andytoshi> like, people were talking about complexities of mixing panic-abort and non-panic-abort libraries, and everyone seems to take for granted that it's possible
15:41 < andytoshi> i guess i could just ask on #rust .. one sec
15:41 < BlueMatt> fair
16:24 -!- itaseski [~itaseski@213.135.176.244] has quit [Quit: Leaving]
17:27 < andytoshi> ah, steve klabnik says that i will force downstream users to use panic=abort
17:27 < andytoshi> so i'll remove that
17:30 < andytoshi> hm, maybe i never implemented it ... i swear i remember doing so
17:55 < andytoshi> dongcarl: can you rebase https://github.com/rust-bitcoin/rust-bitcoin/pull/137 and address the nits? i'd like to get this into 0.14.0
18:01 < andytoshi> dongcarl: but i think i will push BIP173 support out to 0.15, sorry, alongside bitcoin_hashes and (maybe) rust-secp 0.11
18:01 < andytoshi> reason being that i want to test the bip173 stuff against the script_descriptor stuff, which (a) still needs work, (b) needs a bunch of other stuff from 0.14
18:07 -!- itaseski [~itaseski@213.135.176.244] has joined #rust-bitcoin
18:08 < BlueMatt> andytoshi: "Since even valid SecretKeys were able to be invalid tweaks"...hmm? what are the conditions for valid tweak?
18:08 < andytoshi> BlueMatt: a valid tweak is something that results in a valid key after tweaking
18:09 < andytoshi> so specifically, if you try to "tweak" a secret key with its negation, you'll get 0
18:09 < andytoshi> even though both the secret key and its negation were valid keys individually
18:09 < BlueMatt> ah, ok, missed the negative thing
18:09 < andytoshi> yeah. i'm pretty sure that's actually the only case when a tweak is invalid
18:11 < BlueMatt> Do we want to have a tweak-always-valid-if-ThirtyTwoByteHash version?
18:12 < BlueMatt> I guess not, cause secretKey could be hash of the same object
18:12 < BlueMatt> nvm
18:12 < BlueMatt> thats dumb
18:14 < andytoshi> i'm not sure.
18:14 < andytoshi> the secret key would have to be the negative of a hash
18:14 < andytoshi> which would be pretty crazy
18:15 < BlueMatt> well eg i was thinking of cases where you have an attacker-supplied secretkey and something built into a hash
18:15 < andytoshi> and i don't want a From<ThirtyTwoByteHash> impl on SecretKey because (a) i'm not aware of any usecases that directly do this, BIP32 uses halves of a 64-byte hash for example; (b) I worry that people will footgun themselves making secret keys out of public data
18:15 < BlueMatt> well if the attacker could predict the hash then they could supply a key which results in a bad tweak
18:15 < BlueMatt> so it'd not be a great api to epose
18:15 < andytoshi> yeah... the right way to use this is to have the original public key go into the hash, but we can't enforce this at an API level
18:16 < andytoshi> in that case it really is impossible to get a bad tweak (this is what bip32 does, approximately, and what p2contract does)
18:17 < BlueMatt> yea
18:19 < andytoshi> BTW i went through the open PRs in rust-bitcoin and rust-secp and milestone tagged them so it's obvious what's breaking and what's not
18:21 < BlueMatt> andytoshi: do you want to add an iter().collect() on Script in the script deserialize fuzz test?
18:21 < andytoshi> BlueMatt: yeah, i do
18:21 < andytoshi> it's annoying that the failure mode is "run forever"
18:21 < BlueMatt> :)
18:21 < andytoshi> because travis seems to time out accidentally sometimes and it's hard to distinguish
18:22 < BlueMatt> oh? I've never seen that on rust-lightning
18:23 < andytoshi> on rust-bitcoin i've seen it happen once or twice
18:23 < andytoshi> next time it happens i'll flag you and maybe we'll investigate .. it could be an actual problem (though i've never seen it take too long locally)
18:25 < BlueMatt> hmm, yea, if its usually like under a half hour and sometimes it times out completely (>1 hour iirc) then thats probably a sign that one of the targets has some severe performance degradation path
18:32 < andytoshi> kk, running 100M iterations locally then i'll push a fuzz test to #136
18:32 < andytoshi> should be 5-10 minutes unless something's broken
18:32 < BlueMatt> if you remind me I'll regenerate the test cases after the next release
18:32 < BlueMatt> though my fuzzing resources are pretty tied up atm since I've been getting results out of my full_stack_target
18:32 < andytoshi> cool, thanks. and no worries, no rush
18:35 < BlueMatt> wait, huh, why cant PublicKey::combine "be used with anything else in the API"?
18:35 < andytoshi> there's no use for it
18:35 < andytoshi> if you know both secret keys you can add those and then derive a new pubkey
18:35 < andytoshi> (but you shouldn't be doing this, that's bad news)
18:36 < andytoshi> if you don't, well, there are no multisigning functions or anything
18:36 < andytoshi> this function was an artifact of the old schnorr module which was overlooked upstream when that was removed
18:36 < BlueMatt> does upstream want to remove this function?
18:38 < andytoshi> yes
18:38 < BlueMatt> ok!
18:38 < andytoshi> in fact I should PR to do that later today when I switch to C mode..
18:38 < andytoshi> BlueMatt: should i PR to bump rust-secp to 0.10.1 now?
18:38 < andytoshi> the only iffy thing is whether we want to try and get the 64-bit compilation in
18:39 < BlueMatt> given windows compilation is already broken, I dont see it as a big issue https://github.com/rust-bitcoin/rust-secp256k1/issues/45
18:40 < andytoshi> hmm yeah
18:40 < andytoshi> though we should fix that
18:40 < BlueMatt> indeed
18:40 < andytoshi> maybe i'll tag 0.10.1 and then we can try to fix compilation in 0.10.2
18:40 < BlueMatt> fair
18:41 < andytoshi> BTW the script fuzzing still has not stopped .. will honggfuzz time out at some point?
18:41 < BlueMatt> i think travis will time out before honggfuzz does
18:42 < BlueMatt> unless it hits a test case that blows its run-time limit
18:42 < BlueMatt> but I think honggfuzz's default is super high
18:42 < BlueMatt> which we should probably just make lower, tbh
18:42 < andytoshi> i'm running locally
18:42 < andytoshi> but yeah, probably
18:43 < BlueMatt> honggfuzz wont ever time out unless it considers a test as failed due to it being over run-time
18:43 < BlueMatt> if you run without -N it just runs forever anyway
18:44 < andytoshi> hmm, i updated `rand` to 0.4 and `cc` to 1.0 since 0.10.0 ... so strictly speaking i should release a new major version now
18:44 < andytoshi> bleh
18:44 < andytoshi> i just want the freeking debug/display impls
18:45 < BlueMatt> cc at least shouldnt matter
18:45 < BlueMatt> cause its a dev-dep
18:45 < andytoshi> right
18:45 < andytoshi> (although i'm pretty sure it broke windows compilation)
18:45 < BlueMatt> though I guess rand might? I dunno...
18:45 < BlueMatt> ohoh
18:45 < BlueMatt> I mean could just go for 0.11 lol
18:45 < andytoshi> lol yeah, and PR to rust-bitcoin to update i guess
18:46 < andytoshi> might as well, i don't think we'll get rust-bitcoin 0.14 out today
18:46 < BlueMatt> heh, ok
18:46 < andytoshi> and that's a quick turnaround because there are no actual breaking changes
18:48 < BlueMatt> oh, I figured you meant also merge a bunch of the other outstanding prs too
18:49 < andytoshi> oh, yeah
18:49 < andytoshi> we can do that actually
18:50  * andytoshi afk for a few hours
18:58 < andytoshi> yeah i think #49 and #51 should go in 0.11
18:59 < andytoshi> err
18:59 < andytoshi> #50 and #51
18:59 < andytoshi> #49 should wait because we're waiting on upstream
19:00 < BlueMatt> see my comment on #51, I kinda suggest we wait on the same from upstream for 51
19:00 < BlueMatt> wait, and not 42?
19:00 < BlueMatt> if we're gonna bump major why not try the new compilation stuff?
20:15 -!- elichai2 [uid212594@gateway/web/irccloud.com/x-bzfnoksqguxoizcv] has quit [Quit: Connection closed for inactivity]
20:19 < andytoshi> sure, we can do the new compilation stuff
20:19 < andytoshi> i don't feel strongly about it. it just seemed the most risky thing tome
20:19 < andytoshi> oh, and right, i forgo that #51 made a no-caps context
20:19 < andytoshi> yeah, we can wait for upstream on that
20:19 < andytoshi> so only the compilation stuff then?
21:30 < BlueMatt> heh, yea, I guess
21:30 < BlueMatt> btw do you want to email github and get them to remove the "forked from steveklabnik/bitcoin-secp256k1-rs" line?
21:30 < BlueMatt> since thats not really upstream anymore?
21:31  * BlueMatt -> out
22:41 -!- itaseski [~itaseski@213.135.176.244] has quit [Ping timeout: 272 seconds]
23:05 < andytoshi> it was never upstream, i have no idea how that happened
23:06 < andytoshi> but sure i'll email them
23:58 < andytoshi> heh, github says the old upstream was deleted and for some reason it changed to that instead of just becoming independent
23:58 < andytoshi> should be fixed soon