Skip to content

fix: Don't manually close stream on p2p/reqresp/goodbye#14531

Merged
PhilWindle merged 7 commits into
AztecProtocol:masterfrom
mralj:fix/p2p-reqresp-goodbye
May 29, 2025
Merged

fix: Don't manually close stream on p2p/reqresp/goodbye#14531
PhilWindle merged 7 commits into
AztecProtocol:masterfrom
mralj:fix/p2p-reqresp-goodbye

Conversation

@mralj

@mralj mralj commented May 26, 2025

Copy link
Copy Markdown
Contributor

Closes #14515.
The peer connection is closed upon receiving goodbye, so the stream shouldn't be closed manually.

@mralj mralj changed the base branch from next to master May 26, 2025 10:57
alexghr
alexghr reviewed May 27, 2025
View reviewed changes
Comment thread yarn-project/p2p/src/services/reqresp/reqresp.ts
Comment on lines -659 to +665
await stream.close();
if (!streamAlreadyClosed) {
await stream.close();
}

@alexghr alexghr May 27, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm wondering if we could use stream.status instead?

@mralj mralj May 27, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch, I didn't know about the status prop.
Fixed by 83abf6c

PhilWindle
PhilWindle approved these changes May 27, 2025
View reviewed changes

@PhilWindle PhilWindle left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for this!

