Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Sep 13, 2021

This avoids having to remember to set it whenever mocktime is used with
peer connections. Also, it might help avoiding disconnects when
attaching a debugger to a running test.

This avoids having to remember to set it whenever mocktime is used with
peer connections. Also, it might help avoiding disconnects when
attaching a debugger to a running test.
@fanquake fanquake added the Tests label Sep 13, 2021
@fanquake
Copy link
Member

Concept ACK

@DrahtBot
Copy link
Contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #22818 (test: Activate all regtest softforks at height 1, unless overridden by MarcoFalke)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@theStack
Copy link
Contributor

Concept ACK

@laanwj
Copy link
Member

laanwj commented Sep 16, 2021

Concept and code review ACK fad4f44

@laanwj laanwj merged commit 6d76b57 into bitcoin:master Sep 16, 2021
@laanwj
Copy link
Member

laanwj commented Sep 16, 2021

This does show that we apparently have no test for peertimeout.
Eh that's wrong, there's p2p_timeouts.py and p2p_ping.py, but it's already overridden there.

@maflcko maflcko deleted the 2109-testPeerTimeout branch September 16, 2021 14:53
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Sep 19, 2021
fad4f44 test: Set peertimeout in write_config (MarcoFalke)

Pull request description:

  This avoids having to remember to set it whenever mocktime is used with
  peer connections. Also, it might help avoiding disconnects when
  attaching a debugger to a running test.

ACKs for top commit:
  laanwj:
    Concept and code review ACK fad4f44

Tree-SHA512: 00c742571c0524c1b3f55e0217433ef7aa2dccccc12650caab98b4cf9231669f37fc589c7475f28d5725ffe2436c76205920eaece4a47fd27dc8872421a48e5c
@@ -364,6 +364,11 @@ def write_config(config_path, *, n, chain, extra_config="", disable_autoconnect=
f.write("dnsseed=0\n")
f.write("fixedseeds=0\n")
f.write("listenonion=0\n")
# Increase peertimeout to avoid disconnects while using mocktime.
# peertimeout is measured in wall clock time, so setting it to the
# duration of the longest test is sufficient. It can be overriden in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: overriden ==> overridden

Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Feb 2, 2022
Summary:
This avoids having to remember to set it whenever mocktime is used with
peer connections. Also, it might help avoiding disconnects when
attaching a debugger to a running test.

This is a backport of [[bitcoin/bitcoin#22960 | core#22960]]
Depends on D10955

Test Plan:
`ninja check-functional-extended`
`grep -r peertimeout test/functional/`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D10956
@bitcoin bitcoin locked and limited conversation to collaborators Oct 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants