Refactoring and extra logging to better expose flakey behaviour. (#1576)

Signed-off-by: Mark Terry <mark.terry@consensys.net>

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/privacy/methods/eea/EeaSendRawTransaction.java

@@ -14,10 +14,13 @@
  */
 package org.hyperledger.besu.ethereum.api.jsonrpc.internal.privacy.methods.eea;
 
+import static org.apache.logging.log4j.LogManager.getLogger;
 import static org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcEnclaveErrorConverter.convertEnclaveInvalidReason;
 import static org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcErrorConverter.convertTransactionInvalidReason;
 import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError.DECODE_ERROR;
 import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError.PRIVATE_FROM_DOES_NOT_MATCH_ENCLAVE_PUBLIC_KEY;
+import static org.hyperledger.besu.ethereum.privacy.PrivacyGroupUtil.findOffchainPrivacyGroup;
+import static org.hyperledger.besu.ethereum.privacy.PrivacyGroupUtil.findOnchainPrivacyGroup;
 
 import org.hyperledger.besu.enclave.types.PrivacyGroup;
 import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
@@ -40,14 +43,17 @@ import org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason;
 
 import java.util.Optional;
 
+import org.apache.logging.log4j.Logger;
 import org.apache.tuweni.bytes.Bytes;
 import org.apache.tuweni.bytes.Bytes32;
 
 public class EeaSendRawTransaction implements JsonRpcMethod {
 
+  private static final Logger LOG = getLogger();
   private final TransactionPool transactionPool;
   private final PrivacyController privacyController;
   private final EnclavePublicKeyProvider enclavePublicKeyProvider;
+
   /*
    Temporarily adding this flag to this method to avoid being able to use offchain and onchain
    privacy groups at the same time. Later on this check will be done in a better place.
@@ -86,30 +92,25 @@ public class EeaSendRawTransaction implements JsonRpcMethod {
         return new JsonRpcErrorResponse(id, PRIVATE_FROM_DOES_NOT_MATCH_ENCLAVE_PUBLIC_KEY);
       }
 
-      Optional<PrivacyGroup> maybePrivacyGroup = null;
       final Optional<Bytes> maybePrivacyGroupId = privateTransaction.getPrivacyGroupId();
-      if (onchainPrivacyGroupsEnabled) {
+
-        if (!maybePrivacyGroupId.isPresent()) {
+      if (onchainPrivacyGroupsEnabled && maybePrivacyGroupId.isEmpty()) {
-          return new JsonRpcErrorResponse(id, JsonRpcError.ONCHAIN_PRIVACY_GROUP_ID_NOT_AVAILABLE);
+        return new JsonRpcErrorResponse(id, JsonRpcError.ONCHAIN_PRIVACY_GROUP_ID_NOT_AVAILABLE);
-        }
+      }
-        maybePrivacyGroup =
+
-            privacyController.retrieveOnChainPrivacyGroupWithToBeAddedMembers(
+      final Optional<PrivacyGroup> maybePrivacyGroup =
-                maybePrivacyGroupId.get(), enclavePublicKey, privateTransaction);
+          onchainPrivacyGroupsEnabled
-        if (maybePrivacyGroup.isEmpty()) {
+              ? findOnchainPrivacyGroup(
-          return new JsonRpcErrorResponse(id, JsonRpcError.ONCHAIN_PRIVACY_GROUP_DOES_NOT_EXIST);
+                  privacyController, maybePrivacyGroupId, enclavePublicKey, privateTransaction)
-        }
+              : findOffchainPrivacyGroup(privacyController, maybePrivacyGroupId, enclavePublicKey);
-      } else { // !onchainPrivacyGroupEnabled
+
-        if (maybePrivacyGroupId.isPresent()) {
+      if (onchainPrivacyGroupsEnabled && maybePrivacyGroup.isEmpty()) {
-          maybePrivacyGroup =
+        return new JsonRpcErrorResponse(id, JsonRpcError.ONCHAIN_PRIVACY_GROUP_DOES_NOT_EXIST);
-              privacyController.retrieveOffChainPrivacyGroup(
-                  maybePrivacyGroupId.get().toBase64String(), enclavePublicKey);
-        } else {
-          maybePrivacyGroup = Optional.empty();
-        }
       }
 
       final ValidationResult<TransactionInvalidReason> validationResult =
           privacyController.validatePrivateTransaction(privateTransaction, enclavePublicKey);
+
       if (!validationResult.isValid()) {
         return new JsonRpcErrorResponse(
             id, convertTransactionInvalidReason(validationResult.getInvalidReason()));
@@ -140,10 +141,10 @@ public class EeaSendRawTransaction implements JsonRpcMethod {
             id, privateTransaction, privateTransactionLookupId, Address.DEFAULT_PRIVACY);
       }
     } catch (final IllegalArgumentException | RLPException e) {
+      LOG.error(e);
       return new JsonRpcErrorResponse(id, DECODE_ERROR);
     } catch (final Exception e) {
-      final String message = e.getMessage();
+      return new JsonRpcErrorResponse(id, convertEnclaveInvalidReason(e.getMessage()));
-      return new JsonRpcErrorResponse(id, convertEnclaveInvalidReason(message));
     }
   }
 

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/privacy/methods/priv/PrivDistributeRawTransaction.java

@@ -20,6 +20,8 @@ import static org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcErrorConverter.co
 import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError.DECODE_ERROR;
 import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError.ENCLAVE_ERROR;
 import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError.PRIVATE_FROM_DOES_NOT_MATCH_ENCLAVE_PUBLIC_KEY;
+import static org.hyperledger.besu.ethereum.privacy.PrivacyGroupUtil.findOffchainPrivacyGroup;
+import static org.hyperledger.besu.ethereum.privacy.PrivacyGroupUtil.findOnchainPrivacyGroup;
 
 import org.hyperledger.besu.enclave.types.PrivacyGroup;
 import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
@@ -81,30 +83,25 @@ public class PrivDistributeRawTransaction implements JsonRpcMethod {
         return new JsonRpcErrorResponse(id, PRIVATE_FROM_DOES_NOT_MATCH_ENCLAVE_PUBLIC_KEY);
       }
 
-      Optional<PrivacyGroup> maybePrivacyGroup = null;
       final Optional<Bytes> maybePrivacyGroupId = privateTransaction.getPrivacyGroupId();
-      if (onchainPrivacyGroupsEnabled) {
+
-        if (!maybePrivacyGroupId.isPresent()) {
+      if (onchainPrivacyGroupsEnabled && maybePrivacyGroupId.isEmpty()) {
-          return new JsonRpcErrorResponse(id, JsonRpcError.ONCHAIN_PRIVACY_GROUP_ID_NOT_AVAILABLE);
+        return new JsonRpcErrorResponse(id, JsonRpcError.ONCHAIN_PRIVACY_GROUP_ID_NOT_AVAILABLE);
-        }
+      }
-        maybePrivacyGroup =
+
-            privacyController.retrieveOnChainPrivacyGroupWithToBeAddedMembers(
+      final Optional<PrivacyGroup> maybePrivacyGroup =
-                maybePrivacyGroupId.get(), enclavePublicKey, privateTransaction);
+          onchainPrivacyGroupsEnabled
-        if (maybePrivacyGroup.isEmpty()) {
+              ? findOnchainPrivacyGroup(
-          return new JsonRpcErrorResponse(id, JsonRpcError.ONCHAIN_PRIVACY_GROUP_DOES_NOT_EXIST);
+                  privacyController, maybePrivacyGroupId, enclavePublicKey, privateTransaction)
-        }
+              : findOffchainPrivacyGroup(privacyController, maybePrivacyGroupId, enclavePublicKey);
-      } else { // !onchainPrivacyGroupEnabled
+
-        if (maybePrivacyGroupId.isPresent()) {
+      if (onchainPrivacyGroupsEnabled && maybePrivacyGroup.isEmpty()) {
-          maybePrivacyGroup =
+        return new JsonRpcErrorResponse(id, JsonRpcError.ONCHAIN_PRIVACY_GROUP_DOES_NOT_EXIST);
-              privacyController.retrieveOffChainPrivacyGroup(
-                  maybePrivacyGroupId.get().toBase64String(), enclavePublicKey);
-        } else {
-          maybePrivacyGroup = Optional.empty();
-        }
       }
 
       final ValidationResult<TransactionInvalidReason> validationResult =
           privacyController.validatePrivateTransaction(privateTransaction, enclavePublicKey);
+
       if (!validationResult.isValid()) {
         return new JsonRpcErrorResponse(
             id, convertTransactionInvalidReason(validationResult.getInvalidReason()));
@@ -118,6 +115,7 @@ public class PrivDistributeRawTransaction implements JsonRpcMethod {
       LOG.error("Unauthorized privacy multi-tenancy rpc request. {}", e.getMessage());
       return new JsonRpcErrorResponse(id, ENCLAVE_ERROR);
     } catch (final IllegalArgumentException | RLPException e) {
+      LOG.error(e);
       return new JsonRpcErrorResponse(id, DECODE_ERROR);
     } catch (final Exception e) {
       return new JsonRpcErrorResponse(id, convertEnclaveInvalidReason(e.getMessage()));

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/privacy/PrivacyGroupUtil.java

@@ -15,12 +15,14 @@
 package org.hyperledger.besu.ethereum.privacy;
 
 import org.hyperledger.besu.crypto.Hash;
+import org.hyperledger.besu.enclave.types.PrivacyGroup;
 import org.hyperledger.besu.ethereum.rlp.BytesValueRLPOutput;
 
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Comparator;
 import java.util.List;
+import java.util.Optional;
 import java.util.stream.Collectors;
 
 import org.apache.tuweni.bytes.Bytes;
@@ -52,4 +54,25 @@ public class PrivacyGroupUtil {
 
     return Hash.keccak256(bytesValueRLPOutput.encoded());
   }
+
+  public static Optional<PrivacyGroup> findOnchainPrivacyGroup(
+      final PrivacyController privacyController,
+      final Optional<Bytes> maybePrivacyGroupId,
+      final String enclavePublicKey,
+      final PrivateTransaction privateTransaction) {
+    return maybePrivacyGroupId.flatMap(
+        privacyGroupId ->
+            privacyController.retrieveOnChainPrivacyGroupWithToBeAddedMembers(
+                privacyGroupId, enclavePublicKey, privateTransaction));
+  }
+
+  public static Optional<PrivacyGroup> findOffchainPrivacyGroup(
+      final PrivacyController privacyController,
+      final Optional<Bytes> maybePrivacyGroupId,
+      final String enclavePublicKey) {
+    return maybePrivacyGroupId.flatMap(
+        privacyGroupId ->
+            privacyController.retrieveOffChainPrivacyGroup(
+                privacyGroupId.toBase64String(), enclavePublicKey));
+  }
 }