PIE-1792: Added chainId validation to PrivateTransactionValidator (Pe…

…gaSysEng#1741)

* PIE-1792: Added chainId validation to PrivateTransactionValidator

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetProtocolSpecs.java

@@ -29,6 +29,7 @@
 import tech.pegasys.pantheon.ethereum.core.WorldUpdater;
 import tech.pegasys.pantheon.ethereum.mainnet.contractvalidation.MaxCodeSizeRule;
 import tech.pegasys.pantheon.ethereum.privacy.PrivateTransactionProcessor;
+import tech.pegasys.pantheon.ethereum.privacy.PrivateTransactionValidator;
 
 import java.io.IOException;
 import java.math.BigInteger;
@@ -107,15 +108,17 @@ public static ProtocolSpecBuilder<Void> frontierDefinition(
             (gasCalculator,
                 transactionValidator,
                 contractCreationProcessor,
-                messageCallProcessor) ->
+                messageCallProcessor,
+                privateTransactionValidator) ->
                 new PrivateTransactionProcessor(
                     gasCalculator,
                     transactionValidator,
                     contractCreationProcessor,
                     messageCallProcessor,
                     false,
                     stackSizeLimit,
-                    Account.DEFAULT_VERSION))
+                    Account.DEFAULT_VERSION,
+                    new PrivateTransactionValidator(Optional.empty())))
         .difficultyCalculator(MainnetDifficultyCalculators.FRONTIER)
         .blockHeaderValidatorBuilder(MainnetBlockHeaderValidator::create)
         .ommerHeaderValidatorBuilder(MainnetBlockHeaderValidator::createOmmerValidator)
@@ -223,19 +226,22 @@ public static ProtocolSpecBuilder<Void> spuriousDragonDefinition(
                     true,
                     stackSizeLimit,
                     Account.DEFAULT_VERSION))
+        .privateTransactionValidatorBuilder(() -> new PrivateTransactionValidator(chainId))
         .privateTransactionProcessorBuilder(
             (gasCalculator,
                 transactionValidator,
                 contractCreationProcessor,
-                messageCallProcessor) ->
+                messageCallProcessor,
+                privateTransactionValidator) ->
                 new PrivateTransactionProcessor(
                     gasCalculator,
                     transactionValidator,
                     contractCreationProcessor,
                     messageCallProcessor,
                     false,
                     stackSizeLimit,
-                    Account.DEFAULT_VERSION))
+                    Account.DEFAULT_VERSION,
+                    privateTransactionValidator))
         .name("SpuriousDragon");
   }
 
