Current behavior is to set received time to the block header time. This
prevents any possibility to check if received time has been set due to
effective reception of a new block or because of a block loading from
file.
Details
Details
- Reviewers
schancel jasonbcox deadalnix - Group Reviewers
Restricted Project - Commits
- rSTAGING2334ca69a82f: Defaults block index received time to 0 when init from a block header
rABC2334ca69a82f: Defaults block index received time to 0 when init from a block header
make check && ./test/functionnal/test_runner.py --extended
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- header_received_time_defaults_to_0
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 4133 Build 6337: Bitcoin ABC Buildbot (legacy) Build 6336: arc lint + arc unit
Event Timeline
Comment Actions
Why is knowing when the header was loaded from a file useful? The block time is the next-best accurate time we have when the actual received time is not available.
I'd also argue that if you're loading the header from a file, the received time is no longer useful, which is why it was never saved in the first place.
Comment Actions
Your second sentence is what this diff is about.
If loaded from file, the receive time is no longer needed ; but because it is set to the block header time there is no way to know if you need to care about the received time or not.
The block header time is actually a valid header receive time.
If you need to approximate the received time by the block header time it would be easy to check the later against 0 to determine which one is to be trusted.
| src/chain.h | ||
|---|---|---|
| 179 ↗ | (On Diff #6093) | Considering your latest comments, what do you think about this? int64_t GetHeaderReceivedTime(bool guessUnknownTime=false) const {
if (guessUnknownTime && nTimeReceived == 0) {
return GetBlockTime();
}
return nTimeReceived;
} |
Comment Actions
@jasonbcox I defaulted guessUnknownTime to true, so it requires no change on existing code to get the same behavior as prior
| src/test/blockindex_tests.cpp | ||
|---|---|---|
| 126 | guessUnkwnowTime -> guessUnknownTime | |
| src/chain.h | ||
|---|---|---|
| 175 | Is there any callsite that rely on time being non zero ? | |
| src/chain.h | ||
|---|---|---|
| 175 | If none rely on this then just refactor. I don't think there are many callsites so it is worth investigating. | |
| src/chain.h | ||
|---|---|---|
| 175 | Not currently. Fabien and Shae were experimenting with it in other diffs. I figure making this change allows us to continue experiments until the matter of penalizing blocks based on received time is settled. | |
| src/chain.h | ||
|---|---|---|
| 175 | I found a single case where the behaviour was changed when returning 0: in the GetReceivedTimeDiff function below which is called in AcceptBlock for logging only. | |
| src/chain.h | ||
|---|---|---|
| 175 | Adding bool parameter is an antipattern so clearly we are not talking about anything costless here, but actually making the code worse. If there is no reason to make the code worse, then just refactor this and the one callsite. | |