--- Log opened Mon May 11 00:00:17 2020
08:16 -!- jb55 [~jb55@gateway/tor-sasl/jb55] has quit [Ping timeout: 240 seconds]
08:19 -!- jb55 [~jb55@gateway/tor-sasl/jb55] has joined #utreexo
08:37 < adiabat> ja: I added that to parentMany and ran the tests on master, I don't get any panic
08:38 < ja> hmm
08:38 < ja> let me just try again and i'll send a diff
08:40 < ja>  func parentMany(position uint64, rise, forestRows uint8) uint64 {
08:40 < ja> +       if rise+1 > forestRows {
08:40 < ja> +               panic("no go")
08:40 < ja> +       }
08:40 < ja> adiabat: this does crash for me
08:40 < ja> is it because you checked for position and not rise?
09:03 < adiabat> I just did if rise > forestRows {
09:03 < adiabat> I don't think the +1 is needed, as there could be cases when rise == forestRows
09:04 < adiabat> (mask << uint64(forestRows-(rise-1)))) & mask  would become 
09:04 < adiabat> (mask << 1) & mask
09:07 < adiabat> I guess if rise == 0 that would also mess things up
09:09 < adiabat> ... but it doesn't seem to, hm
09:09 < ja> but the line below uses (forestRows-(rise-1))
09:10 < ja> so the panic makes more sense with the +1, no?
09:10 < ja> i mean, we want to panic when the shift amount is negatvie
09:11 < ja> i don't understand what you mean by needed? i mean, any case with a negative shift is something we'd wanna prevent, no?
09:13 < adiabat> I guess there's 2 ways
09:13 < adiabat> if rise is 0, then rise-1 underflows
09:14 < adiabat> otherwise if both are non-zero, then (forestRows-(rise-1)) goes below 0 when rise-1 > forestRows
09:15 < ja> oh right
09:15 < ja> ok, my logic is wrong :O
09:15 < adiabat> but there are cases when rise ==0
09:15 < adiabat> which does cause underflow
09:15 < adiabat> so I think a good way to make it safer
09:15 < adiabat> is to have
09:16 < adiabat> if rise == 0 { return position }
09:16 < adiabat> then panic on rise-1 > forestrows
09:16 < ja> ok, i'll test it now
09:17 < adiabat> yeah having those two still has tests working for me
09:17 < adiabat> and still allows the rise=0 which does seem like that could cause bugs
09:18 < ja> yeah ok, great!
09:18 < adiabat> I can merge that in, or if you're making a PR with modifications here you can add that one
09:18 < ja> i have a PR with some deduplication, but that is unrelated. so it's fine if you put this in
09:19 < ja> (the deduplication PR has already been sent)
09:19 < adiabat> ah I see it, cool
09:19 < adiabat> Hmm need to read this and figure it out :)
09:20 < ja> even though go is pass-by-value, slices are just pointers, so that's why i thought it must be equivalent
22:01 -!- jb55 [~jb55@gateway/tor-sasl/jb55] has quit [Remote host closed the connection]
22:02 -!- jb55 [~jb55@gateway/tor-sasl/jb55] has joined #utreexo
--- Log closed Tue May 12 00:00:18 2020