-
Notifications
You must be signed in to change notification settings - Fork 267
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
Increment network version and change default configs #771
Conversation
@@ -572,7 +572,7 @@ func DefaultRPCConfig() *RPCConfig { | |||
MaxSubscriptionClients: 100, | |||
MaxSubscriptionsPerClient: 5, | |||
SubscriptionBufferSize: defaultSubscriptionBufferSize, | |||
TimeoutBroadcastTxCommit: 10 * time.Second, | |||
TimeoutBroadcastTxCommit: 35 * time.Second, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, this makes sense as well.
config/config.go
Outdated
MaxNumInboundPeers: 120, | ||
MaxNumOutboundPeers: 30, | ||
MaxConnections: 150, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not have any confidence in these settings tbh.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Me either. We can revert these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reverted, but fwiw I originally increased because of https://github.com/tharsis/evmos/pull/541/files and osmosis-labs/osmosis#464
@@ -21,7 +21,7 @@ const ( | |||
var ( | |||
// P2PProtocol versions all p2p behavior and msgs. | |||
// This includes proposer selection. | |||
P2PProtocol uint64 = 8 | |||
P2PProtocol uint64 = 9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to @tychoish UseLegacy
should be interoperable independent with false/true. I would rather clarify this first before trying to enforce this by bumping the version here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense.
closing as we won't be taking this approach |
Replaced int64 with big.int Co-authored-by: Lasaro <lasaro@informal.systems> Co-authored-by: Sergio Mena <sergio@informal.systems> Co-authored-by: Thane Thomson <connect@thanethomson.com>
This PR increments the network version. This will sever the p2p handshake with any node using an older binary. We also take this opportunity to change the default configs, although it should be mentioned that the configs should still be intercepted and changed in the app just to add an extra layer of fool proofness.