diff options
| author | fanquake <fanquake@gmail.com> | 2024-03-22 17:53:46 +0000 |
|---|---|---|
| committer | fanquake <fanquake@gmail.com> | 2024-03-22 17:53:52 +0000 |
| commit | a175efe768892f6e983a7c814a201c506c4d1ce5 (patch) | |
| tree | 834b343840fe39460e31527f6eada0240038096a /src/net_processing.h | |
| parent | 85c8a5ec48b5e2a0ba26bec0acb572dce37271ff (diff) | |
| parent | 032a59748295859845b2a9181ceb1c4ae70bae5c (diff) | |
Merge bitcoin/bitcoin#29704: test: make p2p_handshake robust against timeoffset warnings
032a59748295859845b2a9181ceb1c4ae70bae5c test: make p2p_handshake robust against timeoffset warnings (stickies-v)
Pull request description:
The new `p2p_handshake` test requires that limited nodes are not peered with when the node's system time exceeds ~ 24h of the node's chaintip timestamp, as per [`PeerManagerImpl::GetDesirableServiceFlags`](https://github.com/bitcoin/bitcoin/blob/2ffaa927023f5dc2a7b8d6cfeb4f4810e573b18c/src/net_processing.cpp#L1717).
By patching this test to modify the timestamp of the chaintip as opposed to mocking the node's system time, we make it resilient to future commits where the node raises a warning if it detects its system time is too much out of sync with its outbound peers.
Resolves a silent merge conflict in https://github.com/bitcoin/bitcoin/pull/29623, that is changing the warning behaviour when significant time differences with outbound peers are detected, [failing the test as it's currently in master](https://cirrus-ci.com/task/6553996884705280?logs=ci#L4666).
Considerations/alternatives I've thought of:
- could add `self.setup_clean_chain = True` to `self.set_test_params()`, to avoid creating a new tip with a (much) older date, but it doesn't seem to matter?
- could avoid using `setmocktime` altogether and instead use [`create_block`](https://github.com/bitcoin/bitcoin/blob/2ffaa927023f5dc2a7b8d6cfeb4f4810e573b18c/test/functional/test_framework/blocktools.py#L68) instead, but that seems like it'll be a lot more verbose and I don't think it's worth it?
Big thanks to theStack for his time in discussing this with me offline.
ACKs for top commit:
maflcko:
lgtm ACK 032a59748295859845b2a9181ceb1c4ae70bae5c
theStack:
ACK 032a59748295859845b2a9181ceb1c4ae70bae5c
brunoerg:
crACK 032a59748295859845b2a9181ceb1c4ae70bae5c
BrandonOdiwuor:
Code Review ACK 032a59748295859845b2a9181ceb1c4ae70bae5c
Tree-SHA512: 407564754a100bc9252f5737182de2e111993944ec9a0463a4a43195ce98cd1119de982c8fe5f7531ddb56603043812bf7bf2163a780d30b6df03a072c3308c3
Diffstat (limited to 'src/net_processing.h')
0 files changed, 0 insertions, 0 deletions