Adding inclusion_delay and inactivity to ideal_rewards object

CHANGELOG.md

@@ -4,14 +4,22 @@
 
 ## Current Releases
 
-For information on changes in released versions of Teku, see the [releases page](https://github.com/Consensys/teku/releases).
+For information on changes in released versions of Teku, see
+the [releases page](https://github.com/Consensys/teku/releases).
 
 ## Unreleased Changes
 
 ### Breaking Changes
-- Added new `inactivity` field to `total_rewards` in `/eth/v1/beacon/rewards/attestations/{epoch}` endpoint response.
+
+- The response of the Attestation Rewards method was updated to include two extra
+  fields (`inactivity` and `inclusion_delay`) into the `total_rewards` and `ideal_rewards` objects.
+  This should be considered a breaking change only if you are strictly
+  checking the structure of the json in the response.
 
 ### Additions and Improvements
-- Solve an unintended breaking change in `23.9.1` release which was including an updated leveldb native library not compatible with relative old Linux distributions. Leveldb native library has been updated to support older GLIBC versions (ie Ubuntu 20.04 and Debian 11).
+
+- Solve an unintended breaking change in `23.9.1` release which was including an updated leveldb
+  native library not compatible with relative old Linux distributions. Leveldb native library has
+  been updated to support older GLIBC versions (ie Ubuntu 20.04 and Debian 11).
 
 ### Bug Fixes

data/beaconrestapi/src/integration-test/resources/tech/pegasys/teku/beaconrestapi/beacon/schema/GetAttestationRewards.json

@@ -28,7 +28,8 @@
               "effective_balance",
               "head",
               "target",
-              "source"
+              "source",
+              "inactivity"
             ],
             "properties": {
               "effective_balance": {
@@ -54,6 +55,18 @@
                 "description": "long string",
                 "example": "1",
                 "format": "long"
+              },
+              "inclusion_delay": {
+                "type": "string",
+                "description": "unsigned 64 bit integer",
+                "example": "1",
+                "format": "uint64"
+              },
+              "inactivity": {
+                "type": "string",
+                "description": "long string",
+                "example": "1",
+                "format": "long"
               }
             }
           }
@@ -102,9 +115,9 @@
               },
               "inactivity": {
                 "type": "string",
-                "description": "unsigned 64 bit integer",
+                "description": "long string",
                 "example": "1",
-                "format": "uint64"
+                "format": "long"
               }
             }
           }

data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/rewards/GetAttestationRewards.java

@@ -51,6 +51,9 @@ public class GetAttestationRewards extends RestApiEndpoint {
           .withField("head", LONG_TYPE, IdealAttestationReward::getHead)
           .withField("target", LONG_TYPE, IdealAttestationReward::getTarget)
           .withField("source", LONG_TYPE, IdealAttestationReward::getSource)
+          .withOptionalField(
+              "inclusion_delay", UINT64_TYPE, IdealAttestationReward::getInclusionDelay)
+          .withField("inactivity", LONG_TYPE, IdealAttestationReward::getInactivity)
           .build();
 
   private static final SerializableTypeDefinition<TotalAttestationReward> TOTAL_REWARDS_TYPE =
@@ -61,7 +64,7 @@ public class GetAttestationRewards extends RestApiEndpoint {
           .withField("source", LONG_TYPE, TotalAttestationReward::getSource)
           .withOptionalField(
               "inclusion_delay", UINT64_TYPE, TotalAttestationReward::getInclusionDelay)
-          .withField("inactivity", UINT64_TYPE, TotalAttestationReward::getInactivity)
+          .withField("inactivity", LONG_TYPE, TotalAttestationReward::getInactivity)
           .build();
 
   private static final SerializableTypeDefinition<AttestationRewardsData> DATA_TYPE =

data/beaconrestapi/src/test/java/tech/pegasys/teku/beaconrestapi/handlers/v1/rewards/GetAttestationRewardsTest.java

@@ -49,10 +49,11 @@ class GetAttestationRewardsTest extends AbstractMigratedBeaconHandlerTest {
       idealAttestationReward.addHead(2500L);
       idealAttestationReward.addTarget(5000L);
       idealAttestationReward.addSource(5000L);
+      idealAttestationReward.addInactivity(300L);
     }
     TotalAttestationReward totalAttestationReward =
         new TotalAttestationReward(
-            0L, 2000L, 2000L, 4000L, Optional.of(UInt64.valueOf(2000L)), UInt64.valueOf(300));
+            0L, 2000L, 2000L, 4000L, Optional.of(UInt64.valueOf(2000L)), 300L);
     AttestationRewardsData attestationRewardsData =
         new AttestationRewardsData(
             List.of(idealAttestationReward), List.of(totalAttestationReward));

data/beaconrestapi/src/test/resources/tech/pegasys/teku/beaconrestapi/handlers/v1/rewards/getAttestationRewards.json

@@ -1 +1 @@
-{"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","inactivity":"300"}]}}
+{"execution_optimistic":false,"finalized":false,"data":{"ideal_rewards":[{"effective_balance":"1000000000","head":"2500","target":"5000","source":"5000","inactivity":"300"}],"total_rewards":[{"validator_index":"0","head":"2000","target":"2000","source":"4000","inclusion_delay":"2000","inactivity":"300"}]}}

data/serializer/src/main/java/tech/pegasys/teku/api/migrated/IdealAttestationReward.java

@@ -14,6 +14,7 @@
 package tech.pegasys.teku.api.migrated;
 
 import java.util.Objects;
+import java.util.Optional;
 import tech.pegasys.teku.infrastructure.unsigned.UInt64;
 
 public class IdealAttestationReward {
@@ -22,6 +23,8 @@ public class IdealAttestationReward {
   private long head = 0L;
   private long target = 0L;
   private long source = 0L;
+  private final Optional<UInt64> inclusionDelay = Optional.empty();
+  private long inactivity = 0L;
 
   public IdealAttestationReward(final UInt64 effectiveBalance) {
     this.effectiveBalance = effectiveBalance;
@@ -43,6 +46,14 @@ public long getSource() {
     return source;
   }
 
+  public Optional<UInt64> getInclusionDelay() {
+    return inclusionDelay;
+  }
+
+  public long getInactivity() {
+    return inactivity;
+  }
+
   public void addHead(final long head) {
     this.head += head;
   }
@@ -55,6 +66,10 @@ public void addSource(final long source) {
     this.source += source;
   }
 
+  public void addInactivity(final long inactivity) {
+    this.inactivity += inactivity;
+  }
+
   @Override
   public boolean equals(Object o) {
     if (this == o) {
@@ -69,11 +84,13 @@ public boolean equals(Object o) {
     return effectiveBalance.equals(that.effectiveBalance)
         && head == that.head
         && target == that.target
-        && source == that.source;
+        && source == that.source
+        && inclusionDelay.equals(that.inclusionDelay)
+        && inactivity == that.inactivity;
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(effectiveBalance, head, target, source);
+    return Objects.hash(effectiveBalance, head, target, source, inclusionDelay, inactivity);
   }
 }

data/serializer/src/main/java/tech/pegasys/teku/api/migrated/TotalAttestationReward.java

@@ -13,7 +13,6 @@
 
 package tech.pegasys.teku.api.migrated;
 
-import java.security.InvalidParameterException;
 import java.util.Objects;
 import java.util.Optional;
 import tech.pegasys.teku.infrastructure.unsigned.UInt64;
@@ -28,15 +27,15 @@ public class TotalAttestationReward {
   private final long target;
   private final long source;
   private final Optional<UInt64> inclusionDelay;
-  private final UInt64 inactivity;
+  private final long inactivity;
 
   public TotalAttestationReward(
       long validatorIndex,
       long head,
       long target,
       long source,
       Optional<UInt64> inclusionDelay,
-      UInt64 inactivity) {
+      long inactivity) {
     this.validatorIndex = validatorIndex;
     this.head = head;
     this.target = target;
@@ -53,7 +52,7 @@ public TotalAttestationReward(long validatorIndex, final RewardAndPenalty reward
             .asDetailed()
             .orElseThrow(
                 () ->
-                    new InvalidParameterException(
+                    new IllegalArgumentException(
                         "TotalAttestationRewards requires a DetailedRewardAndPenalty instance"));
 
     this.head =
@@ -65,11 +64,12 @@ public TotalAttestationReward(long validatorIndex, final RewardAndPenalty reward
     this.source =
         detailedRewardAndPenalty.getReward(RewardComponent.SOURCE).longValue()
             - detailedRewardAndPenalty.getPenalty(RewardComponent.SOURCE).longValue();
-    this.inclusionDelay = Optional.empty();
     this.inactivity =
-        detailedRewardAndPenalty
-            .getReward(RewardComponent.INACTIVITY)
-            .minus(detailedRewardAndPenalty.getPenalty(RewardComponent.INACTIVITY));
+        detailedRewardAndPenalty.getReward(RewardComponent.INACTIVITY).longValue()
+            - detailedRewardAndPenalty.getPenalty(RewardComponent.INACTIVITY).longValue();
+
+    // Inclusion delay will always be empty because we don't support phase0 on the Rewards API
+    this.inclusionDelay = Optional.empty();
   }
 
   public long getValidatorIndex() {
@@ -92,7 +92,7 @@ public Optional<UInt64> getInclusionDelay() {
     return inclusionDelay;
   }
 
-  public UInt64 getInactivity() {
+  public long getInactivity() {
     return inactivity;
   }
 
@@ -111,11 +111,12 @@ public boolean equals(Object o) {
         && head == that.head
         && target == that.target
         && source == that.source
-        && inclusionDelay.equals(that.inclusionDelay);
+        && inclusionDelay.equals(that.inclusionDelay)
+        && inactivity == that.inactivity;
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(validatorIndex, head, target, source, inclusionDelay);
+    return Objects.hash(validatorIndex, head, target, source, inclusionDelay, inactivity);
   }
 }

data/serializer/src/test/java/tech/pegasys/teku/api/migrated/TotalAttestationRewardTest.java

@@ -0,0 +1,69 @@
+/*
+ * Copyright Consensys Software Inc., 2023
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ */
+
+package tech.pegasys.teku.api.migrated;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import org.junit.jupiter.api.Test;
+import tech.pegasys.teku.infrastructure.unsigned.UInt64;
+import tech.pegasys.teku.spec.logic.common.statetransition.epoch.AggregatedRewardAndPenalty;
+import tech.pegasys.teku.spec.logic.common.statetransition.epoch.DetailedRewardAndPenalty;
+import tech.pegasys.teku.spec.logic.common.statetransition.epoch.RewardAndPenalty.RewardComponent;
+
+class TotalAttestationRewardTest {
+
+  @Test
+  public void failsToCreateWithAggregatedRewardsAndPenalty() {
+    final AggregatedRewardAndPenalty rewardAndPenalty = new AggregatedRewardAndPenalty();
+    assertThatThrownBy(() -> new TotalAttestationReward(1, rewardAndPenalty))
+        .isInstanceOf(IllegalArgumentException.class);
+  }
+
+  @Test
+  public void successfullyReturnRewardComponents() {
+    final DetailedRewardAndPenalty rewardAndPenalty = new DetailedRewardAndPenalty();
+
+    rewardAndPenalty.reward(RewardComponent.HEAD, UInt64.valueOf(10));
+    rewardAndPenalty.reward(RewardComponent.SOURCE, UInt64.valueOf(10));
+    rewardAndPenalty.reward(RewardComponent.TARGET, UInt64.valueOf(10));
+    rewardAndPenalty.reward(RewardComponent.INACTIVITY, UInt64.valueOf(10));
+
+    rewardAndPenalty.penalize(RewardComponent.HEAD, UInt64.valueOf(5));
+    rewardAndPenalty.penalize(RewardComponent.SOURCE, UInt64.valueOf(4));
+    rewardAndPenalty.penalize(RewardComponent.TARGET, UInt64.valueOf(3));
+    rewardAndPenalty.penalize(RewardComponent.INACTIVITY, UInt64.valueOf(2));
+
+    final TotalAttestationReward totalAttestationReward =
+        new TotalAttestationReward(1L, rewardAndPenalty);
+
+    // Assert that we are correctly calculating reward - penalty for each component
+    assertThat(totalAttestationReward.getHead()).isEqualTo(10L - 5L);
+    assertThat(totalAttestationReward.getSource()).isEqualTo(10L - 4L);
+    assertThat(totalAttestationReward.getTarget()).isEqualTo(10L - 3L);
+    assertThat(totalAttestationReward.getInactivity()).isEqualTo(10L - 2L);
+  }
+
+  @Test
+  public void supportNegativeHeadSourceAndTargetValues() {
+    final DetailedRewardAndPenalty rewardAndPenalty = new DetailedRewardAndPenalty();
+    rewardAndPenalty.reward(RewardComponent.HEAD, UInt64.valueOf(5));
+    rewardAndPenalty.penalize(RewardComponent.HEAD, UInt64.valueOf(10));
+
+    final TotalAttestationReward totalAttestationReward =
+        new TotalAttestationReward(1L, rewardAndPenalty);
+
+    assertThat(totalAttestationReward.getHead()).isEqualTo(5L - 10L);
+  }
+}