fix: private contracts not able to call public contracts that call other public contracts (#2816)

Private accounts are not able to change the state of public accounts. When transferValue is called in MessageCallProcessor it attempts to get a mutable account. This is only required when a transfer of value is happening. If a transfer of value from a private contract to a public contract is attempted an error will be thrown.

Signed-off-by: Antony Denyer <git@antonydenyer.co.uk>

CHANGELOG.md

@@ -20,6 +20,7 @@
 - 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)
 - Update Gas Schedule for Ethereum Classic [#2746](https://github.com/hyperledger/besu/pull/2746)
+- Fix bug with private contracts not able to call public contracts that call public contracts [#2816](https://github.com/hyperledger/besu/pull/2816)
 
 ### Early Access Features
 - \[EXPERIMENTAL\] Added support for QBFT with PKI-backed Block Creation. [#2647](https://github.com/hyperledger/besu/issues/2647)

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

@@ -22,6 +22,8 @@ import org.hyperledger.besu.tests.acceptance.dsl.privacy.ParameterizedEnclaveTes
 import org.hyperledger.besu.tests.acceptance.dsl.privacy.PrivacyNode;
 import org.hyperledger.besu.tests.web3j.generated.CrossContractReader;
 import org.hyperledger.besu.tests.web3j.generated.EventEmitter;
+import org.hyperledger.besu.tests.web3j.generated.RemoteSimpleStorage;
+import org.hyperledger.besu.tests.web3j.generated.SimpleStorage;
 import org.hyperledger.enclave.testutil.EnclaveType;
 
 import java.io.IOException;
@@ -169,4 +171,43 @@ public class PrivateContractPublicStateAcceptanceTest extends ParameterizedEncla
         .returns(
             "0x", e -> ((PrivateTransactionReceipt) e.getTransactionReceipt().get()).getOutput());
   }
+
+  @Test
+  public void privateContractCanCallPublicContractThatCallsPublicContract() throws Exception {
+    final SimpleStorage simpleStorage =
+        transactionNode
+            .getBesu()
+            .execute(contractTransactions.createSmartContract(SimpleStorage.class));
+
+    final RemoteSimpleStorage remoteSimpleStorage =
+        transactionNode
+            .getBesu()
+            .execute(contractTransactions.createSmartContract(RemoteSimpleStorage.class));
+
+    remoteSimpleStorage.setRemote(simpleStorage.getContractAddress()).send();
+
+    final RemoteSimpleStorage reallyRemoteSimpleStorage =
+        transactionNode
+            .getBesu()
+            .execute(contractTransactions.createSmartContract(RemoteSimpleStorage.class));
+
+    reallyRemoteSimpleStorage.setRemote(remoteSimpleStorage.getContractAddress()).send();
+
+    simpleStorage.set(BigInteger.valueOf(42)).send();
+
+    assertThat(simpleStorage.get().send()).isEqualTo(BigInteger.valueOf(42));
+    assertThat(remoteSimpleStorage.get().send()).isEqualTo(BigInteger.valueOf(42));
+    assertThat(reallyRemoteSimpleStorage.get().send()).isEqualTo(BigInteger.valueOf(42));
+
+    final RemoteSimpleStorage privateRemoteSimpleStorage =
+        transactionNode.execute(
+            privateContractTransactions.createSmartContract(
+                RemoteSimpleStorage.class,
+                transactionNode.getTransactionSigningKey(),
+                transactionNode.getEnclaveKey()));
+
+    privateRemoteSimpleStorage.setRemote(reallyRemoteSimpleStorage.getContractAddress()).send();
+
+    assertThat(privateRemoteSimpleStorage.get().send()).isEqualTo(BigInteger.valueOf(42));
+  }
 }

acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/web3j/RemoteSimpleStorage.sol

@@ -0,0 +1,37 @@
+/*
+ * Copyright contributors to Hyperledger Besu
+ *
+ * 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
+ */
+pragma solidity >=0.4.0 <0.6.0;
+
+import "./SimpleStorage.sol";
+
+// compile with:
+// solc RemoteSimpleStorage.sol --bin --abi --optimize --overwrite -o .
+// then create web3j wrappers with:
+// web3j solidity generate -b ./generated/RemoteSimpleStorage.bin -a ./generated/RemoteSimpleStorage.abi -o ../../../../../ -p org.hyperledger.besu.tests.web3j.generated
+contract RemoteSimpleStorage {
+    SimpleStorage public simpleStorage;
+
+    function setRemote(SimpleStorage _simpleStorage) public {
+        simpleStorage = _simpleStorage;
+    }
+
+    function set(uint value) public {
+        simpleStorage.set(value);
+    }
+
+    function get() public view returns (uint) {
+        return simpleStorage.get();
+    }
+}

acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/web3j/generated/RemoteSimpleStorage.java

@@ -0,0 +1,196 @@
+/*
+ * Copyright contributors to Hyperledger Besu
+ *
+ * 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.web3j.generated;
+
+import java.math.BigInteger;
+import java.util.Arrays;
+import java.util.Collections;
+
+import org.web3j.abi.TypeReference;
+import org.web3j.abi.datatypes.Address;
+import org.web3j.abi.datatypes.Function;
+import org.web3j.abi.datatypes.Type;
+import org.web3j.abi.datatypes.generated.Uint256;
+import org.web3j.crypto.Credentials;
+import org.web3j.protocol.Web3j;
+import org.web3j.protocol.core.RemoteCall;
+import org.web3j.protocol.core.RemoteFunctionCall;
+import org.web3j.protocol.core.methods.response.TransactionReceipt;
+import org.web3j.tx.Contract;
+import org.web3j.tx.TransactionManager;
+import org.web3j.tx.gas.ContractGasProvider;
+
+/**
+ * Auto generated code.
+ *
+ * <p><strong>Do not modify!</strong>
+ *
+ * <p>Please use the <a href="https://docs.web3j.io/command_line.html">web3j command line tools</a>,
+ * or the org.web3j.codegen.SolidityFunctionWrapperGenerator in the <a
+ * href="https://github.com/web3j/web3j/tree/master/codegen">codegen module</a> to update.
+ *
+ * <p>Generated with web3j version 4.5.0.
+ */
+@SuppressWarnings("rawtypes")
+public class RemoteSimpleStorage extends Contract {
+  private static final String BINARY =
+      "608060405234801561001057600080fd5b5061034c806100206000396000f3fe608060405234801561001057600080fd5b5060043610610069576000357c01000000000000000000000000000000000000000000000000000000009004806360fe47b11461006e5780636be0d6bf1461009c5780636d4ce63c146100e6578063f1efe24c14610104575b600080fd5b61009a6004803603602081101561008457600080fd5b8101908080359060200190929190505050610148565b005b6100a46101f3565b604051808273ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff16815260200191505060405180910390f35b6100ee610218565b6040518082815260200191505060405180910390f35b6101466004803603602081101561011a57600080fd5b81019080803573ffffffffffffffffffffffffffffffffffffffff1690602001909291905050506102dd565b005b6000809054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff166360fe47b1826040518263ffffffff167c010000000000000000000000000000000000000000000000000000000002815260040180828152602001915050600060405180830381600087803b1580156101d857600080fd5b505af11580156101ec573d6000803e3d6000fd5b5050505050565b6000809054906101000a900473ffffffffffffffffffffffffffffffffffffffff1681565b60008060009054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff16636d4ce63c6040518163ffffffff167c010000000000000000000000000000000000000000000000000000000002815260040160206040518083038186803b15801561029d57600080fd5b505afa1580156102b1573d6000803e3d6000fd5b505050506040513d60208110156102c757600080fd5b8101908080519060200190929190505050905090565b806000806101000a81548173ffffffffffffffffffffffffffffffffffffffff021916908373ffffffffffffffffffffffffffffffffffffffff1602179055505056fea165627a7a723058209de47b3e814c56fb80861a580d360af8738753c53cc6e71163f687ed3eed570f0029";
+
+  public static final String FUNC_SET = "set";
+
+  public static final String FUNC_SIMPLESTORAGE = "simpleStorage";
+
+  public static final String FUNC_GET = "get";
+
+  public static final String FUNC_SETREMOTE = "setRemote";
+
+  @Deprecated
+  protected RemoteSimpleStorage(
+      String contractAddress,
+      Web3j web3j,
+      Credentials credentials,
+      BigInteger gasPrice,
+      BigInteger gasLimit) {
+    super(BINARY, contractAddress, web3j, credentials, gasPrice, gasLimit);
+  }
+
+  protected RemoteSimpleStorage(
+      String contractAddress,
+      Web3j web3j,
+      Credentials credentials,
+      ContractGasProvider contractGasProvider) {
+    super(BINARY, contractAddress, web3j, credentials, contractGasProvider);
+  }
+
+  @Deprecated
+  protected RemoteSimpleStorage(
+      String contractAddress,
+      Web3j web3j,
+      TransactionManager transactionManager,
+      BigInteger gasPrice,
+      BigInteger gasLimit) {
+    super(BINARY, contractAddress, web3j, transactionManager, gasPrice, gasLimit);
+  }
+
+  protected RemoteSimpleStorage(
+      String contractAddress,
+      Web3j web3j,
+      TransactionManager transactionManager,
+      ContractGasProvider contractGasProvider) {
+    super(BINARY, contractAddress, web3j, transactionManager, contractGasProvider);
+  }
+
+  public RemoteFunctionCall<TransactionReceipt> set(BigInteger value) {
+    final Function function =
+        new Function(
+            FUNC_SET,
+            Arrays.<Type>asList(new org.web3j.abi.datatypes.generated.Uint256(value)),
+            Collections.<TypeReference<?>>emptyList());
+    return executeRemoteCallTransaction(function);
+  }
+
+  public RemoteFunctionCall<String> simpleStorage() {
+    final Function function =
+        new Function(
+            FUNC_SIMPLESTORAGE,
+            Arrays.<Type>asList(),
+            Arrays.<TypeReference<?>>asList(new TypeReference<Address>() {}));
+    return executeRemoteCallSingleValueReturn(function, String.class);
+  }
+
+  public RemoteFunctionCall<BigInteger> get() {
+    final Function function =
+        new Function(
+            FUNC_GET,
+            Arrays.<Type>asList(),
+            Arrays.<TypeReference<?>>asList(new TypeReference<Uint256>() {}));
+    return executeRemoteCallSingleValueReturn(function, BigInteger.class);
+  }
+
+  public RemoteFunctionCall<TransactionReceipt> setRemote(String _simpleStorage) {
+    final Function function =
+        new Function(
+            FUNC_SETREMOTE,
+            Arrays.<Type>asList(new org.web3j.abi.datatypes.Address(160, _simpleStorage)),
+            Collections.<TypeReference<?>>emptyList());
+    return executeRemoteCallTransaction(function);
+  }
+
+  @Deprecated
+  public static RemoteSimpleStorage load(
+      String contractAddress,
+      Web3j web3j,
+      Credentials credentials,
+      BigInteger gasPrice,
+      BigInteger gasLimit) {
+    return new RemoteSimpleStorage(contractAddress, web3j, credentials, gasPrice, gasLimit);
+  }
+
+  @Deprecated
+  public static RemoteSimpleStorage load(
+      String contractAddress,
+      Web3j web3j,
+      TransactionManager transactionManager,
+      BigInteger gasPrice,
+      BigInteger gasLimit) {
+    return new RemoteSimpleStorage(contractAddress, web3j, transactionManager, gasPrice, gasLimit);
+  }
+
+  public static RemoteSimpleStorage load(
+      String contractAddress,
+      Web3j web3j,
+      Credentials credentials,
+      ContractGasProvider contractGasProvider) {
+    return new RemoteSimpleStorage(contractAddress, web3j, credentials, contractGasProvider);
+  }
+
+  public static RemoteSimpleStorage load(
+      String contractAddress,
+      Web3j web3j,
+      TransactionManager transactionManager,
+      ContractGasProvider contractGasProvider) {
+    return new RemoteSimpleStorage(contractAddress, web3j, transactionManager, contractGasProvider);
+  }
+
+  public static RemoteCall<RemoteSimpleStorage> deploy(
+      Web3j web3j, Credentials credentials, ContractGasProvider contractGasProvider) {
+    return deployRemoteCall(
+        RemoteSimpleStorage.class, web3j, credentials, contractGasProvider, BINARY, "");
+  }
+
+  @Deprecated
+  public static RemoteCall<RemoteSimpleStorage> deploy(
+      Web3j web3j, Credentials credentials, BigInteger gasPrice, BigInteger gasLimit) {
+    return deployRemoteCall(
+        RemoteSimpleStorage.class, web3j, credentials, gasPrice, gasLimit, BINARY, "");
+  }
+
+  public static RemoteCall<RemoteSimpleStorage> deploy(
+      Web3j web3j, TransactionManager transactionManager, ContractGasProvider contractGasProvider) {
+    return deployRemoteCall(
+        RemoteSimpleStorage.class, web3j, transactionManager, contractGasProvider, BINARY, "");
+  }
+
+  @Deprecated
+  public static RemoteCall<RemoteSimpleStorage> deploy(
+      Web3j web3j,
+      TransactionManager transactionManager,
+      BigInteger gasPrice,
+      BigInteger gasLimit) {
+    return deployRemoteCall(
+        RemoteSimpleStorage.class, web3j, transactionManager, gasPrice, gasLimit, BINARY, "");
+  }
+}

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/worldstate/DefaultMutablePrivateWorldStateUpdater.java

@@ -48,11 +48,6 @@ public class DefaultMutablePrivateWorldStateUpdater implements WorldUpdater {
     return privateWorldUpdater.createAccount(address);
   }
 
-  @Override
-  public EvmAccount getOrCreate(final Address address) {
-    return privateWorldUpdater.getOrCreate(address);
-  }
-
   @Override
   public EvmAccount getAccount(final Address address) {
     final EvmAccount privateAccount = privateWorldUpdater.getAccount(address);

evm/src/main/java/org/hyperledger/besu/evm/processor/MessageCallProcessor.java

@@ -19,7 +19,7 @@ import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.evm.EVM;
 import org.hyperledger.besu.evm.Gas;
 import org.hyperledger.besu.evm.ModificationNotAllowedException;
-import org.hyperledger.besu.evm.account.MutableAccount;
+import org.hyperledger.besu.evm.account.EvmAccount;
 import org.hyperledger.besu.evm.frame.ExceptionalHaltReason;
 import org.hyperledger.besu.evm.frame.MessageFrame;
 import org.hyperledger.besu.evm.precompile.PrecompileContractRegistry;
@@ -27,6 +27,7 @@ import org.hyperledger.besu.evm.precompile.PrecompiledContract;
 import org.hyperledger.besu.evm.tracing.OperationTracer;
 
 import java.util.Collection;
+import java.util.Objects;
 import java.util.Optional;
 
 import com.google.common.collect.ImmutableSet;
@@ -66,7 +67,7 @@ public class MessageCallProcessor extends AbstractMessageProcessor {
         frame.setState(MessageFrame.State.CODE_EXECUTING);
       }
     } catch (ModificationNotAllowedException ex) {
-      LOG.trace("Message call error: atttempt to mutate an immutable account");
+      LOG.trace("Message call error: attempt to mutate an immutable account");
       frame.setExceptionalHaltReason(Optional.of(ExceptionalHaltReason.ILLEGAL_STATE_CHANGE));
       frame.setState(MessageFrame.State.EXCEPTIONAL_HALT);
     }
@@ -89,18 +90,32 @@ public class MessageCallProcessor extends AbstractMessageProcessor {
    * of the world state of this executor.
    */
   private void transferValue(final MessageFrame frame) {
-    final MutableAccount senderAccount =
+    final EvmAccount senderAccount = frame.getWorldUpdater().getSenderAccount(frame);
-        frame.getWorldUpdater().getSenderAccount(frame).getMutable();
+
     // The yellow paper explicitly states that if the recipient account doesn't exist at this
-    // point, it is created.
+    // point, it is created. Even if the value is zero we are still creating an account with 0x!
-    final MutableAccount recipientAccount =
+    final EvmAccount recipientAccount =
-        frame.getWorldUpdater().getOrCreate(frame.getRecipientAddress()).getMutable();
+        frame.getWorldUpdater().getOrCreate(frame.getRecipientAddress());
+
+    if (Objects.equals(frame.getValue(), Wei.ZERO)) {
+      // This is only here for situations where you are calling a public address from a private
+      // address. Without this guard clause we would attempt to get a mutable public address
+      // which isn't possible from a private address and an error would be thrown.
+      // If you are attempting to transfer value from a private address
+      // to public address an error will be thrown.
+      LOG.trace(
+          "Message call from {} to {} has zero value: no fund transferred",
+          frame.getSenderAddress(),
+          frame.getRecipientAddress());
+      return;
+    }
 
     if (frame.getRecipientAddress().equals(frame.getSenderAddress())) {
       LOG.trace("Message call of {} to itself: no fund transferred", frame.getSenderAddress());
     } else {
-      final Wei prevSenderBalance = senderAccount.decrementBalance(frame.getValue());
+      final Wei prevSenderBalance = senderAccount.getMutable().decrementBalance(frame.getValue());
-      final Wei prevRecipientBalance = recipientAccount.incrementBalance(frame.getValue());
+      final Wei prevRecipientBalance =
+          recipientAccount.getMutable().incrementBalance(frame.getValue());
 
       LOG.trace(
           "Transferred value {} for message call from {} ({} -> {}) to {} ({} -> {})",