op-node: log shutdown before releasing client waitgroup

[protolambda]

Description

Fix test issues where the test-logger was being used after the test shut down.

The TestMultiPeerSync defers the .Close() calls on the sync clients, but those then completed the wait-group in the sync client, before logging about the shutdown. This caused a race where the logger would panic because of the unexpected log.

The fix is to move the log before the wg.Done().

Reviewing the code further there also seems to be a case where we could have a warn-log while the client was already shutting down. We now return early in AddPeer if we're closing already, to ensure post-shutdown AddPeer calls cannot accidentally create logs that make the test flake.

[changeset-bot]

⚠️ No Changeset found

Latest commit: e359b31

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[netlify]

✅ Deploy Preview for opstack-docs canceled.

Name	Link
🔨 Latest commit	e359b31
🔍 Latest deploy log	https://app.netlify.com/sites/opstack-docs/deploys/64936c6435ea6a0008b87d76

[ajsutton]

LGTM.

[codecov]

Codecov Report

Merging #6101 (e359b31) into develop (e2d3e59) will decrease coverage by 0.91%.
The diff coverage is 25.00%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #6101      +/-   ##
===========================================
- Coverage    45.49%   44.58%   -0.91%     
===========================================
  Files          434      304     -130     
  Lines        28017    23291    -4726     
  Branches       688        0     -688     
===========================================
- Hits         12745    10384    -2361     
+ Misses       14240    11945    -2295     
+ Partials      1032      962      -70     

Flag	Coverage Δ
bedrock-go-tests	44.58% <25.00%> (-0.13%)	⬇️
cannon-go-tests	?
common-ts-tests	?
contracts-bedrock-tests	?
core-utils-tests	?
fault-detector-tests	?
sdk-tests	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
op-node/p2p/sync.go	62.63% <25.00%> (-6.16%)	⬇️

... and 130 files with indirect coverage changes

[mergify]

This PR has been added to the merge queue, and will be merged soon.

[mergify]

This PR is next in line to be merged, and will be merged as soon as checks pass.

[mergify]

This PR is next in line to be merged, and will be merged as soon as checks pass.