From c9354a9d25906d6ddbe6f8d376b65d6651003993 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Fri, 10 Feb 2023 06:19:42 +0000 Subject: [PATCH] Tweaks to reward APIs (#3957) ## Proposed Changes * Return the effective balance in gwei to align with the spec ([ideal attestation rewards](https://ethereum.github.io/beacon-APIs/?urls.primaryName=dev#/Rewards/getAttestationsRewards)). * Use quoted `i64`s for attestation and sync committee rewards. --- .../beacon_chain/src/attestation_rewards.rs | 27 +++++++------- .../src/lighthouse/attestation_rewards.rs | 2 ++ .../src/lighthouse/sync_committee_rewards.rs | 1 + consensus/serde_utils/src/lib.rs | 2 +- consensus/serde_utils/src/quoted_int.rs | 36 ++++++++++++++++--- 5 files changed, 49 insertions(+), 19 deletions(-) diff --git a/beacon_node/beacon_chain/src/attestation_rewards.rs b/beacon_node/beacon_chain/src/attestation_rewards.rs index 3f3994697..a4a661197 100644 --- a/beacon_node/beacon_chain/src/attestation_rewards.rs +++ b/beacon_node/beacon_chain/src/attestation_rewards.rs @@ -72,6 +72,8 @@ impl BeaconChain { BaseRewardPerIncrement::new(total_active_balance, spec)?; for effective_balance_eth in 0..=32 { + let effective_balance = + effective_balance_eth.safe_mul(spec.effective_balance_increment)?; let base_reward = effective_balance_eth.safe_mul(base_reward_per_increment.as_u64())?; @@ -86,9 +88,9 @@ impl BeaconChain { .safe_div(WEIGHT_DENOMINATOR)?; if !state.is_in_inactivity_leak(previous_epoch, spec) { ideal_rewards_hashmap - .insert((flag_index, effective_balance_eth), (ideal_reward, penalty)); + .insert((flag_index, effective_balance), (ideal_reward, penalty)); } else { - ideal_rewards_hashmap.insert((flag_index, effective_balance_eth), (0, penalty)); + ideal_rewards_hashmap.insert((flag_index, effective_balance), (0, penalty)); } } } @@ -119,12 +121,9 @@ impl BeaconChain { if eligible { let effective_balance = state.get_effective_balance(*validator_index)?; - let effective_balance_eth = - effective_balance.safe_div(spec.effective_balance_increment)?; - for flag_index in 0..PARTICIPATION_FLAG_WEIGHTS.len() { let (ideal_reward, penalty) = ideal_rewards_hashmap - .get(&(flag_index, effective_balance_eth)) + .get(&(flag_index, effective_balance)) .ok_or(BeaconChainError::AttestationRewardsError)?; let voted_correctly = participation_cache .get_unslashed_participating_indices(flag_index, previous_epoch) @@ -160,21 +159,21 @@ impl BeaconChain { let mut ideal_rewards: Vec = ideal_rewards_hashmap .iter() .map( - |((flag_index, effective_balance_eth), (ideal_reward, _penalty))| { - (flag_index, effective_balance_eth, ideal_reward) + |((flag_index, effective_balance), (ideal_reward, _penalty))| { + (flag_index, effective_balance, ideal_reward) }, ) .fold( HashMap::new(), - |mut acc, (flag_index, effective_balance_eth, ideal_reward)| { - let entry = acc.entry(*effective_balance_eth as u32).or_insert( - IdealAttestationRewards { - effective_balance: *effective_balance_eth, + |mut acc, (flag_index, &effective_balance, ideal_reward)| { + let entry = acc + .entry(effective_balance) + .or_insert(IdealAttestationRewards { + effective_balance, head: 0, target: 0, source: 0, - }, - ); + }); match *flag_index { TIMELY_SOURCE_FLAG_INDEX => entry.source += ideal_reward, TIMELY_TARGET_FLAG_INDEX => entry.target += ideal_reward, diff --git a/common/eth2/src/lighthouse/attestation_rewards.rs b/common/eth2/src/lighthouse/attestation_rewards.rs index 3fd59782c..314ffb851 100644 --- a/common/eth2/src/lighthouse/attestation_rewards.rs +++ b/common/eth2/src/lighthouse/attestation_rewards.rs @@ -28,8 +28,10 @@ pub struct TotalAttestationRewards { #[serde(with = "eth2_serde_utils::quoted_u64")] pub head: u64, // attester's reward for target vote in gwei + #[serde(with = "eth2_serde_utils::quoted_i64")] pub target: i64, // attester's reward for source vote in gwei + #[serde(with = "eth2_serde_utils::quoted_i64")] pub source: i64, // TBD attester's inclusion_delay reward in gwei (phase0 only) // pub inclusion_delay: u64, diff --git a/common/eth2/src/lighthouse/sync_committee_rewards.rs b/common/eth2/src/lighthouse/sync_committee_rewards.rs index cdd685065..e215d8e3e 100644 --- a/common/eth2/src/lighthouse/sync_committee_rewards.rs +++ b/common/eth2/src/lighthouse/sync_committee_rewards.rs @@ -8,5 +8,6 @@ pub struct SyncCommitteeReward { #[serde(with = "eth2_serde_utils::quoted_u64")] pub validator_index: u64, // sync committee reward in gwei for the validator + #[serde(with = "eth2_serde_utils::quoted_i64")] pub reward: i64, } diff --git a/consensus/serde_utils/src/lib.rs b/consensus/serde_utils/src/lib.rs index 92b5966c9..5c5dafc66 100644 --- a/consensus/serde_utils/src/lib.rs +++ b/consensus/serde_utils/src/lib.rs @@ -12,4 +12,4 @@ pub mod u64_hex_be; pub mod u8_hex; pub use fixed_bytes_hex::{bytes_4_hex, bytes_8_hex}; -pub use quoted_int::{quoted_u256, quoted_u32, quoted_u64, quoted_u8}; +pub use quoted_int::{quoted_i64, quoted_u256, quoted_u32, quoted_u64, quoted_u8}; diff --git a/consensus/serde_utils/src/quoted_int.rs b/consensus/serde_utils/src/quoted_int.rs index 822acb5ee..0cc35aa31 100644 --- a/consensus/serde_utils/src/quoted_int.rs +++ b/consensus/serde_utils/src/quoted_int.rs @@ -11,7 +11,7 @@ use std::convert::TryFrom; use std::marker::PhantomData; macro_rules! define_mod { - ($int: ty, $visit_fn: ident) => { + ($int: ty) => { /// Serde support for deserializing quoted integers. /// /// Configurable so that quotes are either required or optional. @@ -140,19 +140,25 @@ macro_rules! define_mod { pub mod quoted_u8 { use super::*; - define_mod!(u8, visit_u8); + define_mod!(u8); } pub mod quoted_u32 { use super::*; - define_mod!(u32, visit_u32); + define_mod!(u32); } pub mod quoted_u64 { use super::*; - define_mod!(u64, visit_u64); + define_mod!(u64); +} + +pub mod quoted_i64 { + use super::*; + + define_mod!(i64); } pub mod quoted_u256 { @@ -216,4 +222,26 @@ mod test { fn u256_without_quotes() { serde_json::from_str::("1").unwrap_err(); } + + #[derive(Debug, PartialEq, Serialize, Deserialize)] + #[serde(transparent)] + struct WrappedI64(#[serde(with = "quoted_i64")] i64); + + #[test] + fn negative_i64_with_quotes() { + assert_eq!( + serde_json::from_str::("\"-200\"").unwrap().0, + -200 + ); + assert_eq!( + serde_json::to_string(&WrappedI64(-12_500)).unwrap(), + "\"-12500\"" + ); + } + + // It would be OK if this worked, but we don't need it to (i64s should always be quoted). + #[test] + fn negative_i64_without_quotes() { + serde_json::from_str::("-200").unwrap_err(); + } }