hyperledger#628 Deprecating Private Transaction hash/getHash()

Signed-off-by: Lucas Saldanha <[email protected]>
lucassaldanha · Apr 2, 2020 · ac216cd · ac216cd
1 parent 865b10b
commit ac216cd
Show file tree

Hide file tree

Showing 11 changed files with 36 additions and 24 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## 1.4.3 
 
+### Additions and Improvements
+- Private Transaction `hash` field and `getHash()` method have been deprecated. They will be removed 
+in 1.5.0 release. 
+
 ### Critical Issue for Privacy Users 
 
 A critical issue for privacy users with private transactions created using Hyperledger Besu v1.3.4 

diff --git a/.../besu/ethereum/mainnet/precompiles/privacy/PrivacyPrecompiledContractIntegrationTest.java b/.../besu/ethereum/mainnet/precompiles/privacy/PrivacyPrecompiledContractIntegrationTest.java
@@ -28,6 +28,7 @@
 import org.hyperledger.besu.ethereum.core.Address;
 import org.hyperledger.besu.ethereum.core.Block;
 import org.hyperledger.besu.ethereum.core.BlockDataGenerator;
+import org.hyperledger.besu.ethereum.core.Hash;
 import org.hyperledger.besu.ethereum.core.MutableWorldState;
 import org.hyperledger.besu.ethereum.core.ProcessableBlockHeader;
 import org.hyperledger.besu.ethereum.core.WorldUpdater;
