feat: Implement attestation subnet subscription #616
Conversation
| /// Simple shuffling function based on Ethereum's shuffle algorithm | ||
| fn compute_shuffled_index(index: usize, index_count: usize, seed: [u8; 32]) -> usize { | ||
| assert!(index < index_count, "Index out of bounds"); | ||
| let mut indices = (0..index_count).collect::<Vec<_>>(); | ||
| let mut i = 0; | ||
| while i < index_count { | ||
| let pivot = { | ||
| let mut hasher = Sha256::new(); | ||
| hasher.update(seed); | ||
| hasher.update((i as u32).to_le_bytes()); | ||
| let hash = hasher.finalize(); | ||
| u64::from_le_bytes(hash[..8].try_into().unwrap()) as usize % (index_count - i) | ||
| }; | ||
| indices.swap(i, i + pivot); | ||
| i += 1; | ||
| } | ||
| indices[index] | ||
| } |
There was a problem hiding this comment.
This function has already been implemented please remove this compute_shuffled_index
There was a problem hiding this comment.
okay, that's great, thanks for pointing this, i will remove this implementation and use existing one
There was a problem hiding this comment.
done , removed the function and using the existing implementation now
Addressed the comments and left responses , review requested |
| // Compute and set attestation subnets | ||
| let mut config = config.clone(); | ||
| // Define the get_current_epoch function | ||
| let current_epoch = compute_epoch_at_slot(current_slot); |
There was a problem hiding this comment.
done, made it inline , i hope i understood correctly , code is inlined as :
let subnets =
compute_subscribed_subnets(enr.node_id(), compute_epoch_at_slot(current_slot))?;
Addressed the review comments |
| // Compute and set attestation subnets | ||
| let subnets = | ||
| compute_subscribed_subnets(enr.node_id(), compute_epoch_at_slot(current_slot))?; | ||
| let mut config = config.clone(); | ||
| config.attestation_subnets = AttestationSubnets::new(); | ||
| for subnet_id in subnets { | ||
| config | ||
| .attestation_subnets | ||
| .enable_attestation_subnet(subnet_id)?; | ||
| } |
There was a problem hiding this comment.
this code is currently not doing anything. could you make a function called update_attestation_subnets, which takes in a slot and updates the nodes enr, instead of doing this in the new() function
There was a problem hiding this comment.
yeah, update function seems required here, i didn't use it as i remember from my last PR i removed update function , Should i call it from poll function or it will be called from consensus when the epoch changes
| @@ -137,6 +145,38 @@ impl Decodable for SyncCommitteeSubnets { | |||
| } | |||
| } | |||
|
|
|||
| /// Compute a single subscribed subnet based on node_id, epoch, and index | |||
| pub fn compute_subscribed_subnet(node_id: NodeId, epoch: u64, index: usize) -> anyhow::Result<u8> { | |||
There was a problem hiding this comment.
| pub fn compute_subscribed_subnet(node_id: NodeId, epoch: u64, index: usize) -> anyhow::Result<u8> { | |
| pub fn compute_subscribed_subnet(node_id: NodeId, epoch: u64, index: usize) -> anyhow::Result<u64> { |
SubnetID is supposed to be a u64 not a u8
| // Extract prefix from first 8 bytes of node_id | ||
| let mut node_id_prefix_bytes = [0u8; 8]; | ||
| node_id_prefix_bytes.copy_from_slice(&node_id.raw()[..8]); | ||
| let node_id_prefix = | ||
| u64::from_be_bytes(node_id_prefix_bytes) >> (64 - ATTESTATION_SUBNET_PREFIX_BITS); |
There was a problem hiding this comment.
| // Extract prefix from first 8 bytes of node_id | |
| let mut node_id_prefix_bytes = [0u8; 8]; | |
| node_id_prefix_bytes.copy_from_slice(&node_id.raw()[..8]); | |
| let node_id_prefix = | |
| u64::from_be_bytes(node_id_prefix_bytes) >> (64 - ATTESTATION_SUBNET_PREFIX_BITS); | |
| // Extract prefix from first 8 bytes of node_id | |
| let mut node_id_prefix_bytes = [0u8; 8]; | |
| node_id_prefix_bytes.copy_from_slice(&node_id.raw()[..8]); | |
| let node_id_prefix = | |
| u64::from_be_bytes(node_id_prefix_bytes) >> (NODE_ID_BITS - ATTESTATION_SUBNET_PREFIX_BITS); |
we should be using B256 here no? instead of of converting this to a u64
| let mut node_offset_bytes = [0u8; 8]; | ||
| node_offset_bytes.copy_from_slice(&node_id.raw()[24..32]); | ||
| let node_offset = u64::from_be_bytes(node_offset_bytes) % EPOCHS_PER_SUBNET_SUBSCRIPTION; |
There was a problem hiding this comment.
| let mut node_offset_bytes = [0u8; 8]; | |
| node_offset_bytes.copy_from_slice(&node_id.raw()[24..32]); | |
| let node_offset = u64::from_be_bytes(node_offset_bytes) % EPOCHS_PER_SUBNET_SUBSCRIPTION; | |
| let mut node_offset_bytes = [0u8; 8]; | |
| node_offset_bytes.copy_from_slice(&node_id.raw()[24..32]); | |
| let node_offset = u64::from_be_bytes(node_offset_bytes) % EPOCHS_PER_SUBNET_SUBSCRIPTION; |
same comment here we should be doing B256 % B256, NodeId is a B256
| // Subscription constants | ||
| const SUBNETS_PER_NODE: usize = 2; | ||
| pub const EPOCHS_PER_SUBNET_SUBSCRIPTION: u64 = 256; | ||
| const ATTESTATION_SUBNET_PREFIX_BITS: u32 = 8; |
There was a problem hiding this comment.
shouldn't this be 6? ATTESTATION_SUBNET_EXTRA_BITS = 0
ceillog2(ATTESTATION_SUBNET_COUNT) = 6
| /// Compute a single subscribed subnet based on node_id, epoch, and index | ||
| pub fn compute_subscribed_subnet(node_id: NodeId, epoch: u64, index: usize) -> anyhow::Result<u8> { | ||
| // Extract prefix from first 8 bytes of node_id | ||
| let mut node_id_prefix_bytes = [0u8; 8]; | ||
| node_id_prefix_bytes.copy_from_slice(&node_id.raw()[..8]); | ||
| let node_id_prefix = | ||
| u64::from_be_bytes(node_id_prefix_bytes) >> (64 - ATTESTATION_SUBNET_PREFIX_BITS); | ||
|
|
||
| let mut node_offset_bytes = [0u8; 8]; | ||
| node_offset_bytes.copy_from_slice(&node_id.raw()[24..32]); | ||
| let node_offset = u64::from_be_bytes(node_offset_bytes) % EPOCHS_PER_SUBNET_SUBSCRIPTION; | ||
|
|
||
| let permutation_seed = | ||
| Sha256::digest(((epoch + node_offset) / EPOCHS_PER_SUBNET_SUBSCRIPTION).to_le_bytes()); | ||
|
|
||
| let permutated_prefix = compute_shuffled_index( | ||
| node_id_prefix as usize, | ||
| 1 << ATTESTATION_SUBNET_PREFIX_BITS, | ||
| B256::from_slice(permutation_seed.as_slice()), | ||
| )?; | ||
| Ok(((permutated_prefix + index) % ATTESTATION_SUBNET_COUNT) as u8) | ||
| } |
There was a problem hiding this comment.
anyways let me know about what I said above, I might be missing a few insights let me know
What are you trying to achieve?
Closes #247
How was it implemented/fixed?
Implemented the attestation subnet subscription as per spec => https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/p2p-interface.md#attestation-subnet-subscription.
To-Do