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 inactivity and inclusion_delay fields to Attestation Rewards #340

Merged

Conversation

jimmygchen
Copy link
Contributor

This PR adds the following fields to the AttestationsRewards response:

  1. A inactivity field to both ideal_rewards and total_rewards.
  2. A inclusion_delay field to ideal_rewards

inactivity

Currently the inactivity penalties are not part of the AttestationsRewards response, and is something users of this endpoint (e.g. explorers) have expressed interests in.

I thought about whether it makes sense to combine them into the current source, target and head fields, however I think there are a few reasons to keep them in a separate field:

  • The extra penalty for missing attestations or being slashed during an inactivity leak doesn't really belong to any of those fields
  • IMO it's more consistent with how they are grouped in the consensus spec (altair spec, phase0 spec). There's also no "cancelling" of rewards from Altair, and inactivity penalties are more separate.

ideal_rewards.inclusion_delay

inclusion_delay is part of the individual rewards, but not available under ideal_rewards. This would be useful to indicate the ideal inclusion_delay reward for each effective balance step.

@rolfyone
Copy link
Collaborator

ideal_rewards.inclusion_delay

inclusion_delay is part of the individual rewards, but not available under ideal_rewards. This would be useful to indicate the ideal inclusion_delay reward for each effective balance step.

Isn't this just going to be a constant?

@jimmygchen
Copy link
Contributor Author

Isn't this just going to be a constant?

If I'm not mistaken, (Phase 0) base reward is proportional to effective balance (hence different for each effective balance increment, 32 eth, 31 eth ..etc) and it also changes over time because of total_active_balance, so not really a constant AFAIK.

Here's an example json response snippet taken from Mainnet (running Lighthouse)

{
  "execution_optimistic": false,
  "data": {
    "ideal_rewards": [
      {
        "effective_balance": "1000000000",
        "head": "585",
        "target": "586",
        "source": "586",
        "inclusion_delay": "534",
        "inactivity": "0"
      },
      ...
      {
        "effective_balance": "32000000000",
        "head": "18729",
        "target": "18770",
        "source": "18771",
        "inclusion_delay": "17083",
        "inactivity": "0"
      }
    ],
    "total_rewards": [
      {
        "validator_index": "0",
        "head": "18729",
        "target": "18770",
        "source": "18771",
        "inclusion_delay": "17083",
        "inactivity": "0"
      }
    ]
  }
}

@rolfyone
Copy link
Collaborator

Ok that makes sense, I hadn't looked at the definitions... Looks like a good addition. Although I think some implementations (teku for example) only support from altair, the base_reward is still computed so makes sense.
The concrete example is a good reference - thanks for adding that!

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

2 participants