Skip to content
This repository has been archived by the owner on Jun 12, 2023. It is now read-only.

Confirm there are GRPC subscribers before generating PoC keys in heartbeat #1835

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

PaulVMo
Copy link
Contributor

@PaulVMo PaulVMo commented Oct 24, 2022

This change is indented to prevent poorly configured validators from proposing PoC challenges if they will not be able to receive receipts for those challenges due to their gRPC port being inaccessible. This will enable more of the selected PoC to succeed as validators with the GRPC port blocked will never generate challenges in the first place.

The idea is for the validator to check if it has any GRPC subscribers (i.e. connected hotspots) and only if they do, generate ephemeral PoC keys for including in heartbeat txns. I do this by looking at whether there are any members in the process group for the notifications that hotspots subscribe to. I am open to other ideas for how to do this check. I was not familiar enough with Sibyl and GRPCBOX to figure out if there was an easier/better way for getting a count of connections, streams, or sessions from one of them.

Validators will still heartbeat and will still be eligible for the consensus if gRPC is not open. However, they will miss out on PoC reward and no longer contribute to a lower PoC completion rate.

Note, this is a "self-policing" type approach. Someone could workaround this check by modifying the code and there is no consensus mechanism to prevent this. However, there is precedent for this. The check that the validator is not relayed before generating a heartbeat is in the same situation. However, that has shown to be quite effective because most inaccessible ports are the result of misconfiguration rather than malicious intent.

DEPENDENT ON: helium/sibyl#75

@PaulVMo PaulVMo force-pushed the grpc-check-for-poc branch 2 times, most recently from 938bf9c to d05e0d8 Compare October 24, 2022 20:24
Copy link
Contributor

@andymck andymck left a comment

Choose a reason for hiding this comment

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

I think this is fine as long as the limitations are understood.

@PaulVMo
Copy link
Contributor Author

PaulVMo commented Oct 29, 2022

This may not work, at least not how I imaged. The key proposal length is validated to be exactly the target length, versus <= as I had assumed. So this would cause the heartbeat to fail entirely. That may be a valid behavior although different than what I was originally envisioning / proposed, and a bit more extreme of a penalty for not have GRPC open.

https:/helium/blockchain-core/blob/a34308a576a40594d5057d29570f419516dc87b1/src/transactions/v1/blockchain_txn_validator_heartbeat_v1.erl#L171

@PaulVMo PaulVMo marked this pull request as draft October 29, 2022 13:28
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.

2 participants