Skip to content
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

Jump to bottom

P2p validation on main #3189

Closed
wants to merge 33 commits into from
Closed

P2p validation on main #3189

wants to merge 33 commits into from

Conversation

LeoHChen
Copy link
Contributor

No description provided.

Leo Chen added 29 commits July 2, 2020 05:18
[libp2p] validate pubsub messages
de27567 
Signed-off-by: Leo Chen <leo@harmony.one>

[libp2p] validate consensus message at p2p layer

Signed-off-by: Leo Chen <leo@harmony.one>
[consensus] do pubkey deserialization only once
78cb954 
Signed-off-by: Leo Chen <leo@harmony.one>
[consensus] do not verify every p2p message
d096ffd 
Signed-off-by: Leo Chen <leo@harmony.one>
[p2p] add a buffer in msgChan
85a1bb9 
Signed-off-by: Leo Chen <leo@harmony.one>
[p2p] add counters of valid/invalid messages
10fcf3c 
Signed-off-by: Leo Chen <leo@harmony.one>
[p2p] increase timeout of validator
7f0f52d 
* restore the ticker to the original 3 seconds

Signed-off-by: Leo Chen <leo@harmony.one>
[p2p] sample log p2p validation error messages
39e0d8c 
Signed-off-by: Leo Chen <leo@harmony.one>
[log] sample the cost log of p2p validation
bab70a3
[log] add more log context in err message
1b839de 
Signed-off-by: Leo Chen <leo@harmony.one>
[p2p] output validation context exceed error
9e6c110 
Signed-off-by: Leo Chen <leo@harmony.one>
[context] change log level to warn if deadline exceeded
903b8f3 
Signed-off-by: Leo Chen <leo@harmony.one>
address review comments on harmony-one#3164
b905c0b 
Signed-off-by: Leo Chen <leo@harmony.one>
[p2p] ignore messages not intended for leader/validator
776030d 
those messages are not errors, they won't be processed by consensus message handlers
but till be propagated to p2p network.

Signed-off-by: Leo Chen <leo@harmony.one>
[p2p] not an error when the message is ignored
e5a69cb 
Signed-off-by: Leo Chen <leo@harmony.one>
[p2p] do not invalidate p2p message w/o prefix size
23287e6 
Being a bit conservative, as we have received many message on the network w/o
the prefix size. We need further dive deep into those messages.
So for now, better be conservative.

Signed-off-by: Leo Chen <leo@harmony.one>
[p2p] add a counter of total p2p messages being validated
7ff96eb 
Signed-off-by: Leo Chen <leo@harmony.one>
[p2p] use inline validator
c26712a 
[validation] increase the validation queue size

[validation] increase throttle

Signed-off-by: Leo Chen <leo@harmony.one>
[validation] add ignored message counter
6547024 
Signed-off-by: Leo Chen <leo@harmony.one>
[validation] separate counter of invalid slot messages
85d2ac5 
Signed-off-by: Leo Chen <leo@harmony.one>
[consensus] add IsViewChangingMode function
05e7bd7 
Signed-off-by: Leo Chen <leo@harmony.one>
[validation] fix nil pointer panic in view change mode
75bb430 
Signed-off-by: Leo Chen <leo@harmony.one>
[consensus] unify consensus message checking
dc02481 
Signed-off-by: Leo Chen <leo@harmony.one>
[consensus] move VerifyMessageSig to check.go
ea1e276 
Signed-off-by: Leo Chen <leo@harmony.one>
[validation] remove duplicated check in consensus
b3c68be 
Signed-off-by: Leo Chen <leo@harmony.one>
[go.mod] update libp2p module
1e6d694 
Signed-off-by: Leo Chen <leo@harmony.one>
[validation] use proper return value
79a291e 
Signed-off-by: Leo Chen <leo@harmony.one>
[validation] reject msg with error
33a53fb 
Signed-off-by: Leo Chen <leo@harmony.one>
[log] remove debug log cost
b9f4fd5 
Signed-off-by: Leo Chen <leo@harmony.one>
[counter] count number of p2p messages
cb91ec5 
Signed-off-by: Leo Chen <leo@harmony.one>
@LeoHChen LeoHChen force-pushed the p2p_validation_on_main branch from 5859bbd to cb91ec5 Compare July 2, 2020 07:19
[consensus] remove unused senderKeySanityChecks
2543f58 
Signed-off-by: Leo Chen <leo@harmony.one>
@LeoHChen LeoHChen force-pushed the p2p_validation_on_main branch from 5ed0921 to 2543f58 Compare July 2, 2020 07:50
@LeoHChen LeoHChen requested review from rlan35 and JackyWYX July 2, 2020 07:56
Leo Chen added 3 commits July 2, 2020 07:57
[misc] add comment to SenderSanityCheck func
0d7b538 
Signed-off-by: Leo Chen <leo@harmony.one>
[misc] allocate enough cap for slices
2dd009e 
Signed-off-by: Leo Chen <leo@harmony.one>
[build] fix travis build error
210a571 
Signed-off-by: Leo Chen <leo@harmony.one>
@LeoHChen
Copy link
Contributor Author

LeoHChen commented Jul 2, 2020

image

image

The CPU usage on a validator will be doubled as we do pubkey checking on every p2p messages even though validators may ignore them. This is the tradeoff to reduce the propagation of the invalid messages into the network. We can see clearly that the outgoing network traffic is lower than incoming traffic now as invalid ones are filtered out.

JackyWYX
JackyWYX reviewed Jul 2, 2020
View reviewed changes
Copy link
Contributor

@JackyWYX JackyWYX left a comment

Choose a reason for hiding this comment

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

Great work! Will the update to protobuf in consensus message cause backward incompatibility?

consensus/checks.go Show resolved Hide resolved
node/node_handler.go Show resolved Hide resolved
@LeoHChen LeoHChen requested a review from chainum July 3, 2020 00:55
@LeoHChen
Copy link
Contributor Author

New PR rebased based on latest main branch. #3212

@LeoHChen LeoHChen closed this Jul 10, 2020
@LeoHChen LeoHChen deleted the p2p_validation_on_main branch July 16, 2020 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JackyWYX JackyWYX JackyWYX left review comments

@rlan35 rlan35 Awaiting requested review from rlan35

@chainum chainum Awaiting requested review from chainum

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@LeoHChen @JackyWYX