8000 perf(internal/blocksync): avoid double-calling `types.BlockFromProto` by melekes · Pull Request #2016 · cometbft/cometbft · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

perf(internal/blocksync): avoid double-calling types.BlockFromProto #2016

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jan 26, 2024
Merged

perf(internal/blocksync): avoid double-calling types.BlockFromProto #2016

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f8c9c3c
perf(internal/blocksync): avoid double-calling `types.BlockFromProto`
melekes Jan 11, 2024
987761f
stop the peer if it sends us invalid ext commit
melekes Jan 11, 2024
9351620
add a changelog entry
melekes Jan 11, 2024
97fa720
Merge branch 'main' into anton/1964-blocksync-validate-msg
melekes Jan 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ...unreleased/improvements/2016-blocksync-avoid-double-calling-block-from-proto.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- `[blocksync]` Avoid double-calling `types.BlockFromProto` for performance
reasons ([\#2016](https://github.com/cometbft/cometbft/pull/2016))
7 changes: 3 additions & 4 deletions internal/blocksync/msgs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,9 @@ func ValidateMsg(pb proto.Message) error {
return ErrInvalidHeight{Height: msg.Height, Reason: "negative height"}
}
case *bcproto.BlockResponse:
_, err := types.BlockFromProto(msg.Block)
if err != nil {
return err
}
// Avoid double-calling `types.BlockFromProto` for performance reasons.
// See https://github.com/cometbft/cometbft/issues/1964
return nil
case *bcproto.NoBlockResponse:
if msg.Height < 0 {
return ErrInvalidHeight{Height: msg.Height, Reason: "negative height"}
Expand Down
4 changes: 3 additions & 1 deletion internal/blocksync/reactor.go
View file
Open in desktop
8000
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,8 @@ func (bcR *Reactor) Receive(e p2p.Envelope) {
case *bcproto.BlockResponse:
bi, err := types.BlockFromProto(msg.Block)
if err != nil {
bcR.Logger.Error("Block content is invalid", "err", err)
bcR.Logger.Error("Peer sent us invalid block", "peer", e.Src, "msg", e.Message, "err", err)
bcR.Switch.StopPeerForError(e.Src, err)
return
}
var extCommit *types.ExtendedCommit
Expand All @@ -266,6 +267,7 @@ func (bcR *Reactor) Receive(e p2p.Envelope) {
bcR.Logger.Error("failed to convert extended commit from proto",
"peer", e.Src,
"err", err)
bcR.Switch.StopPeerForError(e.Src, err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we disconnect from peers that send us invalid extended commits. I don't think there is an use case in which a good peer will send us that, so legit.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hardening p2p is always legit

return
}
}
Expand Down
0