Skip to content

Commit

Permalink
feat: Direct call w/o value to system accounts [0.0.361-0.0.750] chan…
Browse files Browse the repository at this point in the history 
…ge output (#12833)

Signed-off-by: Stefan Stefanov <[email protected]>
  • Loading branch information
@stefan-stefanooov
stefan-stefanooov authored May 21, 2024
1 parent 3a4c6a6 commit dd48484
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 51 deletions.
2 changes: 1 addition & 1 deletion hedera-node/docs/system-accounts-operations.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ _Please note that the expected behavior described in this section is valid if th
- success, if the address is a contract, and we are using the correct ABI;
- success with no op, if there is no contract;
- fail, if the address is a contract, and we are **not** using the correct ABI.
- **Hedera:** success with no op.
- **Hedera:** success with no op. (with all gas consumed)

| Address | Calls in Ethereum (ABI) | Calls in Hedera (ABI) |
|------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|--------------------------------------------------------------------------------------------------------------|
Expand Down
34 changes: 21 additions & 13 deletions ...com/hedera/node/app/service/contract/impl/exec/processors/CustomMessageCallProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,29 +123,35 @@ public void start(@NonNull final MessageFrame frame, @NonNull final OperationTra
return;
}

final var evmPrecompile = precompiles.get(codeAddress);
// Check to see if the code address is a system account and possibly halt
if (addressChecks.isSystemAccount(codeAddress)) {
doHaltIfInvalidSystemCall(codeAddress, frame, tracer);
doHaltIfInvalidSystemCall(frame, tracer);
if (alreadyHalted(frame)) {
return;
}
}

// Transfer value to the contract if required and possibly halt
if (transfersValue(frame)) {
doTransferValueOrHalt(frame, tracer);
if (alreadyHalted(frame)) {
if (evmPrecompile == null) {
handleNonExtantSystemAccount(frame, tracer);

return;
}
}

// Handle evm precompiles
final var evmPrecompile = precompiles.get(codeAddress);
if (evmPrecompile != null) {
doExecutePrecompile(evmPrecompile, frame, tracer);
return;
}

// Transfer value to the contract if required and possibly halt
if (transfersValue(frame)) {
doTransferValueOrHalt(frame, tracer);
if (alreadyHalted(frame)) {
return;
}
}

// For mono-service fidelity, we need to consider called contracts
// as a special case eligible for staking rewards
if (isTopLevelTransaction(frame)) {
Expand All @@ -162,6 +168,13 @@ public boolean isImplicitCreationEnabled(@NonNull Configuration config) {
return featureFlags.isImplicitCreationEnabled(config);
}

private void handleNonExtantSystemAccount(
@NonNull final MessageFrame frame, @NonNull final OperationTracer tracer) {
final PrecompileContractResult result = PrecompileContractResult.success(Bytes.EMPTY);
frame.clearGasRemaining();
finishPrecompileExecution(frame, result, PRECOMPILE, (ActionSidecarContentTracer) tracer);
}

private void doExecutePrecompile(
@NonNull final PrecompiledContract precompile,
@NonNull final MessageFrame frame,
Expand Down Expand Up @@ -250,14 +263,9 @@ private void doTransferValueOrHalt(
}

private void doHaltIfInvalidSystemCall(
@NonNull final Address codeAddress,
@NonNull final MessageFrame frame,
@NonNull final OperationTracer operationTracer) {
@NonNull final MessageFrame frame, @NonNull final OperationTracer operationTracer) {
if (transfersValue(frame)) {
doHalt(frame, INVALID_FEE_SUBMITTED, operationTracer);
} else if (precompiles.get(codeAddress) == null) {
final PrecompileContractResult result = PrecompileContractResult.success(Bytes.EMPTY);
finishPrecompileExecution(frame, result, PRECOMPILE, (ActionSidecarContentTracer) operationTracer);
}
}

Expand Down
19 changes: 11 additions & 8 deletions ...a/node/app/service/contract/impl/test/exec/processors/CustomMessageCallProcessorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,9 @@ void callPrngSystemContractInsufficientGas() {

@Test
void callsToNonStandardSystemContractsAreNotSupported() {
final var isHalted = new AtomicBoolean();
givenHaltableFrame(isHalted);
final Deque<MessageFrame> stack = new ArrayDeque<>();
givenCallWithCode(NON_EVM_PRECOMPILE_SYSTEM_ADDRESS);

given(addressChecks.isSystemAccount(NON_EVM_PRECOMPILE_SYSTEM_ADDRESS)).willReturn(true);
when(frame.getValue()).thenReturn(Wei.ZERO);

Expand Down Expand Up @@ -201,8 +201,6 @@ void haltsIfValueTransferFails() {

@Test
void haltsAndTracesInsufficientGasIfPrecompileGasRequirementExceedsRemaining() {
final var isHalted = new AtomicBoolean();
givenHaltableFrame(isHalted);
givenEvmPrecompileCall();
given(nativePrecompile.gasRequirement(INPUT_DATA)).willReturn(GAS_REQUIREMENT);
given(frame.getRemainingGas()).willReturn(1L);
Expand All @@ -221,7 +219,6 @@ void updatesFrameBySuccessfulPrecompileResultWithGasRefund() {
given(nativePrecompile.computePrecompile(INPUT_DATA, frame)).willReturn(result);
given(nativePrecompile.gasRequirement(INPUT_DATA)).willReturn(GAS_REQUIREMENT);
given(frame.getRemainingGas()).willReturn(3L);
given(frame.getState()).willReturn(MessageFrame.State.COMPLETED_SUCCESS);

subject.start(frame, operationTracer);

Expand All @@ -240,7 +237,6 @@ void revertsFrameFromPrecompileResult() {
given(nativePrecompile.computePrecompile(INPUT_DATA, frame)).willReturn(result);
given(nativePrecompile.gasRequirement(INPUT_DATA)).willReturn(GAS_REQUIREMENT);
given(frame.getRemainingGas()).willReturn(3L);
given(frame.getState()).willReturn(MessageFrame.State.REVERT);

subject.start(frame, operationTracer);

Expand Down Expand Up @@ -294,6 +290,15 @@ private void givenHaltableFrame(@NonNull final AtomicBoolean isHalted) {
.getState();
}

private void givenCallWithIsTopLevelTransaction(@NonNull final AtomicBoolean isTopLevelTransaction) {
doAnswer(invocation -> {
isTopLevelTransaction.set(false);
return null;
})
.when(frame)
.setExceptionalHaltReason(any());
}

private void givenCallWithCode(@NonNull final Address contract) {
given(frame.getContractAddress()).willReturn(contract);
}
Expand All @@ -305,11 +310,9 @@ private void givenWellKnownUserSpaceCall() {
}

private void givenEvmPrecompileCall() {
given(addressChecks.isSystemAccount(ADDRESS_6)).willReturn(true);
given(registry.get(ADDRESS_6)).willReturn(nativePrecompile);
given(frame.getContractAddress()).willReturn(ADDRESS_6);
given(frame.getInputData()).willReturn(INPUT_DATA);
given(frame.getValue()).willReturn(Wei.ZERO);
}

private void givenPrngCall(long gasRequirement) {
Expand Down
64 changes: 38 additions & 26 deletions ...ients/src/main/java/com/hedera/services/bdd/suites/contract/evm/Evm46ValidationSuite.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@
import static com.hederahashgraph.api.proto.java.ResponseCodeEnum.SUCCESS;
import static com.swirlds.common.utility.CommonUtils.unhex;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;

import com.google.protobuf.ByteString;
import com.hedera.services.bdd.junit.HapiTest;
Expand Down Expand Up @@ -154,6 +153,8 @@ public List<HapiSpec> getSpecsInSuite() {
directCallToNonExistingNonMirrorAddressResultsInSuccessfulNoOp(),
// EOA -calls-> ExistingCryptoAccount, expect noop success
directCallToExistingCryptoAccountResultsInSuccess(),
// EOA -calls-> SystemAccount, expect noop success
directCallToSystemAccountResultsInSuccessfulNoOp(),
// EOA -callsWValue-> ExistingCryptoAccount, expect successful transfer
directCallWithValueToExistingCryptoAccountResultsInSuccess(),
// EOA -calls-> Reverting, expect revert
Expand Down Expand Up @@ -309,17 +310,15 @@ private HapiSpec selfdestructToExistingMirrorAddressResultsInSuccess() {
cryptoCreate(RECEIVER).exposingCreatedIdTo(receiverId::set),
uploadInitCode(INTERNAL_CALLER_CONTRACT),
contractCreate(INTERNAL_CALLER_CONTRACT).balance(ONE_HBAR))
.when(withOpContext((spec, op) -> {
allRunFor(
spec,
balanceSnapshot("selfdestructTargetAccount", asAccountString(receiverId.get())),
contractCall(
INTERNAL_CALLER_CONTRACT,
SELFDESTRUCT,
mirrorAddrWith(receiverId.get().getAccountNum()))
.gas(GAS_LIMIT_FOR_CALL * 4)
.via(INNER_TXN));
}))
.when(withOpContext((spec, op) -> allRunFor(
spec,
balanceSnapshot("selfdestructTargetAccount", asAccountString(receiverId.get())),
contractCall(
INTERNAL_CALLER_CONTRACT,
SELFDESTRUCT,
mirrorAddrWith(receiverId.get().getAccountNum()))
.gas(GAS_LIMIT_FOR_CALL * 4)
.via(INNER_TXN))))
.then(getAccountBalance(RECEIVER)
.hasTinyBars(changeFromSnapshot("selfdestructTargetAccount", 100000000)));
}
Expand Down Expand Up @@ -403,7 +402,7 @@ private HapiSpec selfdestructToNonExistingMirrorAddressResultsInInvalidSolidityA
}

@HapiTest
private HapiSpec directCallToNonExistingMirrorAddressResultsInSuccessfulNoOp() {
HapiSpec directCallToNonExistingMirrorAddressResultsInSuccessfulNoOp() {

return defaultHapiSpec("directCallToNonExistingMirrorAddressResultsInSuccessfulNoOp")
.given(withOpContext((spec, ctxLog) -> spec.registry()
Expand Down Expand Up @@ -536,6 +535,28 @@ private HapiSpec directCallToExistingCryptoAccountResultsInSuccess() {
.gasUsed(INTRINSIC_GAS_COST + EXTRA_GAS_FOR_FUNCTION_SELECTOR))));
}

@HapiTest
HapiSpec directCallToSystemAccountResultsInSuccessfulNoOp() {
return defaultHapiSpec("directCallToSystemAccountResultsInSuccessfulNoOp")
.given(
cryptoCreate("account").balance(ONE_HUNDRED_HBARS),
withOpContext((spec, opLog) -> spec.registry()
.saveContractId(
"contract",
asContractIdWithEvmAddress(ByteString.copyFrom(
unhex("0000000000000000000000000000000000000275"))))))
.when(withOpContext((spec, ctxLog) -> allRunFor(
spec,
contractCallWithFunctionAbi("contract", getABIFor(FUNCTION, NAME, ERC_721_ABI))
.gas(GAS_LIMIT_FOR_CALL)
.via("callToSystemAddress")
.signingWith("account"))))
.then(getTxnRecord("callToSystemAddress")
.hasPriority(recordWith()
.status(SUCCESS)
.contractCallResult(resultWith().gasUsed(GAS_LIMIT_FOR_CALL))));
}

@HapiTest
private HapiSpec directCallWithValueToExistingCryptoAccountResultsInSuccess() {

Expand Down Expand Up @@ -1411,18 +1432,9 @@ private HapiSpec internalCallToEthereumPrecompile0x2ResultsInSuccess() {
CALL_EXTERNAL_FUNCTION,
mirrorAddrWith(targetId.get().getAccountNum()))
.gas(GAS_LIMIT_FOR_CALL * 4)
.via(INNER_TXN))))
.then(
withOpContext((spec, opLog) -> {
final var lookup = getTxnRecord(INNER_TXN);
allRunFor(spec, lookup);
final var result = lookup.getResponseRecord()
.getContractCallResult()
.getContractCallResult();
assertNotEquals(ByteString.copyFrom(new byte[32]), result);
}),
getAccountBalance(INTERNAL_CALLER_CONTRACT)
.hasTinyBars(changeFromSnapshot("initialBalance", 0)));
.via(INNER_TXN)
.hasKnownStatus(SUCCESS))))
.then(getAccountBalance(INTERNAL_CALLER_CONTRACT).hasTinyBars(changeFromSnapshot("initialBalance", 0)));
}

@HapiTest
Expand Down Expand Up @@ -1499,7 +1511,7 @@ final HapiSpec internalCallWithValueToNonExistingSystemAccount852ResultsInInvali
}

@HapiTest
private HapiSpec internalCallWithValueToSystemAccount564ResultsInSuccessNoopNoTransfer() {
HapiSpec internalCallWithValueToSystemAccount564ResultsInSuccessNoopNoTransfer() {
AtomicReference<AccountID> targetId = new AtomicReference<>();
targetId.set(AccountID.newBuilder().setAccountNum(564L).build());

Expand Down
7 changes: 4 additions & 3 deletions ...src/main/java/com/hedera/services/bdd/suites/contract/traceability/TraceabilitySuite.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4587,7 +4587,7 @@ final HapiSpec traceabilityE2EScenario21() {
.setCallingAccount(TxnUtils.asId(GENESIS, spec))
.setCallOperationType(CallOperationType.OP_CALL)
.setGas(978936)
.setGasUsed(18360)
.setGasUsed(963748)
.setOutput(EMPTY)
/*
For EVM v0.34 use this code block instead:
Expand All @@ -4609,6 +4609,7 @@ final HapiSpec traceabilityE2EScenario21() {
.encodeCallWithArgs(BigInteger.valueOf(234))
.array()))
.setGas(960576)
.setGasUsed(960576)
.setRecipientContract(ContractID.newBuilder()
.setContractNum(0)
.build())
Expand Down Expand Up @@ -5108,7 +5109,7 @@ create2Factory, GET_BYTECODE, asHeadlongAddress(factoryEvmAddress.get()), salt)
.setRecipientContract(
spec.registry()
.getContractId(create2Factory))
.setGasUsed(80135)
.setGasUsed(80193)
.setOutput(EMPTY)
.setInput(
encodeFunctionCall(
Expand All @@ -5123,7 +5124,7 @@ create2Factory, GET_BYTECODE, asHeadlongAddress(factoryEvmAddress.get()), salt)
.setCallingContract(
spec.registry()
.getContractId(create2Factory))
.setGas(3870609)
.setGas(3870552)
// recipient should be the
// original hollow account id as
// a contract
Expand Down

0 comments on commit dd48484

Please sign in to comment.