allow unlocking onchain privacy groups (#2717)

* allow onchain privacy group proxy calls under group lock

Prior to this commit, the only allowed transactions in locked onchain
privacy groups were those making the addParticipants call to *any*
contract--not just the proxy. This commit ensures that:
- while locked, only the group management proxy can be called
- while locked, all functions of the management proxy can be called

Corollary, this commit allows unlocking a group using unlock instead
of implicitely unlocking it via addParticipants. This fixes #2693.

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

* add acceptance test asserting that privacy groups can be unlocked

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

Co-authored-by: Sally MacFarlane <sally.macfarlane@consensys.net>

CHANGELOG.md

@@ -24,6 +24,7 @@
 - Consider effective price and effective priority fee in transaction replacement rules [\#2529](https://github.com/hyperledger/besu/issues/2529)
 - GetTransactionCount should return the latest transaction count if it is greater than the transaction pool [\#2633](https://github.com/hyperledger/besu/pull/2633)
 - Set an idle timeout for metrics connections, to clean up ports when no longer used [\#2748](https://github.com/hyperledger/besu/pull/2748)
+- Onchain privacy groups can be unlocked after being locked without having to add a participant [\#2693](https://github.com/hyperledger/besu/pull/2693)
 
 ### Early Access Features
 

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/privacy/transaction/PrivacyTransactions.java

@@ -78,6 +78,11 @@ public LockOnChainPrivacyGroupTransaction privxLockPrivacyGroupAndCheck(
     return new LockOnChainPrivacyGroupTransaction(privacyGroupId, locker, signer);
   }
 
+  public UnlockOnChainPrivacyGroupTransaction privxUnlockPrivacyGroupAndCheck(
+      final String privacyGroupId, final PrivacyNode locker, final Credentials signer) {
+    return new UnlockOnChainPrivacyGroupTransaction(privacyGroupId, locker, signer);
+  }
+
   public FindPrivacyGroupTransaction findPrivacyGroup(final List<String> nodes) {
     return new FindPrivacyGroupTransaction(nodes);
   }

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/privacy/transaction/UnlockOnChainPrivacyGroupTransaction.java

@@ -0,0 +1,47 @@
+/*
+ * 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.tests.acceptance.dsl.privacy.transaction;
+
+import org.hyperledger.besu.tests.acceptance.dsl.privacy.PrivacyNode;
+import org.hyperledger.besu.tests.acceptance.dsl.transaction.NodeRequests;
+import org.hyperledger.besu.tests.acceptance.dsl.transaction.Transaction;
+
+import java.io.IOException;
+
+import org.web3j.crypto.Credentials;
+import org.web3j.protocol.exceptions.TransactionException;
+import org.web3j.utils.Base64String;
+
+public class UnlockOnChainPrivacyGroupTransaction implements Transaction<String> {
+  private final Base64String privacyGroupId;
+  private final PrivacyNode locker;
+  private final Credentials signer;
+
+  public UnlockOnChainPrivacyGroupTransaction(
+      final String privacyGroupId, final PrivacyNode locker, final Credentials signer) {
+    this.privacyGroupId = Base64String.wrap(privacyGroupId);
+    this.locker = locker;
+    this.signer = signer;
+  }
+
+  @Override
+  public String execute(final NodeRequests node) {
+    try {
+      return node.privacy().privxUnlockPrivacyGroup(locker, privacyGroupId, signer);
+    } catch (final IOException | TransactionException e) {
+      throw new RuntimeException(e);
+    }
+  }
+}

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/transaction/privacy/PrivacyRequestFactory.java

@@ -237,6 +237,29 @@ private Bytes encodeRemoveFromGroupFunctionCall(final Bytes toRemove) {
   public String privxLockPrivacyGroup(
       final PrivacyNode locker, final Base64String privacyGroupId, final Credentials signer)
       throws IOException, TransactionException {
+    return privxLockOrUnlockPrivacyGroup(
+        locker,
+        privacyGroupId,
+        signer,
+        OnChainGroupManagement.LOCK_GROUP_METHOD_SIGNATURE.toHexString());
+  }
+
+  public String privxUnlockPrivacyGroup(
+      final PrivacyNode locker, final Base64String privacyGroupId, final Credentials signer)
+      throws IOException, TransactionException {
+    return privxLockOrUnlockPrivacyGroup(
+        locker,
+        privacyGroupId,
+        signer,
+        OnChainGroupManagement.UNLOCK_GROUP_METHOD_SIGNATURE.toHexString());
+  }
+
+  private String privxLockOrUnlockPrivacyGroup(
+      final PrivacyNode locker,
+      final Base64String privacyGroupId,
+      final Credentials signer,
+      final String callData)
+      throws IOException, TransactionException {
     final BigInteger nonce =
         besuClient
             .privGetTransactionCount(signer.getAddress(), privacyGroupId)
@@ -249,7 +272,7 @@ public String privxLockPrivacyGroup(
             BigInteger.valueOf(1000),
             BigInteger.valueOf(3000000),
             Address.ONCHAIN_PRIVACY_PROXY.toHexString(),
-            OnChainGroupManagement.LOCK_GROUP_METHOD_SIGNATURE.toHexString(),
+            callData,
             Base64String.wrap(locker.getEnclaveKey()),
             privacyGroupId,
             org.web3j.utils.Restriction.RESTRICTED);

acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/web3j/privacy/OnChainPrivacyAcceptanceTest.java

@@ -31,6 +31,8 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
+import java.util.function.BiConsumer;
+import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
 import com.google.common.collect.Lists;
@@ -476,6 +478,71 @@ public void bobCanAddCharlieAfterBeingAddedByAlice() {
     checkOnChainPrivacyGroupExists(privacyGroupId, alice, bob);
   }
 
+  @Test
+  public void canOnlyCallProxyContractWhenGroupLocked() {
+    final String privacyGroupId = createOnChainPrivacyGroup(alice);
+    checkOnChainPrivacyGroupExists(privacyGroupId, alice);
+
+    final EventEmitter eventEmitter =
+        alice.execute(
+            privateContractTransactions.createSmartContractWithPrivacyGroupId(
+                EventEmitter.class,
+                alice.getTransactionSigningKey(),
+                alice.getEnclaveKey(),
+                privacyGroupId));
+
+    privateContractVerifier
+        .validPrivateContractDeployed(
+            eventEmitter.getContractAddress(), alice.getAddress().toString())
+        .verify(eventEmitter);
+
+    final Credentials aliceCredentials = Credentials.create(alice.getTransactionSigningKey());
+
+    final Supplier<String> callContract =
+        () -> {
+          return alice.execute(
+              privateContractTransactions.callOnChainPermissioningSmartContract(
+                  eventEmitter.getContractAddress(),
+                  eventEmitter.value().encodeFunctionCall(),
+                  alice.getTransactionSigningKey(),
+                  alice.getEnclaveKey(),
+                  privacyGroupId));
+        };
+
+    final String lockHash =
+        alice.execute(
+            privacyTransactions.privxLockPrivacyGroupAndCheck(
+                privacyGroupId, alice, aliceCredentials));
+
+    final String callWhileLockedHash = callContract.get();
+
+    final String unlockHash =
+        alice.execute(
+            privacyTransactions.privxUnlockPrivacyGroupAndCheck(
+                privacyGroupId, alice, aliceCredentials));
+
+    final String callAfterUnlockedHash = callContract.get();
+
+    alice.execute(minerTransactions.minerStart());
+    alice.getBesu().verify(ethConditions.miningStatus(true));
+
+    final BiConsumer<String, String> assertThatTransactionReceiptIs =
+        (String hash, String expectedResult) -> {
+          final PrivateTransactionReceipt receipt =
+              alice.execute(privacyTransactions.getPrivateTransactionReceipt(hash));
+          assertThat(receipt.getStatus()).isEqualTo(expectedResult);
+        };
+
+    // when locking a group succeeds ...
+    assertThatTransactionReceiptIs.accept(lockHash, "0x1");
+    // ... calls to contracts fail ...
+    assertThatTransactionReceiptIs.accept(callWhileLockedHash, "0x0");
+    // ... but unlock the group works ...
+    assertThatTransactionReceiptIs.accept(unlockHash, "0x1");
+    // ... and afterwards we can call contracts again
+    assertThatTransactionReceiptIs.accept(callAfterUnlockedHash, "0x1");
+  }
+
   @Test
   public void addMembersToTwoGroupsInTheSameBlock() {
     final String privacyGroupId1 = createOnChainPrivacyGroup(alice);

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/precompiles/privacy/OnChainPrivacyPrecompiledContract.java

@@ -19,6 +19,7 @@
 import org.hyperledger.besu.enclave.types.PrivacyGroup;
 import org.hyperledger.besu.enclave.types.ReceiveResponse;
 import org.hyperledger.besu.ethereum.chain.Blockchain;
+import org.hyperledger.besu.ethereum.core.Address;
 import org.hyperledger.besu.ethereum.core.Hash;
 import org.hyperledger.besu.ethereum.core.MutableWorldState;
 import org.hyperledger.besu.ethereum.core.PrivacyParameters;
@@ -196,7 +197,7 @@ public Bytes compute(final Bytes input, final MessageFrame messageFrame) {
   }
 
   private void sendParticipantRemovedEvent(final PrivateTransaction privateTransaction) {
-    if (privateTransaction.isGroupRemovalTransaction()) {
+    if (isRemovingParticipant(privateTransaction)) {
       // get first participant parameter - there is only one for removal transaction
       final String removedParticipant =
           getRemovedParticipantFromParameter(privateTransaction.getPayload());
@@ -228,24 +229,18 @@ boolean canExecute(
             privateWorldStateArchive,
             privateTransactionProcessor);
 
-    final boolean isAddingParticipant =
-        privateTransaction
-            .getPayload()
-            .toHexString()
-            .startsWith(OnChainGroupManagement.ADD_PARTICIPANTS_METHOD_SIGNATURE.toHexString());
-
-    final boolean isPrivacyGroupLocked =
-        isContractLocked(onchainPrivacyGroupContract, privacyGroupId);
+    final boolean isAddingParticipant = isAddingParticipant(privateTransaction);
+    final boolean isContractLocked = isContractLocked(onchainPrivacyGroupContract, privacyGroupId);
 
-    if (isAddingParticipant && !isPrivacyGroupLocked) {
+    if (isAddingParticipant && !isContractLocked) {
       LOG.debug(
           "Privacy Group {} is not locked while trying to add to group with commitment {}",
           privacyGroupId.toHexString(),
           messageFrame.getTransactionHash());
       return false;
     }
 
-    if (!isAddingParticipant && isPrivacyGroupLocked) {
+    if (isContractLocked && !isTargettingOnchainPrivacyProxy(privateTransaction)) {
       LOG.debug(
           "Privacy Group {} is locked while trying to execute transaction with commitment {}",
           privacyGroupId.toHexString(),
@@ -315,6 +310,27 @@ private String getRemovedParticipantFromParameter(final Bytes input) {
     return input.slice(4).toBase64String();
   }
 
+  private boolean isTargettingOnchainPrivacyProxy(final PrivateTransaction privateTransaction) {
+    return privateTransaction.getTo().isPresent()
+        && privateTransaction.getTo().get().equals(Address.ONCHAIN_PRIVACY_PROXY);
+  }
+
+  private boolean isAddingParticipant(final PrivateTransaction privateTransaction) {
+    return isTargettingOnchainPrivacyProxy(privateTransaction)
+        && privateTransaction
+            .getPayload()
+            .toHexString()
+            .startsWith(OnChainGroupManagement.ADD_PARTICIPANTS_METHOD_SIGNATURE.toHexString());
+  }
+
+  private boolean isRemovingParticipant(final PrivateTransaction privateTransaction) {
+    return isTargettingOnchainPrivacyProxy(privateTransaction)
+        && privateTransaction
+            .getPayload()
+            .toHexString()
+            .startsWith(OnChainGroupManagement.REMOVE_PARTICIPANT_METHOD_SIGNATURE.toHexString());
+  }
+
   protected boolean isContractLocked(
       final OnchainPrivacyGroupContract onchainPrivacyGroupContract, final Bytes32 privacyGroupId) {
     final Optional<Bytes32> canExecuteResult =

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

@@ -16,7 +16,6 @@
 
 import static com.google.common.base.Preconditions.checkState;
 import static org.hyperledger.besu.crypto.Hash.keccak256;
-import static org.hyperledger.besu.ethereum.privacy.group.OnChainGroupManagement.REMOVE_PARTICIPANT_METHOD_SIGNATURE;
 import static org.hyperledger.besu.plugin.data.Restriction.RESTRICTED;
 import static org.hyperledger.besu.plugin.data.Restriction.UNRESTRICTED;
 import static org.hyperledger.besu.plugin.data.Restriction.UNSUPPORTED;
@@ -206,14 +205,6 @@ public static PrivateTransaction readFrom(final RLPInput input) throws RLPExcept
     }
   }
 
-  public boolean isGroupRemovalTransaction() {
-    return this.getTo().isPresent()
-        && this.getTo().get().equals(Address.ONCHAIN_PRIVACY_PROXY)
-        && this.getPayload()
-            .toHexString()
-            .startsWith(REMOVE_PARTICIPANT_METHOD_SIGNATURE.toHexString());
-  }
-
   private static Object resolvePrivateForOrPrivacyGroupId(final RLPInput item) {
     return item.nextIsList() ? item.readList(RLPInput::readBytes) : item.readBytes();
   }

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/privacy/group/OnChainGroupManagement.java

@@ -31,4 +31,5 @@ public class OnChainGroupManagement {
   public static final Bytes GET_VERSION_METHOD_SIGNATURE = Bytes.fromHexString("0x0d8e6e2c");
   public static final Bytes LOCK_GROUP_METHOD_SIGNATURE = Bytes.fromHexString("0xf83d08ba");
   public static final Bytes REMOVE_PARTICIPANT_METHOD_SIGNATURE = Bytes.fromHexString("0xfd017797");
+  public static final Bytes UNLOCK_GROUP_METHOD_SIGNATURE = Bytes.fromHexString("0xa69df4b5");
 }