From 78bff2853abfd052e1bfb5b45e3ad41705b9ea7b Mon Sep 17 00:00:00 2001 From: Michael Tinker Date: Tue, 20 Feb 2024 09:31:57 -0600 Subject: [PATCH] chore: match mono-service `contractCallResult`/`contractID` externalization (#11578) Signed-off-by: Michael Tinker --- .../contract/impl/exec/CallOutcome.java | 41 +++++++++++++++---- .../impl/handlers/ContractCallHandler.java | 5 ++- .../impl/handlers/ContractCreateHandler.java | 3 +- .../handlers/EthereumTransactionHandler.java | 7 +++- .../impl/test/exec/CallOutcomeTest.java | 14 +++++-- .../map/benchmark/BenchmarkAccount.java | 2 +- .../fixtures/map/benchmark/BenchmarkKey.java | 2 +- .../map/dummy/DummyFCQueueElement.java | 2 +- .../fixtures/map/dummy/FCMapComplexValue.java | 2 +- .../test/fixtures/map/dummy/FCQValue.java | 2 +- .../test/fixtures/map/dummy/SimpleValue.java | 2 +- .../merkle/test/fixtures/map/pta/MapKey.java | 2 +- .../test/fixtures/map/pta/MerkleMapKey.java | 2 +- .../fixtures/map/pta/TransactionRecord.java | 2 +- 14 files changed, 65 insertions(+), 23 deletions(-) diff --git a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/CallOutcome.java b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/CallOutcome.java index 6937ea326b51..f0e761f49478 100644 --- a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/CallOutcome.java +++ b/hedera-node/hedera-smart-contract-service-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/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractCallHandler.java b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractCallHandler.java index ff14ddbe7b92..4ba1af5669e4 100644 --- a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractCallHandler.java +++ b/hedera-node/hedera-smart-contract-service-impl/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/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractCreateHandler.java b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractCreateHandler.java index b12995319765..14f88444ac03 100644 --- a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/ContractCreateHandler.java +++ b/hedera-node/hedera-smart-contract-service-impl/src/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/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/EthereumTransactionHandler.java b/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/EthereumTransactionHandler.java index cb70202eb3ac..265e3dbbd3ea 100644 --- a/hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/handlers/EthereumTransactionHandler.java +++ b/hedera-node/hedera-smart-contract-service-impl/src/main/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/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/exec/CallOutcomeTest.java b/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/exec/CallOutcomeTest.java index bd0b12f55fd5..808bdc912e6a 100644 --- a/hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/exec/CallOutcomeTest.java +++ b/hedera-node/hedera-smart-contract-service-impl/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/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/benchmark/BenchmarkAccount.java b/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/benchmark/BenchmarkAccount.java index 83d383400b8f..0da0ad6f3adb 100644 --- a/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/benchmark/BenchmarkAccount.java +++ b/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/benchmark/BenchmarkAccount.java @@ -28,7 +28,7 @@ */ public class BenchmarkAccount extends PartialMerkleLeaf implements Keyed, MerkleLeaf { - private static final long CLASS_ID = 0x6b7ca4c97dbade4cL; + private static final long CLASS_ID = 0x6b7ca4c97dbade4dL; /** * The balance of the account. diff --git a/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/benchmark/BenchmarkKey.java b/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/benchmark/BenchmarkKey.java index fbaf9c0b8b21..a7c59450c988 100644 --- a/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/benchmark/BenchmarkKey.java +++ b/platform-sdk/swirlds-merkle/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/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/dummy/DummyFCQueueElement.java b/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/dummy/DummyFCQueueElement.java index 0316e8148b8b..654aa6eae5ed 100644 --- a/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/dummy/DummyFCQueueElement.java +++ b/platform-sdk/swirlds-merkle/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/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/dummy/FCMapComplexValue.java b/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/dummy/FCMapComplexValue.java index a48f4b0104f6..7878584a0782 100644 --- a/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/dummy/FCMapComplexValue.java +++ b/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/dummy/FCMapComplexValue.java @@ -29,7 +29,7 @@ */ public class FCMapComplexValue extends PartialNaryMerkleInternal implements Keyed, 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/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/dummy/FCQValue.java b/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/dummy/FCQValue.java index 44e81547884c..44dcaaecab51 100644 --- a/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/dummy/FCQValue.java +++ b/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/dummy/FCQValue.java @@ -39,7 +39,7 @@ public class FCQValue 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/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/dummy/SimpleValue.java b/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/dummy/SimpleValue.java index 477aabd327c5..36983b4986bc 100644 --- a/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/dummy/SimpleValue.java +++ b/platform-sdk/swirlds-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/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/pta/MapKey.java b/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/pta/MapKey.java index 23c386d54c85..7a82fd6d1d42 100644 --- a/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/pta/MapKey.java +++ b/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/pta/MapKey.java @@ -28,7 +28,7 @@ public class MapKey implements SelfSerializable, Comparable, 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/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/pta/MerkleMapKey.java b/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/pta/MerkleMapKey.java index c2e137a81588..ab9b06982f61 100644 --- a/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/pta/MerkleMapKey.java +++ b/platform-sdk/swirlds-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/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/pta/TransactionRecord.java b/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/pta/TransactionRecord.java index fb95528ba6bd..b1a6f443a390 100644 --- a/platform-sdk/swirlds-merkle/src/testFixtures/java/com/swirlds/merkle/test/fixtures/map/pta/TransactionRecord.java +++ b/platform-sdk/swirlds-merkle/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;