Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HIP-801: Implement integration tests covering new services #8344

Conversation

victor-yanev
Copy link
Contributor

@victor-yanev victor-yanev commented May 21, 2024

Description:

This PR adds integration tests which cover different positive and negative scenarios for the new processOpcodeCall method in ContractCallService:

  • an expected OpcodesProcessingResult is generated by re-executing the transaction with the same parameters and filling the expected opcodes in the OpcodesProcessingResult from the OpcodeTracer
  • each scenario executes a contract and compares the generated opcodes with the ones from the expected OpcodesProcessingResult and also checks for any pre-known data based on the contract function which is being called (e.g., a certain pre-known expected output or revert reason)
  • given a contract revert, tests also verify that the method does not throw an exception, but rather returns an OpcodesProcessingResult with the following values:
    • isSuccessful: false
    • output: Bytes.EMPTY
    • reason: should be filled with the hex-encoded revert reason
    • opcodes: all executed opcodes leading to the revert, where the last entry in the list includes the revert reason

Related issue(s):

Fixes #7341

Notes for reviewer:

NOTE: The tests are currently executed with new OpcodeTracerOptions(false, false, false) due to the long execution time needed to fill the data for stack, memory and storage when they are set to true

NOTE: Keep in mind that OpcodeTracerTest.java already contains unit tests which cover all edge cases for the logic which populates the opcodes list inside OpcodeTracer given different options for stack, memory, storage, given exceptional halt, revert, etc.)

