Skip to content

Conversation

cloudgray
Copy link
Contributor

Description

Problem

  • When the caller contract calls the precompile, the error message isn’t propagated back to it.
  • Unlike the geth opcode opRevert, which returns the error message in its return data and ErrExecutionReverted as the error, the Cosmos/EVM precompile currently returns nil as return data and a custom error type as the error.
  • We need to make precompile errors behave like opRevert: return the error message in the return data and use ErrExecutionReverted as the error.

Solution

  1. Modify return values on precompile error
    • Always return ErrExecutionReverted as the error.
    • Encode the original error message with the Revert selector ("Error(string)") as the return data.
    • Use evm.Interpreter.SetReturnData() to set that encoded data into EVMInterpreter.returnData.
  2. Modify return values in ApplyMessageWithConfig
    • If err is ErrExecutionReverted, set res.Ret to evm.Interpreter.ReturnData().
  3. Update the test util function
    • Previously, on error the code logged the revert reason via emitted events (which never occur when an error is thrown).
    • Now, pull the EthereumTxResponse.Ret returned in step 2, decode it with abi.UnpackRevert, and print that as the reason.

Result

As-Is

err_msg_empty In the original code, errors thrown by the precompile were not propagated to the caller contract.

To-Be

revert_with_reason
Now, errors returned by the precompile are returned in the caller contract’s return data, and the test utilities have been updated so that the test code can verify them.

Closes: #223


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • tackled an existing issue or discussed with a team member
  • left instructions on how to review the changes
  • targeted the main branch

Reviewers Checklist

All items are required.
Please add a note if the item is not applicable
and please add your handle next to the items reviewed
if you only reviewed selected items.

I have...

  • added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • confirmed all author checklist items have been addressed
  • confirmed that this PR does not change production code
  • reviewed content
  • tested instructions (if applicable)
  • confirmed all CI checks have passed

@cloudgray cloudgray self-assigned this Jun 24, 2025
@cloudgray cloudgray added the enhancement New feature or request label Jul 2, 2025
@cloudgray cloudgray marked this pull request as ready for review July 2, 2025 12:54
txArgs.Amount = delAmt.BigInt()
txArgs.To = &contractTwoAddr

