net/libp2p: Enforce outbound request-response timeout limits

[lexnv]

This PR enforces that outbound requests are finished within the specified protocol timeout.

The stable2412 version running libp2p 0.52.4 contains a bug which does not track request timeouts properly:

• fix(request-response): Report dial IO errors to the user libp2p/rust-libp2p#5429

The issue has been detected while submitting libp2p -> litep2p requests in kusama. This aims to check that pending outbound requests have not timedout. Although the issue has been fixed in libp2p, there might be other cases where this may happen. For example:

• fix(request-response): Report failure when streams are at capacity libp2p/rust-libp2p#5417

For more context see: #7076 (comment)

1. Ideally, the force-timeout mechanism in this PR should never be triggered in production. However, origin/stable2412 occasionally encounters this issue. When this happens, 2 warnings may be generated:

• one warning introduced by this PR wrt force timeout terminating the request
• possible one warning when the libp2p decides (if at all) to provide the response back to substrate (as mentioned by @alexggh here and here

2. This implementation does not propagate to the substrate service the RequestFinished { error: .. }. That event is only used internally by substrate to increment metrics. However, we don't have the peer information available to propagate the event properly when we force-timeout the request. Considering this should most likely not happen in production (origin/master) and that we'll be able to extract information by warnings, I would say this is a good tradeoff for code simplicity:

polkadot-sdk/substrate/client/network/src/service.rs

Line 1543 in 06e3b5c

RequestFailure::Network(OutboundFailure::Timeout) => "timeout",

Testing

Added a new test to ensure the timeout is reached properly, even if libp2p does not produce a response in due time.

I've also transitioned the tests to using tokio::test due to a limitation of CI

--- TRY 1 STDERR:        sc-network request_responses::tests::max_response_size_exceeded ---
thread 'request_responses::tests::max_response_size_exceeded' panicked at /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.40.0/src/time/interval.rs:139:26:
there is no reactor running, must be called from the context of a Tokio 1.x runtime

cc @paritytech/networking

[alexggh]

Overall, looks good to me, one thing I would do is burn it in on the kusama validator over the weekend, just to see if this triggers in real work conditions, our assumption it doesn't but we might be surprised.

[alexggh]

Looks alright to me.

[alexggh]

I don't understand why we need this change.

[lexnv]

Just a tiny change to not pass the runtime when building the swarm and let tokio::spawn below do the job

[bkchr]

I would change the collect & remove code path to a simple retain.

[bkchr]

Why not retain? Then we don't need collect & remove.

[lexnv]

There's a tiny borrow-checker issue coming from the sender consuming self: retain will give us &mut req and the req.response oneshot::Sender consumes self when we send out the send(Err(RequestFailure::Network(OutboundFailure::Timeout))).

We could still avoid the extra alloc by adding an Option over the sender, will have a look if it complicates the code, thanks

[bkchr]

So this could only be triggered by a bug in libp2p/litep2p?

[lexnv]

Yep this could be triggered only by a bug in libp2p (libp2p/rust-libp2p#5429). I expected that the latest libp2p version would solve this issue, however, I could reproduce it on our libp2p kusama validator (and locally):

2025-01-21 05:43:14.885  WARN tokio-runtime-worker sub-libp2p: Requests [(ProtocolRequestId { protocol: OnHeap("/b0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafe/sync/2"), request_id: OutboundRequestId(53207) }, Some(20.479579184s))] force detected as timeout.

2025-01-21 05:43:18.178  WARN tokio-runtime-worker sub-libp2p: Received `RequestResponseEvent::OutboundFailure` with unexpected request id OutboundRequestId(53207) error Timeout from PeerId("12D3KooWJZuPc7KPtJcbYurX7brn3booPwbK53ucJhhPmuc1zZPt")

In this case libp2p produces a timeout event ~4s after we have detected the request as timed out. I'll also have a look at our poll implementation, maybe there's something there

[paritytech-workflow-stopper]

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/12907455229
Failed job name: cargo-clippy

[paritytech-cmd-bot-polkadot-sdk]

Created backport PR for stable2407:

• [stable2407] Backport #7222 #7300 with remaining conflicts!

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-7222-to-stable2407
git worktree add --checkout .worktree/backport-7222-to-stable2407 backport-7222-to-stable2407
cd .worktree/backport-7222-to-stable2407
git reset --hard HEAD^
git cherry-pick -x fd64a1e7768ba6e8676cbbf25c4e821a901c0a7f
git push --force-with-lease

[paritytech-cmd-bot-polkadot-sdk]

Created backport PR for stable2409:

• [stable2409] Backport #7222 #7301 with remaining conflicts!

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-7222-to-stable2409
git worktree add --checkout .worktree/backport-7222-to-stable2409 backport-7222-to-stable2409
cd .worktree/backport-7222-to-stable2409
git reset --hard HEAD^
git cherry-pick -x fd64a1e7768ba6e8676cbbf25c4e821a901c0a7f
git push --force-with-lease

[paritytech-cmd-bot-polkadot-sdk]

Created backport PR for stable2412:

• stable2412/backport #7222: Enforce libp2p outbound request-response timeout limits #7302 with remaining conflicts!

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-7222-to-stable2412
git worktree add --checkout .worktree/backport-7222-to-stable2412 backport-7222-to-stable2412
cd .worktree/backport-7222-to-stable2412
git reset --hard HEAD^
git cherry-pick -x fd64a1e7768ba6e8676cbbf25c4e821a901c0a7f
git push --force-with-lease