@@ -312,15 +318,17 @@ public static ProtocolSpecBuilder<Void> istanbulDefinition(
             (gasCalculator,
                 transactionValidator,
                 contractCreationProcessor,
-                messageCallProcessor) ->
+                messageCallProcessor,
+                privateTransactionValidator) ->
                 new PrivateTransactionProcessor(
                     gasCalculator,
                     transactionValidator,
                     contractCreationProcessor,
                     messageCallProcessor,
                     false,
                     stackSizeLimit,
-                    ISTANBUL_ACCOUNT_VERSION))
+                    ISTANBUL_ACCOUNT_VERSION,
+                    privateTransactionValidator))
         .contractCreationProcessorBuilder(
             (gasCalculator, evm) ->
                 new MainnetContractCreationProcessor(

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/ProtocolScheduleBuilder.java

@@ -14,6 +14,7 @@
 
 import tech.pegasys.pantheon.config.GenesisConfigOptions;
 import tech.pegasys.pantheon.ethereum.core.PrivacyParameters;
+import tech.pegasys.pantheon.ethereum.privacy.PrivateTransactionValidator;
 
 import java.math.BigInteger;
 import java.util.Optional;
@@ -24,6 +25,7 @@
 import org.apache.logging.log4j.Logger;
 
 public class ProtocolScheduleBuilder<C> {
+
   private static final Logger LOG = LogManager.getLogger();
   private final GenesisConfigOptions config;
   private final Function<ProtocolSpecBuilder<Void>, ProtocolSpecBuilder<C>> protocolSpecAdapter;
@@ -163,6 +165,8 @@ private void addProtocolSpec(
                 protocolSpecAdapter
                     .apply(definition)
                     .privacyParameters(privacyParameters)
+                    .privateTransactionValidatorBuilder(
+                        () -> new PrivateTransactionValidator(protocolSchedule.getChainId()))
                     .build(protocolSchedule)));
   }
 

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/ProtocolSpecBuilder.java

@@ -24,6 +24,7 @@
 import tech.pegasys.pantheon.ethereum.mainnet.MainnetBlockProcessor.TransactionReceiptFactory;
 import tech.pegasys.pantheon.ethereum.mainnet.precompiles.privacy.PrivacyPrecompiledContract;
 import tech.pegasys.pantheon.ethereum.privacy.PrivateTransactionProcessor;
+import tech.pegasys.pantheon.ethereum.privacy.PrivateTransactionValidator;
 import tech.pegasys.pantheon.ethereum.vm.EVM;
 import tech.pegasys.pantheon.ethereum.vm.GasCalculator;
 
@@ -56,6 +57,7 @@ public class ProtocolSpecBuilder<T> {
   private MiningBeneficiaryCalculator miningBeneficiaryCalculator;
   private PrivacyParameters privacyParameters;
   private PrivateTransactionProcessorBuilder privateTransactionProcessorBuilder;
+  private PrivateTransactionValidatorBuilder privateTransactionValidatorBuilder;
 
   public ProtocolSpecBuilder<T> gasCalculator(final Supplier<GasCalculator> gasCalculatorBuilder) {
     this.gasCalculatorBuilder = gasCalculatorBuilder;
@@ -160,6 +162,12 @@ public ProtocolSpecBuilder<T> privateTransactionProcessorBuilder(
     return this;
   }
 
+  public ProtocolSpecBuilder<T> privateTransactionValidatorBuilder(
+      final PrivateTransactionValidatorBuilder privateTransactionValidatorBuilder) {
+    this.privateTransactionValidatorBuilder = privateTransactionValidatorBuilder;
+    return this;
+  }
+
   public ProtocolSpecBuilder<T> blockProcessorBuilder(
       final BlockProcessorBuilder blockProcessorBuilder) {
     this.blockProcessorBuilder = blockProcessorBuilder;
@@ -211,6 +219,7 @@ public <R> ProtocolSpecBuilder<R> changeConsensusContextType(
         .gasCalculator(gasCalculatorBuilder)
         .evmBuilder(evmBuilder)
         .transactionValidatorBuilder(transactionValidatorBuilder)
+        .privateTransactionValidatorBuilder(privateTransactionValidatorBuilder)
         .contractCreationProcessorBuilder(contractCreationProcessorBuilder)
         .privacyParameters(privacyParameters)
         .precompileContractRegistryBuilder(precompileContractRegistryBuilder)
@@ -236,6 +245,7 @@ public ProtocolSpec<T> build(final ProtocolSchedule<T> protocolSchedule) {
     checkNotNull(gasCalculatorBuilder, "Missing gasCalculator");
     checkNotNull(evmBuilder, "Missing operation registry");
     checkNotNull(transactionValidatorBuilder, "Missing transaction validator");
+    checkNotNull(privateTransactionValidatorBuilder, "Missing private transaction validator");
     checkNotNull(contractCreationProcessorBuilder, "Missing contract creation processor");
     checkNotNull(precompileContractRegistryBuilder, "Missing precompile contract registry");
     checkNotNull(messageCallProcessorBuilder, "Missing message call processor");
@@ -274,9 +284,15 @@ public ProtocolSpec<T> build(final ProtocolSchedule<T> protocolSchedule) {
 
     // Set private Tx Processor
     if (privacyParameters.isEnabled()) {
+      final PrivateTransactionValidator privateTransactionValidator =
+          privateTransactionValidatorBuilder.apply();
       final PrivateTransactionProcessor privateTransactionProcessor =
           privateTransactionProcessorBuilder.apply(
-              gasCalculator, transactionValidator, contractCreationProcessor, messageCallProcessor);
+              gasCalculator,
+              transactionValidator,
+              contractCreationProcessor,
+              messageCallProcessor,
+              privateTransactionValidator);
       Address address = Address.privacyPrecompiled(privacyParameters.getPrivacyAddress());
       PrivacyPrecompiledContract privacyPrecompiledContract =
           (PrivacyPrecompiledContract)
@@ -332,7 +348,12 @@ PrivateTransactionProcessor apply(
         GasCalculator gasCalculator,
         TransactionValidator transactionValidator,
         AbstractMessageProcessor contractCreationProcessor,
-        AbstractMessageProcessor messageCallProcessor);
+        AbstractMessageProcessor messageCallProcessor,
+        PrivateTransactionValidator privateTransactionValidator);
+  }
+
+  public interface PrivateTransactionValidatorBuilder {
+    PrivateTransactionValidator apply();
   }
 
   public interface BlockProcessorBuilder {

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

@@ -31,7 +31,9 @@
 import tech.pegasys.pantheon.ethereum.worldstate.WorldStateArchive;
 import tech.pegasys.pantheon.util.bytes.BytesValues;
 
+import java.math.BigInteger;
 import java.util.List;
+import java.util.Optional;
 import java.util.stream.Collectors;
 
 import org.apache.logging.log4j.LogManager;
@@ -50,15 +52,16 @@ public class PrivateTransactionHandler {
   private final WorldStateArchive privateWorldStateArchive;
   private final PrivateTransactionValidator privateTransactionValidator;
 
-  public PrivateTransactionHandler(final PrivacyParameters privacyParameters) {
+  public PrivateTransactionHandler(
+      final PrivacyParameters privacyParameters, final Optional<BigInteger> chainId) {
     this(
         new Enclave(privacyParameters.getEnclaveUri()),
         Address.privacyPrecompiled(privacyParameters.getPrivacyAddress()),
         privacyParameters.getSigningKeyPair(),
         privacyParameters.getEnclavePublicKey(),
         privacyParameters.getPrivateStateStorage(),
         privacyParameters.getPrivateWorldStateArchive(),
-        new PrivateTransactionValidator());
+        new PrivateTransactionValidator(chainId));
   }
 
   public PrivateTransactionHandler(

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

@@ -163,15 +163,16 @@ public PrivateTransactionProcessor(
       final AbstractMessageProcessor messageCallProcessor,
       final boolean clearEmptyAccounts,
       final int maxStackSize,
-      final int createContractAccountVersion) {
+      final int createContractAccountVersion,
+      final PrivateTransactionValidator privateTransactionValidator) {
     this.gasCalculator = gasCalculator;
     this.transactionValidator = transactionValidator;
-    this.privateTransactionValidator = new PrivateTransactionValidator();
     this.contractCreationProcessor = contractCreationProcessor;
     this.messageCallProcessor = messageCallProcessor;
     this.clearEmptyAccounts = clearEmptyAccounts;
     this.maxStackSize = maxStackSize;
     this.createContractAccountVersion = createContractAccountVersion;
+    this.privateTransactionValidator = privateTransactionValidator;
   }
 
   @SuppressWarnings("unused")

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

@@ -13,20 +13,41 @@
 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.INVALID_SIGNATURE;
 import static tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason.PRIVATE_NONCE_TOO_LOW;
+import static tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason.REPLAY_PROTECTED_SIGNATURES_NOT_SUPPORTED;
+import static tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason.WRONG_CHAIN_ID;
 
 import tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason;
 import tech.pegasys.pantheon.ethereum.mainnet.ValidationResult;
 
+import java.math.BigInteger;
+import java.util.Optional;
+
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
-class PrivateTransactionValidator {
+public class PrivateTransactionValidator {
 
   private static final Logger LOG = LogManager.getLogger();
 
+  private final Optional<BigInteger> chainId;
+
+  public PrivateTransactionValidator(final Optional<BigInteger> chainId) {
+    this.chainId = chainId;
+  }
+
   public ValidationResult<TransactionInvalidReason> validate(
       final PrivateTransaction transaction, final Long accountNonce) {
+
+    LOG.debug("Validating private transaction {} signature ", transaction.hash());
+
+    ValidationResult<TransactionInvalidReason> signatureValidationResult =
+        validateTransactionSignature(transaction);
+    if (!signatureValidationResult.isValid()) {
+      return signatureValidationResult;
+    }
+
     final long transactionNonce = transaction.getNonce();
 
     LOG.debug("Validating actual nonce {} with expected nonce {}", transactionNonce, accountNonce);
@@ -49,4 +70,33 @@ public ValidationResult<TransactionInvalidReason> validate(
 
     return ValidationResult.valid();
   }
+
+  public ValidationResult<TransactionInvalidReason> validateTransactionSignature(
+      final PrivateTransaction transaction) {
+    if (chainId.isPresent()
+        && (transaction.getChainId().isPresent() && !transaction.getChainId().equals(chainId))) {
+      return ValidationResult.invalid(
+          WRONG_CHAIN_ID,
+          String.format(
+              "transaction was meant for chain id %s and not this chain id %s",
+              transaction.getChainId().get(), chainId.get()));
+    }
+
+    if (!chainId.isPresent() && transaction.getChainId().isPresent()) {
+      return ValidationResult.invalid(
+          REPLAY_PROTECTED_SIGNATURES_NOT_SUPPORTED,
+          "replay protected signatures is not supported");
+    }
+
+    // org.bouncycastle.math.ec.ECCurve.AbstractFp.decompressPoint throws an
+    // IllegalArgumentException for "Invalid point compression" for bad signatures.
+    try {
+      transaction.getSender();
+    } catch (final IllegalArgumentException e) {
+      return ValidationResult.invalid(
+          INVALID_SIGNATURE, "sender could not be extracted from transaction signature");
+    }
+
+    return ValidationResult.valid();
+  }
 }