alvarolm / curvecp Goto Github PK

For the additive increase step in congestion.go, the calculation 
s.txThrottle*s.txThrottle*s.txThrottle can overflow:

  // These maths taken straight from the implementation. I'm
  // not sure I get why it adjusts the way it does, but I
  // want to make progress.
  const timeConstant = 2251799813685248 // 2^51
  if s.txThrottle < 16*time.Millisecond {
    // N = N - cN^3, avoids 1 division and is close to the next curve.
    s.txThrottle -= s.txThrottle * s.txThrottle * s.txThrottle / timeConstant
  } else {
    // N = N/(1 + cN^2)
    s.txThrottle = s.txThrottle / (1 + s.txThrottle*s.txThrottle/timeConstant)
  }

Since s.txThrottle here can still be up to 15999999, and 15999999^3 exceeds 
2^63, the calculation will overflow and wrap around.

The second expression can actually overflow too, but only if the connection 
gets throttled down to less than about one block every three seconds.

Also of note is that since you're using integer arithmetic here, there's going 
to be some loss of precision compared to curvecpmessage.c's floating point 
calculation for the second case, which will limit how quickly the throughput 
grows.  It also means once txThrottle is less than about 47ms, it'll stop 
reducing further because txThrottle*txThrottle/timeConstant will truncate to 0.


Probably the simplest thing to do is just always use the floating point 
calculation strategy:

    // N = N/(1 + cN^2)
    d := float64(s.txThrottle)
    s.txThrottle = time.Duration(d/(1+d*d*(1.0/(1<<51))) + 0.5)

(+0.5 is so it rounds and reaches a fixed point before collapsing to 0)