@alexghr alexghr added the ci-external Allow CI to run on this external pull request. label May 27, 2025
@mralj mralj changed the title Fix: Don't manually close stream on p2p/reqresp/goodbye fix!: Don't manually close stream on p2p/reqresp/goodbye May 28, 2025
@PhilWindle PhilWindle changed the title fix!: Don't manually close stream on p2p/reqresp/goodbye fix: Don't manually close stream on p2p/reqresp/goodbye May 29, 2025
@PhilWindle PhilWindle added this pull request to the merge queue May 29, 2025
Merged via the queue into AztecProtocol:master with commit 38cec7d May 29, 2025
5 checks passed
This was referenced May 29, 2025
Closed
Closed
@mralj mralj deleted the fix/p2p-reqresp-goodbye branch May 29, 2025 09:22
@AztecBot AztecBot mentioned this pull request May 30, 2025
Merged
charlielye pushed a commit that referenced this pull request Jun 2, 2025
@AztecBot
chore(master): release 0.87.6 (#14651)
93bd0bf 
🤖 I have created a new Aztec Packages release
---


##
[0.87.6](v0.87.5...v0.87.6)
(2025-06-02)


### ⚠ BREAKING CHANGES

* app benches and unified PXE creation
([#14504](#14504))
* Removes normalize() calls on pairing points
([#14285](#14285))

### Features

* Adds StarknetZK WASM bindings to bb.js
([#14372](#14372))
([224b219](224b219))
* allow mempool to overflow before evicting
([#14641](#14641))
([f81425d](f81425d))
* app benches and unified PXE creation
([#14504](#14504))
([aae5ab0](aae5ab0))
* **bb:** memory tracking for microbenchmarks
([#14445](#14445))
([fae2961](fae2961))
* enable provers to run in node chart
([#14405](#14405))
([94edd35](94edd35))
* Ignore P2P messages previously seen
([#14665](#14665))
([2e50588](2e50588))
* measure oracles
([#14552](#14552))
([9cc6b54](9cc6b54))
* Removes normalize() calls on pairing points
([#14285](#14285))
([942b948](942b948))
* Request missing txs from block proposal sender
([#14341](#14341))
([3f97e8b](3f97e8b))
* unbundled bb.js
([#14401](#14401))
([e0d9662](e0d9662))
* validate notes in a single node roundtrip
([#14650](#14650))
([d467955](d467955))
* validating partial note sender
([#14379](#14379))
([de9880c](de9880c)),
closes
[#14363](#14363)


### Bug Fixes

* allow returning of tuples from contract funcs
([#14553](#14553))
([1c0955c](1c0955c))
* asan-fast caching
([#14468](#14468))
([fbd9ed6](fbd9ed6))
* attempt fix concurrency
([f31e706](f31e706))
* attempt to fix merge group base config
([6aa342b](6aa342b))
* bug in `TxProvingResult` schema
([#14498](#14498))
([#14530](#14530))
([0ac26fa](0ac26fa))
* bump defaults
([#14474](#14474))
([ade9a56](ade9a56))
* do not run concurrent idb transactions
([#14609](#14609))
([5a6f36a](5a6f36a))
* **docs:** Update create pxe interface
([#14587](#14587))
([b3b9c05](b3b9c05))
* **docs:** Update getting started with testnet page
([#14536](#14536))
([66fc3eb](66fc3eb))
* don't create ./out when verifying
([#14556](#14556))
([ccb9981](ccb9981))
* Don't manually close stream on p2p/reqresp/goodbye
([#14531](#14531))
([38cec7d](38cec7d))
* eccvm_circuit_builder overrun
([#14484](#14484))
([5e01c07](5e01c07))
* enable bundling txs with block proposals
([#14649](#14649))
([39b8c4e](39b8c4e))
* error on goodbye
([#14679](#14679))
([5112f5f](5112f5f))
* issues with syncNotes --> syncPrivateState renaming
([#14442](#14442))
([78de410](78de410))
* Merge queue instances have pr name for uniquness in e.g. reorgs.
([#14562](#14562))
([1f012a9](1f012a9))
* nope
([470882a](470882a))
* parse LOG_LEVELS in bb
([#14674](#14674))
([88203d6](88203d6))
* **playground:** fix sfpc version when creating account
([#14481](#14481))
([5486a22](5486a22))
* prevent world-state from spamming the logs
([#14594](#14594))
([8c725de](8c725de))
* processing events in contracts with no notes
([#14528](#14528))
([1bab9b4](1bab9b4)),
closes
[#14499](#14499)
* retrieve L1 to L2 messages in batches
([#14586](#14586))
([2be7f1b](2be7f1b))
* slack for flakes
([61829bc](61829bc))
* target branch
([e463723](e463723))
* test tracking
([#14513](#14513))
([2d6fc3a](2d6fc3a))
* try to fix release please
([f31c07e](f31c07e))
* try to fix release please
([d402c27](d402c27))
* try to fix release please
([cb959b8](cb959b8))
* try to fix release please
([227dbd2](227dbd2))
* update sponsored fpc address in playground
([#14472](#14472))
([4c14bc9](4c14bc9))
* wip merge master to next
([249ab4c](249ab4c))
* wip merge master to next
([b7850c9](b7850c9))


### Miscellaneous

* add testnet compat test
([#14601](#14601))
([1fbb350](1fbb350))
* Aztec simulator into PXE
([#14598](#14598))
([a999c8c](a999c8c))
* **bb:** avoid compile-time hash-to-curve
([#14177](#14177))
([d3863ff](d3863ff))
* capture RPC calls + correct timings
([#14633](#14633))
([5ee54ff](5ee54ff))
* Disable flood publish by default
([#14635](#14635))
([af8d879](af8d879))
* **docs:** Add scheduled typesense index job
([#14615](#14615))
([5f42e1b](5f42e1b))
* **docs:** Update alpha-testnet docs to use version 0.87.2
([#14501](#14501))
([c86d3b0](c86d3b0))
* drop txs per block to 8
([#14627](#14627))
([369f210](369f210))
* master-to-next
([#14454](#14454))
([06ac335](06ac335))
* **master:** release 0.87.3
([#14440](#14440))
([3ca26cd](3ca26cd))
* **master:** release 0.87.3
([#14582](#14582))
([8cea842](8cea842))
* **master:** release 0.87.4
([#14583](#14583))
([8077f63](8077f63))
* New prover chart for alpha-testnet
([#14514](#14514))
([031d7cb](031d7cb))
* partial notes optimization with nullifier
([#14432](#14432))
([cc90823](cc90823))
* playground should release also at refname. attempt to make RP do
prerelease version.
([3c4fba5](3c4fba5))
* Protocol breaking changes must now go to next branch.
([#14423](#14423))
([44304b6](44304b6))
* release 0.87.3
([8340415](8340415))
* release please v4
([bf97805](bf97805))
* remove some expected failures
([#13843](#13843))
([2593e28](2593e28))
* Revert "fix: processing events in contracts with no notes
([#14528](#14528))"
([#14596](#14596))
([3278e61](3278e61))
* select random rollup versions from distinct sets
([#14568](#14568))
([fe787cd](fe787cd))
* stdlib field pre-audit pt.0
([#14413](#14413))
([52458c2](52458c2))
* trick release-please
([cbcad3d](cbcad3d))
* trick release-please
([a0c3a21](a0c3a21))


### Documentation

* comment on partial note reuse
([#14383](#14383))
([42200c8](42200c8))
* create v0.87.4 as latest
([#14607](#14607))
([78c2469](78c2469))
* fix image
([#14537](#14537))
([388e79b](388e79b)),
closes
[#14163](#14163)
* update cli ref in node doc
([#14574](#14574))
([d54a8af](d54a8af))
* update node version instructions
([#14412](#14412))
([a2b1b9d](a2b1b9d))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
@mralj mralj mentioned this pull request Jun 10, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexghr alexghr alexghr approved these changes

@PhilWindle PhilWindle PhilWindle approved these changes

Assignees

No one assigned

Labels

ci-external Allow CI to run on this external pull request.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] ReqResp error on Goodbye

3 participants

@mralj @alexghr @PhilWindle