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

Add endpoints for block and attestation rewards #260

Merged
merged 26 commits into from Nov 22, 2022

Conversation

kevinbogner
Copy link
Contributor

@kevinbogner kevinbogner commented Nov 7, 2022

Issue: sigp/lighthouse#3661

Currently, there is no way to get block rewards paid on the consensus side. Therefore, we propose two endpoints.

/eth/v1/beacon/rewards/blocks/{block_id}

Get block rewards

{
  "execution_optimistic": false,
  "finalized": false,
  "data": {
    "proposer_index": "123",
    "total": "123",
    "attestations": "123",
    "sync_aggregate": "123",
    "proposer_slashings": "123",
    "attester_slashings": "123"
  }
}

/eth/v1/beacon/rewards/attestations/{epoch}

Get attestation rewards

{
  "execution_optimistic": false,
  "finalized": false,
  "data": [
    {
      "ideal_rewards": [
        {
          "effective_balance": "1000000000",
          "head": "2500",
          "target": "5000",
          "source": "5000"
        }
      ],
      "total_rewards": [
        {
          "validator_index": "0",
          "head": "2000",
          "target": "2000",
          "source": "4000",
          "inclusion_delay": "2000"
        }
      ]
    }
  ]
}

Copy link
Collaborator

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

This is awesome! Thanks @naviechan & @kevinbogner!

In addition to the minor edits I've proposed the main thing missing is an API for the sync committee rewards. I didn't describe one in my original messages, but it should be similar to the others, maybe something like:

/eth/v1/beacon/blocks/{block_id}/sync_committee_rewards?id=...
{
    "data": [
        {
            "validator_index": "0",
            "reward": "-1000",
        },
        {
            "validator_index": "1",
            "reward": "1000",
        },
    ]
}

The function that defines these rewards is here: https://github.com/ethereum/consensus-specs/blob/dev/specs/altair/beacon-chain.md#sync-aggregate-processing

I'd be open to speccing that endpoint in a separate PR if you like! Maybe one of you could take that on while the other updates the existing endpoints

apis/beacon/blocks/rewards.yaml Outdated Show resolved Hide resolved
apis/beacon/blocks/rewards.yaml Outdated Show resolved Hide resolved
types/rewards.yaml Outdated Show resolved Hide resolved
types/rewards.yaml Outdated Show resolved Hide resolved
types/rewards.yaml Outdated Show resolved Hide resolved
apis/beacon/attestations/rewards.yaml Outdated Show resolved Hide resolved
types/rewards.yaml Outdated Show resolved Hide resolved
apis/beacon/attestations/rewards.yaml Outdated Show resolved Hide resolved
apis/beacon/attestations/rewards.yaml Outdated Show resolved Hide resolved
apis/beacon/blocks/rewards.yaml Outdated Show resolved Hide resolved
types/rewards.yaml Outdated Show resolved Hide resolved
Copy link
Collaborator

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

We're getting super close to being ready to merge. I think this is probably the final round of review from me before we can pass it to the other maintainers for merging!

apis/beacon/attestations/rewards.yaml Outdated Show resolved Hide resolved
apis/beacon/attestations/rewards.yaml Outdated Show resolved Hide resolved
apis/beacon/attestations/rewards.yaml Outdated Show resolved Hide resolved
apis/beacon/blocks/rewards.yaml Outdated Show resolved Hide resolved
apis/beacon/blocks/rewards.yaml Outdated Show resolved Hide resolved
apis/beacon/blocks/rewards.yaml Outdated Show resolved Hide resolved
types/rewards.yaml Outdated Show resolved Hide resolved
@michaelsproul michaelsproul changed the title WIP: Add endpoints for per validator rewards WIP: Add endpoints for block and attestation rewards Nov 14, 2022
kevinbogner and others added 2 commits November 14, 2022 09:56
wording changes from sproul

Co-authored-by: Michael Sproul <micsproul@gmail.com>
@kevinbogner kevinbogner marked this pull request as ready for review November 14, 2022 15:59
@kevinbogner kevinbogner changed the title WIP: Add endpoints for block and attestation rewards Add endpoints for block and attestation rewards Nov 14, 2022
michaelsproul
michaelsproul previously approved these changes Nov 16, 2022
Copy link
Collaborator

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

Ship it!

@rolfyone
Copy link
Collaborator

as a nit, can we updated the endpoints to :
/eth/v1/beacon/rewards/blocks/{block_id}
/eth/v1/beacon/rewards/attestations/{epoch}

@kevinbogner
Copy link
Contributor Author

Thanks @rolfyone
I changed the endpoints and the folder structure. It should look cleaner now

@zah
Copy link

zah commented Nov 16, 2022

In the Nimbus codebase, we have a set of routines which can produce not only the obtained rewards, but also the maximum reward that was possible if your attestations were perfect. This allows the creation of UIs and reports which focus on the deviation from perfect behavior. In other words, if a validator produces a wrong head vote for example (while hitting source and target correctly), this can be seen as an opportunity cost (a negative score) instead of as a positive increase of the balance.

@michaelsproul
Copy link
Collaborator

@zah Great idea! I think all the clients will end up with that data on-hand while computing the attestation rewards, it's just a matter of how to present it.

We could take advantage of the fact that the rewards are fixed per effective balance with something like:

{
    "ideal_rewards": [
        {
            "effective_balance": "1000000000",
            "head": "2500",
            "target": "5000",
            "source": "5000"
        },
        {
            "effective_balance": "2000000000",
            "head": "2500",
            "target": "5000",
            "source": "5000"
        }
    ],
    "rewards": [
        {
            "validator_index": "0",
            "effective_balance": "32000000000",
            "head": "0",
            "target": "5000",
            "source": "5000",
        }
    ],
}

Or maybe a null-able field per reward component?

{
    "rewards": [
        {
            "validator_index": "0",
            "head": "0",
            "ideal_head": "2500",
            "target": "5000",
            "source": "5000",
        },
        {
            "validator_index": "1",
            "head": "0",
            "ideal_head": "2500",
            "target": "-5000",
            "ideal_target": "5000",
            "source": "-5000",
            "ideal_source": "5000",
        }
    ],
}

This has the advantage of being smaller over the wire when the majority of requested validators are behaving optimally, however it introduces more duplication when there is a lot of sub-optimal voting. I think I have a slight preference for the first effective_balance approach.

Another option would be an entirely separate API for the ideal reward information. However I don't think this would really be simpler, and it would likely be less efficient.

types/rewards.yaml Outdated Show resolved Hide resolved
types/rewards.yaml Outdated Show resolved Hide resolved
apis/beacon/rewards/attestations.yaml Outdated Show resolved Hide resolved
Copy link
Collaborator

@rolfyone rolfyone left a comment

Choose a reason for hiding this comment

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

LGTM

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.

None yet

5 participants