Storing fewer time samples makes a node's time offset more accurate following system clock adjustments, uses less memory and avoids a possible memory exhaustion attack.
The only part of this patch I'm not entirely pleased with is the choice of 200 as the limit. In my tests, that's roughly two hours of network activity. I chose two hours because it seemed about right, but have no rational basis for the decision.
The full list of time samples is rarely useful outside of debugging.
The node's time offset, however is useful for discovering local clock
drift, so it's displayed in all logging modes.
Remembering all time samples makes nTimeOffset slow to respond to
system clock corrections. For instance, I start my node with a system
clock that's 30 minutes slow and run it for a few days. During that
time, I accumulate 10,000 offset samples with a median of 1800
seconds. Now I correct my system clock. Without this change, my node
must collect another 10,000 samples before nTimeOffset is correct
again. With this change, I must only accumulate 100 samples to
correct the offset.
Storing unlimited time samples also allows an attacker with many IP
addresses (ex, a large botnet) to perform a memory exhaustion attack
against Bitcoin nodes. The attacker sends a version message from each
IP to his target, consuming more of the target's memory each time.
Time samples are small, so this attack might be impractical under the
old code, but it's impossible with the new code.
ACK. I support hunting down any data structures that could become arbitrarily large and figuring out how to reasonably bound them.