With the current implementation, the integration tests execute in 1min 35sec, where the following scenarios are covered:

  • evmDynamicCallsTokenFunctions
    [1] function=MINT_FUNGIBLE_TOKEN
    [2] function=MINT_NFT
    [3] function=BURN_FUNGIBLE_TOKEN
    [4] function=BURN_NFT
    [5] function=WIPE_FUNGIBLE_TOKEN
    [6] function=WIPE_NFT
    [7] function=PAUSE_UNPAUSE_FUNGIBLE_TOKEN
    [8] function=FREEZE_UNFREEZE_FUNGIBLE_TOKEN
    [9] function=PAUSE_UNPAUSE_NFT
    [10] function=FREEZE_UNFREEZE_NFT
    [11] function=ASSOCIATE_TRANSFER_NFT
    [12] function=ASSOCIATE_TRANSFER_FUNGIBLE_TOKEN
    [13] function=ASSOCIATE_DISSOCIATE_TRANSFER_FUNGIBLE_TOKEN_FAIL
    [14] function=ASSOCIATE_DISSOCIATE_TRANSFER_NFT_FAIL
    [15] function=ASSOCIATE_TRANSFER_NFT_EXCEPTION
    [16] function=APPROVE_FUNGIBLE_TOKEN_GET_ALLOWANCE
    [17] function=APPROVE_NFT_GET_ALLOWANCE
    [18] function=APPROVE_FUNGIBLE_TOKEN_TRANSFER_FROM_GET_ALLOWANCE
    [19] function=APPROVE_FUNGIBLE_TOKEN_TRANSFER_FROM_GET_ALLOWANCE_2
    [20] function=APPROVE_NFT_TOKEN_TRANSFER_FROM_GET_ALLOWANCE
    [21] function=APPROVE_NFT_TOKEN_TRANSFER_FROM_GET_ALLOWANCE_2
    [22] function=APPROVE_FUNGIBLE_TOKEN_TRANSFER_GET_ALLOWANCE
    [23] function=APPROVE_NFT_TRANSFER_GET_ALLOWANCE
    [24] function=APPROVE_CRYPTO_TRANSFER_FUNGIBLE_GET_ALLOWANCE
    [25] function=APPROVE_CRYPTO_TRANSFER_NFT_GET_ALLOWANCE
    [26] function=APPROVE_FOR_ALL_TRANSFER_FROM_NFT_GET_ALLOWANCE
    [27] function=APPROVE_FOR_ALL_TRANSFER_NFT_GET_ALLOWANCE
    [28] function=APPROVE_FOR_ALL_CRYPTO_TRANSFER_NFT_GET_ALLOWANCE
    [29] function=TRANSFER_NFT_GET_ALLOWANCE_OWNER_OF
    [30] function=TRANSFER_FUNGIBLE_TOKEN_GET_BALANCE
    [31] function=TRANSFER_NFT_GET_OWNER
    [32] function=CRYPTO_TRANSFER_FUNFIBLE_TOKEN_GET_OWNER
    [33] function=CRYPTO_TRANSFER_NFT_GET_OWNER
    [34] function=GRANT_KYC_REVOKE_KYC_FUNGIBLE
    [35] function=GRANT_KYC_REVOKE_KYC_NFT
    [36] function=ADDRESS_THIS
  • evmPrecompileSupportedModificationTokenFunctions
    [1] function=TRANSFER_FROM
    [2] function=APPROVE
    [3] function=DELETE_ALLOWANCE
    [4] function=DELETE_ALLOWANCE_NFT
    [5] function=APPROVE_NFT
    [6] function=SET_APPROVAL_FOR_ALL
    [7] function=ASSOCIATE_TOKEN
    [8] function=ASSOCIATE_TOKENS
    [9] function=HRC_ASSOCIATE_REDIRECT
    [10] function=MINT_TOKEN
    [11] function=MINT_NFT_TOKEN
    [12] function=DISSOCIATE_TOKEN
    [13] function=DISSOCIATE_TOKENS
    [14] function=HRC_DISSOCIATE_REDIRECT
    [15] function=BURN_TOKEN
    [16] function=BURN_NFT_TOKEN
    [17] function=WIPE_TOKEN
    [18] function=WIPE_NFT_TOKEN
    [19] function=REVOKE_TOKEN_KYC
    [20] function=GRANT_TOKEN_KYC
    [21] function=DELETE_TOKEN
    [22] function=FREEZE_TOKEN
    [23] function=UNFREEZE_TOKEN
    [24] function=PAUSE_TOKEN
    [25] function=UNPAUSE_TOKEN
    [26] function=CREATE_FUNGIBLE_TOKEN
    [27] function=CREATE_FUNGIBLE_TOKEN_WITH_CUSTOM_FEES
    [28] function=CREATE_NON_FUNGIBLE_TOKEN
    [29] function=CREATE_NON_FUNGIBLE_TOKEN_WITH_CUSTOM_FEES
    [30] function=TRANSFER_TOKEN_WITH
    [31] function=TRANSFER_TOKENS
    [32] function=TRANSFER_TOKENS_WITH_ALIAS
    [33] function=CRYPTO_TRANSFER_TOKENS
    [34] function=CRYPTO_TRANSFER_TOKENS_WITH_ALIAS
    [35] function=CRYPTO_TRANSFER_HBARS_AND_TOKENS
    [36] function=CRYPTO_TRANSFER_HBARS
    [37] function=CRYPTO_TRANSFER_NFT
    [38] function=TRANSFER_NFT_TOKENS
    [39] function=TRANSFER_NFT_TOKEN
    [40] function=TRANSFER_FROM_NFT
    [41] function=UPDATE_TOKEN_INFO
    [42] function=UPDATE_TOKEN_EXPIRY
    [43] function=UPDATE_TOKEN_KEYS_CONTRACT_ADDRESS
    [44] function=UPDATE_TOKEN_KEYS_DELEGATABLE_CONTRACT_ID
    [45] function=UPDATE_TOKEN_KEYS_ED25519
    [46] function=UPDATE_TOKEN_KEYS_ECDSA
  • failedNestedCallWithHardcodedResult
    [1] function=GET_TOKEN_INFO_HISTORICAL
    [2] function=GET_TOKEN_INFO
    [3] function=HTS_GET_APPROVED_HISTORICAL
    [4] function=HTS_GET_APPROVED
    [5] function=MINT_TOKEN_HISTORICAL
    [6] function=MINT_TOKEN

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

konstantinabl and others added 30 commits May 7, 2024 18:29
Signed-off-by: Konstantina Blazhukova <[email protected]>
Signed-off-by: Victor Yanev <[email protected]>
Signed-off-by: Konstantina Blazhukova <[email protected]>
…nto 8173-implement-opcode-logger-tracing-logic

Need changes from other branch to debug
Signed-off-by: Konstantina Blazhukova <[email protected]>
Signed-off-by: Victor Yanev <[email protected]>
Signed-off-by: Victor Yanev <[email protected]>
Signed-off-by: Konstantina Blazhukova <[email protected]>
konstantinabl and others added 2 commits June 10, 2024 11:41
This reverts commit 965a85f.

Signed-off-by: Victor Yanev <[email protected]>
Signed-off-by: Victor Yanev <[email protected]>
@victor-yanev victor-yanev force-pushed the 7341-implement-integration-and-acceptance-tests-covering-opcode-tracer branch from 02095aa to cbd5de3 Compare June 10, 2024 08:42
@victor-yanev victor-yanev changed the title test: Implement integration tests covering ContractDebugService test: Implement integration tests covering new services (HIP-801) Jun 10, 2024
@victor-yanev
Copy link
Contributor Author

