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

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

Merged
merged 7 commits into from
May 10, 2023
Merged

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

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7cb1630
Attnet revamp draft
AgeManning Mar 30, 2023
0dd8db7
Make linter happy. Add `SUBNET_DURATION_IN_EPOCHS` definition.
hwwhww Mar 30, 2023
a0d0337
Correct subnet subscription duration variable
AgeManning Apr 6, 2023
6e423f6
Stagger node rotations
AgeManning Apr 12, 2023
745d529
Add `compute_subscribed_subnets` unittests and fix typing errors
hwwhww Apr 25, 2023
79b8a9a
Apply suggestions from code review
hwwhww May 4, 2023
5cb2733
Add custom types `NodeID` and `SubnetID` and constant `NODE_ID_BITS`
hwwhww May 4, 2023
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
13 changes: 6 additions & 7 deletions specs/phase0/validator.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -612,23 +612,22 @@ Because Phase 0 does not have shards and thus does not have Shard Committees, th

* 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.
* Select these subnets based on their node-id as specified by the following `compute_subscribed_subnets(node_id,epoch)` function.
hwwhww marked this conversation as resolved.
Show resolved Hide resolved

```python
def compute_subnet(node_id: int, epoch: Epoch, index: int) -> int:
def compute_subscribed_subnet(node_id: int, epoch: Epoch, index: int) -> int:
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the maximum size of node_id?

Copy link
Member

Choose a reason for hiding this comment

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

I believe it was assumed UInt256. Not sure though what's the most appropriate type for pyspec

node_id_prefix = node_id >> (256 - ATTESTATION_SUBNET_PREFIX_BITS)
permutation_seed = hash(uint_to_bytes(epoch // EPOCHS_PER_SUBNET_SUBSCRIPTION))
node_offset = node_id % EPOCHS_PER_SUBNET_SUBSCRIPTION
AgeManning marked this conversation as resolved.
Show resolved Hide resolved
permutation_seed = hash(uint_to_bytes((epoch + node_offset) // 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)]
def compute_subscribed_subnets(node_id: int, epoch: Epoch) -> Sequence[int]:
return [compute_subscribed_subnet(node_id, epoch, idx) for idx in range(SUBNETS_PER_NODE)]
hwwhww marked this conversation as resolved.
Show resolved Hide resolved
```

*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.

*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