PIE-1781: Enforce nonce validation on PrivateTransactionProcessor (#1713)

Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionHandler.java

@@ -12,9 +12,6 @@
  */
 package tech.pegasys.pantheon.ethereum.privacy;
 
-import static tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason.INCORRECT_PRIVATE_NONCE;
-import static tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason.PRIVATE_NONCE_TOO_LOW;
-
 import tech.pegasys.pantheon.crypto.SECP256K1;
 import tech.pegasys.pantheon.enclave.Enclave;
 import tech.pegasys.pantheon.enclave.types.ReceiveRequest;
@@ -51,6 +48,7 @@ public class PrivateTransactionHandler {
   private final String enclavePublicKey;
   private final PrivateStateStorage privateStateStorage;
   private final WorldStateArchive privateWorldStateArchive;
+  private final PrivateTransactionValidator privateTransactionValidator;
 
   public PrivateTransactionHandler(final PrivacyParameters privacyParameters) {
     this(
@@ -59,7 +57,8 @@ public class PrivateTransactionHandler {
         privacyParameters.getSigningKeyPair(),
         privacyParameters.getEnclavePublicKey(),
         privacyParameters.getPrivateStateStorage(),
-        privacyParameters.getPrivateWorldStateArchive());
+        privacyParameters.getPrivateWorldStateArchive(),
+        new PrivateTransactionValidator());
   }
 
   public PrivateTransactionHandler(
@@ -68,7 +67,8 @@ public class PrivateTransactionHandler {
       final SECP256K1.KeyPair nodeKeyPair,
       final String enclavePublicKey,
       final PrivateStateStorage privateStateStorage,
-      final WorldStateArchive privateWorldStateArchive) {
+      final WorldStateArchive privateWorldStateArchive,
+      final PrivateTransactionValidator privateTransactionValidator) {
     this.enclave = enclave;
     this.privacyPrecompileAddress = privacyPrecompileAddress;
     this.nodeKeyPair = nodeKeyPair;
@@ -76,6 +76,7 @@ public class PrivateTransactionHandler {
     this.enclavePublicKey = enclavePublicKey;
     this.privateStateStorage = privateStateStorage;
     this.privateWorldStateArchive = privateWorldStateArchive;
+    this.privateTransactionValidator = privateTransactionValidator;
   }
 
   public String sendToOrion(final PrivateTransaction privateTransaction) throws Exception {
@@ -130,26 +131,8 @@ public class PrivateTransactionHandler {
 
   public ValidationResult<TransactionInvalidReason> validatePrivateTransaction(
       final PrivateTransaction privateTransaction, final String privacyGroupId) {
-    final long actualNonce = privateTransaction.getNonce();
+    return privateTransactionValidator.validate(
-    final long expectedNonce = getSenderNonce(privateTransaction.getSender(), privacyGroupId);
+        privateTransaction, getSenderNonce(privateTransaction.getSender(), privacyGroupId));
-    LOG.debug("Validating actual nonce {} with expected nonce {}", actualNonce, expectedNonce);
-    if (expectedNonce > actualNonce) {
-      return ValidationResult.invalid(
-          PRIVATE_NONCE_TOO_LOW,
-          String.format(
-              "private transaction nonce %s does not match sender account nonce %s.",
-              actualNonce, expectedNonce));
-    }
-
-    if (expectedNonce != actualNonce) {
-      return ValidationResult.invalid(
-          INCORRECT_PRIVATE_NONCE,
-          String.format(
-              "private transaction nonce %s does not match sender account nonce %s.",
-              actualNonce, expectedNonce));
-    }
-
-    return ValidationResult.valid();
   }
 
   private SendRequest createSendRequest(final PrivateTransaction privateTransaction) {

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionProcessor.java

@@ -12,8 +12,6 @@
  */
 package tech.pegasys.pantheon.ethereum.privacy;
 
-import static tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason.NONCE_TOO_LOW;
-
 import tech.pegasys.pantheon.ethereum.chain.Blockchain;
 import tech.pegasys.pantheon.ethereum.core.Account;
 import tech.pegasys.pantheon.ethereum.core.Address;
@@ -53,6 +51,8 @@ public class PrivateTransactionProcessor {
   @SuppressWarnings("unused")
   private final TransactionValidator transactionValidator;
 
+  private final PrivateTransactionValidator privateTransactionValidator;
+
   private final AbstractMessageProcessor contractCreationProcessor;
 
   private final AbstractMessageProcessor messageCallProcessor;
@@ -166,6 +166,7 @@ public class PrivateTransactionProcessor {
       final int createContractAccountVersion) {
     this.gasCalculator = gasCalculator;
     this.transactionValidator = transactionValidator;
+    this.privateTransactionValidator = new PrivateTransactionValidator();
     this.contractCreationProcessor = contractCreationProcessor;
     this.messageCallProcessor = messageCallProcessor;
     this.clearEmptyAccounts = clearEmptyAccounts;
@@ -193,13 +194,10 @@ public class PrivateTransactionProcessor {
             ? maybePrivateSender
             : privateWorldState.createAccount(senderAddress, 0, Wei.ZERO);
 
-    if (transaction.getNonce() < sender.getNonce()) {
+    final ValidationResult<TransactionInvalidReason> validationResult =
-      return Result.invalid(
+        privateTransactionValidator.validate(transaction, sender.getNonce());
-          ValidationResult.invalid(
+    if (!validationResult.isValid()) {
-              NONCE_TOO_LOW,
+      return Result.invalid(validationResult);
-              String.format(
-                  "transaction nonce %s below sender account nonce %s",
-                  transaction.getNonce(), sender.getNonce())));
     }
 
     final long previousNonce = sender.incrementNonce();

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionValidator.java

@@ -0,0 +1,52 @@
+/*
+ * Copyright 2019 ConsenSys AG.
+ *
+ * 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.pantheon.ethereum.privacy;
+
+import static tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason.INCORRECT_PRIVATE_NONCE;
+import static tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason.PRIVATE_NONCE_TOO_LOW;
+
+import tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason;
+import tech.pegasys.pantheon.ethereum.mainnet.ValidationResult;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+class PrivateTransactionValidator {
+
+  private static final Logger LOG = LogManager.getLogger();
+
+  public ValidationResult<TransactionInvalidReason> validate(
+      final PrivateTransaction transaction, final Long accountNonce) {
+    final long transactionNonce = transaction.getNonce();
+
+    LOG.debug("Validating actual nonce {} with expected nonce {}", transactionNonce, accountNonce);
+
+    if (accountNonce > transactionNonce) {
+      return ValidationResult.invalid(
+          PRIVATE_NONCE_TOO_LOW,
+          String.format(
+              "private transaction nonce %s does not match sender account nonce %s.",
+              transactionNonce, accountNonce));
+    }
+
+    if (accountNonce != transactionNonce) {
+      return ValidationResult.invalid(
+          INCORRECT_PRIVATE_NONCE,
+          String.format(
+              "private transaction nonce %s does not match sender account nonce %s.",
+              transactionNonce, accountNonce));
+    }
+
+    return ValidationResult.valid();
+  }
+}

ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionHandlerTest.java

@@ -61,6 +61,7 @@ public class PrivateTransactionHandlerTest {
 
   private PrivateTransactionHandler privateTransactionHandler;
   private PrivateTransactionHandler brokenPrivateTransactionHandler;
+  private PrivateTransactionValidator privateTransactionValidator;
 
   private static final Transaction PUBLIC_TRANSACTION =
       Transaction.builder()
@@ -89,6 +90,12 @@ public class PrivateTransactionHandlerTest {
     return mockEnclave;
   }
 
+  PrivateTransactionValidator mockPrivateTransactionValidator() {
+    PrivateTransactionValidator validator = mock(PrivateTransactionValidator.class);
+    when(validator.validate(any(), any())).thenReturn(ValidationResult.valid());
+    return validator;
+  }
+
   @Before
   public void setUp() throws Exception {
     PrivateStateStorage privateStateStorage = mock(PrivateStateStorage.class);
@@ -102,6 +109,8 @@ public class PrivateTransactionHandlerTest {
     when(worldStateArchive.getMutable(any(Hash.class))).thenReturn(Optional.of(mutableWorldState));
     when(mutableWorldState.get(any(Address.class))).thenReturn(account);
 
+    privateTransactionValidator = mockPrivateTransactionValidator();
+
     privateTransactionHandler =
         new PrivateTransactionHandler(
             mockEnclave(),
@@ -109,7 +118,8 @@ public class PrivateTransactionHandlerTest {
             KEY_PAIR,
             OrionKeyUtils.loadKey("orion_key_0.pub"),
             privateStateStorage,
-            worldStateArchive);
+            worldStateArchive,
+            privateTransactionValidator);
     brokenPrivateTransactionHandler =
         new PrivateTransactionHandler(
             brokenMockEnclave(),
@@ -117,7 +127,8 @@ public class PrivateTransactionHandlerTest {
             KEY_PAIR,
             OrionKeyUtils.loadKey("orion_key_0.pub"),
             privateStateStorage,
-            worldStateArchive);
+            worldStateArchive,
+            privateTransactionValidator);
   }
 
   @Test
@@ -173,8 +184,10 @@ public class PrivateTransactionHandlerTest {
 
   @Test
   public void nonceTooLowError() throws Exception {
-    final PrivateTransaction transaction = buildLegacyPrivateTransaction(0);
+    when(privateTransactionValidator.validate(any(), any()))
+        .thenReturn(ValidationResult.invalid(PRIVATE_NONCE_TOO_LOW));
 
+    final PrivateTransaction transaction = buildLegacyPrivateTransaction(0);
     final String enclaveKey = privateTransactionHandler.sendToOrion(transaction);
     final String privacyGroupId =
         privateTransactionHandler.getPrivacyGroup(enclaveKey, transaction);
@@ -185,6 +198,9 @@ public class PrivateTransactionHandlerTest {
 
   @Test
   public void incorrectNonceError() throws Exception {
+    when(privateTransactionValidator.validate(any(), any()))
+        .thenReturn(ValidationResult.invalid(INCORRECT_PRIVATE_NONCE));
+
     final PrivateTransaction transaction = buildLegacyPrivateTransaction(2);
 
     final String enclaveKey = privateTransactionHandler.sendToOrion(transaction);

ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionValidatorTest.java

@@ -0,0 +1,76 @@
+/*
+ * Copyright 2019 ConsenSys AG.
+ *
+ * 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.pantheon.ethereum.privacy;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason.INCORRECT_PRIVATE_NONCE;
+import static tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason.PRIVATE_NONCE_TOO_LOW;
+
+import tech.pegasys.pantheon.ethereum.core.Address;
+import tech.pegasys.pantheon.ethereum.core.Wei;
+import tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason;
+import tech.pegasys.pantheon.ethereum.mainnet.ValidationResult;
+import tech.pegasys.pantheon.util.bytes.BytesValue;
+
+import java.math.BigInteger;
+
+import org.junit.Before;
+import org.junit.Test;
+
+public class PrivateTransactionValidatorTest {
+
+  private PrivateTransactionValidator validator;
+
+  @Before
+  public void before() {
+    validator = new PrivateTransactionValidator();
+  }
+
+  @Test
+  public void transactionWithNonceLowerThanAccountNonceShouldReturnLowNonceError() {
+    ValidationResult<TransactionInvalidReason> validationResult =
+        validator.validate(privateTransactionWithNonce(1L), 2L);
+
+    assertThat(validationResult).isEqualTo(ValidationResult.invalid(PRIVATE_NONCE_TOO_LOW));
+  }
+
+  @Test
+  public void transactionWithNonceGreaterThanAccountNonceShouldReturnIncorrectNonceError() {
+    ValidationResult<TransactionInvalidReason> validationResult =
+        validator.validate(privateTransactionWithNonce(3L), 2L);
+
+    assertThat(validationResult).isEqualTo(ValidationResult.invalid(INCORRECT_PRIVATE_NONCE));
+  }
+
+  @Test
+  public void transactionWithNonceMatchingThanAccountNonceShouldReturnValidTransactionResult() {
+    ValidationResult<TransactionInvalidReason> validationResult =
+        validator.validate(privateTransactionWithNonce(1L), 1L);
+
+    assertThat(validationResult).isEqualTo(ValidationResult.valid());
+  }
+
+  private static PrivateTransaction privateTransactionWithNonce(final long nonce) {
+    return PrivateTransaction.builder()
+        .nonce(nonce)
+        .gasPrice(Wei.of(1000))
+        .gasLimit(3000000)
+        .to(Address.fromHexString("0x627306090abab3a6e1400e9345bc60c78a8bef57"))
+        .value(Wei.ZERO)
+        .payload(BytesValue.fromHexString("0x"))
+        .sender(Address.fromHexString("0xfe3b557e8fb62b89f4916b721be55ceb828dbd73"))
+        .chainId(BigInteger.valueOf(2018))
+        .restriction(Restriction.RESTRICTED)
+        .build();
+  }
+}