Fix sidecar ids for calls to missing contracts

Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>
hashgraph · Apr 29, 2024 · d3e9fcc · d3e9fcc
1 parent d668e78
commit d3e9fcc
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 42 deletions.
diff --git a/...pl/src/main/java/com/hedera/node/app/service/contract/impl/exec/TransactionProcessor.java b/...pl/src/main/java/com/hedera/node/app/service/contract/impl/exec/TransactionProcessor.java
@@ -316,8 +316,6 @@ private InvolvedParties partiesWhenContractNotRequired(
                         new InvolvedParties(sender, relayer, contractIDToBesuAddress(transaction.contractIdOrThrow()));
             }
         } else {
-            // In order to be EVM equivalent, we need to gracefully handle calls to potentially non-existent contracts
-            // and thus create a receiver address even if it may not exist in the ledger
             updater.setContractNotRequired();
             parties = new InvolvedParties(
                     sender,

diff --git a/...-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/utils/ActionStack.java b/...-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/utils/ActionStack.java
@@ -22,15 +22,14 @@
 import static com.hedera.hapi.streams.ContractActionType.CREATE;
 import static com.hedera.hapi.streams.codec.ContractActionProtoCodec.RECIPIENT_UNSET;
 import static com.hedera.node.app.service.contract.impl.exec.failure.CustomExceptionalHaltReason.INVALID_SOLIDITY_ADDRESS;
+import static com.hedera.node.app.service.contract.impl.exec.utils.FrameUtils.proxyUpdaterFor;
 import static com.hedera.node.app.service.contract.impl.utils.ConversionUtils.asNumberedContractId;
 import static com.hedera.node.app.service.contract.impl.utils.ConversionUtils.hederaIdNumOfContractIn;
 import static com.hedera.node.app.service.contract.impl.utils.ConversionUtils.hederaIdNumOfOriginatorIn;
-import static com.hedera.node.app.service.contract.impl.utils.ConversionUtils.hederaIdNumberIn;
 import static com.hedera.node.app.service.contract.impl.utils.ConversionUtils.pbjToBesuAddress;
 import static com.hedera.node.app.service.contract.impl.utils.ConversionUtils.tuweniToPbjBytes;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.Objects.requireNonNull;
-import static org.hyperledger.besu.evm.frame.MessageFrame.State.EXCEPTIONAL_HALT;
 import static org.hyperledger.besu.evm.frame.MessageFrame.Type.CONTRACT_CREATION;
 import static org.hyperledger.besu.evm.frame.MessageFrame.Type.MESSAGE_CALL;
 
@@ -192,12 +191,13 @@ private ContractAction finalFormOf(@NonNull final ContractAction action, @NonNul
                 } else {
                     builder.output(tuweniToPbjBytes(frame.getOutputData()));
                     if (action.targetedAddress() != null) {
-                        final var lazyCreatedAddress = pbjToBesuAddress(action.targetedAddressOrThrow());
-                        try {
-                            builder.recipientAccount(accountIdWith(hederaIdNumberIn(frame, lazyCreatedAddress)));
-                        } catch (final IllegalArgumentException | NullPointerException | ArithmeticException e) {
-                            // handle non-existing to/receiver address
-                            builder.recipientAccount((AccountID) null);
+                        final var maybeCreatedAddress = pbjToBesuAddress(action.targetedAddressOrThrow());
+                        final var maybeCreatedAccount = proxyUpdaterFor(frame).getHederaAccount(maybeCreatedAddress);
+                        // Fill in the account of id of a successful lazy creation; but just leave
+                        // the targeted address in case of a failed lazy-creation or a call to a
+                        // non-existent address
+                        if (maybeCreatedAccount != null) {
+                            builder.recipientAccount(maybeCreatedAccount.hederaId());
                         }
                     }
                 }
@@ -282,7 +282,7 @@ private void completePush(@NonNull ContractAction.Builder builder, @NonNull fina
         } else {
             try {
                 builder.recipientContract(contractIdWith(hederaIdNumOfContractIn(frame)));
-            } catch (NullPointerException e) {
+            } catch (NullPointerException ignore) {
                 builder.targetedAddress(tuweniToPbjBytes(frame.getContractAddress()));
             }
         }

diff --git a/...impl/src/main/java/com/hedera/node/app/service/contract/impl/hevm/HederaWorldUpdater.java b/...impl/src/main/java/com/hedera/node/app/service/contract/impl/hevm/HederaWorldUpdater.java
@@ -288,11 +288,6 @@ void externalizeSystemContractResults(
     @NonNull
     ExchangeRate currentExchangeRate();
 
-    /**
-     * Revert the last child record.
-     */
-    void revertChildRecords();
-
     /**
      * Sets the world updater to not check for the existence of the contractId
      * in the ledger when the getHederaContractId() method is called

diff --git a/...impl/src/main/java/com/hedera/node/app/service/contract/impl/state/ProxyWorldUpdater.java b/...impl/src/main/java/com/hedera/node/app/service/contract/impl/state/ProxyWorldUpdater.java
@@ -175,20 +175,16 @@ public ProxyWorldUpdater(
     public ContractID getHederaContractId(@NonNull final Address address) {
         requireNonNull(address);
         final var account = (HederaEvmAccount) get(address);
-        // As an important special case, return the pending creation's contract ID if
-        // its address matches and there is no extant account; but still prioritize
-        // existing accounts of course
         if (account == null) {
-            // If configured to allow non-existent contracts, return the address as a contract ID if the account is
-            // not found.
-            if (!contractMustBePresent) {
-                return isLongZero(address) ? asNumberedContractId(address) : asEvmContractId(address);
-            }
+            // Also return ids for pending creations
             if (pendingCreation != null && pendingCreation.address().equals(address)) {
                 return ContractID.newBuilder()
                         .contractNum(pendingCreation.number())
                         .build();
             } else {
+                if (!contractMustBePresent) {
+                    return isLongZero(address) ? asNumberedContractId(address) : asEvmContractId(address);
+                }
                 throw new IllegalArgumentException("No contract pending or extant at " + address);
             }
         }
@@ -410,16 +406,6 @@ public void revert() {
         reverted = true;
     }
 
-    /**
-     * {@inheritDoc}
-     */
-    @Override
-    public void revertChildRecords() {
-        if (recordListCheckPoint != null) {
-            enhancement.operations().revertRecordsFrom(recordListCheckPoint);
-        }
-    }
-
     /**
      * {@inheritDoc}
      */

diff --git a/.../test/java/com/hedera/node/app/service/contract/impl/test/exec/utils/ActionStackTest.java b/.../test/java/com/hedera/node/app/service/contract/impl/test/exec/utils/ActionStackTest.java
@@ -59,12 +59,14 @@
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoInteractions;
 
+import com.hedera.hapi.node.base.AccountID;
 import com.hedera.hapi.node.base.ContractID;
 import com.hedera.hapi.streams.ContractAction;
 import com.hedera.hapi.streams.ContractActionType;
 import com.hedera.node.app.service.contract.impl.exec.utils.ActionStack;
 import com.hedera.node.app.service.contract.impl.exec.utils.ActionWrapper;
 import com.hedera.node.app.service.contract.impl.exec.utils.ActionsHelper;
+import com.hedera.node.app.service.contract.impl.state.HederaEvmAccount;
 import com.hedera.node.app.service.contract.impl.state.ProxyWorldUpdater;
 import com.hedera.node.app.service.contract.impl.utils.ConversionUtils;
 import com.hedera.pbj.runtime.io.buffer.Bytes;
@@ -94,6 +96,9 @@ class ActionStackTest {
     @Mock
     private Account account;
 
+    @Mock
+    private HederaEvmAccount evmAccount;
+
     @Mock
     private Operation operation;
 
@@ -140,7 +145,7 @@ void getsActionsFromWrappers() {
         allActions.add(new ActionWrapper(ContractAction.DEFAULT));
         allActions.add(new ActionWrapper(ContractAction.DEFAULT));
         final var actions = subject.asContractActions();
-        assertEquals(actions.contractActionsOrThrow(), List.of(ContractAction.DEFAULT, ContractAction.DEFAULT));
+        assertEquals(actions.contractActions(), List.of(ContractAction.DEFAULT, ContractAction.DEFAULT));
     }
 
     @Test
@@ -414,7 +419,7 @@ void finalizationLazyCallWithUnresolvedAddress() {
         final var gasUsed = REMAINING_GAS / 3;
         given(parentFrame.getRemainingGas()).willReturn(REMAINING_GAS - gasUsed);
         given(parentFrame.getOutputData()).willReturn(pbjToTuweniBytes(OUTPUT_DATA));
-        givenUnresolvableEvmAddress();
+        given(parentFrame.getWorldUpdater()).willReturn(worldUpdater);
 
         final var wrappedAction = new ActionWrapper(LAZY_CREATE_ACTION);
         allActions.add(wrappedAction);
@@ -426,7 +431,8 @@ void finalizationLazyCallWithUnresolvedAddress() {
         assertEquals(wrappedAction, allActions.get(0));
         final var finalAction = allActions.get(0).get();
         assertEquals(gasUsed, finalAction.gasUsed());
-        assertEquals(null, finalAction.recipientAccount());
+        assertNull(finalAction.recipientAccount());
+        assertEquals(tuweniToPbjBytes(EIP_1014_ADDRESS), finalAction.targetedAddress());
         assertTrue(actionsStack.isEmpty());
     }
 
@@ -474,7 +480,7 @@ void emptyRevertReasonUsedIfMissing() {
 
     @Test
     void tracksTopLevelCreationAsExpected() {
-        givenResolvableEvmAddress();
+        givenPresentEvmAddress();
 
         given(parentFrame.getType()).willReturn(CONTRACT_CREATION);
         given(parentFrame.getOriginatorAddress()).willReturn(NON_SYSTEM_LONG_ZERO_ADDRESS);
@@ -504,7 +510,7 @@ void tracksTopLevelCreationAsExpected() {
 
     @Test
     void tracksTopLevelCallToEoaAsExpected() {
-        givenResolvableEvmAddress();
+        givenPresentEvmAddress();
 
         given(worldUpdater.get(EIP_1014_ADDRESS)).willReturn(account);
         given(parentFrame.getType()).willReturn(MessageFrame.Type.MESSAGE_CALL);
@@ -599,11 +605,15 @@ void tracksIntermediateCallAsExpected() {
 
     private void givenResolvableEvmAddress() {
         given(parentFrame.getWorldUpdater()).willReturn(worldUpdater);
-        given(worldUpdater.getHederaContractId(EIP_1014_ADDRESS)).willReturn(CALLED_CONTRACT_ID);
+        given(worldUpdater.getHederaAccount(EIP_1014_ADDRESS)).willReturn(evmAccount);
+        given(evmAccount.hederaId())
+                .willReturn(AccountID.newBuilder()
+                        .accountNum(CALLED_CONTRACT_ID.contractNumOrThrow())
+                        .build());
     }
 
-    private void givenUnresolvableEvmAddress() {
+    private void givenPresentEvmAddress() {
         given(parentFrame.getWorldUpdater()).willReturn(worldUpdater);
-        given(worldUpdater.getHederaContractId(EIP_1014_ADDRESS)).willReturn(null);
+        given(worldUpdater.getHederaContractId(EIP_1014_ADDRESS)).willReturn(CALLED_CONTRACT_ID);
     }
 }