Skip to content

Commit

Permalink
Fix more tests in ContractCallServiceTests (#10382)
Browse files Browse the repository at this point in the history
This PR fixes some tests in `ContractCallServicesTests`
- made changes to capture the result from handleFailedResult but still throw an exception
- Lowered the initial balance of 0.0.2 to avoid FAILED_INVALID because it was set to maximum and wasn't able to receive reward
- Fix INSUFFICIENT_TX_FEE

---------

Signed-off-by: Nikolay Nikolov <[email protected]>
  • Loading branch information
nickeynikolovv authored Feb 17, 2025
1 parent 5393ea9 commit 69b0527
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -128,22 +128,26 @@ protected HederaEvmTransactionProcessingResult callContract(CallServiceParameter
protected HederaEvmTransactionProcessingResult doProcessCall(
CallServiceParameters params, long estimatedGas, boolean restoreGasToThrottleBucket)
throws MirrorEvmTransactionException {
HederaEvmTransactionProcessingResult result = null;

try {
HederaEvmTransactionProcessingResult result;
if (!mirrorNodeEvmProperties.isModularizedServices()) {
result = mirrorEvmTxProcessor.execute(params, estimatedGas);
} else {
result = transactionExecutionService.execute(params, estimatedGas, gasUsedCounter);
}
if (!restoreGasToThrottleBucket) {
return result;
}

restoreGasToBucket(result, params.getGas());
return result;
} catch (IllegalStateException | IllegalArgumentException e) {
throw new MirrorEvmTransactionException(e.getMessage(), EMPTY, EMPTY);
} catch (MirrorEvmTransactionException e) {
// This result is needed in case of exception to be still able to call restoreGasToBucket method
result = e.getResult();
throw e;
} finally {
if (restoreGasToThrottleBucket) {
restoreGasToBucket(result, params.getGas());
}
}
return result;
}

private void restoreGasToBucket(HederaEvmTransactionProcessingResult result, long gasLimit) {
Expand All @@ -152,7 +156,7 @@ private void restoreGasToBucket(HederaEvmTransactionProcessingResult result, lon
// of the gasLimit value back in the bucket.
final var gasLimitToRestoreBaseline =
(long) (Math.floorDiv(gasLimit, gasUnit) * throttleProperties.getGasLimitRefundPercent() / 100f);
if (!result.isSuccessful() && gasLimit == result.getGasUsed()) {
if (result == null || (!result.isSuccessful() && gasLimit == result.getGasUsed())) {
gasLimitBucket.addTokens(gasLimitToRestoreBaseline);
} else {
// The transaction was successful or reverted, so restore the remaining gas back in the bucket or
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ public class TransactionExecutionService {
private static final AccountID TREASURY_ACCOUNT_ID =
AccountID.newBuilder().accountNum(2).build();
private static final Duration TRANSACTION_DURATION = new Duration(15);
private static final long CONTRACT_CREATE_TX_FEE = 100_000_000L;

private final State mirrorNodeState;
private final MirrorNodeEvmProperties mirrorNodeEvmProperties;
Expand Down Expand Up @@ -145,7 +146,12 @@ private HederaEvmTransactionProcessingResult handleFailedResult(
var detail = maybeDecodeSolidityErrorStringToReadableMessage(errorMessage);
updateErrorGasUsedMetric(gasUsedCounter, result.gasUsed(), 1);
if (ContractCallContext.get().getOpcodeTracerOptions() == null) {
throw new MirrorEvmTransactionException(status.protoName(), detail, errorMessage.toHexString());
throw new MirrorEvmTransactionException(
status.protoName(),
detail,
errorMessage.toHexString(),
HederaEvmTransactionProcessingResult.failed(
result.gasUsed(), 0L, 0L, Optional.of(errorMessage), Optional.empty()));
} else {
// If we are in an opcode trace scenario, we need to return a failed result in order to get the
// opcode list from the ContractCallContext. If we throw an exception instead of returning a result,
Expand Down Expand Up @@ -181,6 +187,7 @@ private TransactionBody buildContractCreateTransactionBody(
.gas(estimatedGas)
.autoRenewPeriod(new Duration(maxLifetime))
.build())
.transactionFee(CONTRACT_CREATE_TX_FEE)
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,9 @@ protected void setup() {
.entity()
.customize(e -> e.id(2L)
.num(2L)
.balance(5000000000000000000L)
// The balance should not be set to max value 5000000000000000000L, because if we use it as a
// node operator it would not be able to receive rewards and can cause failures
.balance(1000000000000000000L)
.createdTimestamp(genesisRecordFile.getConsensusStart())
.timestampRange(Range.atLeast(genesisRecordFile.getConsensusStart())))
.persist();
Expand Down Expand Up @@ -144,13 +146,15 @@ protected long gasUsedAfterExecution(final ContractExecutionParameters servicePa
try {
return contractExecutionService.callContract(serviceParameters).getGasUsed();
} catch (MirrorEvmTransactionException e) {
var result = e.getResult();

// Some tests expect to fail but still want to capture the gas used
if (result != null) {
return result.getGasUsed();
if (mirrorNodeEvmProperties.isModularizedServices()) {
return e.getResult().getGasUsed();
} else {
var result = e.getResult();
if (result != null) {
return result.getGasUsed();
}
}

throw e;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;

import com.google.common.collect.Range;
import com.hedera.mirror.common.domain.entity.Entity;
import com.hedera.mirror.common.domain.entity.EntityId;
import com.hedera.mirror.web3.evm.contracts.execution.MirrorEvmTxProcessor;
Expand Down Expand Up @@ -715,6 +716,20 @@ void estimateGasForDirectCreateContractDeploy() {
final var serviceParameters = testWeb3jService.serviceParametersForTopLevelContractCreate(
contract.getContractBinary(), ETH_ESTIMATE_GAS, senderAddress);
final var actualGas = 175242L;
domainBuilder
.entity()
.customize(e -> e.id(801L)
.num(801L)
.createdTimestamp(genesisRecordFile.getConsensusStart())
.timestampRange(Range.atLeast(genesisRecordFile.getConsensusStart())))
.persist();
domainBuilder
.entity()
.customize(e -> e.id(800L)
.num(800L)
.createdTimestamp(genesisRecordFile.getConsensusStart())
.timestampRange(Range.atLeast(genesisRecordFile.getConsensusStart())))
.persist();

// When
final var result = contractExecutionService.processCall(serviceParameters);
Expand Down Expand Up @@ -832,6 +847,10 @@ void ercPrecompileExceptionalHaltReturnsExpectedGasToBucket(final CallType callT
// Given
final var token = tokenPersist();
final var contract = testWeb3jService.deploy(ERCTestContract::deploy);
final var payer = accountEntityWithEvmAddressPersist();
accountBalancePersist(payer, payer.getCreatedTimestamp());
testWeb3jService.setSender(toAddress(payer.toEntityId()).toHexString());

final var functionCall = contract.send_approve(
toAddress(token.getId()).toHexString(), SPENDER_ALIAS.toHexString(), BigInteger.valueOf(2));

Expand Down Expand Up @@ -871,6 +890,10 @@ void ercPrecompileExceptionalHaltReturnsExpectedGasToBucket(final CallType callT
void ercPrecompileContractRevertReturnsExpectedGasToBucket(
final CallType callType, final long gasLimit, final int gasUnit) {
// Given
final var payer = accountEntityWithEvmAddressPersist();
accountBalancePersist(payer, payer.getBalance());
testWeb3jService.setSender(toAddress(payer.toEntityId()).toHexString());

final var contract = testWeb3jService.deploy(ERCTestContract::deploy);
final var functionCall = contract.call_nameNonStatic(Address.ZERO.toHexString());
given(throttleProperties.getGasUnit()).willReturn(gasUnit);
Expand Down

0 comments on commit 69b0527

Please sign in to comment.