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

Adds operator ejections dataapi endpoint #810

Merged
merged 12 commits into from
Oct 16, 2024
Merged

Conversation

pschork
Copy link
Contributor

@pschork pschork commented Oct 16, 2024

  • Default look back is 1 day.
  • Max lookback is 90 days
  • Optional operator_id can be specified to limit ejection query to specific operator.
curl -X 'GET' \
  'https://dataapi-preprod-holesky.eigenda.xyz/api/v1/operators-info/operator-ejections?days=90' \
  -H 'accept: application/json'
{
  "ejections": [
    {
      "operator_id": "0x45fc52054b0b4b95f9188959eac542abf08537b62575fb0c89a8e42a77a93497",
      "quorum": 1,
      "block_number": 1956080,
      "block_timestamp": "2024-07-19T01:48:48Z",
      "transaction_hash": "0x3d0e493e09e960a8d373fdb904ab7cc6ece5d7f7385602a791e14aa44b2fed2e"
    },
    {
      "operator_id": "0x45fc52054b0b4b95f9188959eac542abf08537b62575fb0c89a8e42a77a93497",
      "quorum": 2,
      "block_number": 1956080,
      "block_timestamp": "2024-07-19T01:48:48Z",
      "transaction_hash": "0x3d0e493e09e960a8d373fdb904ab7cc6ece5d7f7385602a791e14aa44b2fed2e"
    },
    {
      "operator_id": "0xf512db9a07eefc22336c0de9f0dd8cb71400d0718e9ed00cb4af2643dc64325e",
      "quorum": 1,
      "block_number": 2126464,
      "block_timestamp": "2024-08-13T18:47:12Z",
      "transaction_hash": "0x23ba5240a21cb7e56a8deb971833d188ff5e30562bed5fb70e44b2a624809367"
    },
    {
      "operator_id": "0x45fc52054b0b4b95f9188959eac542abf08537b62575fb0c89a8e42a77a93497",
      "quorum": 1,
      "block_number": 2290044,
      "block_timestamp": "2024-09-07T18:47:12Z",
      "transaction_hash": "0x72044cb9bd5a05c5de466d698ffeff475d2e2e2df104e73b67bfac72bcc7814a"
    },
    {
      "operator_id": "0x45fc52054b0b4b95f9188959eac542abf08537b62575fb0c89a8e42a77a93497",
      "quorum": 2,
      "block_number": 2290044,
      "block_timestamp": "2024-09-07T18:47:12Z",
      "transaction_hash": "0x72044cb9bd5a05c5de466d698ffeff475d2e2e2df104e73b67bfac72bcc7814a"
    },
    {
      "operator_id": "0x45fc52054b0b4b95f9188959eac542abf08537b62575fb0c89a8e42a77a93497",
      "quorum": 1,
      "block_number": 2329091,
      "block_timestamp": "2024-09-13T18:47:24Z",
      "transaction_hash": "0x987fcdd225d72f2f22f9ae4eb32eaeb2f52dc301e585e7c1c503382c8c0e07cd"
    },
    {
      "operator_id": "0x45fc52054b0b4b95f9188959eac542abf08537b62575fb0c89a8e42a77a93497",
      "quorum": 2,
      "block_number": 2329091,
      "block_timestamp": "2024-09-13T18:47:24Z",
      "transaction_hash": "0x987fcdd225d72f2f22f9ae4eb32eaeb2f52dc301e585e7c1c503382c8c0e07cd"
    }
  ]
}

Checks

  • I've made sure the lint is passing in this PR.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, in that case, please comment that they are not relevant.
  • I've checked the new test coverage and the coverage percentage didn't drop.
  • Testing Strategy
    • Unit tests
    • Integration tests
    • This PR is not tested :(

@pschork pschork force-pushed the pschork/dataapi_ejections branch from 97a2e2d to 9b8a1d7 Compare October 16, 2024 05:07
@pschork pschork force-pushed the pschork/dataapi_ejections branch from 3591ccf to 3dfbef9 Compare October 16, 2024 07:41
@pschork pschork force-pushed the pschork/dataapi_ejections branch from b273283 to 5a7683d Compare October 16, 2024 08:14
@pschork pschork marked this pull request as ready for review October 16, 2024 08:18
Copy link
Contributor

@jianoaix jianoaix left a comment

Choose a reason for hiding this comment

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

It will be useful to also return whether the ejection hit rate limiting which is logged as well cc @0x0aa0

// @Summary Fetch list of operator ejections over last N days.
// @Tags OperatorsInfo
// @Produce json
// @Param days query int false "Lookback in days [default: 1]"
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be more helpful passing hours

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 think days is preferable as it doesn't require math to query weekly/monthey/quarterly ejections and the typical cadence of ejections is <= 1 per day (mainnet had 3 ejections over last 7 days. testnet had 6 ejections over last 7 days).

Copy link
Contributor

Choose a reason for hiding this comment

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

It can be any number of times per day. And the query can be interested in a number of hours.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whats the use case where you only want ejections in the last hour, but don't want ejections from the previous 24hr, and you are unable to filter results by block timestamp?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's all possible, but just adding friction

disperser/dataapi/server.go Outdated Show resolved Hide resolved
disperser/dataapi/subgraph_client.go Outdated Show resolved Hide resolved
disperser/dataapi/subgraph/api.go Show resolved Hide resolved
@pschork pschork merged commit abadc9b into master Oct 16, 2024
8 checks passed
// @Produce json
// @Param days query int false "Lookback in days [default: 1]"
// @Param operator_id query string false "Operator ID filter [default: all operators]"
// @Param first query int false "Return first N ejections [default: 1000]"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure we want to expose these two at API, they can be internal controls

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason subgraph has these is to force pagination on the caller to limit response from getting too big. I think we should pass pagination onto our callers as well. We shouldn't do pagination for them and thus return a huge response payload.
I considered setting internal limits and not exposing these, but it seems like we should allow people to find every ejection that has ever occurred - in which case we should just provide them the tools to do it.

Copy link
Contributor

Choose a reason for hiding this comment

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

This deviates from the existing API norms and creates inconsistent API experience.
Adding more parameters also complicates things, even this implementation of pagination is not quite correct (a second call will is not strictly relative to the first one). I would prefer a simpler semantics with a caveat that it'll provide at most MIN{N, pastNumHoursWindow} items.

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.

3 participants