Skip to content

fix(mocknet): make stream deadline methods noop instead of returning error #3471

Merged
MarcoPolo merged 1 commit intolibp2p:masterfrom
walldiss:fix/mocknet-deadline-noop
Mar 5, 2026
Merged

fix(mocknet): make stream deadline methods noop instead of returning error #3471
MarcoPolo merged 1 commit intolibp2p:masterfrom
walldiss:fix/mocknet-deadline-noop

Conversation

@walldiss
Copy link
Contributor

Summary

  • Make SetDeadline, SetReadDeadline, and SetWriteDeadline on mocknet streams return nil instead of an error
  • Mocknet uses io.Pipe which doesn't support deadlines, but returning an error breaks callers that treat SetWriteDeadline failure as fatal
  • Since writes on in-memory pipes complete immediately, deadline enforcement is unnecessary and the noop is safe

Context

go-libp2p-pubsub added SetWriteDeadline calls in handleSendingMessages (part of libp2p/go-libp2p-pubsub#631). The deadline error is treated as fatal. The stream is reset and the hello packet is never sent. This prevents gossipsub peers from discovering each other's subscriptions in any mocknet-based test.

@MarcoPolo
Copy link
Collaborator

We recently released the simlibp2p package under x/simlibp2p. That's the preferred way to simulate networks in tests as it supports everything a real QUIC transport supports and properly models congestion.

I think we should deprecate our mocknet package. We aren't using it anywhere within go-libp2p, and I don't want to spend time maintaining it when we have a better alternative.

That said, I don't strongly oppose the general idea here, and I'm a bit surprised it's taken this long to find this issue.

Since writes on in-memory pipes complete immediately, deadline enforcement is unnecessary and the noop is safe

Deadlines aren't only about how fast writes happen. They protect a node from peers that are (possibly maliciously) stalling a stream.

These changes wouldn't work for testing those cases. But maybe that's fine

@Wondertan
Copy link
Contributor

This is a quick way to solve the issue that doesn't require major testing refactoring. We are aware of simnet and that mocknet looms with deprecation.

@MarcoPolo
Copy link
Collaborator

Please gofmt the file @walldiss, and we can merge this.

@walldiss
Copy link
Contributor Author

walldiss commented Mar 5, 2026

There seems to be some CI issue related to testing setup: go: go.mod requires go >= 1.25.7 (running go 1.24.13;. @MarcoPolo anything needed from my end?

@MarcoPolo
Copy link
Collaborator

nope, that's a known issue that I haven't had a chance to fix it.

@MarcoPolo MarcoPolo merged commit 67f4f58 into libp2p:master Mar 5, 2026
8 of 9 checks passed
@MarcoPolo
Copy link
Collaborator

thanks for the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants