refactor GoQuorum private transaction detection (hyperledger#2644)

Signed-off-by: Taccat Isid <taccatisid@protonmail.com>

Co-authored-by: Taccat Isid <taccatisid@protonmail.com>
Co-authored-by: Sally MacFarlane <sally.macfarlane@consensys.net>

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/Transaction.java

@@ -28,6 +28,7 @@
 import org.hyperledger.besu.ethereum.rlp.RLP;
 import org.hyperledger.besu.ethereum.rlp.RLPInput;
 import org.hyperledger.besu.ethereum.rlp.RLPOutput;
+import org.hyperledger.besu.ethereum.transaction.GoQuorumPrivateTransactionDetector;
 import org.hyperledger.besu.plugin.data.Quantity;
 import org.hyperledger.besu.plugin.data.TransactionType;
 
@@ -635,11 +636,10 @@ public boolean isGoQuorumPrivateTransaction() {
     if (chainId.isPresent()) {
       return false;
     }
-    return v.map(
-            value ->
-                GO_QUORUM_PRIVATE_TRANSACTION_V_VALUE_MIN.equals(value)
-                    || GO_QUORUM_PRIVATE_TRANSACTION_V_VALUE_MAX.equals(value))
-        .orElse(false);
+    if (!v.isPresent()) {
+      return false;
+    }
+    return GoQuorumPrivateTransactionDetector.isGoQuorumPrivateTransactionV(v.get());
   }
 
   private static Bytes32 computeSenderRecoveryHash(

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/encoding/TransactionDecoder.java

@@ -15,15 +15,13 @@
 package org.hyperledger.besu.ethereum.core.encoding;
 
 import static com.google.common.base.Preconditions.checkNotNull;
-import static org.hyperledger.besu.ethereum.core.Transaction.GO_QUORUM_PRIVATE_TRANSACTION_V_VALUE_MAX;
 import static org.hyperledger.besu.ethereum.core.Transaction.GO_QUORUM_PRIVATE_TRANSACTION_V_VALUE_MIN;
 import static org.hyperledger.besu.ethereum.core.Transaction.REPLAY_PROTECTED_V_BASE;
 import static org.hyperledger.besu.ethereum.core.Transaction.REPLAY_PROTECTED_V_MIN;
 import static org.hyperledger.besu.ethereum.core.Transaction.REPLAY_UNPROTECTED_V_BASE;
 import static org.hyperledger.besu.ethereum.core.Transaction.REPLAY_UNPROTECTED_V_BASE_PLUS_1;
 import static org.hyperledger.besu.ethereum.core.Transaction.TWO;
 
-import org.hyperledger.besu.config.GoQuorumOptions;
 import org.hyperledger.besu.crypto.SECPSignature;
 import org.hyperledger.besu.crypto.SignatureAlgorithm;
 import org.hyperledger.besu.crypto.SignatureAlgorithmFactory;
@@ -33,6 +31,7 @@
 import org.hyperledger.besu.ethereum.core.Wei;
 import org.hyperledger.besu.ethereum.rlp.RLP;
 import org.hyperledger.besu.ethereum.rlp.RLPInput;
+import org.hyperledger.besu.ethereum.transaction.GoQuorumPrivateTransactionDetector;
 import org.hyperledger.besu.plugin.data.TransactionType;
 
 import java.math.BigInteger;
@@ -104,7 +103,7 @@ static Transaction decodeFrontier(final RLPInput input) {
     final BigInteger v = input.readBigIntegerScalar();
     final byte recId;
     Optional<BigInteger> chainId = Optional.empty();
-    if (isGoQuorumPrivateTransaction(v)) {
+    if (GoQuorumPrivateTransactionDetector.isGoQuorumPrivateTransactionV(v)) {
       builder.v(v);
       recId = v.subtract(GO_QUORUM_PRIVATE_TRANSACTION_V_VALUE_MIN).byteValueExact();
     } else if (v.equals(REPLAY_UNPROTECTED_V_BASE) || v.equals(REPLAY_UNPROTECTED_V_BASE_PLUS_1)) {
@@ -205,10 +204,4 @@ static Transaction decodeEIP1559(final RLPInput input) {
     input.leaveList();
     return transaction;
   }
-
-  private static boolean isGoQuorumPrivateTransaction(final BigInteger v) {
-    return GoQuorumOptions.goQuorumCompatibilityMode
-        && (v.equals(GO_QUORUM_PRIVATE_TRANSACTION_V_VALUE_MAX)
-            || v.equals(GO_QUORUM_PRIVATE_TRANSACTION_V_VALUE_MIN));
-  }
 }

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/transaction/GoQuorumPrivateTransactionDetector.java

@@ -0,0 +1,37 @@
+/*
+ * Copyright 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.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package org.hyperledger.besu.ethereum.transaction;
+
+import static org.hyperledger.besu.ethereum.core.Transaction.GO_QUORUM_PRIVATE_TRANSACTION_V_VALUE_MAX;
+import static org.hyperledger.besu.ethereum.core.Transaction.GO_QUORUM_PRIVATE_TRANSACTION_V_VALUE_MIN;
+
+import org.hyperledger.besu.config.GoQuorumOptions;
+
+import java.math.BigInteger;
+
+public class GoQuorumPrivateTransactionDetector {
+  /**
+   * Returns whether or not <i>v</i> indicates a GoQuorum private transaction.
+   *
+   * @param v the v value of a transaction
+   * @return true if GoQuorum private transaction, false otherwise
+   */
+  public static final boolean isGoQuorumPrivateTransactionV(final BigInteger v) {
+    return GoQuorumOptions.goQuorumCompatibilityMode
+        && ((v.compareTo(GO_QUORUM_PRIVATE_TRANSACTION_V_VALUE_MAX) <= 0)
+            && (v.compareTo(GO_QUORUM_PRIVATE_TRANSACTION_V_VALUE_MIN)) >= 0);
+  }
+}

ethereum/core/src/test/java/org/hyperledger/besu/ethereum/core/TransactionGoQuorumTest.java

@@ -20,6 +20,8 @@
 import org.hyperledger.besu.ethereum.rlp.RLP;
 import org.hyperledger.besu.ethereum.rlp.RLPInput;
 
+import java.math.BigInteger;
+
 import org.apache.tuweni.bytes.Bytes;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
@@ -73,6 +75,18 @@ public void givenGoQuorumTransactionV38_assertThatIsGoQuorumFlagIsTrue() {
     assertThat(transaction.hasCostParams()).isFalse();
   }
 
+  @Test
+  public void givenTransactionWithChainId_assertThatIsGoQuorumFlagIsFalse() {
+    final Transaction transaction = Transaction.builder().chainId(BigInteger.valueOf(0)).build();
+    assertThat(transaction.isGoQuorumPrivateTransaction()).isFalse();
+  }
+
+  @Test
+  public void givenTransactionWithoutChainIdAndV37_assertThatIsGoQuorumFlagIsTrue() {
+    final Transaction transaction = Transaction.builder().v(BigInteger.valueOf(37)).build();
+    assertThat(transaction.isGoQuorumPrivateTransaction()).isTrue();
+  }
+
   private static RLPInput toRLP(final String bytes) {
     return RLP.input(Bytes.fromHexString(bytes));
   }

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolTest.java

@@ -36,6 +36,7 @@
 import static org.mockito.Mockito.verifyZeroInteractions;
 import static org.mockito.Mockito.when;
 
+import org.hyperledger.besu.config.GoQuorumOptions;
 import org.hyperledger.besu.crypto.KeyPair;
 import org.hyperledger.besu.crypto.SignatureAlgorithmFactory;
 import org.hyperledger.besu.ethereum.ProtocolContext;
@@ -78,6 +79,7 @@
 import java.util.Optional;
 import java.util.Set;
 
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
@@ -160,6 +162,12 @@ public void setUp() {
     blockchain.observeBlockAdded(transactionPool);
   }
 
+  @After
+  public void tearDown() {
+    GoQuorumOptions.goQuorumCompatibilityMode =
+        GoQuorumOptions.GOQUORUM_COMPATIBILITY_MODE_DEFAULT_VALUE;
+  }
+
   @Test
   public void mainNetValueTransferSucceeds() {
     final Transaction transaction =
@@ -847,6 +855,8 @@ public void shouldRejectLocalTransactionIfFeeCapExceeded() {
 
   @Test
   public void shouldRejectGoQuorumTransactionWithNonZeroValue() {
+    GoQuorumOptions.goQuorumCompatibilityMode = true;
+
     final EthProtocolManager ethProtocolManager = EthProtocolManagerTestUtil.create();
     final EthContext ethContext = ethProtocolManager.ethContext();
     final PeerTransactionTracker peerTransactionTracker = new PeerTransactionTracker();