Upgrade Go and libp2p versions #3771
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Package `go-libp2p-core` has been deprecated and moved to `go-libp2p`. Here we adjust all imports accordingly. See: https://github.com/libp2p/go-libp2p/releases/tag/v0.22.0
The `go-libp2p`'s TLS implementation used within the `transport` struct has a new constructor that accepts the security protocol ID and a list of muxers. Regarding the first, it's enough to pass the TLS protocol ID exposed by `go-libp2p`. For the second, it's enough to pass `nil` as the Keep client does not use Optimized Stream Multiplexer Selection so no mutex is determined upfront. Regarding `authenticatedConnection`, there is a need to implement an additional method `ConnState` that exposes `ConnectionState`. This can be fetched from the secure connection established by the TLS encryption layer. See: - https://github.com/libp2p/go-libp2p/releases/tag/v0.24.0 - https://github.com/libp2p/specs/blob/50e2cd49c3261eeb71936e4e1b371bd3aa3d7d62/connections/inlined-muxer-negotiation.md
Version of the `golang.org/x/exp` package has been bumped up. The returned type of the compare function used in `slices.SortFunc` was changed from `bool` to `int` somewhere between.
While working on the previous change, we realized that this test is flaky and behaves differently due to non-deterministic operator address construction. Here we fix that problem.
This test used a dummy curve point that was not on the curve. Since Go 1.19, such a behavior leads to a panic. Here we fix that by using a proper point that is on the curve. See: https://tip.golang.org/doc/go1.19 > Minor changes to the library > crypto/elliptic
`go-libp2p` versions prior to `v0.24.0` allowed to pass either a constructed security transport instance or a constructor producing such an instance. Starting from `v0.24.0` only the latter option is supported. See: https://github.com/libp2p/go-libp2p/releases/tag/v0.24.0
We are also taking an opportunity and bump `protoc-gen-go` to the recent version.
Warnings were all about usage of functions that are deprecated in Go 1.20. We are replacing them as recommended.
Setup of the security transport done after the recent libp2p upgrade was wrong. We configured libp2p host to use `/keep/handshake/1.0.0` as the security protocol but, in the same time, the security protocol implementation (our own `net/libp2p.transport`) returned `/tls/1.0.0` as ID. Here we fix that by using `/keep/handshake/1.0.0` everywhere and re-factoring the security transport constructor passed to the `libp2p.Security` option. Libp2p injects necessary parameters there and the constructor of `net/libp2p.transport` should use them to create the instance. Moreover, we are taking the opportunity and do some housekeeping around Keep-specific protocol names. We now have `securityProtocolID` which holds the ID of the encrypted transport protocol and `authProtocolID` which denotes the custom authentication protocol used by Keep clients. We are leaving original values to maintain backward compatibility with older clients.
tomaszslabon
approved these changes
Feb 8, 2024
tomaszslabon
added a commit
that referenced
this pull request
Feb 8, 2024
Refs: #3770 Depends on: #3771 Recent libp2p versions (we started to use them in #3771) introduced a way to set the seen messages cache TTL and strategy. Here we leverage those settings to reduce the excessive message flooding effect that sometimes occurs on mainnet. This pull request consists of two steps ### Use longer TTL for pubsub seen messages cache Once a message is received and validated, pubsub re-broadcasts it to other peers and puts it into the seen messages cache. This way, subsequent arrivals of the same message are not re-broadcasted unnecessarily. This mechanism is important for the network to avoid excessive message flooding. The default value used by libp2p is 2 minutes. However, Keep client messaging sessions are quite time-consuming so, we use a longer TTL of 5 minutes to reduce flooding risk even further. Worth noting that this time cannot be too long as the cache may grow excessively and impact memory consumption. ### Use `LastSeen` as seen messages cache strategy By default, the libp2p seen messages cache uses the `FirstSeen` strategy which expires an entry once TTL elapses from when it was added. This means that if a single message is being received frequently and consistently, pubsub will re-broadcast it every TTL, rather than never re-broadcasting it. In the context of the Keep client which additionally uses app-level retransmissions, that often leads to a strong message amplification in the broadcast channel which causes a significant increase in the network load. As the problem is quite common (see libp2p/go-libp2p-pubsub#502), the libp2p team added a new `LastSeen` strategy which behaves differently. This strategy expires an entry once TTL elapses from the last time the message was touched by a cache write (`Add`) or read (`Has`) operation. That gives the desired behavior of never re-broadcasting a message that was already seen within the last TTL period. This reduces the risk of unintended over-amplification.
lukasz-zimnoch
added a commit
that referenced
this pull request
Feb 12, 2024
This pull request backports #3771 to the `releases/mainnet/v2.0.0-m7` branch.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refs: #3770
Closes: #3761
Here we upgrade all libp2p libraries to the recent versions. To make it possible, we were also forced to bump the Go version from 1.18 to 1.20. This is the minimum version supported by recent libp2p packages.
I recommend reviewing commit by commit where specific changes are described in detail. Here is a brief summary of what has been done:
Upgrade Go from 1.18 to 1.20
Upgrade of Go resulted in a need to:
slices.SortFunccompare function we are using in one unit test. This is because the version of thegolang.org/x/exppackage had to be bumped up as well. The returned type of the compare function used inslices.SortFuncwas changed frombooltointsomewhere between (5a980c7)TestCoordinationExecutor_Coordinatewhich started to be flaky due to a changed behavior ofecdsa.GenerateKey. Since Go 1.20, the returned key no longer depends deterministically on the bytes read from the provided RNG, and may change between calls and/or between versions (2ed7179)TestWalletRegistry_getWalletByPublicKeyHash_NotFoundwhich used a dummy curve point. Since Go 1.19, such a behavior leads to a panic (50b6bd6)gofmtversion (3c2274e)staticcheckversion used by CI and fix the new warnings about deprecated standard library functions by replacing them as recommended (a87eea3)Upgrade of libp2p libraries
Upgrade of libp2p packages forced us to:
go-libp2p-coreimports to bego-libp/coreas this package was moved to thego-libp2pmonorepo (95d60b8)transportandauthenticatedConnectionimplementations to expose some additional functions required by libp2p interfaces (ac01765)transportdifferently due to the changes around libp2pSecurityoption (110fbb3, 6953b79)