--- Log opened Tue Jan 12 00:00:13 2021
03:20 -!- DeanWeen [~dean@gateway/tor-sasl/deanguss] has quit [Ping timeout: 240 seconds]
03:20 -!- DeanGuss [~dean@gateway/tor-sasl/deanguss] has joined #rust-bitcoin
04:44 -!- tibo_ [~tibo@2400:4050:2a83:7000:1865:a0f0:c0c4:130f] has quit [Remote host closed the connection]
05:17 -!- tibo [~tibo@2400:4050:2a83:7000:cdf0:dfbb:ddb:300] has joined #rust-bitcoin
05:17 -!- tibo [~tibo@2400:4050:2a83:7000:cdf0:dfbb:ddb:300] has quit [Remote host closed the connection]
05:24 -!- titanbiscuit [~tbisk@104.200.131.173] has quit [Ping timeout: 256 seconds]
05:28 -!- belcher_ is now known as belcher
05:41 -!- da39a3ee5e6b4b0d [~da39a3ee5@mx-ll-171.5.28-247.dynamic.3bb.co.th] has joined #rust-bitcoin
06:14 -!- belcher_ [~belcher@unaffiliated/belcher] has joined #rust-bitcoin
06:16 -!- belcher [~belcher@unaffiliated/belcher] has quit [Ping timeout: 264 seconds]
06:16 -!- belcher_ is now known as belcher
06:19 -!- jonatack [~jon@88.124.242.136] has quit [Ping timeout: 240 seconds]
06:21 -!- jonatack [~jon@88.124.242.136] has joined #rust-bitcoin
06:26 -!- jonatack [~jon@88.124.242.136] has quit [Ping timeout: 256 seconds]
06:26 -!- jonatack [~jon@134.19.179.235] has joined #rust-bitcoin
06:29 -!- belcher [~belcher@unaffiliated/belcher] has quit [Quit: Leaving]
06:50 -!- belcher [~belcher@unaffiliated/belcher] has joined #rust-bitcoin
08:53 -!- da39a3ee5e6b4b0d [~da39a3ee5@mx-ll-171.5.28-247.dynamic.3bb.co.th] has quit [Quit: My MacBook has gone to sleep. ZZZzzz…]
08:55 -!- da39a3ee5e6b4b0d [~da39a3ee5@2403:6200:8876:255c:8c1d:42fb:24ab:a8e9] has joined #rust-bitcoin
08:59 -!- da39a3ee5e6b4b0d [~da39a3ee5@2403:6200:8876:255c:8c1d:42fb:24ab:a8e9] has quit [Ping timeout: 246 seconds]
09:12 -!- jeremyrubin [~jr@2601:645:c200:14:91a5:d8cb:fe32:d136] has joined #rust-bitcoin
09:37 < stevenroose> byteorder dep fix for base64-compat: https://github.com/stevenroose/rust-base64-compat/pull/1
09:38 < stevenroose> I wonder if that would cause problems for downstream users. In principle it should be able to work without problems. I think it only causes problems if there's a byteorder 2.0 at some point.
09:42 < stevenroose> rebased the io::Error PR: https://github.com/rust-bitcoin/rust-bitcoin/pull/494
10:42 < BlueMatt> yay, its now actually supported to link rust and C code when targeting wasm :)https://github.com/rust-lang/rust/pull/79998
10:43 < BlueMatt> (not if you're using wasm-bindgen, but it is for wasm32-wasi or wasm32-unknown-emscripten)
11:16 < jeremyrubin> What would we think about changing/adding a new API for SigHashCache where scriptcode is a struct wrapped script that can only be made by calling a function where you have to specify that it is wpkh / the last codeseparator position or None?
11:18 < andytoshi> can i have a second ack for https://github.com/rust-bitcoin/rust-bitcoin/pull/494
11:18 < andytoshi> sanketcell: sgeisler:
11:19 < andytoshi> jeremyrubin: this sounds good in principle
11:19 < andytoshi> though i'm not sure how you specify "the last codeseparator position", i guess you just give a byte index
11:19 < andytoshi> this will run into a problem we have now though which is that there is no script slice type
11:20 < jeremyrubin> i just finished a big goose chase where I was like "why can't my psbt return a witness_script for this input"
11:20 < jeremyrubin> when in fact, because it was wpkh there was no witness_script
11:21 < andytoshi> heh interesting
11:21 < jeremyrubin> so making the API a bit more direct about converting a script to scriptcode would improve the straightforwardness of the api
11:21 < andytoshi> i don't fully follow .. how is scriptcode and "wpkh has no witness script" connected?
11:21 < jeremyrubin> ah
11:21 < andytoshi> lol holy shit elichai2 that was fast
11:21 < jeremyrubin> say I have a psbt
11:22 < andytoshi> ok lemme rebase the 0.26 PR and we'll have a release :)
11:22 < jeremyrubin> and I want to sign it using some custom signing code
11:22 < jeremyrubin> i need to generate a SigHashCache for that output
11:23 < jeremyrubin> I can detect it is a witness output by looking for the witness_utxo
11:23 < sanket1729> looks it elichai2 got it in. Are we ready for a release?
11:23 < jeremyrubin> and then, in most cases, the scriptcode is just the witness_script
11:24 < andytoshi> sanketcell: yeah
11:24 < andytoshi> jeremyrubin: ah! interesting
11:24 < andytoshi> yeah that's an interesting footgun
11:24 < jeremyrubin> well "most cases"
11:24 < andytoshi> i'm alllmost tempted to say "the witness script for wpkh is the pubkey"
11:24 < andytoshi> though i suppose that'd be differently wrong if you use it as a scriptcode
11:24 < jeremyrubin> except for P2WPKH
11:24 < jeremyrubin> yeah
11:24 < jeremyrubin> the scriptcode is something else
11:24 < jeremyrubin> honestly
11:25 < andytoshi> but like, rust-miniscript has a .script_code method that does the right thing here
11:25 < jeremyrubin> PSBT should just have a scriptcode field maybe heh
11:25 < andytoshi> maybe it doesn't, in the released version
11:26 < jeremyrubin> Anyways... scriptcode is definitely a different "type of thing" from a script, so it does probably make sense to make sure you can't just plug a script in without properly blessing it
11:28 < andytoshi> yeah agreed
11:29 < andytoshi> this may be a rust-bitcoin 0.27 thing
11:29 < andytoshi> we could make two new ScriptRef and ScriptCode types which are slices into Script
11:30 < andytoshi> https://github.com/rust-bitcoin/rust-bitcoin/pull/533 release PR for 0.26
11:31 < elichai2> andytoshi: I haven't even seen it here lol, I accidentally opened github at the right time hehe
11:31 < jeremyrubin> it's hard to get exactly right
11:31 < jeremyrubin> ideally, e.g., SigHashCache would take in whatever raw data
11:31 < jeremyrubin> and then whenever you actually ask for a sighash you need to provide the codesep value
11:32 < jeremyrubin> Maybe instead of taking scriptcode it should take witness_script : option<script>
11:32 < jeremyrubin> and if it is None, it treats it as wpkh
11:35 < andytoshi> that sounds weird
11:35 < andytoshi> i don't think we should bless wpkh like that
11:35 < andytoshi> we should just have a scriptcode type
11:35 < sanket1729> yeah. I think the wrapped script approach is better
11:35 < sanket1729> yep, a new type
11:35 < BlueMatt> why is briansmith such a dick?
11:36 < andytoshi> i guess scriptcode would be a wrapped Cow<[u8]> ... for wpkh it has to be a new script
11:36 < andytoshi> BlueMatt: where?
11:36 < BlueMatt> I mean i think I've had two interactions with him, both negative, but he's on the thread about wasm-bindgen/wasm-C abi incompatibility bitching about it without bothering to read about the issue in any meaningful way
11:41 < BlueMatt> "why isnt this supported" on an issue where all the regular contributors have responded to most things with "you can do change X or Y, but I
11:41 < BlueMatt> m too busy to do it"
11:41 < BlueMatt> like, sheesh, dude, learn what open source means
11:57 < elichai2> BlueMatt: He moved between 2 things so quickly that I don't quite get what he's trying to get at
11:58 < elichai2> like he moved from understanding the dichotomy you made between bindgen and rustc/clang, to talking about wasi and wasi syscalls
11:58 < BlueMatt> he's trying to get at the fact that he wants someone to fix something, and he doesn't want to do it himself.
11:58 < BlueMatt> my previous experience with him was him responding to people offering to implement a feature for him with "nah, but I'll do it if you pay me"
11:58 < BlueMatt> I dont understand why anyone would use ring.
11:59 < elichai2> ring is the only sane AES-GCM implementation
12:00 < elichai2> (in rust)
12:00 < BlueMatt> doesnt it just call out to C copied from boringssl?
12:00 < BlueMatt> also, last i looked, its api was completely batshit.
12:01 < elichai2> it's bindings to the boringssl code, yeah
12:02 < elichai2> and yeah the API is really weird
12:02 < elichai2> altough it's allocation free which is nice
12:02 < BlueMatt> heh, right
12:04 < elichai2> but I hate on RustCrypto's generic attitude more
12:04 < BlueMatt> heh, i havent looked at rustcrypto since they kicked back up after the original author of the things stopped working on them.
12:05 < elichai2> there are 2 rustcrypto projects, there's the abandon rust-crypto and there's RustCrypto
12:05  * BlueMatt sticks with his own transliterated chacha20poly1305 implementation for now
12:06 < BlueMatt> right, i havent looked at rustcrypto since it forked into RustCrypto
12:06 < elichai2> I didn't know/remember that RustCrypto kicked a maintainer
12:06 < BlueMatt> at least not materially
12:06 < elichai2> oh ok
12:06 < BlueMatt> nono, I'm just saying its a fork of rust-crypto
12:06 < BlueMatt> which is dead cause its maintainer stopped working on it
12:06 < elichai2> yeah I don't think it makes any sense to have an AEAD generic over both the MAC and the encryption
12:06 < elichai2> and the size of the nonce, and the size of the MAC, and the size of the key etc
12:07 < BlueMatt> heh, that does seem overengineered.
12:07 < BlueMatt> though not explicitly unsafe
12:07 < elichai2> well it's unsafe if you accidentally make it SHA1
12:07 < BlueMatt> and its better than looking up algorithms by passing in a fucking string.
12:07 < BlueMatt> heh, right, but they should remove unsafe or hide it behind a LOL_BAD_CRYPTO feature
12:08 < BlueMatt> errr, sha1
12:08 < elichai2> oh yeah strings for algorithms are bad
12:08 < BlueMatt> which iirc ring does
12:08 < elichai2> even not just plain unsafe like SHA1, but if you don't notice you might accidentally use poly1305 as a general MAC because it implements some trait for AEAD
12:09 < elichai2> BlueMatt: really? I didn't know that
12:09 < BlueMatt> it did at some point...maybe not anymore? dunno
12:09 < BlueMatt> last i looked at it it did, which resulted in me never looking at it again, cause, like, wtf
12:09 < elichai2> anyway I don't trust current rust's AES implementations. and for chacha20poly1305 it's easier to just implement it yourself safely
12:10 < BlueMatt> right.
12:10 < elichai2> but their Hash implementations are good
12:10 < elichai2> I use them a lot
12:10 < BlueMatt> point being, all crypto implementations in rust are somewhat scary, doubly so ring.
13:52 < andytoshi> i kinda like cryptoxide
13:52 < andytoshi> which explicitly does not support aes :P
13:53 < andytoshi> it's RustCrypto minus all the weird trait stuff, minus all the non-constant-time stuff, minus all the deps
14:27 < jeremyrubin> btw i found a bug in the Amount serializing code
14:28 < jeremyrubin> you have to also include     #[serde(skip_serializing_if = "Option::is_none")]
14:28 < jeremyrubin> if you are using Option<Amount>
14:29 < jeremyrubin> The tests do not currently cover this
14:30 < jeremyrubin> because they never check round trip ser/de equality
14:30 < thomaseizinger> <andytoshi "i kinda like cryptoxide"> I like that they went for a single crate approach with many feature flags instead of having a lot of tiny crates. Makes maintenance and usage so much easier.
14:31 < andytoshi> jeremyrubin: do we have fuzztests that check this?
14:31 < andytoshi> i'm assuming not, but we should..
14:32 < jeremyrubin> idts
14:32 < jeremyrubin> it's kinda bad because presumably any use case of opt is seriously broken at the moment
14:33 < jeremyrubin> it can be fixed a couple ways... or maybe no one is using it but me and we can remove it & come up with a better way to handle
14:34 < andytoshi> *shrug* i doubt anyone is really using Amount except as temporary wrapper for Display
14:34 < jeremyrubin> basically {"a" : null} is rejected on deser but {} is accepted
14:34 < jeremyrubin> and serializers output {"a" : null}
14:34 < andytoshi> lol oops
14:35 < andytoshi> can we just add that #[serde] guard?
14:35 < andytoshi> i forget if we're using serde_derive or what
14:35 < jeremyrubin> nope
14:35 < andytoshi> lol. so annoying
14:35 < jeremyrubin> it's done in a super hack way
14:36 < andytoshi> i'll take a look in 5-10 minutes ... my workstation is really bogged down right now
14:36 < jeremyrubin> https://github.com/rust-bitcoin/rust-bitcoin/pull/501 is helpful context
14:36 < andytoshi> i think 456 might be too many rustcs to run at once.
14:36 < jeremyrubin> lol wat
14:37 < andytoshi> well i'm checking all the commits of a liquid PR, with multiple cargo versions, all in fresh workspaces in parallel
14:37 < andytoshi> and also the rebased versions of all those commits
14:37 < jeremyrubin> yikes
14:37 < andytoshi> and there's a big dep tree to rebuild :P
14:38 < andytoshi> i mean, not that big, but with that kinda multiplier..
14:38 < jeremyrubin> no rush
14:39 < andytoshi> ok it looks like enough of them finished tha ti'm good now
14:39 < jeremyrubin> it is fixable by adding the attribute every time you use an Amount
14:39 < jeremyrubin> i think that no one has complained means im the first to use it; so i think a breaking change is not a big deal
14:39 < andytoshi> man the amd epyc is incredible
14:39 < andytoshi> jeremyrubin: yeah, i believe you're the first to use this
14:39 < jeremyrubin> and a more generic fix is needed anyways for e.g. Vec<Amount>
14:39 < thomaseizinger> <andytoshi "well i'm checking all the commit"> I think there should be a way of getting GitHub actions to check each individual commit so this doesn't need to happen locally. Although it might also be a bit of a resource waste if it happens unconditionally ..
14:40 < andytoshi> thomaseizinger: there is no way github could ever be trustworthy enough that i'd trust its CI without running stuff locally
14:41 < andytoshi> although, this is blockstream code i'm playing with and we have our own CI boxes....i just have so much compute power that doing this seemed like an easy way to shortcut some of it
14:41 < andytoshi> jeremyrubin: so, what is the actual issue in rust-bitcoin here? do we use Option<Amount> somewhere?
14:42 < andytoshi> and if you use it in your own code, what could the library do about that?
14:42 < andytoshi> ah found it
14:42 < andytoshi> in serde_as_btc_opt
14:43 < andytoshi> where we totally are using serde_derive..
14:43 < andytoshi> oh, wait, i'm looking at a unit test
14:43 < thomaseizinger> Fair enough, back when I was initially thinking about the idea, the main concern was that each patch should be properly formatted and at the very least build to make bisecting easier, i.e. not very security sensitive checks but annoying to do locally.
14:44 < andytoshi> thomaseizinger: well, you ideally want it to build with all the features .. and you want the unit tests to pass
14:44 < andytoshi> i originally would fuzz-test every commit but that got to be a bit much even for me
14:46 < andytoshi> jeremyrubin: ah -- the bug is that we won't deserialize "null" as None
14:46 < andytoshi> ok, that's not too hard to fix, we can probably even get it into 0.26
14:51 < jeremyrubin> yeah it's definitely fixable. Although I will say that I still think the problem is the cludgy lack of a default serialization, which prevents generics containing Amount from working (e.g., Vec<Amount>)
14:52 < jeremyrubin> There's also another bug BTW which is that Amount::max_value() cannot be serialized/deserialized in json
14:52 < jeremyrubin> actually I think it can be serialized, but not deserialized
14:52 < jeremyrubin> We may also want to switch to representing the numbers as strings
14:53 < jeremyrubin> or something else
14:53 < jeremyrubin> Or we should change max value to be, e.g., 21e6
14:54 < andytoshi> yes, it can be serialized, it's just that we don't deserialize what we serialize
14:54 < jeremyrubin> maybe max_valid_value?
14:54 < andytoshi> it's pretty overt,
14:55 < andytoshi> we call `.serialize_none` and then our desirialize impl always produces a Some. really make syou think
14:55 < andytoshi> yeah i wouldn't mind reducing max_value
14:56 < andytoshi> in fairness there's a cargo feature `arbitrary_precision` in serde-json that'll let you deserialize bignums
14:56 < andytoshi> but i think it'd be better to just fix max-vaule
14:57 < jeremyrubin> well one annoying thing
14:57 < andytoshi> i'm not sure why you describe the lack of default serilaizaiton as "kludgy", it was a deliberate design decision and would be technically trivial to fix
14:57 < jeremyrubin> from_sat type signature has to change
14:57 < jeremyrubin> from_sat(u64) -> Amount ---> from_sat(u64) -> Result<Amount>
14:58 < jeremyrubin> It's kludgy because it breaks a bunch of other downstream things that would be nice to work
14:59 < andytoshi> well, that's just serde
14:59 < jeremyrubin> Which is the serialization library we're trying to use ;)
14:59 < jeremyrubin> Maybe we could add a wrapper
14:59 < jeremyrubin> SerAsFloat
14:59 < jeremyrubin> SerAsInt
15:00 < jeremyrubin> and SerAsFloat(Amount) impls Serialize?
15:00 < jeremyrubin> and also impls some sort of deref coercion
15:00 < jeremyrubin> you can use transparent serialization so there is no overhead
15:01 < andytoshi> i mean, we're not trying very hard, serde is feature-gated and Amount is a second-class type
15:01 < andytoshi> but yeah, agreed that having wrapper types would be better for serialization
15:01 < thomaseizinger> what is the advantage of wrapper types over the serde modules we have now?
15:01 < andytoshi> thomaseizinger: jeremyrubin: says that they don't work with Vec<Amount>
15:02 < jeremyrubin> Or (Amount, Amount) is also broken for ex
15:02 < thomaseizinger> there is this: https://docs.rs/serde_with/1.6.0/serde_with/
15:02 < jeremyrubin> You have to implement the proper functions for every concrete type tho
15:03 < jeremyrubin> you can review the exisiting code for Amount, as it is done for Option<Amount>
15:03 -!- DeanGuss [~dean@gateway/tor-sasl/deanguss] has quit [Remote host closed the connection]
15:04 < andytoshi> yeah that is shitty that we did that (and it led to this bug)
15:04 < andytoshi> thomaseizinger: so, in the codebase you can grep for `mod opt` to find special-purpose code we wrote for Option<Amount> .. and you can see a unit test for this by grepping `serde_as_btc_opt`
15:05 < thomaseizinger> I see, yeah the `serde(with)` design leads to an exploding number of modules if you want to support all these combinations
15:05 < andytoshi> is there a reasonable way we could avoid that? (and also avoid special-casing Vec<Amount>, (Amount, Amount), etc ad infinitum)
15:05 < andytoshi> yeah :/
15:05 < thomaseizinger> I think the crate I linked above is meant to fix this: https://docs.rs/serde_with/1.6.0/serde_with/#advanced-serde_as-usage
15:06 < andytoshi> true, but then that's an extra dep people need in order to use our Amount type
15:06 < thomaseizinger> Not necessarily. I think we might get away with providing said wrapper types and then people are free to either use `serde_with` to avoid the boilerplate of converting or convert themselves.
15:07 < jeremyrubin> There's also the third angle which is that amount should in some contexts be acceptable to get in any user-preferred denomination
15:08 < jeremyrubin> but maybe that can be handled in a completely different way
15:08 < jeremyrubin> I think the Wrapper types SerAsF64<T> SerAsU64<T> which work on SignedAmount and Amount would work.
15:09 < jeremyrubin> they would internally use the mod serde, but we then wouldn't need to export them
15:09 < andytoshi> i mean, ideally we could just have more serialization modules :/ but apparently you can't use these with containers
15:09 < andytoshi> which sounds like a bug in serde to me
15:09 < jeremyrubin> Ah you could have more modules
15:09 < jeremyrubin> but you need one per type
15:09 < andytoshi> then you have a combinatorial explosion
15:09 < thomaseizinger> well, infinitely more modules to be precise
15:09 < andytoshi> yeah :P
15:09 < jeremyrubin> correct
15:09 < thomaseizinger> `Vec<Option<Amount>>`, `Option<Vec<Amount>>`, ...
15:11 < thomaseizinger> <andytoshi "which sounds like a bug in serde"> Here is the issue if you want to read up on what people have been experimenting with: https://github.com/serde-rs/serde/issues/723
15:11 < jeremyrubin> The main question I have is basically if a wrapper type and no default is really helping over just having a default ser
15:11 < andytoshi> yes, any default ser is guaranteed to be wrong a huge proportion of the time
15:12 < jeremyrubin> Sure -- I guess also we could skip the wrapper types and just be pure concrete
15:12 < jeremyrubin> e.g. SignedAmountF64
15:14 < thomaseizinger> We've always used the Amount type to serialize to satoshis inside a string to avoid precision errors on the client side if you consume the API through JS for example
15:14 < thomaseizinger> that could potentially be a "safe" default
15:15 < thomaseizinger>  * that could potentially be a safe default
15:15 < andytoshi> one could argue that this is a bug in bitcoin core, that they are using noninteger json numbers for financial quantites. but that ship has long since sailed
15:15 < andytoshi> thomaseizinger: yeah, that would be one of my votes for a default
15:15 < andytoshi> but it wouldn't be compatible with Core, so..
15:15 < thomaseizinger> has it? what is the policy with breaking changes on the RPC interface?
15:16 < jeremyrubin> Well I also think that if we're going to rip up amount we should switch rust-bitcoin Amount to default signed since it's weird to have the default be core type incompatible...
15:16 < jeremyrubin> but I think changing max_value also helps there
15:16 < andytoshi> yeah, i think if we changed max_value there wouldn't really be any incompatibilty
15:16 < andytoshi> but we maybe want to be signed anyway
15:16 < andytoshi> so you can do fee calculations sensibly
15:17 -!- tibo [~tibo@2400:4050:2a83:7000:5580:8901:6bce:e45b] has joined #rust-bitcoin
15:19 < jeremyrubin> Well confirming the wrapper approach successfully serializes/deserializes with no external annotations needed
15:20 < andytoshi> ok, but then you need conversion functions for every container
15:21 < andytoshi> would be nice if rust had higher kinded types :/
15:21 < thomaseizinger> I wonder what it would take to integrate with the `serde_as` feature if we already have wrapper types? Can we do that in a way such that we don't need a dependency on `serde_with`?
15:21 < jeremyrubin> Hmmm I think you can get around some of it... you could at least impl all the default things (Eq, etc) for Wrapper<T> Wrapper<T2>?
15:21 < andytoshi> i though tthat serde_as was part of serde_with
15:22 < thomaseizinger> it is, `serde_with` is the crate, `serde_as` is the attribute that it provides, sorry if that was ambiguous
15:23 < andytoshi> oh, i see, you're saying that users could maybe have the choice of writing their own conversion functions or they could just use serde_as
15:23 < andytoshi> that's kinda a shitty choice
15:24 < andytoshi> although .. i only ever use serde for json stuff where i'm typically using types that i need to convert anyway
15:24 < andytoshi> for real serialialization i write my own traits
15:26 < thomaseizinger> pushing as much as possible towards serde is nice for writing HTTP APIs where your handler gets passed the already deserialized type, having to convert / extract again within the handler quickly amounts to boilerplate ..
15:26 < andytoshi> yeah
15:28 < andytoshi> so part of me wants to say "serde is just there for http and json, let's just do something, it doesn't matter what because if the user cared they wouldn't use serde"
15:28 < andytoshi> but unfortunately Bitcoin Core does something which is ridiculous
15:29 < andytoshi> even if you've accepted http+json as your transport
15:29 < andytoshi> :(
15:30 < jeremyrubin> I think that it's OK if we just accept that it's gonna suck because it sucks
15:30 < andytoshi> +1 ... but does that then mean, "serialize as floats"? that seems like too much suckage
15:30 < jeremyrubin> My favorite is tagged enum
15:31 < jeremyrubin> because it sucks but at least is 100% unambiguous
15:31 < andytoshi> right but can you give such a thing to Core
15:31 < jeremyrubin> ah so being able to like parse a JSON from rpc or something?
15:32 < andytoshi> well, parsing is not so bad .. we could just say "if the user gives us a float we'll deal with it"
15:32 < andytoshi> but we need to make a choice about how to serialize
15:32 < jeremyrubin> we could make core *always* include a . if it is a float
15:32 < jeremyrubin> i mean not old versions, but going forward
15:33 < andytoshi> so the numbers Core produces always do have a . and 8 decimal places
15:33 < jeremyrubin> unless they are sats?
15:33 < andytoshi> ah, yeah, there are a few places where they use sats aren't there
15:33 < jeremyrubin> not sure
15:33 < jeremyrubin> (shakes fist at lighnting network using fractional sats)
15:34 < andytoshi> i think there might be. and there might be an option you can give it to make it output sats always, people talk about that a lot..
15:34 < andytoshi> in any case. serde does not give us the ability to distinguish between "a number with a . and 8 decimal places" and "a number"
15:34 < andytoshi> so i guess, even when deserializing we have to make a decision
15:34 < jeremyrubin> Ah it's not that hard to implement such a type I think?
15:34 < andytoshi> i'm not sure what you mean
15:35 < jeremyrubin> We could implement a type which treats things with a . as different from no dot
15:35 < jeremyrubin> no dot ==> sats. dot ==> bitcoin?
15:35 < andytoshi> no, we can't, because serde does not let us distinguish those cases
15:36 < andytoshi> oh maaybe we can try to parse as an int type, and failing that try to parse as a f64, and failing that try to parse as a string
15:36 < andytoshi> but IIRC serde makes that very hard to do
15:36 < jeremyrubin> also json 1.0 will deser int
15:37 < andytoshi> lol, and just drop the data after the . ?
15:37 < jeremyrubin> IIRC
15:37 < jeremyrubin> JS doesn't even have integers bruh
15:38 < jeremyrubin> ah andytoshi isee what you mean above
15:38 < jeremyrubin> it would be something that would only work on e.g. json formats, but not binary formats
15:39 < andytoshi> i think even with json we might have trouble .. but yeah.
15:45 -!- mode/#rust-bitcoin [+o andytoshi] by ChanServ
15:45 < jeremyrubin> hmmm
15:45 < jeremyrubin> what if we do wrappers
15:45 < jeremyrubin> and give them all a trait IsAmount
15:46 -!- andytoshi changed the topic of #rust-bitcoin to: Discussion of rust-bitcoin, rust-lightning and related crates | For rust-miniscript see ##miniscript | For general bitcoin discussion #bitcoin | Channel logs at http://gnusha.org/rust-bitcoin/
15:46 < jeremyrubin> and all the methods are implemented generically on IsAmount?
15:46 <@andytoshi> (it is a year today since tamas passed, i realize i left the /topic up the entire time)
15:47 <@andytoshi> jeremyrubin: we could do that, but i think it'd still cause ergonomic problems with users having to fix their serialization format in their data structures
15:47 <@andytoshi> maybe we should just bite the bullet. it's pretty common already to have auxiliary structs for serialization
15:48 <@andytoshi> and even to have different ones for serialization and deserialization, to avoid borrowing pain
15:48 -!- mode/#rust-bitcoin [-o andytoshi] by andytoshi
15:48 < thomaseizinger> why can't core change to returning satoshis everywhere?
15:48 < thomaseizinger> there are ways of transitioning an API so that it is not immediately a breaking change from one version to the next. for example, you can introduce a header (`Feature-Flags: "satoshi-amounts"`) that, if present, returns numbers in satoshis. clients can then opt-in this behaviour and say 3 versions down the line, it default behaviour changes.
15:48 < thomaseizinger> not sure what the policy of core is here but it seems a bit ridiculous to put all this effort into a bad historical decision ..
15:48 < jeremyrubin> "Why can't core change" lol
15:48 < andytoshi> also my reaction :P
15:49 < andytoshi> thomaseizinger: the HTTP header thing is a cool idea, you could propose that
15:50 < thomaseizinger> just as a GitHub issue?
15:51 < andytoshi> i think, you could ask on #bitcoin-core-dev if people would accept such a PR
15:51 < andytoshi> you don't actually have to amke the PR (though it won't get done if you don't..)
15:51 < jeremyrubin> TBH it wouldn't fix our problem here
15:51 < andytoshi> but if you can get someone to say they like it, you could then cite the logs in future
15:51 < andytoshi> when trying to convince somebody else to do a PR
15:52 < andytoshi> jeremyrubin: well, it'd let us just say "by default we serialize as sats"
15:52 < thomaseizinger> yeah, I am not sure if people want to have me coding C++ ..
15:52 < andytoshi> hehe
15:53 < thomaseizinger> also for my own sanity I'd rather not
15:53 < andytoshi> so, unfortunately i don't think anyone working on Core has the inclination to take this up
15:53 < andytoshi> because they'd get bikeshedded to death and they've had this conversaiton so many times they don't wanna think about it
15:53 < andytoshi> i wouldn't mind writing it but i don't have much time
15:53  * jeremyrubin ponders in tonal
15:54 < andytoshi> lol
15:54 < jeremyrubin> i bet luke would like a header that lets you specify tonal
15:54 < jeremyrubin> thomaseizinger: if you put that in your issue and tag him he might do it :shrug:
15:55 < thomaseizinger> I am afraid I don't quite follow ..
15:56 < thomaseizinger> as in, I don't get the joke 😅
15:56 < jeremyrubin> it's not really a joke https://en.bitcoin.it/wiki/Tonal_Bitcoin
15:57 < thomaseizinger> ah okay, I was looking up "tonal" and didn't receive any useful search result which led me to think that it is some kind of language joke :D
16:06 < jeremyrubin> One *wild* option: feature flag to enable default Amount serialization as btc or float
16:07 < andytoshi> lol, no, we can't do that becasue then random other deps could turn on that flag
16:07 < jeremyrubin> Compiler wouldn't catch multiple impls of Serialization?
16:07 < jeremyrubin> Ah i guess if you don't have *any* set
16:08 < andytoshi> what would you do if you had multiple impls?
16:08 < jeremyrubin> compiler fails to compile
16:08 < andytoshi> just say "these dependencies cannot be in the same tree" and fork all your deps
16:08 < andytoshi> lol
16:08 < jeremyrubin> hehehehe
16:09 < andytoshi> we could just feature gate the whole module and put an english language essay in there about why rust,serde,Core,etc are conspiring to make this impossible
16:09 < jeremyrubin> and make it print out to stdout
16:11  * jeremyrubin global config variable
16:12 < jeremyrubin> (sorry now I'm just putting out bad ideas to see if any stick)
16:13 < jeremyrubin> DeserializeSeed?
16:14 < andytoshi> hmmm, that's a thing i've never looked into
16:14 < jeremyrubin> That allows you to pass context into deserialization, but I don't see how it would happen automatically
16:17 < jeremyrubin> Sadly looks experimental
16:19 < jeremyrubin> i give up for now
16:19 < jeremyrubin> I guess just to summarize:
16:19 < jeremyrubin> 1) Bug: deserialize opt always returns Some
16:19 < jeremyrubin> 2) Bug: max_value cannot be repr'd in JSON
16:20 < jeremyrubin> 3) Some bigger API change because Numbers Suck
16:54 < andytoshi> yeah .. so maybe for now we just fix 1 and 2
17:21 -!- da39a3ee5e6b4b0d [~da39a3ee5@2403:6200:8876:255c:8c1d:42fb:24ab:a8e9] has joined #rust-bitcoin
19:18 -!- DeanGuss [~dean@gateway/tor-sasl/deanguss] has joined #rust-bitcoin
21:29 -!- titanbiscuit [~tbisk@104.200.131.173] has joined #rust-bitcoin
22:14 -!- jeremyrubin [~jr@2601:645:c200:14:91a5:d8cb:fe32:d136] has quit [Ping timeout: 260 seconds]
23:46 -!- jeremyrubin [~jr@2601:645:c200:14:91a5:d8cb:fe32:d136] has joined #rust-bitcoin
--- Log closed Wed Jan 13 00:00:14 2021