fix: remove all dynamic peer discovery #3289
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces several updates to the dependency management and telemetry functionalities within the project. The Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
go.mod (1)
Based on the GitHub security advisory results, I can generate the final response as the verification is complete.
Security audit of fixed dependency versions reveals critical concerns
The project's dependencies have known security vulnerabilities:
github.com/ethereum/go-ethereum v1.13.15: Currently using the patched version that fixes a HIGH severity DoS vulnerability via malicious p2p messages.
github.com/libp2p/go-libp2p v0.27.8: Using a version that patches the HIGH severity RSA key attack vulnerability, but is vulnerable to a HIGH severity OOM attack (requires upgrade to v0.27.4 or later).🔗 Analysis chain
Line range hint
6-373: Verify security implications of fixed dependency versions.Several dependencies have fixed versions. While this ensures reproducible builds, it's important to verify that these versions don't have known security vulnerabilities.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for known vulnerabilities in key dependencies # Check for security advisories for ethereum/go-ethereum gh api graphql -f query=' { securityVulnerabilities(first: 5, ecosystem: GO, package: "github.com/ethereum/go-ethereum") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }' # Check for security advisories for libp2p gh api graphql -f query=' { securityVulnerabilities(first: 5, ecosystem: GO, package: "github.com/libp2p/go-libp2p") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }'Length of output: 2860
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
go.mod(1 hunks)zetaclient/metrics/telemetry.go(0 hunks)zetaclient/tss/healthcheck.go(0 hunks)zetaclient/tss/service.go(0 hunks)
💤 Files with no reviewable changes (3)
- zetaclient/tss/service.go
- zetaclient/tss/healthcheck.go
- zetaclient/metrics/telemetry.go
🔇 Additional comments (2)
go.mod (2)
Line range hint 1-4: LGTM: Go toolchain version is appropriately specified.
The Go toolchain version is explicitly set to 1.22.8, which is a good practice for ensuring consistent builds across different environments.
374-374: Verify the impact of go-tss dependency update.
The replacement of gitlab.com/thorchain/tss/go-tss with github.com/zeta-chain/go-tss aligns with the PR objective of removing dynamic peer discovery. However, we should verify that this change doesn't introduce any breaking changes.
✅ Verification successful
Let me gather more information about the specific changes in the go-tss dependency.
Let me try a different approach to analyze the go-tss usage.
Go-TSS dependency update verified as safe
Based on the code analysis, the update from gitlab.com/thorchain/tss/go-tss to github.com/zeta-chain/go-tss is safe for the following reasons:
- The codebase maintains compatibility with the TSS interface, using the same core types and methods (
TssServer,KeygenConfig, etc.). - The implementation in
zetaclient/tss/setup.goshows that the TSS server initialization and configuration remain consistent with the original interface. - The change aligns with removing dynamic peer discovery as evidenced by the explicit peer whitelisting in
NewTSSServer.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for significant changes between the old and new go-tss versions
# Find files importing the old go-tss package
rg -l "gitlab.com/thorchain/tss/go-tss"
# Find files importing the new go-tss package
rg -l "github.com/zeta-chain/go-tss"
# Look for any TSS-related configuration or initialization code
ast-grep --pattern 'type $_ struct {
$$$
tss $_
$$$
}'
Length of output: 11351
Script:
#!/bin/bash
# Check the actual usage of go-tss in the codebase
rg -A 5 "NewTssServer|NewTssKeySign"
# Check for any configuration or initialization changes required
rg -A 5 "tss.Config|tss.KeygenConfig"
# Look for any specific imports or usage patterns that might be affected
ast-grep --pattern 'import $_ "gitlab.com/thorchain/tss/go-tss"'
ast-grep --pattern 'import $_ "github.com/zeta-chain/go-tss"'
Length of output: 209
Script:
#!/bin/bash
# Check the implementation details in tss-related files
cat zetaclient/tss/config.go
cat zetaclient/tss/setup.go
# Look for specific TSS server initialization patterns
rg -A 10 "func New.*Server" zetaclient/tss/
# Check for any configuration structures
rg -A 10 "type Config struct" zetaclient/tss/
Length of output: 13844
gartnera
force-pushed
the
remove-dynamic-peer-discovery
branch
from
edcd2d7 to
588abe0
Compare
We've had to revert the new private peer discovery for two releases. It didn't work on live networks because it's very hard to discover which port is actually publicly accessible on our zetaclients because of the sentry architecture.
We've been using a static address book for awhile now which isn't that hard to manage on the operations side.
Related to #3277
go-tss change: zeta-chain/go-tss#40
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
SetKnownPeersmethod, enhancing peer management.