Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

*: update v0.17 branch with latest code for release #2528

Merged
merged 10 commits into from
Aug 9, 2023

Conversation

corverroos and others added 10 commits August 9, 2023 16:02
Makes DKG P2P receive timeout configurable and increases it from the default 5s to 1min allowing peers to broadcast messages much earlier and having them wait longer for the receiver to handle to properly.

This was encountered in large DKG where some peers are much slower than others, still doing key generation even though others are already sharing node sigs and erroring after 5s since the slow peer doensn't have a lock hash yet to verify the received node sigs.

category: refactor
ticket: none
We didn't actually set timeouts when sending libp2p messages, only when receiving message. Not setting timeouts is bad practice. See more info on this [here](https://dev.to/bearer/the-importance-of-request-timeouts-l3n) and [here](https://medium.com/booking-com-development/io-socket-timeout-socket-timeout-made-easy-4dfd43e777f7).

This sets the send timeout equivalent to the read timeout, allowing up to 1s latency between nodes, so read timeout+2s.

category: misc
ticket: none
Before this commit, calling `exchange()` would listen for new incoming messages and discard everything that didn't carry the expected `sigType`.

In a DKG one node might fall behind the others (for example, while DKG'ing 3000 validator keys) and send `sigTypes` that the other peers have already received threshold for.

This PR makes sure a given instance of `exchanger` will hold onto all messages whose `sigTypes` are specified during its initialization phase.

Calling `exchange()` will return as soon as there are enough messages for the specified `sigType`, but different `sigType`s messages will still be stored and will be returned when requested by means of another `exchange()` call, with a different `sigType` argument.

category: bug
ticket: #2491

Closes #2491.
Introduces explicit sync steps in DKG, allowing all peers to wait for other to complete each step before continuing to next step. This prevents fast peers timing out sending stuff to slow peers. It also mitigates against shutdown races.

category: bug
ticket: #887
Wait until all peer's validator signatures over exchanger-handled data.

This should fix nightly test.

category: bug
ticket: none
Integrates p2p fuzz to compose.

category: test
ticket: #2375
Also fixes latest `govulncheck` issues.


category: misc
ticket: none
Make every peer participating in the DKG wait for the "slow" one, and synchronize the `Successfully completed DKG ceremony 🎉` message output.


category: misc
ticket: none
Fixes soft shutdown by cancelling long running proxied requests.

This is because long lived connections are not interrupted by `http.Server.Shutdown`:
```
// Shutdown gracefully shuts down the server without interrupting any
// active connections. Shutdown works by first closing all open
// listeners, then closing all idle connections, and then waiting
// indefinitely for connections to return to idle and then shut down...
```

category: bug
ticket: #2435
Add the `LaunchpadURLForLock` method, which generates a Launchpad cluster dashboard URL for a given cluster lock.

Reference this method in `dkg` and `create cluster` commands so that at the end of those processes, if the user has specified `--publish`, the Launchpad dashboard URL will be printed on the console.

category: feature
ticket: #2423 

Closes #2423.
@github-actions github-actions bot added the branch-release PR or Issue linked to a release branch (not main) label Aug 9, 2023
@codecov
Copy link

codecov bot commented Aug 9, 2023

Codecov Report

Patch coverage: 72.69% and project coverage change: +0.17% 🎉

Comparison is base (1382a92) 53.54% compared to head (7352fde) 53.72%.

Additional details and impacted files
@@              Coverage Diff               @@
##           main-v0.17    #2528      +/-   ##
==============================================
+ Coverage       53.54%   53.72%   +0.17%     
==============================================
  Files             198      198              
  Lines           26756    26931     +175     
==============================================
+ Hits            14326    14468     +142     
- Misses          10645    10662      +17     
- Partials         1785     1801      +16     
Files Changed Coverage Δ
app/app.go 6.78% <0.00%> (ø)
dkg/bcast/server.go 70.42% <ø> (ø)
testutil/compose/config.go 72.72% <ø> (ø)
testutil/compose/run.go 81.00% <33.33%> (-3.22%) ⬇️
dkg/dkg.go 57.12% <52.70%> (-0.95%) ⬇️
cmd/createcluster.go 59.82% <52.94%> (-0.24%) ⬇️
p2p/sender.go 77.72% <60.00%> (+0.06%) ⬆️
dkg/sync/server.go 70.18% <70.00%> (+0.12%) ⬆️
dkg/exchanger.go 84.88% <90.74%> (+4.49%) ⬆️
app/obolapi/api.go 65.51% <91.30%> (+17.69%) ⬆️
... and 5 more

... and 6 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gsora gsora added the merge when ready Indicates bulldozer bot may merge when all checks pass label Aug 9, 2023
@obol-bulldozer obol-bulldozer bot merged commit 4c90ca0 into main-v0.17 Aug 9, 2023
@obol-bulldozer obol-bulldozer bot deleted the gsora/v017_updates branch August 9, 2023 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch-release PR or Issue linked to a release branch (not main) merge when ready Indicates bulldozer bot may merge when all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants