integration: stabilize pre-verack disconnect cycles
What the developer wrote
Authored by Olaoluwa Osuntokun
integration: stabilize pre-verack disconnect cycles
In this commit, we make the pre-verack lifecycle test count only peers that
btcd has admitted and processed through version exchange.
The source handshake limit can close a rapid follow-up socket while the
previous disconnect is still unwinding. This made the client-side version
write fail with EPIPE before the intended lifecycle path ran. We now wait for
a version response from btcd, retry rejected attempts under a deadline, and
still disconnect without sending verack.
This preserves all 50 peerDone without peerAdd cycles while removing the
scheduler-dependent admission race from the test.
What changed, and why it matters
This change only modifies an integration test file. It makes a test more reliable by waiting for a version response and retrying connection attempts, rather than changing any production code that handles real Bitcoin peer connections. There is no security fix or vulnerability here.
No security action needed. Treat as a normal test reliability improvement.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit refactors dialAndSendVersion into dialPreVerackPeer in integration/sync_race_test.go. The helper now performs a full version exchange (send version, read version response) and returns errors instead of using require.NoError. The test loop retries failed dial/version attempts under a deadline. No production code in btcd’s P2P stack, sync manager, or peer lifecycle handling is changed. The commit message frames this as stabilizing a flaky test by removing a scheduler-dependent admission race from the test itself.
Changed components
integration/sync_race_test.goInspect captured patch +58 / −19
diff --git a/integration/sync_race_test.go b/integration/sync_race_test.go
index cac1cc6..b678fa1 100644
--- a/integration/sync_race_test.go
+++ b/integration/sync_race_test.go
@@ -357,22 +357,27 @@ func TestSyncManagerRaceCorruption(t *testing.T) {
done, heightBefore, heightAfter)
}
-// dialAndSendVersion connects to nodeAddr and sends a version
-// message, returning the open connection. The caller is
-// responsible for closing it.
-func dialAndSendVersion(
- t *testing.T, nodeAddr string,
-) net.Conn {
-
- t.Helper()
-
+// dialPreVerackPeer connects to nodeAddr and exchanges version messages without
+// sending verack. The caller is responsible for closing the returned
+// connection.
+func dialPreVerackPeer(nodeAddr string) (net.Conn, error) {
conn, err := net.DialTimeout("tcp", nodeAddr, 5*time.Second)
- require.NoError(t, err)
+ if err != nil {
+ return nil, err
+ }
+ connected := false
+ defer func() {
+ if !connected {
+ _ = conn.Close()
+ }
+ }()
_ = conn.SetDeadline(time.Now().Add(5 * time.Second))
nodeTCP, err := net.ResolveTCPAddr("tcp", nodeAddr)
- require.NoError(t, err)
+ if err != nil {
+ return nil, err
+ }
you := wire.NewNetAddress(
nodeTCP, wire.SFNodeNetwork|wire.SFNodeWitness,
@@ -391,9 +396,22 @@ func dialAndSendVersion(
err = wire.WriteMessage(
conn, msgVersion, wire.ProtocolVersion, wire.SimNet,
)
- require.NoError(t, err)
+ if err != nil {
+ return nil, err
+ }
- return conn
+ msg, _, err := wire.ReadMessage(
+ conn, wire.ProtocolVersion, wire.SimNet,
+ )
+ if err != nil {
+ return nil, err
+ }
+ if _, ok := msg.(*wire.MsgVersion); !ok {
+ return nil, fmt.Errorf("expected version message, got %T", msg)
+ }
+
+ connected = true
+ return conn, nil
}
// TestPreVerackDisconnect verifies that a peer disconnecting
@@ -409,15 +427,36 @@ func TestPreVerackDisconnect(t *testing.T) {
nodeAddr := harness.P2PAddress()
- // Connect and send version, then disconnect before receiving or
- // sending verack. This is expected to produce a peerDone without
- // a preceding peerAdd in the lifecycle channel.
- const preVerackAttempts = 50
+ // Connect and exchange version messages, then disconnect without sending
+ // verack. This is expected to produce a peerDone without a preceding
+ // peerAdd in the lifecycle channel.
+ const (
+ preVerackAttempts = 50
+ preVerackRetryTimeout = 5 * time.Second
+ preVerackRetryDelay = 10 * time.Millisecond
+ )
+ retries := 0
for i := 0; i < preVerackAttempts; i++ {
- conn := dialAndSendVersion(t, nodeAddr)
- conn.Close()
+ deadline := time.Now().Add(preVerackRetryTimeout)
+ for {
+ conn, err := dialPreVerackPeer(nodeAddr)
+ if err == nil {
+ require.NoError(t, conn.Close())
+ break
+ }
+
+ if time.Now().After(deadline) {
+ t.Fatalf("pre-verack attempt %d did not complete: %v",
+ i+1, err)
+ }
+
+ retries++
+ time.Sleep(preVerackRetryDelay)
+ }
}
+ t.Logf("completed %d pre-verack disconnects with %d retries",
+ preVerackAttempts, retries)
// Allow the node time to process all the disconnects.
time.Sleep(2 * time.Second)
Why this scored 14/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.