@@ -96,6 +97,7 @@ private PrivateTransactionProcessor mockPrivateTxProcessor() {
             nullable(WorldUpdater.class),
             nullable(WorldUpdater.class),
             nullable(ProcessableBlockHeader.class),
+            nullable(Hash.class),
             nullable(PrivateTransaction.class),
             nullable(Address.class),
             nullable(OperationTracer.class),

diff --git a/...erledger/besu/ethereum/mainnet/precompiles/privacy/OnChainPrivacyPrecompiledContract.java b/...erledger/besu/ethereum/mainnet/precompiles/privacy/OnChainPrivacyPrecompiledContract.java
@@ -85,6 +85,8 @@ public Bytes compute(final Bytes input, final MessageFrame messageFrame) {
       return Bytes.EMPTY;
     }
 
+    final Hash pmtHash = messageFrame.getTransactionHash();
+
     final String key = input.slice(0, 32).toBase64String();
 
     final ReceiveResponse receiveResponse;
@@ -111,10 +113,7 @@ public Bytes compute(final Bytes input, final MessageFrame messageFrame) {
 
     final Bytes32 privacyGroupId = Bytes32.wrap(maybeGroupId.get());
 
-    LOG.debug(
-        "Processing private transaction {} in privacy group {}",
-        privateTransaction.getHash(),
-        privacyGroupId);
+    LOG.debug("Processing private transaction {} in privacy group {}", pmtHash, privacyGroupId);
 
     final ProcessableBlockHeader currentBlockHeader = messageFrame.getBlockHeader();
     final Hash currentBlockHash = ((BlockHeader) currentBlockHeader).getHash();
@@ -153,14 +152,14 @@ public Bytes compute(final Bytes input, final MessageFrame messageFrame) {
     if (result.isInvalid() || !result.isSuccessful()) {
       LOG.error(
           "Failed to process private transaction {}: {}",
-          privateTransaction.getHash(),
+          pmtHash,
           result.getValidationResult().getErrorMessage());
       return Bytes.EMPTY;
     }
 
     if (messageFrame.isPersistingPrivateState()) {
       persistPrivateState(
-          messageFrame.getTransactionHash(),
+          pmtHash,
           currentBlockHash,
           privateTransaction,
           privacyGroupId,
@@ -272,6 +271,7 @@ protected PrivateTransactionProcessor.Result checkCanExecute(
         publicWorldState,
         canExecuteUpdater,
         currentBlockHeader,
+        messageFrame.getTransactionHash(),
         buildSimulationTransaction(
             privacyGroupId, privateWorldStateUpdater, canExecuteMethodSignature),
         messageFrame.getMiningBeneficiary(),

diff --git a/...org/hyperledger/besu/ethereum/mainnet/precompiles/privacy/PrivacyPrecompiledContract.java b/...org/hyperledger/besu/ethereum/mainnet/precompiles/privacy/PrivacyPrecompiledContract.java
@@ -109,6 +109,8 @@ public Bytes compute(final Bytes input, final MessageFrame messageFrame) {
       return Bytes.EMPTY;
     }
 
+    final Hash pmtHash = messageFrame.getTransactionHash();
+
     final String key = input.toBase64String();
     final ReceiveResponse receiveResponse;
     try {
@@ -127,10 +129,7 @@ public Bytes compute(final Bytes input, final MessageFrame messageFrame) {
     final Bytes32 privacyGroupId =
         Bytes32.wrap(Bytes.fromBase64String(receiveResponse.getPrivacyGroupId()));
 
-    LOG.debug(
-        "Processing private transaction {} in privacy group {}",
-        privateTransaction.getHash(),
-        privacyGroupId);
+    LOG.debug("Processing private transaction {} in privacy group {}", pmtHash, privacyGroupId);
 
     final Hash currentBlockHash = ((BlockHeader) messageFrame.getBlockHeader()).getHash();
 
@@ -149,15 +148,14 @@ public Bytes compute(final Bytes input, final MessageFrame messageFrame) {
     if (result.isInvalid() || !result.isSuccessful()) {
       LOG.error(
           "Failed to process private transaction {}: {}",
-          privateTransaction.getHash(),
+          pmtHash,
           result.getValidationResult().getErrorMessage());
       return Bytes.EMPTY;
     }
 
     if (messageFrame.isPersistingPrivateState()) {
-
       persistPrivateState(
-          messageFrame.getTransactionHash(),
+          pmtHash,
           currentBlockHash,
           privateTransaction,
           privacyGroupId,
@@ -237,6 +235,7 @@ PrivateTransactionProcessor.Result processPrivateTransaction(
         messageFrame.getWorldState(),
         privateWorldStateUpdater,
         messageFrame.getBlockHeader(),
+        messageFrame.getTransactionHash(),
         privateTransaction,
         messageFrame.getMiningBeneficiary(),
         new DebugOperationTracer(TraceOptions.DEFAULT),

diff --git a/...ain/java/org/hyperledger/besu/ethereum/privacy/PrivateGroupRehydrationBlockProcessor.java b/...ain/java/org/hyperledger/besu/ethereum/privacy/PrivateGroupRehydrationBlockProcessor.java
@@ -128,14 +128,15 @@ public AbstractBlockProcessor.Result processBlock(
         LOG.debug(
             "Pre-rehydrate root hash: {} for tx {}",
             disposablePrivateState.rootHash(),
-            privateTransaction.getHash());
+            transaction.getHash());
 
         final PrivateTransactionProcessor.Result privateResult =
             privateTransactionProcessor.processTransaction(
                 blockchain,
                 worldStateUpdater.updater(),
                 privateStateUpdater,
                 blockHeader,
+                transaction.getHash(),
                 privateTransaction,
                 miningBeneficiary,
                 OperationTracer.NO_TRACING,

diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/privacy/PrivateTransaction.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/privacy/PrivateTransaction.java
@@ -87,6 +87,8 @@ public class PrivateTransaction {
   protected volatile Address sender;
 
   // Caches the hash used to uniquely identify the transaction.
+  // This field will be removed in 1.5.0
+  @Deprecated(since = "1.4.3")
   protected volatile Hash hash;
 
   public static Builder builder() {
@@ -414,8 +416,11 @@ public BigInteger getV() {
   /**
    * Returns the transaction hash.
    *
+   * @deprecated All private transactions should be identified by their correspondent PMT hash.
    * @return the transaction hash
    */
+  // This field will be removed in 1.5.0
+  @Deprecated(since = "1.4.3")
   public Hash getHash() {
     if (hash == null) {
       final Bytes rlp = RLP.encode(this::writeTo);

diff --git a/...core/src/main/java/org/hyperledger/besu/ethereum/privacy/PrivateTransactionProcessor.java b/...core/src/main/java/org/hyperledger/besu/ethereum/privacy/PrivateTransactionProcessor.java
@@ -19,6 +19,7 @@
 import org.hyperledger.besu.ethereum.core.Address;
 import org.hyperledger.besu.ethereum.core.DefaultEvmAccount;
 import org.hyperledger.besu.ethereum.core.Gas;
+import org.hyperledger.besu.ethereum.core.Hash;
 import org.hyperledger.besu.ethereum.core.Log;
 import org.hyperledger.besu.ethereum.core.MutableAccount;
 import org.hyperledger.besu.ethereum.core.ProcessableBlockHeader;
@@ -181,6 +182,7 @@ public Result processTransaction(
       final WorldUpdater publicWorldState,
       final WorldUpdater privateWorldState,
       final ProcessableBlockHeader blockHeader,
+      final Hash pmtHash,
       final PrivateTransaction transaction,
       final Address miningBeneficiary,
       final OperationTracer operationTracer,
@@ -248,7 +250,7 @@ public Result processTransaction(
               .miningBeneficiary(miningBeneficiary)
               .blockHashLookup(blockHashLookup)
               .maxStackSize(maxStackSize)
-              .transactionHash(transaction.getHash())
+              .transactionHash(pmtHash)
               .build();
 
     } else {
@@ -279,7 +281,7 @@ public Result processTransaction(
               .miningBeneficiary(miningBeneficiary)
               .blockHashLookup(blockHashLookup)
               .maxStackSize(maxStackSize)
-              .transactionHash(transaction.getHash())
+              .transactionHash(pmtHash)
               .build();
     }
 

diff --git a/...core/src/main/java/org/hyperledger/besu/ethereum/privacy/PrivateTransactionSimulator.java b/...core/src/main/java/org/hyperledger/besu/ethereum/privacy/PrivateTransactionSimulator.java
@@ -122,6 +122,7 @@ private Optional<PrivateTransactionProcessor.Result> process(
             publicWorldState.updater(),
             disposablePrivateState.updater(),
             header,
+            Hash.ZERO, // PMT hash is not needed as this private transaction doesn't exist
             transaction,
             protocolSpec.getMiningBeneficiaryCalculator().calculateBeneficiary(header),
             new DebugOperationTracer(TraceOptions.DEFAULT),

diff --git a/...core/src/main/java/org/hyperledger/besu/ethereum/privacy/PrivateTransactionValidator.java b/...core/src/main/java/org/hyperledger/besu/ethereum/privacy/PrivateTransactionValidator.java
@@ -38,25 +38,21 @@ public ValidationResult<TransactionValidator.TransactionInvalidReason> validate(
       final PrivateTransaction transaction,
       final Long accountNonce,
       final boolean allowFutureNonces) {
-    LOG.debug("Validating private transaction fields of {}", transaction.getHash());
+    LOG.debug("Validating private transaction {}", transaction);
     final ValidationResult<TransactionInvalidReason> privateFieldsValidationResult =
         validatePrivateTransactionFields(transaction);
     if (!privateFieldsValidationResult.isValid()) {
       LOG.debug(
-          "Private Transaction fields are invalid {}, {}",
-          transaction.getHash(),
+          "Private Transaction fields are invalid {}",
           privateFieldsValidationResult.getErrorMessage());
       return privateFieldsValidationResult;
     }
 
-    LOG.debug("Validating the signature of Private Transaction {} ", transaction.getHash());
-
     final ValidationResult<TransactionValidator.TransactionInvalidReason>
         signatureValidationResult = validateTransactionSignature(transaction);
     if (!signatureValidationResult.isValid()) {
       LOG.debug(
-          "Private Transaction {}, failed validation {}, {}",
-          transaction.getHash(),
+          "Private Transaction failed signature validation {}, {}",
           signatureValidationResult.getInvalidReason(),
           signatureValidationResult.getErrorMessage());
       return signatureValidationResult;

diff --git a/...m/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/PrivacyBlockProcessorTest.java b/...m/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/PrivacyBlockProcessorTest.java
@@ -194,7 +194,7 @@ private ProtocolSpec mockProtocolSpec() {
     final PrivateTransactionProcessor mockPrivateTransactionProcessor =
         mock(PrivateTransactionProcessor.class);
     when(mockPrivateTransactionProcessor.processTransaction(
-            any(), any(), any(), any(), any(), any(), any(), any(), any()))
+            any(), any(), any(), any(), any(), any(), any(), any(), any(), any()))
         .thenReturn(
             PrivateTransactionProcessor.Result.successful(
                 Collections.emptyList(), 0, Bytes.EMPTY, ValidationResult.valid()));

diff --git a/...hyperledger/besu/ethereum/mainnet/precompiles/privacy/PrivacyPrecompiledContractTest.java b/...hyperledger/besu/ethereum/mainnet/precompiles/privacy/PrivacyPrecompiledContractTest.java
@@ -28,6 +28,7 @@
 import org.hyperledger.besu.ethereum.core.Address;
 import org.hyperledger.besu.ethereum.core.Block;
 import org.hyperledger.besu.ethereum.core.BlockDataGenerator;
+import org.hyperledger.besu.ethereum.core.Hash;
 import org.hyperledger.besu.ethereum.core.Log;
 import org.hyperledger.besu.ethereum.core.MutableWorldState;
 import org.hyperledger.besu.ethereum.core.PrivateTransactionDataFixture;
@@ -79,6 +80,7 @@ private PrivateTransactionProcessor mockPrivateTxProcessor() {
             nullable(WorldUpdater.class),
             nullable(WorldUpdater.class),
             nullable(ProcessableBlockHeader.class),
+            nullable((Hash.class)),
             nullable(PrivateTransaction.class),
             nullable(Address.class),
             nullable(OperationTracer.class),