LGTM!

It will be also a good idea to manually test the /opcodes endpoint results against the evm cli tool from besu and compare the results for the same smart contract execution (for all cases except HTS precompiles, which are not native and not supported for besu). In this way, we will have 2 different sources of the same execution and compare their opcode traces.

We have automated this in the hedera-smart-contracts repo: hashgraph/hedera-smart-contracts#786

@natanasow
Copy link
Contributor

LGTM!

It will be also a good idea to manually test the /opcodes endpoint results against the evm cli tool from besu and compare the results for the same smart contract execution (for all cases except HTS precompiles, which are not native and not supported for besu). In this way, we will have 2 different sources of the same execution and compare their opcode traces.

There is a dedicated task/PR in the hedera-smart-contracts repo that should cover these cases, I guess.

@victor-yanev victor-yanev added this to the 0.107.0 milestone Jun 10, 2024
xin-hedera
xin-hedera previously approved these changes Jun 10, 2024
Copy link
Contributor

@xin-hedera xin-hedera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there still seems to be spotless formatting issue. perhaps at a later time we should run spotless on all web3 files and fix in a separate PR.

@steven-sheehy steven-sheehy modified the milestones: 0.107.0, 0.108.0 Jun 10, 2024
@konstantinabl konstantinabl force-pushed the 8173-implement-opcode-logger-tracing-logic branch from 7e2375a to 943724c Compare June 12, 2024 14:47
steven-sheehy
steven-sheehy previously approved these changes Jun 13, 2024
@steven-sheehy steven-sheehy changed the title test: Implement integration tests covering new services (HIP-801) HIP-801: Implement integration tests covering new services Jun 13, 2024
Base automatically changed from 8173-implement-opcode-logger-tracing-logic to main June 25, 2024 14:49
@steven-sheehy steven-sheehy dismissed stale reviews from xin-hedera, mgoelswirlds, IvanKavaldzhiev, kselveliev, and themself June 25, 2024 14:49

The base branch was changed.

@steven-sheehy steven-sheehy removed this from the 0.108.0 milestone Jun 25, 2024
…sts-covering-opcode-tracer

# Conflicts:
#	build.gradle.kts
#	hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/common/ContractCallContext.java
#	hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/evm/config/EvmConfiguration.java
#	hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/evm/contracts/execution/MirrorEvmTxProcessor.java
#	hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/evm/contracts/execution/MirrorEvmTxProcessorImpl.java
#	hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/evm/contracts/execution/traceability/OpcodeTracer.java
#	hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/repository/ContractActionRepository.java
#	hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/service/ContractCallService.java
#	hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/service/ContractDebugService.java
#	hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/service/ContractExecutionService.java
#	hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/service/OpcodeServiceImpl.java
#	hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/service/model/ContractDebugParameters.java
#	hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/service/model/ContractExecutionParameters.java
#	hedera-mirror-web3/src/main/java/com/hedera/node/app/service/evm/contracts/execution/HederaEvmTxProcessor.java
#	hedera-mirror-web3/src/test/java/com/hedera/mirror/web3/controller/OpcodesControllerTest.java
#	hedera-mirror-web3/src/test/java/com/hedera/mirror/web3/evm/contracts/execution/MirrorEvmTxProcessorTest.java
#	hedera-mirror-web3/src/test/java/com/hedera/mirror/web3/evm/contracts/execution/traceability/OpcodeTracerTest.java
#	hedera-mirror-web3/src/test/java/com/hedera/mirror/web3/service/ContractCallNativePrecompileTest.java
#	hedera-mirror-web3/src/test/java/com/hedera/mirror/web3/service/ContractCallTestSetup.java
Signed-off-by: Victor Yanev <[email protected]>
Copy link

@steven-sheehy steven-sheehy merged commit f2774e4 into main Jun 26, 2024
29 checks passed
@steven-sheehy steven-sheehy deleted the 7341-implement-integration-and-acceptance-tests-covering-opcode-tracer branch June 26, 2024 14:23
@steven-sheehy steven-sheehy added this to the 0.109.0 milestone Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
limechain Work planned for the LimeChain team test Test infrastructure, automated tests required, etc web3 Area: Web3 API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement integration and acceptance tests covering opcode tracer
8 participants