regression test: infinite path challenge resending (#442)

* Write regression test

* Add comments to regression test

* Add comment on how we fixed the test

Author: matheus23

quinn-proto/proptest-regressions/tests/proptest.txt

@@ -27,3 +27,4 @@ cc a22f4683ab811ac9a4299c8148f2ce58c7c54abacc95c32849bdd12a781388b3
 cc a49af8f0de194989d376da3b88838abb79793f3f4b236877c1ce6b16f57d6d21
 cc b2c0170aa51218d842490b6f561ccc1de08918f17f461f4808c5bbc6f7479ec5
 cc e4f16be75773cb09f37f3cd699d03aa34c8f24ead942f1a499a63abd3784a7ba
+cc 1c853ee3d7dc5cbed43db750b243688ab677fbd0f4d5dc4e75bc85288600f6fa

quinn-proto/src/tests/proptest.rs

@@ -788,3 +788,90 @@ fn regression_peer_ignored_path_abandon() {
         pair.server_conn_mut(server_ch)
     )));
 }
+
+/// A regression test that used to put quinn into a state of sending PATH_CHALLENGE
+/// from the client side indefinitely.
+///
+/// The test uses passive migrations to establish a situation in which the client
+/// expects the server to respond to a PATH_CHALLENGE that was sent on the interface
+/// 1.1.1.1, but the server cannot respond to it anymore, because the client was
+/// involuntarily migrated to path 1.1.1.2.
+///
+/// Here's a log line that shows the client skipping the path response due to it
+/// being delivered "on the wrong network path" (after migration).
+///
+/// > DEBUG client:pkt{path_id=1}:recv{space=Data pn=3}:frame{ty=PATH_RESPONSE}:
+/// > iroh_quinn_proto::connection: 4704:
+/// > ignoring invalid PATH_RESPONSE
+/// >  response=PATH_RESPONSE(ece9dc07f89ded7e)
+/// >  network_path=(local: ::ffff:1.1.1.2, remote: [::ffff:2.2.2.0]:4433)
+/// >  expected=(local: ::ffff:1.1.1.1, remote: [::ffff:2.2.2.0]:4433)
+///
+/// The client will then never clear out the PATH_CHALLENGE from the "pending"
+/// challenges, and so it will never fully clear the path challenge timer.
+///
+/// This issue was fixed by making sure to clear out challenges that were probing
+/// 4-tuples that are different from the current network path.
+#[test]
+fn regression_never_idle4() {
+    let prefix = "regression_never_idle4";
+    let seed = [0u8; 32];
+    let interactions = vec![
+        // Open path 1 with the same remote address as path 0
+        TestOp::OpenPath {
+            side: Side::Client,
+            status: PathStatus::Backup,
+            addr_idx: 0,
+        },
+        // Sets path 0 to backup, but generally just sends *something*
+        TestOp::PathSetStatus {
+            side: Side::Client,
+            path_idx: 0,
+            status: PathStatus::Backup,
+        },
+        // Sends the two packets (opening path 1 & setting path status on path 0)
+        TestOp::Drive { side: Side::Client },
+        // But loses those two packets
+        TestOp::DropInbound { side: Side::Server },
+        // Client's interface 0 now migrates from 1.1.1.0 to 1.1.1.1
+        TestOp::PassiveMigration {
+            side: Side::Client,
+            addr_idx: 0,
+        },
+        TestOp::AdvanceTime,
+        // Client closes path 0, path 1 is now the only remaining path for the client.
+        // It will now always choose path 1 to send, even though it's not validated.
+        TestOp::ClosePath {
+            side: Side::Client,
+            path_idx: 0,
+            error_code: 0,
+        },
+        // Send out the packet containing the PATH_ABANDON
+        TestOp::Drive { side: Side::Client },
+        // Migrate the first interface from 1.1.1.1 to 1.1.1.2 now.
+        TestOp::PassiveMigration {
+            side: Side::Client,
+            addr_idx: 0,
+        },
+    ];
+    let routes = RoutingTable::from_routes(
+        vec![
+            ("[::ffff:1.1.1.0]:44433".parse().unwrap(), 0),
+            ("[::ffff:1.1.1.1]:44433".parse().unwrap(), 0),
+        ],
+        vec![("[::ffff:2.2.2.0]:4433".parse().unwrap(), 0)],
+    );
+
+    let _guard = subscribe();
+    let mut pair = setup_deterministic_with_multipath(seed, routes, prefix);
+    let (client_ch, server_ch) =
+        run_random_interaction(&mut pair, interactions, multipath_transport_config(prefix));
+
+    assert!(!pair.drive_bounded(100), "connection never became idle");
+    assert!(allowed_error(poll_to_close(
+        pair.client_conn_mut(client_ch)
+    )));
+    assert!(allowed_error(poll_to_close(
+        pair.server_conn_mut(server_ch)
+    )));
+}