-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
cmd/utils: disable snap protocol for fast node #2234
Conversation
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.
LGTM
cmd/utils/flags.go
Outdated
@@ -1954,6 +1954,12 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) { | |||
cfg.EnableTrustProtocol = true | |||
} | |||
|
|||
// A node without trie is not able to provide snap data, so it should disable snap protocol. |
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.
if disable
fast node can do snap sync?
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.
If you meant when a node starts with a fast node, which does full sync, then later on becomes a full node and does snap sync again, will it work? I'm not entirely sure, my intuition is that it doesn't work.
80daa47
to
e6280a1
Compare
e6280a1
to
8430242
Compare
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.
👍
Description
This PR adds a logic check such that if the node is fast node (i.e.
--tries-verify-mode
is notlocal
), then it will disable the snap protocol by default.Rationale
When serving snap data (i.e.
AccountRange
,StorageRange
,TrieNodes
), access to a complete state trie is necessary. However, for fast nodes, since they only store the snapshot, they can't service these requests. Disabling this snap protocol by default may help reduce network calls and processing time for the nodes in general.