Add adjustedtime to getinfo RPC call #2160

Provides a method to get the network adjusted time from the RPC interface.

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/635c3829c6a4ebae311d43881ea88265e444a4c7 for binaries and test log.

Whats the motivation here? (I have no real objection to it— it's probably a good idea.. though our whole adjustedtime handling needs some love— I'm just curious what motivated it)

(As far as love goes: we should probably forget adjustments from peers which we are no longer connected to— and we should probably do something(???) to address the attack where we accept inbound and a hostile party makes 100 connections to us to push our adjusted time to the edge of the accepted window)

Here are the issues I think about for this seemingly trivial one-line change:

Is adjusted time valid before you're connected to any peers? Check code: yes, will be current time.

Are there any issues with calling GetAdjustedTime from multiple threads? Check code: nTimeOffSet in util is not protected by a mutex, so there could be an issue, although worst-case would just be an incorrect time reported fleetingly.

Maybe we should protect nTimeOffset with a mutex; the assumption right now is probably that the only code that cares about adjusted time holds either the cs_main or a network mutex. I'd have to spend a lot longer than I'm willing to right now to figure out if that is true.

Other than all that... seems like a good idea.

Is the adjusted time that interesting? I think that the measured adjustment itself is more useful.

gmaxwell: I was just experimenting with nLockTime and noticed that there currently isn't any way to get the network adjusted time over RPC. (or even at all?) In theory a nLockTime-using application might care, but 1) none exist yet and 2) mining is too unpredictable for it to matter much and 3) what's more interesting is to know if your clock is probably way off. I added the data to getinfo simply because that seems to be the "general state overview" call, and it is in wallet.cpp so it's at least vagely related to the end-user use-case.

gavin: Whether or not the time is valid is irrelevant: the purpose is to know the value of GetAdjustedTime(). Regarding thread safety nTimeOffset is only ever set in two places in AddTimeData and in both cases it's the correct value. On some 32-bit archs the two halves can be written separately, however the maximum range of nTimeOffset fits within 32-bits so it wouldn't be an issue except when transitioning from -1 to 0. (twos complement) Of course it could still be wrong, but with RPC latency you'd still be wrong anyway. That said, it's the kind of thing that could break in the future if the code changes. And yes, I should have thought about all of those issues first. :)

sipa: That's a good point and because it doesn't look like a time value people would be more likely to understand what the value actually meant before using it for something. I could just call it "timeoffset" and return nTimeOffset directly.

gmaxwell: Forgetting sounds reasonable. Regarding pushing: well, what's the big deal? The acceptance window is two hours, and AddTimeData doesn't allow nTimeOffset to be larger than +-70minutes. Lets suppose it gets pushed back as far as possible: on reception we reject a block with a time > 2 hours from now, thus so long as our clock is less than 50mins behind we're ok. (the mining simply ensures the timestamp will be valid regardless) If the clock is pushed forward and we're not mining nTimeOffset is irrelevant as the condition is for the block time to be simply > GetMedianTimePast(). If we are mining our blocks will be valid if our clocks are no more than 50mins ahead. Similarly that problem can be easily fixed by changing UpdateTime to never set the block time more than, say, an hour or something newer than the last block. (this assumes there isn't a large contingent of hashing power actually trying to increase the difficulty)

Getting a bit off topic here, but I only just realized it... there isn't actually anything in Bitcoin preventing block timestamps from being too early other than collective miner self-interest... and while it's a theoretical problem now, it's conceivable that as the block reward drops to nothing the incentives to push the block timestamps backwards in time could get miners to do exactly that. Of course you just need one person to put a correct timestamp in, but it could make the trustworthiness of the timestamps more like within a day or two. It also could really throw a wrench in some of the niftier "item renting via the blockchain" schemes, although of course in that case you might see mining pools getting paid to put correct timestamps in the chain as well.

The timestamps must go forward eventually as they must be greater than the median of the last ten, and stacking them all up would make the difficulty skyrocket. Sadly, they can't really be /forced/ more tightly than that without endangering convergence. If it were ever an issue one could implement a soft (non-forking) that says nodes will delay forward a block from more than X minutes in the past for at least Y minutes or until buried by Z more blocks, causing delayed blocks to get orphaned.

what's the big deal? The acceptance window is two hours, and AddTimeData doesn't allow nTimeOffset to be larger than +-70minutes.

Because abs(-70) + abs(70) = 140 which is greater than 120. This is a vulnerability. You can push a miner one way, and a merchant another way and make a merchant reject the longest chain to put them on a fork, a fork you can create by pushing a miner the same direction as the merchant (thus also rejecting the opposite sign miner). It's sort of a far out attack that isn't much worth worrying about, but it ought to be fixed.

@sipa: Modified to return the offset itself.

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/8686f6467c9db2606706baca33842c97ff7621f8 for binaries and test log.