reverReasonCheck := execRevertedCheck.WithErrContains(
Copy link
Member

Choose a reason for hiding this comment

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

nit: revert typo

Copy link
Member

@vladjdk vladjdk left a comment

Choose a reason for hiding this comment

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

This is great, thank you!

@vladjdk vladjdk merged commit 76d8d10 into cosmos:main Jul 4, 2025
18 of 19 checks passed
Eric-Warehime pushed a commit that referenced this pull request Jul 17, 2025
commit 43cff363c5b90c9652960da00e62b737681804c2
Author: Kyuhyeon Choi <[email protected]>
Date:   Fri Jul 11 12:41:09 2025 +0900

    test(precmopiles): apply revert reason check

    This commit is for resolving cherry-pick conflict.

commit ec5a79edcc32c211e79ef04220153d60f68a9472
Author: Haber <[email protected]>
Date:   Sat Jul 5 05:37:53 2025 +0900

    fix(precompiles): return data for revert (#224)

    * fix(vm/keeper): add return data of ApplyMessageWithConfig for ErrExecutionReverted

    * fix(precompile): modify return data of distribution precompile for revert error

    * test: remove redundant test case

    * chore(precompiles/staking): modify description for integration test case

    * chore: fix lint

    * fix(precompiles): modify return data of precompiles for revert error

    * fix: broken test cases after modifying precompile err to revert err

    * refactor:(precompiles) convert error that precompile.Run returns to ErrExecutionReverted

    * wip: test(precompiles/staking): fix test cases

    * chore: compile latest test contracts

    * fix(precompiles/staking): check revert reason or integration test cases

    * test(precompiles/staking): fix unit test

    * test(precompiles/distribution): improve integration test

    * test(precompiles/erc20): improve integration test

    * test(precompiles/ics20): improve integration test

    * test(precompiles/slashing): add slashing integration test for proof of audit issue fix

    * chore: fix lint

    * chore: fix lint

commit a7cdcc537e0a33f36e8de36f0247bfe3a2a1d9dc
Author: Kyuhyeon Choi <[email protected]>
Date:   Thu Jul 10 19:45:25 2025 +0900

    fix(precompiles/common): hex address parsing method

commit 5a937ecd545703515993e6af47def0574a398897
Author: Kyuhyeon Choi <[email protected]>
Date:   Tue Jul 8 09:49:35 2025 +0900

    chore: fix lint

commit c32a6e5128c592d5fc791fc13733fc52e18553bc
Author: Kyuhyeon Choi <[email protected]>
Date:   Mon Jul 7 23:09:24 2025 +0900

    feat(precompiles): add BalanceHandler to handle native balance change (#201)

    * feat(precompiles): add BalanceHandler to handle native balance change

    * refactor: remove parts of calling SetBalanceChangeEntries

    * chore: fix lint

    * chore(precompiles/distribution): remove unused helper function

    * chore(precompiles): modify comments

    * chore: restore modification to be applied later

    * chore: fix typo

    * chore: resolve conflict

    * chore: fix lint

    * test(precompiles/common) add unit test cases

    * chore: fix lint

    * fix(test): precompile test case that intermittently fails

    * refactor: move mock evm keeper to x/vm/types/mocks

    * chore: add KVStoreKeys() method to mock evmKeeper

    * refactoring balance handling

    * test(precompile/common): improve unit test for balance handler

    * refactor(precompiles): separate common logic

    * Revert "refactor(precompiles): separate common logic"

    This reverts commit 25b89f3.

    * Revert "Merge pull request #1 from zsystm/poc/precompiles-balance-handler"

    This reverts commit 46cd527, reversing
    changes made to b532fd5.

    ---------

    Co-Authored-By: zsystm <[email protected]>
    Co-Authored-By: Vlad J <[email protected]>

commit 37e0f9a8cdd362c78d411bc4291e61b64bbc3dbc
Author: Haber <[email protected]>
Date:   Mon Jul 7 10:28:01 2025 +0900

    feat(precompiles): add BalanceHandler to handle native balance change (#201)

    * feat(precompiles): add BalanceHandler to handle native balance change

    * refactor: remove parts of calling SetBalanceChangeEntries

    * chore: fix lint

    * chore(precompiles/distribution): remove unused helper function

    * chore(precompiles): modify comments

    * chore: restore modification to be applied later

    * chore: fix typo

    * chore: resolve conflict

    * chore: fix lint

    * test(precompiles/common) add unit test cases

    * chore: fix lint

    * fix(test): precompile test case that intermittently fails

    * refactor: move mock evm keeper to x/vm/types/mocks

    * chore: add KVStoreKeys() method to mock evmKeeper

    * refactoring balance handling

    * test(precompile/common): improve unit test for balance handler

    * refactor(precompiles): separate common logic

    * Revert "refactor(precompiles): separate common logic"

    This reverts commit 25b89f3.

    * Revert "Merge pull request #1 from zsystm/poc/precompiles-balance-handler"

    This reverts commit 46cd527, reversing
    changes made to b532fd5.

    ---------

    Co-authored-by: zsystm <[email protected]>
    Co-authored-by: Vlad J <[email protected]>

commit a5bf7d1f255ab43a28ac19d3348d56f20d3dbe29
Author: Haber <[email protected]>
Date:   Thu Jun 12 11:17:44 2025 +0900

    refactor(precompiles): apply journal-based revert approach (#205)

    * refactor(precompiles): apply journal-based revert approach

    * refactor: remove unused Snapshot type

    * chore: fix lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Precompile Errors Not Propagated to the Caller Contract
2 participants