--- Log opened Tue Jan 21 00:00:15 2020
03:04 -!- Kiel71Mosciski [~Kiel71Mos@ns334669.ip-5-196-64.eu] has joined #rust-bitcoin
04:03 -!- belcher [~belcher@unaffiliated/belcher] has joined #rust-bitcoin
04:20 -!- dr-orlovsky [~dr-orlovs@194.230.155.171] has quit [Read error: Connection reset by peer]
04:21 -!- dr-orlovsky [~dr-orlovs@194.230.155.171] has joined #rust-bitcoin
04:34 -!- jonatack [~jon@2a01:e0a:53c:a200:bb54:3be5:c3d0:9ce5] has quit [Quit: jonatack]
05:31 -!- Kiel71Mosciski [~Kiel71Mos@ns334669.ip-5-196-64.eu] has quit [Ping timeout: 265 seconds]
05:53 < elichai2> Is there any string amount that can actually be size 50 *and valid*?
06:03 -!- Rebekah66Bauch [~Rebekah66@ns334669.ip-5-196-64.eu] has joined #rust-bitcoin
06:08 < elichai2> should this pass? `assert_eq!(p("5500000000000000000.", sat), Ok(Amount::from_sat(5_500_000_000_000_000_000)));` andytoshi
06:13 -!- Rebekah66Bauch [~Rebekah66@ns334669.ip-5-196-64.eu] has quit [Ping timeout: 268 seconds]
06:32 < stevenroose> ah size 50
06:33 < stevenroose> elichai2: probably the longest valid amount string is "18446744073709551616000 msat" as that's u64::max_value() * 1000
06:35 < stevenroose> elichai2: I don't remember if we put any arbitrary restrictions other than the upstream/underlying restrictions. It's arguable that ridiculously high amounts don't make much sense.
06:36 < stevenroose> But well. Let's say you want to (just for fun) calculate what the fee would be to broadcast a tx that is as big as the entire blockchain projected in the year 2100, you could reach a ridiculously high amount :D
06:37 < stevenroose> Other than that, anything an order of magnitude or 2 above 21 MBTC doesn't make much sense
06:37 < elichai2> that's still 28 chars so not close to the 50 limit
06:52 < instagibbs> elichai2, thanks :P
07:02 < instagibbs> elichai2, if you notice missing tests of varying kinds make sure to file issues :)
07:29 -!- jonatack [~jon@82.102.27.171] has joined #rust-bitcoin
08:08 -!- jonatack [~jon@82.102.27.171] has quit [Ping timeout: 265 seconds]
08:21 < elichai2> found an overflow bug in https://docs.rs/bitcoin/0.23.0/bitcoin/util/amount/struct.SignedAmount.html#method.fmt_value_in
08:22 < elichai2> SignedAmount::from_sat(i64::min_value()).fmt_value_in() will cause an overflow because calling `i64::abs()` overflows if called on minimum value
08:22 < elichai2> https://doc.rust-lang.org/std/primitive.i64.html#method.abs
08:23 < elichai2> the problem is that this method returns `fmt::Result` which returns an empty Error struct, so the usual thing people do there is unwrap
08:26 < elichai2> that bug is also in all other "to_string" functions we have which call it without checking denomination including `to_string_with_denomination` and Display
08:36 < elichai2> Id I'll replace  `self.as_sat().abs() as u64` with `u64::max_value - self.as_sat() as u64 + 1` it will overflow with SignedAmount::from_sat(0) instead
08:36 < elichai2> So I think adding a branch is the only way :/
08:52 < stevenroose> elichai2: does that mean SignedAmount::abs also overflows?
08:52 < elichai2> yes
08:52 < elichai2> I'm adding `checked_abs`
08:53 < stevenroose> and returning Option<SignedAmount> on ::abs?
08:53 < stevenroose> elichai2: tbh, I was quite sure there must have snuck in at least one bug into that code. doing numbers is so tricky
08:53 < elichai2> like this: https://doc.rust-lang.org/std/primitive.i64.html#method.checked_abs
08:54 < stevenroose> elichai2: if you do, please amend documentation of .abs() then as well
08:55 < elichai2> ok, altough all the rest of the functions don't mention if and when they overflow, they just point to libstd's docs
08:55 < elichai2> see the warning: https://docs.rs/bitcoin/0.23.0/bitcoin/util/amount/struct.SignedAmount.html
08:56 < stevenroose> ah cool. I remember writing that :D
08:57 < stevenroose> so you can use u64::max_value().checked_sub(self.as_sat() as u64).unwrap_or(0), right?
08:57 < stevenroose> so you can use u64::max_value().checked_sub(self.as_sat() as u64).map(|v| v+1).unwrap_or(0), right?
08:59 < elichai2> yeah I debated which would be better. I thought keeping the `abs` call might makes sense so right now I wrote:
08:59 < elichai2> `let sats = self.as_sat().checked_abs().map(|a: i64| a as u64).unwrap_or_else(|| {u64::max_value() - self.as_sat() as u64 +1});`
08:59 < elichai2> but yours is shorter :P
09:00 < stevenroose> ah because my call makes an assumption about when the checked_abs returns None, true..
09:00 < elichai2> hmm no yours doesn't work, you need to change `map` to something that can also return Option
09:01 < stevenroose> perhaps for readability it's even better to do it very explicitly
09:01 < elichai2> because the addition can also overflow
09:01 < elichai2> so `and_then`
09:02 < stevenroose> yeah but then I think it becomes too hard to read, no?
09:02 < stevenroose> I'd prefer 2 nested if blocks with a line of reasoning in that case I think
09:02 < elichai2> ` u64::max_value().checked_sub(self.as_sat() as u64).map(|v| v.checked_add(1)).unwrap_or(0)`
09:02 < elichai2> what do you feel about how I wrote it right now? https://gist.github.com/rust-play/f90dcbbe98446d8aaabdfe128e09cf2a
09:02 -!- jonatack [~jon@2a01:e0a:53c:a200:bb54:3be5:c3d0:9ce5] has joined #rust-bitcoin
09:03 < elichai2> ***` u64::max_value().checked_sub(self.as_sat() as u64).and_then(|v| v.checked_add(1)).unwrap_or(0)`
09:04 < elichai2> evalr: let a = i64::min_value(); u64::max_value().checked_sub(a as u64).and_then(|v| v.checked_add(1)).unwrap_or(0)
09:04 -evalr:#rust-bitcoin- 9223372036854775808
09:05 < elichai2> wat
09:06 < stevenroose> whaaat?? we have a rust bot
09:06 < elichai2> yeah I added him :)
09:06 < stevenroose> that's epic
09:06 < elichai2> yessss
09:07 < stevenroose> https://gist.github.com/stevenroose/f47e6e63f63c1922b938bc50c9f8e64f
09:07 < stevenroose> how about something like that that's more explicit?
09:07 < stevenroose> (I took the number you put in the comment.)
09:08 < stevenroose> The assertion should be trivial, but one never knows :D
09:09 < elichai2> weird. https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=2146ebdc24fd11fdfa7bec8319c78c06
09:10 < elichai2> well my number is wrong loool
09:10 < elichai2> I think I tested with i32
09:10 < elichai2> that's i32::min_value() not i64::min_value()
09:16 < elichai2> I know what's wrong with the second one hehe
09:16 < elichai2> the `+1` works when it's negative, if the original number is positive then it needs to be `-1`
09:45 < elichai2> stevenroose: if you are interested this is the branch where i'm adding tests https://github.com/elichai/rust-bitcoin/tree/2020-01-tests (I'll PR when I finish fixing most of mutagen's complains hehe)
10:59 -!- Dean_Guss [~dean@gateway/tor-sasl/deanguss] has quit [Remote host closed the connection]
11:00 -!- Dean_Guss [~dean@gateway/tor-sasl/deanguss] has joined #rust-bitcoin
11:55 -!- mauz555 [~mauz555@2a01:e35:8ab1:dea0:91e5:e440:c318:3628] has joined #rust-bitcoin
12:31 -!- mryandao [~mryandao@gateway/tor-sasl/mryandao] has quit [Remote host closed the connection]
12:32 -!- mryandao [~mryandao@gateway/tor-sasl/mryandao] has joined #rust-bitcoin
12:44 -!- DeanWeen [~dean@gateway/tor-sasl/deanguss] has joined #rust-bitcoin
12:47 -!- Dean_Guss [~dean@gateway/tor-sasl/deanguss] has quit [Ping timeout: 240 seconds]
13:50 -!- mauz555 [~mauz555@2a01:e35:8ab1:dea0:91e5:e440:c318:3628] has quit []
18:54 -!- mryandao [~mryandao@gateway/tor-sasl/mryandao] has quit [Ping timeout: 240 seconds]
18:55 -!- DeanWeen [~dean@gateway/tor-sasl/deanguss] has quit [Ping timeout: 240 seconds]
18:55 -!- drolmer [~drolmer@unaffiliated/drolmer] has quit [Quit: adios]
18:56 -!- ghost43 [~daer@gateway/tor-sasl/daer] has quit [Ping timeout: 240 seconds]
19:12 -!- mryandao [~mryandao@gateway/tor-sasl/mryandao] has joined #rust-bitcoin
19:12 -!- ghost43 [~daer@gateway/tor-sasl/daer] has joined #rust-bitcoin
19:27 -!- drolmer [~drolmer@unaffiliated/drolmer] has joined #rust-bitcoin
21:50 -!- dongcarl6 [~dongcarl@unaffiliated/dongcarl] has joined #rust-bitcoin
21:52 -!- dongcarl [~dongcarl@unaffiliated/dongcarl] has quit [Ping timeout: 260 seconds]
21:52 -!- dongcarl6 is now known as dongcarl
--- Log closed Wed Jan 22 00:00:16 2020