chore: match mono-service contractCallResult/contractID externali…

…zation (#11578) Signed-off-by: Michael Tinker <[email protected]>
hashgraph · Feb 20, 2024 · 78bff28 · 78bff28
1 parent c98292e
commit 78bff28
Show file tree

Hide file tree

Showing 14 changed files with 65 additions and 23 deletions.
diff --git a/...ervice-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/CallOutcome.java b/...ervice-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/CallOutcome.java
@@ -48,6 +48,16 @@ public record CallOutcome(
         @Nullable ContractActions actions,
         @Nullable ContractStateChanges stateChanges) {
 
+    /**
+     * Enumerates whether to externalize the result of aborted calls; needed for
+     * mono-service fidelity, since only a top-level {@code EthereumTransaction}
+     * would externalize the result of an aborted call there.
+     */
+    public enum ExternalizeAbortResult {
+        YES,
+        NO
+    }
+
     public boolean hasStateChanges() {
         return stateChanges != null
                 && !stateChanges.contractStateChangesOrElse(emptyList()).isEmpty();
@@ -88,11 +98,17 @@ public boolean isSuccess() {
      * Adds the call details to the given record builder.
      *
      * @param recordBuilder the record builder
+     * @param externalizeAbortResult whether to externalize the result of aborted calls
      */
-    public void addCallDetailsTo(@NonNull final ContractCallRecordBuilder recordBuilder) {
-        recordBuilder.contractID(recipientId);
-        // Intentionally omit result on aborted calls (i.e., when gasUsed = 0)
-        if (result.gasUsed() != 0L) {
+    public void addCallDetailsTo(
+            @NonNull final ContractCallRecordBuilder recordBuilder,
+            @NonNull final ExternalizeAbortResult externalizeAbortResult) {
+        requireNonNull(recordBuilder);
+        requireNonNull(externalizeAbortResult);
+        if (!callWasAborted()) {
+            recordBuilder.contractID(recipientId);
+        }
+        if (shouldExternalizeResult(externalizeAbortResult)) {
             recordBuilder.contractCallResult(result);
         }
         recordBuilder.withCommonFieldsSetFrom(this);
@@ -103,10 +119,13 @@ public void addCallDetailsTo(@NonNull final ContractCallRecordBuilder recordBuil
      *
      * @param recordBuilder the record builder
      */
-    public void addCreateDetailsTo(@NonNull final ContractCreateRecordBuilder recordBuilder) {
+    public void addCreateDetailsTo(
+            @NonNull final ContractCreateRecordBuilder recordBuilder,
+            @NonNull final ExternalizeAbortResult externalizeAbortResult) {
+        requireNonNull(recordBuilder);
+        requireNonNull(externalizeAbortResult);
         recordBuilder.contractID(recipientIdIfCreated());
-        // Intentionally omit result on aborted calls (i.e., when gasUsed = 0)
-        if (result.gasUsed() != 0L) {
+        if (shouldExternalizeResult(externalizeAbortResult)) {
             recordBuilder.contractCreateResult(result);
         }
         recordBuilder.withCommonFieldsSetFrom(this);
@@ -134,4 +153,12 @@ public long tinybarGasCost() {
     private boolean representsTopLevelCreation() {
         return isSuccess() && requireNonNull(result).hasEvmAddress();
     }
+
+    private boolean shouldExternalizeResult(@NonNull final ExternalizeAbortResult externalizeAbortResult) {
+        return !callWasAborted() || externalizeAbortResult == ExternalizeAbortResult.YES;
+    }
+
+    private boolean callWasAborted() {
+        return result.gasUsed() == 0L;
+    }
 }
diff --git a/...src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractCallHandler.java b/...src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractCallHandler.java
@@ -24,6 +24,7 @@
 import com.hedera.hapi.node.base.HederaFunctionality;
 import com.hedera.hapi.node.base.SubType;
 import com.hedera.node.app.hapi.utils.fee.SmartContractFeeBuilder;
+import com.hedera.node.app.service.contract.impl.exec.CallOutcome.ExternalizeAbortResult;
 import com.hedera.node.app.service.contract.impl.exec.TransactionComponent;
 import com.hedera.node.app.service.contract.impl.records.ContractCallRecordBuilder;
 import com.hedera.node.app.service.mono.fees.calculation.contract.txns.ContractCallResourceUsage;
@@ -59,7 +60,9 @@ public void handle(@NonNull final HandleContext context) throws HandleException
         final var outcome = component.contextTransactionProcessor().call();
 
         // Assemble the appropriate top-level record for the result
-        outcome.addCallDetailsTo(context.recordBuilder(ContractCallRecordBuilder.class));
+        // (FUTURE) Remove ExternalizeAbortResult.NO, this is only
+        // for mono-service fidelity during differential testing
+        outcome.addCallDetailsTo(context.recordBuilder(ContractCallRecordBuilder.class), ExternalizeAbortResult.NO);
 
         throwIfUnsuccessful(outcome.status());
     }

diff --git a/...c/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractCreateHandler.java b/...c/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractCreateHandler.java
@@ -26,6 +26,7 @@
 import com.hedera.hapi.node.base.HederaFunctionality;
 import com.hedera.hapi.node.base.SubType;
 import com.hedera.node.app.hapi.utils.fee.SmartContractFeeBuilder;
+import com.hedera.node.app.service.contract.impl.exec.CallOutcome.ExternalizeAbortResult;
 import com.hedera.node.app.service.contract.impl.exec.TransactionComponent;
 import com.hedera.node.app.service.contract.impl.records.ContractCreateRecordBuilder;
 import com.hedera.node.app.service.mono.fees.calculation.contract.txns.ContractCreateResourceUsage;
@@ -64,7 +65,7 @@ public void handle(@NonNull final HandleContext context) throws HandleException
         final var outcome = component.contextTransactionProcessor().call();
 
         // Assemble the appropriate top-level record for the result
-        outcome.addCreateDetailsTo(context.recordBuilder(ContractCreateRecordBuilder.class));
+        outcome.addCreateDetailsTo(context.recordBuilder(ContractCreateRecordBuilder.class), ExternalizeAbortResult.NO);
 
         throwIfUnsuccessful(outcome.status());
     }

diff --git a/...n/java/com/hedera/node/app/service/contract/impl/handlers/EthereumTransactionHandler.java b/...n/java/com/hedera/node/app/service/contract/impl/handlers/EthereumTransactionHandler.java
@@ -29,6 +29,7 @@
 import com.hedera.hapi.node.contract.EthereumTransactionBody;
 import com.hedera.node.app.hapi.utils.ethereum.EthTxSigs;
 import com.hedera.node.app.hapi.utils.fee.SmartContractFeeBuilder;
+import com.hedera.node.app.service.contract.impl.exec.CallOutcome.ExternalizeAbortResult;
 import com.hedera.node.app.service.contract.impl.exec.TransactionComponent;
 import com.hedera.node.app.service.contract.impl.infra.EthTxSigsCache;
 import com.hedera.node.app.service.contract.impl.infra.EthereumCallDataHydration;
@@ -119,9 +120,11 @@ public void handle(@NonNull final HandleContext context) throws HandleException
         context.recordBuilder(EthereumTransactionRecordBuilder.class)
                 .ethereumHash(Bytes.wrap(ethTxData.getEthereumHash()));
         if (ethTxData.hasToAddress()) {
-            outcome.addCallDetailsTo(context.recordBuilder(ContractCallRecordBuilder.class));
+            outcome.addCallDetailsTo(
+                    context.recordBuilder(ContractCallRecordBuilder.class), ExternalizeAbortResult.YES);
         } else {
-            outcome.addCreateDetailsTo(context.recordBuilder(ContractCreateRecordBuilder.class));
+            outcome.addCreateDetailsTo(
+                    context.recordBuilder(ContractCreateRecordBuilder.class), ExternalizeAbortResult.YES);
         }
 
         throwIfUnsuccessful(outcome.status());

diff --git a/...pl/src/test/java/com/hedera/node/app/service/contract/impl/test/exec/CallOutcomeTest.java b/...pl/src/test/java/com/hedera/node/app/service/contract/impl/test/exec/CallOutcomeTest.java
@@ -52,18 +52,26 @@ class CallOutcomeTest {
     private ContractCreateRecordBuilder contractCreateRecordBuilder;
 
     @Test
-    void onlySetsCallResultIfNotAborted() {
+    void doesNotSetAbortCallResultIfNotRequested() {
         final var abortedCall =
                 new CallOutcome(ContractFunctionResult.DEFAULT, INSUFFICIENT_GAS, CALLED_CONTRACT_ID, 123L, null, null);
-        abortedCall.addCallDetailsTo(contractCallRecordBuilder);
+        abortedCall.addCallDetailsTo(contractCallRecordBuilder, CallOutcome.ExternalizeAbortResult.NO);
         verify(contractCallRecordBuilder, never()).contractCallResult(any());
     }
 
+    @Test
+    void setsAbortCallResultIfRequested() {
+        final var abortedCall =
+                new CallOutcome(ContractFunctionResult.DEFAULT, INSUFFICIENT_GAS, CALLED_CONTRACT_ID, 123L, null, null);
+        abortedCall.addCallDetailsTo(contractCallRecordBuilder, CallOutcome.ExternalizeAbortResult.YES);
+        verify(contractCallRecordBuilder).contractCallResult(any());
+    }
+
     @Test
     void onlySetsCreateResultIfNotAborted() {
         final var abortedCreate =
                 new CallOutcome(ContractFunctionResult.DEFAULT, INSUFFICIENT_GAS, null, 123L, null, null);
-        abortedCreate.addCreateDetailsTo(contractCreateRecordBuilder);
+        abortedCreate.addCreateDetailsTo(contractCreateRecordBuilder, CallOutcome.ExternalizeAbortResult.NO);
         verify(contractCreateRecordBuilder, never()).contractCreateResult(any());
     }
 

diff --git a/...rc/testFixtures/java/com/swirlds/merkle/test/fixtures/map/benchmark/BenchmarkAccount.java b/...rc/testFixtures/java/com/swirlds/merkle/test/fixtures/map/benchmark/BenchmarkAccount.java
@@ -28,7 +28,7 @@
  */
 public class BenchmarkAccount extends PartialMerkleLeaf implements Keyed<BenchmarkKey>, MerkleLeaf {
 
-    private static final long CLASS_ID = 0x6b7ca4c97dbade4cL;
+    private static final long CLASS_ID = 0x6b7ca4c97dbade4dL;
 
     /**
      * The balance of the account.

diff --git a/...le/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/benchmark/BenchmarkKey.java b/...le/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/benchmark/BenchmarkKey.java
@@ -27,7 +27,7 @@
  */
 public class BenchmarkKey implements SelfSerializable {
 
-    private static final long CLASS_ID = 0x41db5e9d036e36fdL;
+    private static final long CLASS_ID = 0x41db5e9d036e36feL;
 
     /**
      * The actual key.

diff --git a/...src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/dummy/DummyFCQueueElement.java b/...src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/dummy/DummyFCQueueElement.java
@@ -29,7 +29,7 @@
  */
 public class DummyFCQueueElement implements FastCopyable, SerializableHashable {
 
-    private static final long CLASS_ID = 0x1fc41d4f294c4114L;
+    private static final long CLASS_ID = 0x1fc41d4f294c4115L;
 
     private static final class ClassVersion {
         public static final int ORIGINAL = 1;

diff --git a/...e/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/dummy/FCMapComplexValue.java b/...e/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/dummy/FCMapComplexValue.java
@@ -29,7 +29,7 @@
  */
 public class FCMapComplexValue extends PartialNaryMerkleInternal implements Keyed<SerializableLong>, MerkleInternal {
 
-    private static final long CLASS_ID = 0xea7f1d984a6b3d22L;
+    private static final long CLASS_ID = 0xea7f1d984a6b3d23L;
 
     private static class ClassVersion {
         public static final int ORIGINAL = 1;

diff --git a/...lds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/dummy/FCQValue.java b/...lds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/dummy/FCQValue.java
@@ -39,7 +39,7 @@ public class FCQValue<T extends FastCopyable & SerializableHashable> extends Par
 
     protected static final int FCQ_VALUE_TYPE = 5;
 
-    private static final long CLASS_ID = 0x9ce2813e6b425a9bL;
+    private static final long CLASS_ID = 0x9ce2813e6b425a9cL;
 
     private static class ClassVersion {
         public static final int ORIGINAL = 1;

diff --git a/...-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/dummy/SimpleValue.java b/...-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/dummy/SimpleValue.java
@@ -26,7 +26,7 @@
 
 public class SimpleValue extends PartialMerkleLeaf implements MerkleLeaf {
 
-    private static final long CLASS_ID = 0x9475f1a4061a5329L;
+    private static final long CLASS_ID = 0x9475f1a4061a532aL;
 
     private static class ClassVersion {
         public static final int ORIGINAL = 1;

diff --git a/...swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/pta/MapKey.java b/...swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/pta/MapKey.java
@@ -28,7 +28,7 @@
 
 public class MapKey implements SelfSerializable, Comparable<MapKey>, FastCopyable {
 
-    public static final long CLASS_ID = 0x63302b26cc321421L;
+    public static final long CLASS_ID = 0x63302b26cc321422L;
 
     private static class ClassVersion {
         public static final int ORIGINAL = 1;

diff --git a/...s-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/pta/MerkleMapKey.java b/...s-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/pta/MerkleMapKey.java
@@ -27,7 +27,7 @@
  */
 public class MerkleMapKey extends PartialMerkleLeaf implements MerkleLeaf {
 
-    public static final long CLASS_ID = 0x66ff4872c3ae8c5eL;
+    public static final long CLASS_ID = 0x66ff4872c3ae8c5fL;
 
     private static final class ClassVersion {
 

diff --git a/...kle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/pta/TransactionRecord.java b/...kle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/pta/TransactionRecord.java
@@ -26,7 +26,7 @@
 import java.util.Objects;
 
 public class TransactionRecord extends AbstractSerializableHashable implements FastCopyable, Serializable {
-    private static final long CLASS_ID = 0xcdd5ad651cf2c4d7L;
+    private static final long CLASS_ID = 0xcdd5ad651cf2c4d8L;
 
     private static class ClassVersion {
         public static final int ORIGINAL = 1;