feat: implement /eth/v1/validator/sync_committee_subscriptions endpoi…

[bomanaps]

What are you trying to achieve?

closes #236

How was it implemented/fixed?

This pull request adds a handler for the Beacon API endpoint POST /eth/v1/validator/sync_committee_subscriptions.
Run node:

cargo run --bin ream node 

Make post request

curl -X POST http://localhost:5052/eth/v1/validator/sync_committee_subscriptions \
  -H "Content-Type: application/json" \
  -d '[
    {
      "validator_index": 1234,
      "sync_committee_indices": [0, 1, 2],
      "until_epoch": 100
    }
  ]'

To-Do

• I have read CONTRIBUTING.md.
• This PR uses conventional commits.

[bomanaps]

Hello @KolbyML please can you help me review this? thanks.

[KolbyML]

Hello @KolbyML please can you help me review this? thanks.

Hi @bomanaps this PR isn't updating the subnets we are subscribed to in our Enr

[bomanaps]

this PR isn't updating the subnets we are subscribed to in our Enr

Yes am currently stuck on that and I need your input because from our current implementation of update_sync_committee_subnets is incomplete because the Discv5 struct takes ownership of the CombinedKey and doesn't expose it, which is making it impossible to update the ENR without either storing a duplicate key (with performance and security implications) or modifying the discv5 crate to expose the key or provide an ENR update method, or maybe am missing something?

[bomanaps]

this PR isn't updating the subnets we are subscribed to in our Enr

Yes am currently stuck on that and I need your input because from our current implementation of update_sync_committee_subnets is incomplete because the Discv5 struct takes ownership of the CombinedKey and doesn't expose it, which is making it impossible to update the ENR without either storing a duplicate key (with performance and security implications) or modifying the discv5 crate to expose the key or provide an ENR update method, or maybe am missing something?

If I keep a copy of the CombinedKey in the Discovery struct it will require CombinedKey to implement Clone, which it doesn't

[KolbyML]

this PR isn't updating the subnets we are subscribed to in our Enr

Yes am currently stuck on that and I need your input because from our current implementation of update_sync_committee_subnets is incomplete because the Discv5 struct takes ownership of the CombinedKey and doesn't expose it, which is making it impossible to update the ENR without either storing a duplicate key (with performance and security implications) or modifying the discv5 crate to expose the key or provide an ENR update method, or maybe am missing something?

You can modify the Enr like this https://docs.rs/discv5/0.9.1/discv5/struct.Discv5.html#method.enr_insert

[bomanaps]

Hello @KolbyML please can you help me review this once more, thanks

[KolbyML]

@bomanaps can you rebase to fix the conflicts and make CI pass first

[bomanaps]

can you rebase to fix the conflicts and make CI pass first

Done

[KolbyML]

Left some directional feedback

[KolbyML]

@bomanaps can you rebase the PR, I should be able to give it another look when I wake up tomorrow

[bomanaps]

@bomanaps can you rebase the PR, I should be able to give it another look when I wake up tomorrow

Done

[KolbyML]

Left some high level feedback, feel free to ask questions

[KolbyML]

Left some feedback

[KolbyML]

A lot of the PR looks improved, there are a few things I need to read more deeply on my next review

[KolbyML]

@bomanaps could you squash all the commits so CI passes

[bomanaps]

@bomanaps could you squash all the commits so CI passes

Done