Skip to content
This repository was archived by the owner on Mar 5, 2024. It is now read-only.

Add validation for stale heartbeats #1393

Merged
merged 1 commit into from
Aug 17, 2022

Conversation

PaulVMo
Copy link
Contributor

@PaulVMo PaulVMo commented Jun 14, 2022

This change proposes adding a new validation to validator heartbeat transactions to ensure they are not older than the heartbeat interval + grace period. This change will help ensure that 1) validators who are significantly behind the blockchain (i.e. still syncing) do not submit PoC proposals or gateway reactivations and 2) PoC public keys stored in the ledger do not outlive the private keys stored locally by the validator (and thus causing invalid PoCs).

Currently, heartbeats are validated 1) to make sure they are not ahead of the consensus group validating the transaction and 2) to make sure they are not too soon after the previous heartbeat for that validator. The proposed change in this PR adds another check to make sure that the transaction height is not too old, using the heartbeat interval + grace period as the maximum age. While the miner includes a check to not submit a heartbeat if still synching, this is not currently enforced on chain.

This is likely an edge case, as most validator heartbeats are added to the chain within 2-3 blocks (see below for stats). However, during times of heavily load or in the case of an improperly functioning validator, the difference between the heartbeat transaction height and the height at which it is absorbed, may be significant. The table below shows the count of heartbeat transactions grouped by the difference between the chain height and the heartbeat transaction height in blocks for the last 100000 heartbeats.

Diff.   Count
2       45541
3       33080
4       11860
5        4341
6        1749
7         854
8         433
9         321
10        229
11-150    117
151+       94

Copy link
Contributor

@evanmcc evanmcc left a comment

Choose a reason for hiding this comment

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

This looks good but needs a test.

@PaulVMo PaulVMo force-pushed the validate-heartbeat-age branch from e47ee58 to 0fa9881 Compare June 15, 2022 17:28
@PaulVMo
Copy link
Contributor Author

PaulVMo commented Jun 15, 2022

thanks evan. let me know if this tests are what you had in mind.

@PaulVMo PaulVMo force-pushed the validate-heartbeat-age branch from 823eb8f to 5f1907b Compare August 17, 2022 16:50
@PaulVMo
Copy link
Contributor Author

PaulVMo commented Aug 17, 2022

Resolved merge conflict and squashed including re-running ct suite for validator heartbeats.

@evanmcc evanmcc merged commit b6aca66 into helium:master Aug 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants