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.

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

Attnet revamp: Subnet backbone structure based on beacon nodes #3312

Merged
merged 7 commits into from
May 10, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
33 changes: 24 additions & 9 deletions specs/phase0/validator.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,12 @@ All terminology, constants, functions, and protocol mechanics defined in the [Ph

| Name | Value | Unit | Duration |
| - | - | :-: | :-: |
| `TARGET_AGGREGATORS_PER_COMMITTEE` | `2**4` (= 16) | validators | |
| `RANDOM_SUBNETS_PER_VALIDATOR` | `2**0` (= 1) | subnets | |
| `EPOCHS_PER_RANDOM_SUBNET_SUBSCRIPTION` | `2**8` (= 256) | epochs | ~27 hours |
| `TARGET_AGGREGATORS_PER_COMMITTEE` | `2**4` (= 16) | validators |
| `EPOCHS_PER_SUBNET_SUBSCRIPTION` | `2**8` (= 256) | epochs | ~27 hours |
| `ATTESTATION_SUBNET_COUNT` | `64` | The number of attestation subnets used in the gossipsub protocol. |
| `ATTESTATION_SUBNET_EXTRA_BITS` | `0` | The number of extra bits of a NodeId to use when mapping to a subscribed subnet |
| `SUBNETS_PER_NODE` | `2` | The number of long-lived subnets a beacon node should be subscribed to. |
| `ATTESTATION_SUBNET_PREFIX_BITS` | `(ceillog2(ATTESTATION_SUBNET_COUNT) + ATTESTATION_SUBNET_EXTRA_BITS)` | |

## Containers

Expand Down Expand Up @@ -606,15 +608,28 @@ def get_aggregate_and_proof_signature(state: BeaconState,

## Phase 0 attestation subnet stability

Because Phase 0 does not have shards and thus does not have Shard Committees, there is no stable backbone to the attestation subnets (`beacon_attestation_{subnet_id}`). To provide this stability, each validator must:
Because Phase 0 does not have shards and thus does not have Shard Committees, there is no stable backbone to the attestation subnets (`beacon_attestation_{subnet_id}`). To provide this stability, each beacon node should:

* Randomly select and remain subscribed to `RANDOM_SUBNETS_PER_VALIDATOR` attestation subnets
* Maintain advertisement of the randomly selected subnets in their node's ENR `attnets` entry by setting the randomly selected `subnet_id` bits to `True` (e.g. `ENR["attnets"][subnet_id] = True`) for all persistent attestation subnets
* Set the lifetime of each random subscription to a random number of epochs between `EPOCHS_PER_RANDOM_SUBNET_SUBSCRIPTION` and `2 * EPOCHS_PER_RANDOM_SUBNET_SUBSCRIPTION]`. At the end of life for a subscription, select a new random subnet, update subnet subscriptions, and publish an updated ENR
* Remain subscribed to `SUBNETS_PER_NODE` for `EPOCHS_PER_SUBNET_SUBSCRIPTION` epochs.
* Maintain advertisement of the selected subnets in their node's ENR `attnets` entry by setting the selected `subnet_id` bits to `True` (e.g. `ENR["attnets"][subnet_id] = True`) for all persistent attestation subnets.
* Select these subnets based on their node-id as specified by the following `compute_subnets(node_id,epoch)` function.

*Note*: Short lived beacon committee assignments should not be added in into the ENR `attnets` entry.
```python
def compute_subnet(node_id: int, epoch: Epoch, index: int) -> int:
node_id_prefix = node_id >> (256 - ATTESTATION_SUBNET_PREFIX_BITS)
permutation_seed = hash(uint_to_bytes(epoch // EPOCHS_PER_SUBNET_SUBSCRIPTION))
permutated_prefix = compute_shuffled_index(node_id_prefix, 1 << ATTESTATION_SUBNET_PREFIX_BITS, permutation_seed)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be worth putting an historic randao mix (that can be retrieved from state) in here?

It would define how long before a node_id can be grinded to do some attack.

To not run into issues with potential trailing finality windows, you'd probably need this to be on the order of a month or more. which then degrades the utility

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm happy to add this complexity in if we feel it warrants it. We had a small discussion on this in the issue.

My current thinking is that if someone wants to attack a subnet in this manner now, its pretty easy to do without having to generate node-ids. You could just make a bunch of nodes and set your ENR field to a subnet (I realize that the argument that the vulnerability currently exists is not a good argument to not fix it 😅 )

In the new form, I guess if we modify our discovery to start searching for these peer-ids, maybe there is a greater chance of being eclipsed as we are in effect splitting the DHT into smaller sub-DHTs. I think the security guarantees are the same, in that if there are some honest nodes discovery should find them also.

Mixing in RANDAO then ties in fork-choice (as you've pointed out). If it's too new and peers are temporarily on different forks, I'd imagine we'd start penalizing our peers for not being on the right subnets etc.

Do you have a suggestion for mixing in the RANDO. Like maybe current_epoch - 20 or something?

Copy link
Contributor

Choose a reason for hiding this comment

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

The problem with using anything moderately recent is that a chain split past that depth (non-finality period) would segment the p2p network because people (from your perspective) wouldn't be on the correct subnets and yuo would downscore. In practice, if there is actually a split, there is likely some sort of partition anyway.

But it does begin to seem dangerous for recent windows

Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I agree.

My thoughts on this is to leave it deterministic for the time being. Perhaps we could add it in in a later PR. I imagine if we decide to set ATTESTATION_SUBNET_EXTRA_BITS to something non-zero, we could add it in then with that change.

Copy link
Member

Choose a reason for hiding this comment

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

Do you have a suggestion for mixing in the RANDO. Like maybe current_epoch - 20 or something?

The number 20 doesn't have to be fixed. I think using the randao mix of the finalized_checkpoint of the head block is the best idea now, since it's unlikely to be reverted.

return (permutated_prefix + index) % ATTESTATION_SUBNET_COUNT
```

```python
def compute_subnets(node_id: int, epoch: Epoch) -> Sequence[int]:
return [compute_subnet(node_id, epoch, idx) for idx in range(SUBNETS_PER_NODE)]
```

*Note*: Nodes should subscribe to new subnets and remain subscribed to old subnets for at least one epoch. Nodes should pick a random duration to unsubscribe from old subnets to smooth the transition on the exact epoch boundary of which the shuffling changes.
AgeManning marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer to stagger subnet subscriptions to avoid the network churning all at once (and to avoid the issue of having to double-up on subnets for some amount of time).

If the node-id dictated the epoch to churn -- essentially defining a (256, 512] period in which you keep your subscription, then we'd not have the churn all at once

Copy link
Contributor

Choose a reason for hiding this comment

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

The random local offset could work too if it's spaced enough but then we lose the determinism (and penalization) in that period

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, seems this is an active convo on the pr

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added Jim's suggestion for now.


*Note*: When preparing for a hard fork, a validator must select and subscribe to random subnets of the future fork versioning at least `EPOCHS_PER_RANDOM_SUBNET_SUBSCRIPTION` epochs in advance of the fork. These new subnets for the fork are maintained in addition to those for the current fork until the fork occurs. After the fork occurs, let the subnets from the previous fork reach the end of life with no replacements.
*Note*: When preparing for a hard fork, a validator must select and subscribe to subnets of the future fork versioning at least `EPOCHS_PER_SUBNET_SUBSCRIPTION` epochs in advance of the fork. These new subnets for the fork are maintained in addition to those for the current fork until the fork occurs. After the fork occurs, let the subnets from the previous fork reach the end of life with no replacements.

## How to avoid slashing

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def test_time(spec, state):
@with_all_phases
@spec_state_test
def test_networking(spec, state):
assert spec.RANDOM_SUBNETS_PER_VALIDATOR <= spec.ATTESTATION_SUBNET_COUNT
assert spec.SUBNETS_PER_NODE <= spec.ATTESTATION_SUBNET_COUNT


@with_all_phases
Expand Down