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

exporter: decideds endpoint; fix signers; fix resolving role; cache domain; fix convert #1766

Merged
merged 98 commits into from
Oct 30, 2024

Conversation

olegshmuelov
Copy link
Contributor

@olegshmuelov olegshmuelov commented Sep 30, 2024

Changes to Exporter:

  • port feat: (API) decided instances endpoint #1632
    • add decided instances endpoint (GET /v1/exporter/decideds)
    • add X-SSV-Node-Version header
  • fix role type casting when using the convert package
  • allow saving partial signatures of length not equal to 3f+1
  • cache domain requests to beacon node within one epoch
  • make resolving beacon role type by roots work
  • fix wrong participants rendering by merging committees on new quorum instead of overwriting

@olegshmuelov olegshmuelov self-assigned this Sep 30, 2024
@olegshmuelov olegshmuelov marked this pull request as ready for review September 30, 2024 15:20
@@ -16,7 +16,7 @@ var HoleskyStage = NetworkConfig{
GenesisEpoch: 1,
RegistrySyncOffset: new(big.Int).SetInt64(84599),
RegistryContractAddr: "0x0d33801785340072C452b994496B19f196b7eE15",
AlanForkEpoch: 99999999,
//AlanForkEpoch: 99999999,
Copy link
Contributor

Choose a reason for hiding this comment

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

lets not merge this

Copy link
Contributor

Choose a reason for hiding this comment

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

reverted

api/handlers/exporter.go Show resolved Hide resolved
api/handlers/exporter.go Show resolved Hide resolved
api/handlers/exporter.go Outdated Show resolved Hide resolved
api/handlers/exporter.go Outdated Show resolved Hide resolved
api/handlers/exporter.go Outdated Show resolved Hide resolved
api/handlers/exporter.go Outdated Show resolved Hide resolved
api/handlers/exporter.go Outdated Show resolved Hide resolved
@@ -16,8 +16,8 @@ var HoleskyStage = NetworkConfig{
GenesisEpoch: 1,
RegistrySyncOffset: new(big.Int).SetInt64(84599),
RegistryContractAddr: "0x0d33801785340072C452b994496B19f196b7eE15",
AlanForkEpoch: 999999999,
DiscoveryProtocolID: [6]byte{'s', 's', 'v', 'd', 'v', '5'},
//AlanForkEpoch: 999999999,
Copy link
Contributor

Choose a reason for hiding this comment

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

should not be in this PR even though stage is going to be post-fork from now..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should not be in this PR even though stage is going to be post-fork from now..

yes, should be reverted once argo sensor is deleted

Copy link
Contributor

Choose a reason for hiding this comment

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

next time just please create a separate deploy branch to keep the main PR clean.

Copy link
Contributor

Choose a reason for hiding this comment

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

reverted

api/handlers/exporter.go Outdated Show resolved Hide resolved
Copy link
Contributor

@nkryuchkov nkryuchkov left a comment

Choose a reason for hiding this comment

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

Requesting changes not to forget to revert AlanForkEpoch once it's tested


type DomainCache struct {
beaconNode beacon.BeaconNode
cache *ttlcache.Cache[domainCacheKey, phase0.Domain]
Copy link
Contributor

Choose a reason for hiding this comment

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

seems to be threadsafe from looking at the code

"github.com/ssvlabs/ssv/protocol/v2/blockchain/beacon"
)

type DomainCache struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

nice addition, we should probably use this in the actual runners and not just exporter! of course in a different PR.

Copy link
Contributor

@y0sher y0sher left a comment

Choose a reason for hiding this comment

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

@olegshmuelov @nkryuchkov
Good job on figuring out all the quirks here! this is a big transition in exporter.

@y0sher y0sher changed the title Exporter Api (Alan post-fork) feat: exporter http api and bug fixes Oct 30, 2024
@nkryuchkov nkryuchkov changed the title feat: exporter http api and bug fixes exporter: add decideds endpoint; fix signers; fix resolving role; cache domain; fix convert Oct 30, 2024
@nkryuchkov nkryuchkov changed the title exporter: add decideds endpoint; fix signers; fix resolving role; cache domain; fix convert exporter: decideds endpoint; fix signers; fix resolving role; cache domain; fix convert Oct 30, 2024
Copy link
Contributor

@moshe-blox moshe-blox left a comment

Choose a reason for hiding this comment

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

LGTM

well done @olegshmuelov @nkryuchkov @y0sher 💪

let's not forget to let @andrew-blox know that this should be QA'd before release (this will probably go in v2.0.2 after the fork, except for private early release to exporter operators)

@nkryuchkov nkryuchkov merged commit a020f5a into stage Oct 30, 2024
6 checks passed
@nkryuchkov nkryuchkov deleted the exporter-api-alan-post-fork branch October 30, 2024 15:38
@olegshmuelov olegshmuelov restored the exporter-api-alan-post-fork branch October 30, 2024 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants