Errorprone 2.3.3 upgrades (#1110)

Biggest change is that UnusedVariable and UnusedMethod went to WARN by default. Since our build is a no warning build this means we either need to turn them off or fix them. I mostly opted for the latter. Test code was mostly fixed, unused loggers were deleted, and other shipped code was mostly suppressed. Two less noisy fixes to not use `SortedSet` and to use zero based comparable results instead of -1, 0, and 1. Also a compiler nit in errorprone was suppressed, per the description it won't affect us.
PegaSysEng · Mar 19, 2019 · 171928d · 171928d
1 parent 3fffd60
commit 171928d
Show file tree

Hide file tree

Showing 51 changed files with 159 additions and 307 deletions.
diff --git a/...tance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/account/Account.java b/...tance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/account/Account.java
@@ -75,4 +75,19 @@ public Condition balanceEquals(final Amount expectedBalance) {
     return new ExpectAccountBalance(
         eth, this, expectedBalance.getValue(), expectedBalance.getUnit());
   }
+
+  @Override
+  public String toString() {
+    return "Account{"
+        + "eth="
+        + eth
+        + ", name='"
+        + name
+        + '\''
+        + ", keyPair="
+        + keyPair
+        + ", nonce="
+        + nonce
+        + '}';
+  }
 }
diff --git a/...nce-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/blockchain/Amount.java b/...nce-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/blockchain/Amount.java
@@ -46,7 +46,7 @@ public Unit getUnit() {
   public Amount subtract(final Amount subtracting) {
 
     final Unit denominator;
-    if (unit.getWeiFactor().compareTo(subtracting.unit.getWeiFactor()) == -1) {
+    if (unit.getWeiFactor().compareTo(subtracting.unit.getWeiFactor()) < 0) {
       denominator = unit;
     } else {
       denominator = subtracting.unit;

diff --git a/...a/tech/pegasys/pantheon/tests/acceptance/dsl/transaction/account/TransferTransaction.java b/...a/tech/pegasys/pantheon/tests/acceptance/dsl/transaction/account/TransferTransaction.java
@@ -95,7 +95,7 @@ private BigInteger convertGasPriceToWei(final Amount unconverted) {
     final BigInteger price =
         Convert.toWei(unconverted.getValue(), unconverted.getUnit()).toBigInteger();
 
-    if (MINIMUM_GAS_PRICE.compareTo(price) == 1) {
+    if (MINIMUM_GAS_PRICE.compareTo(price) > 0) {
       throw new IllegalArgumentException(
           String.format(
               "Gas price: %s WEI, is below the accepted minimum: %s WEI",

diff --git a/build.gradle b/build.gradle
@@ -20,7 +20,7 @@ plugins {
   id 'com.github.hierynomus.license' version '0.15.0'
   id 'io.spring.dependency-management' version '1.0.7.RELEASE'
   id 'me.champeau.gradle.jmh' version '0.4.8' apply false
-  id 'net.ltgt.errorprone' version '0.6.1'
+  id 'net.ltgt.errorprone' version '0.7.1'
   id 'net.researchgate.release' version '2.7.0'
 }
 

diff --git a/...src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/RoundChangeTest.java b/...src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/tests/RoundChangeTest.java
@@ -258,7 +258,7 @@ public void subsequentRoundChangeMessagesFromPeerDoNotOverwritePriorMessage() {
             new ConsensusRoundIdentifier(1, 2),
             context.createBlockForProposalFromChainHead(2, ARBITRARY_BLOCKTIME));
 
-    List<SignedData<RoundChangePayload>> roundChangeMessages = Lists.newArrayList();
+    final List<SignedData<RoundChangePayload>> roundChangeMessages = Lists.newArrayList();
     // Create a roundChange containing a PreparedCertificate
     roundChangeMessages.add(
         peers
@@ -311,9 +311,9 @@ public void roundChangeExpiryForNonCurrentRoundIsDiscarded() {
   public void illegallyConstructedRoundChangeMessageIsDiscarded() {
     final ConsensusRoundIdentifier targetRound = new ConsensusRoundIdentifier(1, 4);
 
-    final RoundChange rc1 = peers.getNonProposing(0).injectRoundChange(targetRound, empty());
-    final RoundChange rc2 = peers.getNonProposing(1).injectRoundChange(targetRound, empty());
-    final RoundChange rc3 = peers.getNonProposing(2).injectRoundChange(targetRound, empty());
+    peers.getNonProposing(0).injectRoundChange(targetRound, empty());
+    peers.getNonProposing(1).injectRoundChange(targetRound, empty());
+    peers.getNonProposing(2).injectRoundChange(targetRound, empty());
 
     // create illegal RoundChangeMessage
     final PreparedRoundArtifacts illegalPreparedRoundArtifacts =

diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftProtocolSchedule.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftProtocolSchedule.java
@@ -16,7 +16,6 @@
 
 import tech.pegasys.pantheon.config.GenesisConfigOptions;
 import tech.pegasys.pantheon.config.IbftConfigOptions;
-import tech.pegasys.pantheon.consensus.common.EpochManager;
 import tech.pegasys.pantheon.ethereum.MainnetBlockValidator;
 import tech.pegasys.pantheon.ethereum.core.PrivacyParameters;
 import tech.pegasys.pantheon.ethereum.core.Wei;
@@ -35,9 +34,7 @@ public class IbftProtocolSchedule {
 
   public static ProtocolSchedule<IbftContext> create(final GenesisConfigOptions config) {
     final IbftConfigOptions ibftConfig = config.getIbftLegacyConfigOptions();
-    final long epochLength = ibftConfig.getEpochLength();
     final long blockPeriod = ibftConfig.getBlockPeriodSeconds();
-    final EpochManager epochManager = new EpochManager(epochLength);
 
     return new ProtocolScheduleBuilder<>(
             config,

diff --git a/...ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/protocol/IbftProtocolManager.java b/...ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/protocol/IbftProtocolManager.java
@@ -25,13 +25,9 @@
 import java.util.Arrays;
 import java.util.List;
 
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
-
 public class IbftProtocolManager implements ProtocolManager {
   private final IbftEventQueue ibftEventQueue;
 
-  private final Logger LOG = LogManager.getLogger();
   private final PeerConnectionTracker peers;
 
   /**

diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/RoundTimerTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/RoundTimerTest.java
@@ -17,9 +17,9 @@
 import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
-import static org.mockito.internal.verification.VerificationModeFactory.times;
 
 import tech.pegasys.pantheon.consensus.ibft.ibftevent.RoundExpiry;
 

diff --git a/...rc/test/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftBlockCreatorTest.java b/...rc/test/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftBlockCreatorTest.java
@@ -20,7 +20,6 @@
 import static tech.pegasys.pantheon.ethereum.core.InMemoryStorageProvider.createInMemoryWorldStateArchive;
 
 import tech.pegasys.pantheon.config.GenesisConfigFile;
-import tech.pegasys.pantheon.consensus.common.VoteTally;
 import tech.pegasys.pantheon.consensus.ibft.IbftBlockHashing;
 import tech.pegasys.pantheon.consensus.ibft.IbftBlockHeaderValidationRulesetFactory;
 import tech.pegasys.pantheon.consensus.ibft.IbftContext;
@@ -69,8 +68,6 @@ public void createdBlockPassesValidationRulesAndHasAppropriateHashAndMixHash() {
       initialValidatorList.add(AddressHelpers.ofValue(i));
     }
 
-    final VoteTally voteTally = new VoteTally(initialValidatorList);
-
     final ProtocolSchedule<IbftContext> protocolSchedule =
         IbftProtocolSchedule.create(
             GenesisConfigFile.fromConfig("{\"config\": {\"spuriousDragonBlock\":0}}")

diff --git a/...st/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManagerTest.java b/...st/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManagerTest.java
@@ -95,7 +95,6 @@ public class IbftBlockHeightManagerTest {
 
   @Captor private ArgumentCaptor<Optional<PreparedRoundArtifacts>> preparedRoundArtifactsCaptor;
 
-  private final List<KeyPair> validatorKeys = Lists.newArrayList();
   private final List<Address> validators = Lists.newArrayList();
   private final List<MessageFactory> validatorMessageFactory = Lists.newArrayList();
 
@@ -105,7 +104,7 @@ public class IbftBlockHeightManagerTest {
 
   private void buildCreatedBlock() {
 
-    IbftExtraData extraData =
+    final IbftExtraData extraData =
         new IbftExtraData(
             BytesValue.wrap(new byte[32]), emptyList(), Optional.empty(), 0, validators);
 
@@ -118,7 +117,6 @@ private void buildCreatedBlock() {
   public void setup() {
     for (int i = 0; i < 3; i++) {
       final KeyPair key = KeyPair.generate();
-      validatorKeys.add(key);
       validators.add(Util.publicKeyToAddress(key.getPublicKey()));
       validatorMessageFactory.add(new MessageFactory(key));
     }
@@ -146,7 +144,7 @@ public void setup() {
     when(roundFactory.createNewRound(any(), anyInt()))
         .thenAnswer(
             invocation -> {
-              final int round = (int) invocation.getArgument(1);
+              final int round = invocation.getArgument(1);
               final ConsensusRoundIdentifier roundId = new ConsensusRoundIdentifier(1, round);
               final RoundState createdRoundState =
                   new RoundState(roundId, finalState.getQuorum(), messageValidator);
@@ -183,14 +181,13 @@ public void setup() {
   public void startsABlockTimerOnStartIfLocalNodeIsTheProoserForRound() {
     when(finalState.isLocalNodeProposerForRound(any())).thenReturn(true);
 
-    final IbftBlockHeightManager manager =
-        new IbftBlockHeightManager(
-            headerTestFixture.buildHeader(),
-            finalState,
-            roundChangeManager,
-            roundFactory,
-            clock,
-            messageValidatorFactory);
+    new IbftBlockHeightManager(
+        headerTestFixture.buildHeader(),
+        finalState,
+        roundChangeManager,
+        roundFactory,
+        clock,
+        messageValidatorFactory);
 
     verify(blockTimer, times(1)).startTimer(any(), any());
   }

diff --git a/...s/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRoundTest.java b/...s/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRoundTest.java
@@ -32,9 +32,7 @@
 import tech.pegasys.pantheon.consensus.ibft.blockcreation.IbftBlockCreator;
 import tech.pegasys.pantheon.consensus.ibft.network.IbftMessageTransmitter;
 import tech.pegasys.pantheon.consensus.ibft.payload.MessageFactory;
-import tech.pegasys.pantheon.consensus.ibft.payload.ProposalPayload;
 import tech.pegasys.pantheon.consensus.ibft.payload.RoundChangeCertificate;
-import tech.pegasys.pantheon.consensus.ibft.payload.SignedData;
 import tech.pegasys.pantheon.consensus.ibft.validation.MessageValidator;
 import tech.pegasys.pantheon.crypto.SECP256K1;
 import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair;
@@ -82,7 +80,6 @@ public class IbftRoundTest {
   @Mock private MessageValidator messageValidator;
   @Mock private RoundTimer roundTimer;
 
-  @Captor private ArgumentCaptor<SignedData<ProposalPayload>> payloadArgCaptor;
   @Captor private ArgumentCaptor<Block> blockCaptor;
 
   private Block proposedBlock;
@@ -120,17 +117,16 @@ public void setup() {
   @Test
   public void onConstructionRoundTimerIsStarted() {
     final RoundState roundState = new RoundState(roundIdentifier, 3, messageValidator);
-    final IbftRound round =
-        new IbftRound(
-            roundState,
-            blockCreator,
-            protocolContext,
-            blockImporter,
-            subscribers,
-            localNodeKeys,
-            messageFactory,
-            transmitter,
-            roundTimer);
+    new IbftRound(
+        roundState,
+        blockCreator,
+        protocolContext,
+        blockImporter,
+        subscribers,
+        localNodeKeys,
+        messageFactory,
+        transmitter,
+        roundTimer);
     verify(roundTimer, times(1)).startTimer(roundIdentifier);
   }
 
@@ -233,11 +229,11 @@ public void twoValidatorNetworkSendsPrepareOnProposalReceptionThenSendsCommitOnC
         messageFactory.createCommit(roundIdentifier, proposedBlock.getHash(), remoteCommitSeal));
 
     // Should import block when both commit seals are available.
-    ArgumentCaptor<Block> capturedBlock = ArgumentCaptor.forClass(Block.class);
+    final ArgumentCaptor<Block> capturedBlock = ArgumentCaptor.forClass(Block.class);
     verify(blockImporter, times(1)).importBlock(any(), capturedBlock.capture(), any());
 
     // Ensure imported block contains both commit seals.
-    IbftExtraData importedExtraData =
+    final IbftExtraData importedExtraData =
         IbftExtraData.decode(capturedBlock.getValue().getHeader().getExtraData());
     assertThat(importedExtraData.getSeals()).containsOnly(remoteCommitSeal, localCommitSeal);
   }

diff --git a/...c/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeManagerTest.java b/...c/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeManagerTest.java
@@ -32,13 +32,9 @@
 import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair;
 import tech.pegasys.pantheon.ethereum.BlockValidator;
 import tech.pegasys.pantheon.ethereum.BlockValidator.BlockProcessingOutputs;
-import tech.pegasys.pantheon.ethereum.ProtocolContext;
-import tech.pegasys.pantheon.ethereum.chain.MutableBlockchain;
 import tech.pegasys.pantheon.ethereum.core.Address;
 import tech.pegasys.pantheon.ethereum.core.Block;
-import tech.pegasys.pantheon.ethereum.core.BlockHeader;
 import tech.pegasys.pantheon.ethereum.core.Util;
-import tech.pegasys.pantheon.ethereum.worldstate.WorldStateArchive;
 
 import java.util.Collections;
 import java.util.List;
@@ -63,7 +59,7 @@ public class RoundChangeManagerTest {
   private final ConsensusRoundIdentifier ri2 = new ConsensusRoundIdentifier(2, 2);
   private final ConsensusRoundIdentifier ri3 = new ConsensusRoundIdentifier(2, 3);
   private final List<Address> validators = Lists.newArrayList();
-  private ProposalBlockConsistencyValidator proposalConsistencyValidator =
+  private final ProposalBlockConsistencyValidator proposalConsistencyValidator =
       mock(ProposalBlockConsistencyValidator.class);
 
   @Before
@@ -73,18 +69,13 @@ public void setup() {
     validators.add(Util.publicKeyToAddress(validator1Key.getPublicKey()));
     validators.add(Util.publicKeyToAddress(validator2Key.getPublicKey()));
 
-    final ProtocolContext<IbftContext> protocolContext =
-        new ProtocolContext<>(
-            mock(MutableBlockchain.class), mock(WorldStateArchive.class), mock(IbftContext.class));
-
     @SuppressWarnings("unchecked")
-    BlockValidator<IbftContext> blockValidator =
+    final BlockValidator<IbftContext> blockValidator =
         (BlockValidator<IbftContext>) mock(BlockValidator.class);
     when(blockValidator.validateAndProcessBlock(any(), any(), any(), any()))
         .thenReturn(Optional.of(new BlockProcessingOutputs(null, null)));
-    BlockHeader parentHeader = mock(BlockHeader.class);
 
-    RoundChangePayloadValidator.MessageValidatorForHeightFactory messageValidatorFactory =
+    final RoundChangePayloadValidator.MessageValidatorForHeightFactory messageValidatorFactory =
         mock(RoundChangePayloadValidator.MessageValidatorForHeightFactory.class);
 
     when(messageValidatorFactory.createAt(ri1))
@@ -119,7 +110,7 @@ public void setup() {
 
   private RoundChange makeRoundChangeMessage(
       final KeyPair key, final ConsensusRoundIdentifier round) {
-    MessageFactory messageFactory = new MessageFactory(key);
+    final MessageFactory messageFactory = new MessageFactory(key);
     return messageFactory.createRoundChange(round, Optional.empty());
   }
 

diff --git a/...java/tech/pegasys/pantheon/consensus/ibft/validation/RoundChangeMessageValidatorTest.java b/...java/tech/pegasys/pantheon/consensus/ibft/validation/RoundChangeMessageValidatorTest.java
@@ -12,18 +12,15 @@
  */
 package tech.pegasys.pantheon.consensus.ibft.validation;
 
-import static java.util.Collections.emptyList;
 import static java.util.Optional.empty;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 
 import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier;
-import tech.pegasys.pantheon.consensus.ibft.TestHelpers;
 import tech.pegasys.pantheon.consensus.ibft.messagewrappers.RoundChange;
 import tech.pegasys.pantheon.consensus.ibft.payload.MessageFactory;
 import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair;
-import tech.pegasys.pantheon.ethereum.core.Block;
 
 import org.junit.Test;
 
@@ -34,7 +31,6 @@ public class RoundChangeMessageValidatorTest {
   private final KeyPair keyPair = KeyPair.generate();
   private final MessageFactory messageFactory = new MessageFactory(keyPair);
   private final ConsensusRoundIdentifier roundIdentifier = new ConsensusRoundIdentifier(1, 1);
-  private final Block block = TestHelpers.createProposalBlock(emptyList(), roundIdentifier);
 
   private ProposalBlockConsistencyValidator proposalBlockConsistencyValidator =
       mock(ProposalBlockConsistencyValidator.class);

diff --git a/...legacy/src/main/java/tech/pegasys/pantheon/consensus/ibftlegacy/IbftProtocolSchedule.java b/...legacy/src/main/java/tech/pegasys/pantheon/consensus/ibftlegacy/IbftProtocolSchedule.java
@@ -16,7 +16,6 @@
 
 import tech.pegasys.pantheon.config.GenesisConfigOptions;
 import tech.pegasys.pantheon.config.IbftConfigOptions;
-import tech.pegasys.pantheon.consensus.common.EpochManager;
 import tech.pegasys.pantheon.consensus.ibft.IbftContext;
 import tech.pegasys.pantheon.ethereum.MainnetBlockValidator;
 import tech.pegasys.pantheon.ethereum.core.PrivacyParameters;
@@ -36,22 +35,18 @@ public class IbftProtocolSchedule {
 
   public static ProtocolSchedule<IbftContext> create(final GenesisConfigOptions config) {
     final IbftConfigOptions ibftConfig = config.getIbftLegacyConfigOptions();
-    final long epochLength = ibftConfig.getEpochLength();
     final long blockPeriod = ibftConfig.getBlockPeriodSeconds();
-    final EpochManager epochManager = new EpochManager(epochLength);
 
     return new ProtocolScheduleBuilder<>(
             config,
             DEFAULT_CHAIN_ID,
-            builder -> applyIbftChanges(blockPeriod, epochManager, builder),
+            builder -> applyIbftChanges(blockPeriod, builder),
             PrivacyParameters.noPrivacy())
         .createProtocolSchedule();
   }
 
   private static ProtocolSpecBuilder<IbftContext> applyIbftChanges(
-      final long secondsBetweenBlocks,
-      final EpochManager epochManager,
-      final ProtocolSpecBuilder<Void> builder) {
+      final long secondsBetweenBlocks, final ProtocolSpecBuilder<Void> builder) {
     return builder
         .<IbftContext>changeConsensusContextType(
             difficultyCalculator -> ibftBlockHeaderValidator(secondsBetweenBlocks),

diff --git a/...t/java/tech/pegasys/pantheon/consensus/ibftlegacy/blockcreation/IbftBlockCreatorTest.java b/...t/java/tech/pegasys/pantheon/consensus/ibftlegacy/blockcreation/IbftBlockCreatorTest.java
@@ -20,7 +20,6 @@
 import static tech.pegasys.pantheon.ethereum.core.InMemoryStorageProvider.createInMemoryWorldStateArchive;
 
 import tech.pegasys.pantheon.config.GenesisConfigFile;
-import tech.pegasys.pantheon.consensus.common.VoteTally;
 import tech.pegasys.pantheon.consensus.ibft.IbftContext;
 import tech.pegasys.pantheon.consensus.ibftlegacy.IbftBlockHeaderValidationRulesetFactory;
 import tech.pegasys.pantheon.consensus.ibftlegacy.IbftExtraData;
@@ -75,8 +74,6 @@ public void headerProducedPassesValidationRules() {
             Address.fromHexString(String.format("%020d", 4)),
             localAddr);
 
-    final VoteTally voteTally = new VoteTally(initialValidatorList);
-
     final ProtocolSchedule<IbftContext> protocolSchedule =
         IbftProtocolSchedule.create(
             GenesisConfigFile.fromConfig("{\"config\": {\"spuriousDragonBlock\":0}}")

diff --git a/.../pantheon/consensus/ibftlegacy/headervalidationrules/IbftExtraDataValidationRuleTest.java b/.../pantheon/consensus/ibftlegacy/headervalidationrules/IbftExtraDataValidationRuleTest.java
@@ -17,7 +17,6 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static tech.pegasys.pantheon.consensus.ibft.IbftContextBuilder.setupContextWithValidators;
 
-import tech.pegasys.pantheon.consensus.common.VoteTally;
 import tech.pegasys.pantheon.consensus.ibft.IbftContext;
 import tech.pegasys.pantheon.consensus.ibftlegacy.IbftBlockHashing;
 import tech.pegasys.pantheon.consensus.ibftlegacy.IbftExtraData;
@@ -186,7 +185,6 @@ public void proposerNotInValidatorListFailsValidation() {
         Lists.newArrayList(
             AddressHelpers.calculateAddressWithRespectTo(proposerAddress, 1), proposerAddress);
 
-    final VoteTally voteTally = new VoteTally(validators);
     final ProtocolContext<IbftContext> context =
         new ProtocolContext<>(null, null, setupContextWithValidators(validators));
 
@@ -312,7 +310,6 @@ private boolean subExecution(final int validatorCount, final int committerCount)
     }
 
     Collections.sort(validators);
-    final VoteTally voteTally = new VoteTally(validators);
     BlockHeader header = createProposedBlockHeader(proposerKeyPair, validators);
     final IbftExtraData commitedExtraData =
         createExtraDataWithCommitSeals(header, committerKeys.subList(0, committerCount));