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

fix(evm)!: Fungible Token conversions #2003

Merged
merged 29 commits into from
Aug 15, 2024
Merged

Conversation

k-yang
Copy link
Member

@k-yang k-yang commented Aug 14, 2024

Purpose / Abstract

  • Closes epic(evm): fungible tokens: Coin to ERC20 and ERC20 to Coin #1898
  • Fixes a bug where tokens were being minted on the ERC-20 contract when the x/evm module is not the owner
  • Also fixes a bug where tokens were being minted instead of unescrowed on the Cosmos side
  • Refactored and added positive+negative test scenarios for all 4 cases
  • renamed some proto messages so it's state breaking

BREAKING CHANGE: renames proto messages

Summary by CodeRabbit

  • New Features

    • Introduced a bug fix for FunToken conversions between Cosmos and EVM, improving interoperability and accuracy.
    • Added enhancements to the ConvertCoinToEvm functionality, improving clarity and robustness in token conversion processes.
  • Bug Fixes

    • Resolved issues that affected the functionality of FunToken, enhancing the reliability of operations.
  • Documentation

    • Updated the CHANGELOG to reflect recent changes and improvements.
  • Tests

    • Expanded test coverage for FunToken functionality, including scenarios for conversion and error handling.
    • Improved error reporting in test assertions for better debugging.

Copy link
Contributor

coderabbitai bot commented Aug 14, 2024

Caution

Review failed

The pull request is closed.

Walkthrough

This update enhances the functionality of the Nibiru EVM by refining the conversion processes between native coins and ERC20 tokens, incorporating bug fixes and improved error handling. Key changes include renaming functions for clarity, adjusting return types for better memory management, and introducing new methods to encapsulate conversion logic. The overall goal is to improve interoperability and maintainability within the x/evm module, ensuring robust interactions between different fungible token standards.

Changes

File/Path Change Summary
CHANGELOG.md Added entry for a bug fix related to FunToken conversions between Cosmos and EVM.
proto/.../events.proto Renamed EventSendFunTokenToEvm to EventConvertCoinToEvm to reflect updated functionality.
proto/.../tx.proto Renamed SendFunTokenToEvm to ConvertCoinToEvm for clarity in method functionality.
x/evm/cli/tx.go Replaced command SendFunTokenToEvm() with ConvertCoinToEvm() to align with updated logic.
x/evm/evmmodule/genesis_test.go Updated test to use ConvertCoinToEvm, changing logic to reflect new functionality.
x/evm/keeper/erc20.go Improved error handling in Mint and Transfer methods; adjusted signatures for clarity.
x/evm/keeper/funtoken_from_coin.go Changed return types to pointers, updated error handling, and renamed methods for consistency.
x/evm/keeper/funtoken_from_coin_test.go Renamed test cases to reflect new focus on fun tokens; added content to cover new conversion logic.
x/evm/keeper/funtoken_from_erc20.go Changed return types for consistency; updated error handling for metadata retrieval.
x/evm/keeper/msg_server.go Refactored CreateFunToken and SendFunTokenToEvm methods for clarity, introduced new private methods for conversion.
x/evm/precompile/funtoken.go Adjusted logic in bankSend to burn tokens instead of minting under certain conditions.
x/evm/precompile/funtoken_test.go Updated tests to reflect new token handling logic, including funding operations and expected balance adjustments.

Assessment against linked issues

Objective Addressed Explanation
Extend functionality for creating FunToken mappings ( #1942, #1941)
Improve conversion processes between coins and ERC20 tokens (#1898)
Refactor and clarify existing functions and methods (#1944)
Implement robust error handling (#1943)
Enhance test coverage for new functionality (#1882)

🐰 In the meadow where tokens dance,
FunTokens leap in a joyful prance.
Converting coins with a happy cheer,
EVM's magic brings them near!
Bugs are fixed, the code is bright,
Hopping forward, all feels right! 🌼✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@k-yang k-yang changed the title fix(evm): Fungible Token conversions fix(evm)!: Fungible Token conversions Aug 14, 2024
@k-yang k-yang marked this pull request as ready for review August 14, 2024 22:10
@k-yang k-yang requested a review from a team as a code owner August 14, 2024 22:10
@k-yang k-yang self-assigned this Aug 14, 2024
@k-yang k-yang added the x: evm Relates to Nibiru EVM or the EVM Module label Aug 14, 2024
@k-yang k-yang added this to the Audit milestone Aug 14, 2024
@k-yang k-yang marked this pull request as draft August 14, 2024 22:15
@k-yang k-yang marked this pull request as ready for review August 14, 2024 22:22
Copy link

codecov bot commented Aug 14, 2024

Codecov Report

Attention: Patch coverage is 65.94203% with 47 lines in your changes missing coverage. Please review.

Project coverage is 65.85%. Comparing base (dbf26bb) to head (ee1b14b).
Report is 1 commits behind head on main.

Files Patch % Lines
x/evm/keeper/msg_server.go 69.51% 13 Missing and 12 partials ⚠️
x/evm/precompile/funtoken.go 18.18% 8 Missing and 1 partial ⚠️
x/evm/keeper/erc20.go 71.42% 5 Missing and 1 partial ⚠️
x/evm/msg.go 0.00% 3 Missing ⚠️
x/evm/keeper/funtoken_from_coin.go 80.00% 2 Missing ⚠️
x/evm/keeper/funtoken_from_erc20.go 80.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2003      +/-   ##
==========================================
- Coverage   65.86%   65.85%   -0.01%     
==========================================
  Files         261      261              
  Lines       16371    16435      +64     
==========================================
+ Hits        10783    10824      +41     
- Misses       4789     4802      +13     
- Partials      799      809      +10     
Files Coverage Δ
x/evm/evmtest/erc20.go 95.83% <100.00%> (ø)
x/evm/keeper/funtoken_from_coin.go 77.77% <80.00%> (+5.55%) ⬆️
x/evm/keeper/funtoken_from_erc20.go 77.35% <80.00%> (-0.42%) ⬇️
x/evm/msg.go 65.63% <0.00%> (ø)
x/evm/keeper/erc20.go 81.81% <71.42%> (-0.85%) ⬇️
x/evm/precompile/funtoken.go 48.19% <18.18%> (-1.81%) ⬇️
x/evm/keeper/msg_server.go 73.92% <69.51%> (-2.04%) ⬇️

... and 1 file with indirect coverage changes

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (5)
x/evm/keeper/funtoken_from_coin.go (1)

32-34: Add Test Coverage for Error Handling in deployERC20ForBankCoin.

The error handling for the deployERC20ForBankCoin function is not currently covered by tests. Consider adding test cases to ensure robust error handling.

Analysis chain

Coverage needed for error handling.

The error handling for deployERC20ForBankCoin is not covered by tests. Consider adding tests to ensure robust error handling.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify test coverage for error handling in `deployERC20ForBankCoin`.

# Test: Search for test functions that cover error scenarios in `deployERC20ForBankCoin`.
rg --type go -A 5 'deployERC20ForBankCoin' -- 'x/evm/keeper'

Length of output: 994


Script:

#!/bin/bash
# Description: Search for test files and functions in the `x/evm/keeper` directory.

# Find all Go test files in the `x/evm/keeper` directory.
fd --type f --extension go --exec rg --files-with-matches 'func.*Test' -- 'x/evm/keeper'

# Search for test functions that mention `deployERC20ForBankCoin` or handle errors related to it.
rg --type go 'deployERC20ForBankCoin' -- 'x/evm/keeper' --glob '*_test.go'

Length of output: 163463


Script:

#!/bin/bash
# Description: Search for test functions that reference `deployERC20ForBankCoin` in the `x/evm/keeper` directory.

# Correct the search command to find references in test files.
rg --type go 'deployERC20ForBankCoin' -- 'x/evm/keeper/*_test.go'

Length of output: 128


Script:

#!/bin/bash
# Description: Search for test functions that reference `deployERC20ForBankCoin` in the `x/evm/keeper` directory.

# Find all Go test files in the `x/evm/keeper` directory and search for `deployERC20ForBankCoin` within them.
fd --type f --extension go --exec rg 'deployERC20ForBankCoin' {}

Length of output: 170

Tools
GitHub Check: codecov/patch

[warning] 34-34: x/evm/keeper/funtoken_from_coin.go#L34
Added line #L34 was not covered by tests

x/evm/cli/tx.go (2)

102-103: Clarify Command Description.

The command description for ConvertCoinToEvm mentions "send-funtoken-to-erc20" in the Use field. Consider updating this to better reflect the conversion functionality, as it may imply a broader capability than just sending.

- Use:   "send-funtoken-to-erc20 [to_eth_addr] [coin] [flags]",
+ Use:   "convert-coin-to-evm [to_eth_addr] [coin] [flags]",

30-30: Update References to ConvertCoinToEvm for Consistency

While the function name ConvertCoinToEvm has been updated, there are still references to MsgSendFunTokenToEvm in comments and as part of function parameters and types. Ensure that these references are also updated for consistency across the codebase.

  • Files with outdated references:
    • x/evm/tx.pb.go: Function parameters and comments still reference MsgSendFunTokenToEvm.
    • x/evm/cli/tx.go: Comment references MsgSendFunTokenToEvm.

Please update these references to maintain consistency with the new command name.

Analysis chain

Ensure Consistency in Command Naming.

The function ConvertCoinToEvm replaces SendFunTokenToEvm. Make sure that all references to this command in documentation and other parts of the codebase are updated to reflect this change.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to `ConvertCoinToEvm` are updated in the codebase.

# Test: Search for the function usage. Expect: Only occurrences of the new command.
rg --type go $'ConvertCoinToEvm'

Length of output: 5034

x/evm/precompile/funtoken_test.go (1)

68-68: Ensure Consistent Update of bankDenom to "unibi" Across the Codebase

The variable bankDenom is not consistently set to "unibi" across all related tests and code sections. Please review and update the following instances to ensure uniformity:

  • x/evm/keeper/funtoken_from_coin_test.go: Multiple occurrences with different values such as "sometoken".
  • x/evm/keeper/erc20_test.go: Uses "ibc/btc".
  • x/evm/evmtest/eth_test.go: Uses "token".

Please update these instances to "unibi" where applicable to maintain consistency throughout the codebase.

Analysis chain

Update Bank Denomination for Consistency.

The bankDenom variable is changed to "unibi". Ensure that this change is consistently applied across all related tests and documentation.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to `bankDenom` are updated to "unibi" in the codebase.

# Test: Search for the variable usage. Expect: Only occurrences of the new denomination.
rg --type go $'bankDenom = "unibi"'

Length of output: 36


Script:

#!/bin/bash
# Description: Broadly search for all occurrences of `bankDenom` to verify its consistent update.

# Search for any occurrence of `bankDenom` and capture the context around it.
rg --type go 'bankDenom' -A 3 -B 3

Length of output: 23539

x/evm/keeper/msg_server.go (1)

557-562: Improve error handling for multiple fun tokens.

The function currently returns an error if multiple fun tokens are found. Consider logging additional context to aid debugging.

-  return nil, fmt.Errorf("multiple funtokens for bank denom \"%s\" found", msg.BankCoin.Denom)
+  k.Logger(ctx).Error("multiple funtokens found", "denom", msg.BankCoin.Denom)
+  return nil, fmt.Errorf("multiple funtokens for bank denom \"%s\" found", msg.BankCoin.Denom)
Tools
GitHub Check: codecov/patch

[warning] 559-559: x/evm/keeper/msg_server.go#L559
Added line #L559 was not covered by tests


[warning] 562-562: x/evm/keeper/msg_server.go#L562
Added line #L562 was not covered by tests

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between dbf26bb and 1d5114f.

Files ignored due to path filters (2)
  • x/evm/events.pb.go is excluded by !**/*.pb.go
  • x/evm/tx.pb.go is excluded by !**/*.pb.go
Files selected for processing (15)
  • CHANGELOG.md (1 hunks)
  • proto/eth/evm/v1/events.proto (1 hunks)
  • proto/eth/evm/v1/tx.proto (1 hunks)
  • x/evm/cli/tx.go (2 hunks)
  • x/evm/evmmodule/genesis_test.go (1 hunks)
  • x/evm/evmtest/erc20.go (1 hunks)
  • x/evm/keeper/erc20.go (3 hunks)
  • x/evm/keeper/erc20_test.go (1 hunks)
  • x/evm/keeper/funtoken_from_coin.go (2 hunks)
  • x/evm/keeper/funtoken_from_coin_test.go (3 hunks)
  • x/evm/keeper/funtoken_from_erc20.go (5 hunks)
  • x/evm/keeper/funtoken_from_erc20_test.go (2 hunks)
  • x/evm/keeper/msg_server.go (4 hunks)
  • x/evm/precompile/funtoken.go (1 hunks)
  • x/evm/precompile/funtoken_test.go (4 hunks)
Files skipped from review due to trivial changes (1)
  • x/evm/keeper/erc20_test.go
Additional context used
GitHub Check: codecov/patch
x/evm/keeper/funtoken_from_coin.go

[warning] 34-34: x/evm/keeper/funtoken_from_coin.go#L34
Added line #L34 was not covered by tests


[warning] 39-39: x/evm/keeper/funtoken_from_coin.go#L39
Added line #L39 was not covered by tests

x/evm/keeper/funtoken_from_erc20.go

[warning] 38-38: x/evm/keeper/funtoken_from_erc20.go#L38
Added line #L38 was not covered by tests


[warning] 43-43: x/evm/keeper/funtoken_from_erc20.go#L43
Added line #L43 was not covered by tests

x/evm/precompile/funtoken.go

[warning] 160-161: x/evm/precompile/funtoken.go#L160-L161
Added lines #L160 - L161 were not covered by tests


[warning] 163-168: x/evm/precompile/funtoken.go#L163-L168
Added lines #L163 - L168 were not covered by tests

x/evm/keeper/erc20.go

[warning] 58-58: x/evm/keeper/erc20.go#L58
Added line #L58 was not covered by tests


[warning] 79-79: x/evm/keeper/erc20.go#L79
Added line #L79 was not covered by tests


[warning] 89-89: x/evm/keeper/erc20.go#L89
Added line #L89 was not covered by tests


[warning] 366-366: x/evm/keeper/erc20.go#L366
Added line #L366 was not covered by tests


[warning] 374-374: x/evm/keeper/erc20.go#L374
Added line #L374 was not covered by tests

x/evm/keeper/msg_server.go

[warning] 559-559: x/evm/keeper/msg_server.go#L559
Added line #L559 was not covered by tests


[warning] 562-562: x/evm/keeper/msg_server.go#L562
Added line #L562 was not covered by tests


[warning] 633-633: x/evm/keeper/msg_server.go#L633
Added line #L633 was not covered by tests


[warning] 636-636: x/evm/keeper/msg_server.go#L636
Added line #L636 was not covered by tests


[warning] 657-657: x/evm/keeper/msg_server.go#L657
Added line #L657 was not covered by tests


[warning] 660-660: x/evm/keeper/msg_server.go#L660
Added line #L660 was not covered by tests


[warning] 663-663: x/evm/keeper/msg_server.go#L663
Added line #L663 was not covered by tests


[warning] 675-675: x/evm/keeper/msg_server.go#L675
Added line #L675 was not covered by tests


[warning] 678-678: x/evm/keeper/msg_server.go#L678
Added line #L678 was not covered by tests


[warning] 684-684: x/evm/keeper/msg_server.go#L684
Added line #L684 was not covered by tests


[warning] 687-687: x/evm/keeper/msg_server.go#L687
Added line #L687 was not covered by tests


[warning] 692-692: x/evm/keeper/msg_server.go#L692
Added line #L692 was not covered by tests


[warning] 698-698: x/evm/keeper/msg_server.go#L698
Added line #L698 was not covered by tests

Additional comments not posted (26)
proto/eth/evm/v1/events.proto (1)

58-59: Improved naming for clarity.

Renaming EventSendFunTokenToEvm to EventConvertCoinToEvm enhances the clarity of the event's purpose, aligning with the functionality described in the PR objectives.

x/evm/evmtest/erc20.go (1)

25-25: Enhanced error reporting in assertions.

Adding a formatted message to the assert.Zero call improves the clarity of test failure reports by showing expected and actual balances.

x/evm/keeper/funtoken_from_coin.go (4)

17-19: Improved error handling and return type.

Changing the return type of createFunTokenFromCoin to a pointer enhances error handling by explicitly indicating when no valid FunToken is created.


26-28: Ensure metadata existence for bank coins.

The check for bankMetadata ensures that bank coins have associated metadata, which is crucial for creating FunTokens.


56-59: Consistent naming for unexported methods.

Renaming DeployERC20ForBankCoin to deployERC20ForBankCoin aligns with Go's convention for unexported methods, improving code consistency.


39-39: Coverage needed for ERC20 registration check.

The check for existing FunToken mappings for an ERC20 address is not covered by tests. Consider adding tests to validate this logic.

Verification successful

Test coverage for ERC20 registration check is present.

The funtoken_state_test.go file includes a test case that verifies the scenario where ERC20 addresses are already registered, indicating that the necessary test coverage for the ERC20 registration check is in place.

  • File: x/evm/keeper/funtoken_state_test.go
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify test coverage for ERC20 registration checks.

# Test: Search for test functions that cover scenarios where ERC20 addresses are already registered.
rg --type go -A 5 'ERC20Addr.ExactMatch' -- 'x/evm/keeper'

Length of output: 2179

Tools
GitHub Check: codecov/patch

[warning] 39-39: x/evm/keeper/funtoken_from_coin.go#L39
Added line #L39 was not covered by tests

x/evm/precompile/funtoken_test.go (3)

117-119: Check Balance Assertions.

The assertions for ERC20 balances and bank balances should align with the expected outcomes after the conversion and send operations. Ensure that these checks accurately reflect the intended behavior.

Verification successful

Balance Assertions Verified

The balance assertions in funtoken_test.go correctly reflect the expected outcomes after the conversion and send operations. The ERC20 and bank balance checks align with the intended behavior.

  • The sender's ERC20 balance is correctly set to 69_000.
  • The EVM module's ERC20 balance remains 0.
  • The bank balance for the random account is accurately checked to be 420.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify balance assertions are correct.

# Test: Search for balance assertions in test files. Expect: Consistency with expected outcomes.
rg --type go $'AssertERC20BalanceEqual'

Length of output: 2202


Script:

#!/bin/bash
# Search for the context around the balance assertions in funtoken_test.go to understand the operations leading to these checks.
rg --type go -A 10 'AssertERC20BalanceEqual' x/evm/precompile/funtoken_test.go

Length of output: 818


76-81: Ensure Adequate Account Funding.

The test now includes funding the sender's Nibiru address with 69,420 units of "unibi". Verify that this amount is appropriate for the test scenario and aligns with the expected outcomes.


83-91: Validate Conversion Functionality.

The call to ConvertCoinToEvm is a critical part of the test. Ensure that the conversion logic is correctly implemented and that the test adequately checks for expected outcomes.

x/evm/evmmodule/genesis_test.go (1)

79-79: Ensure Correct Function Integration.

The integration of ConvertCoinToEvm in the TestExportInitGenesis test case should be verified to ensure it aligns with the test's objectives and logic.

x/evm/keeper/funtoken_from_erc20.go (2)

29-46: LGTM! Ensure test coverage for error handling.

The change to return a pointer improves performance by avoiding unnecessary copying of ERC20Metadata. Ensure that error handling paths are covered by tests.

Tools
GitHub Check: codecov/patch

[warning] 38-38: x/evm/keeper/funtoken_from_erc20.go#L38
Added line #L38 was not covered by tests


[warning] 43-43: x/evm/keeper/funtoken_from_erc20.go#L43
Added line #L43 was not covered by tests


Line range hint 92-172:
LGTM! Ensure comprehensive test coverage.

The change to return a pointer is consistent and appropriate. The function's logic is sound, with thorough checks for existing mappings and metadata validation. Ensure all error paths are covered by tests.

x/evm/precompile/funtoken.go (1)

153-172: LGTM! Ensure test coverage for conditional logic.

The logic change to handle token supply tracking based on IsMadeFromCoin is correct and enhances reliability. Ensure both branches of the conditional logic are covered by tests.

Tools
GitHub Check: codecov/patch

[warning] 160-161: x/evm/precompile/funtoken.go#L160-L161
Added lines #L160 - L161 were not covered by tests


[warning] 163-168: x/evm/precompile/funtoken.go#L163-L168
Added lines #L163 - L168 were not covered by tests

x/evm/keeper/funtoken_from_erc20_test.go (2)

32-141: LGTM! Ensure test cases cover all edge cases.

The refactoring improves readability and maintainability. Ensure that test cases cover all potential edge cases and scenarios.


158-254: LGTM! Ensure comprehensive test coverage.

The refactoring improves readability and maintainability. The test cases are comprehensive, covering both successful and error scenarios.

proto/eth/evm/v1/tx.proto (1)

30-33: Rename RPC method for clarity.

The RPC method SendFunTokenToEvm has been renamed to ConvertCoinToEvm. This change improves the clarity of the method's purpose, aligning it more closely with its functionality of converting coins to their ERC20 representation.

x/evm/keeper/funtoken_from_coin_test.go (2)

Line range hint 24-166: Enhance test coverage and accuracy.

The test function has been renamed and updated to include checks for insufficient funds, invalid denominations, and already registered tokens. The dynamic computation of contract addresses improves the accuracy of the tests.


169-295: Introduce comprehensive tests for conversion logic.

The new test function TestConvertCoinToEvmAndBack provides thorough coverage of the conversion process, including balance checks and event verification. It ensures robustness in handling both successful and unsuccessful conversion scenarios.

x/evm/keeper/erc20.go (3)

76-92: Enhance clarity in Transfer method.

The method now returns a boolean indicating success, improving clarity and robustness in error handling.

However, some lines are not covered by tests. Consider adding tests to ensure these paths are exercised.

Verification successful

Test Coverage for Transfer Method Found

The Transfer method is covered by tests, including scenarios for both successful transfers and error handling, such as insufficient balance. This indicates that the method's error handling paths are exercised in the test suite.

  • File: x/evm/keeper/erc20_test.go
    • Tests for insufficient funds and successful transfers are present.

No further action is required regarding test coverage for the Transfer method.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify test coverage for the `Transfer` method's error handling.

# Test: Search for the `Transfer` method usage in test files. Expect: Tests covering error scenarios.
rg --type go 'Transfer(' --context 5

Length of output: 107


Script:

#!/bin/bash
# Description: Verify test coverage for the `Transfer` method's error handling.

# Test: Search for the `Transfer` method usage in test files. Expect: Tests covering error scenarios.
rg --type go 'Transfer\(' --context 5

Length of output: 14933

Tools
GitHub Check: codecov/patch

[warning] 79-79: x/evm/keeper/erc20.go#L79
Added line #L79 was not covered by tests


[warning] 89-89: x/evm/keeper/erc20.go#L89
Added line #L89 was not covered by tests


58-58: Improve error clarity in Mint method.

The method now returns a detailed error message when ABI packing fails, enhancing the clarity of error handling.

However, this line is not covered by tests. Consider adding tests to ensure this error path is exercised.

Verification successful

Test Coverage Found for Mint Method

The Mint method in x/evm/keeper/erc20.go is covered by tests in x/evm/keeper/erc20_test.go. The tests include scenarios for both successful and failed minting operations, which implies that the error handling path is being exercised.

  • File: x/evm/keeper/erc20_test.go
    • Tests for minting failure due to non-owner execution.
    • Tests for successful minting.

This indicates that the error handling in the Mint method is adequately covered by tests.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify test coverage for the `Mint` method's error handling.

# Test: Search for the `Mint` method usage in test files. Expect: Tests covering error scenarios.
rg --type go 'Mint(' --context 5

Length of output: 95


Script:

#!/bin/bash
# Description: Verify test coverage for the `Mint` method's error handling.

# Test: Search for the `Mint` method usage in test files. Expect: Tests covering error scenarios.
rg --type go 'Mint\(' --context 5

Length of output: 9378

Tools
GitHub Check: codecov/patch

[warning] 58-58: x/evm/keeper/erc20.go#L58
Added line #L58 was not covered by tests


351-377: Improve readability and error handling in LoadERC20BigInt.

The parameter renaming and improved error handling enhance the method's readability and robustness.

However, some lines are not covered by tests. Consider adding tests to ensure these paths are exercised.

Tools
GitHub Check: codecov/patch

[warning] 366-366: x/evm/keeper/erc20.go#L366
Added line #L366 was not covered by tests


[warning] 374-374: x/evm/keeper/erc20.go#L374
Added line #L374 was not covered by tests

x/evm/keeper/msg_server.go (4)

549-572: Verify test coverage for ConvertCoinToEvm.

The function has been renamed and updated with improved error handling. Ensure that the test suite covers these changes, especially the error paths.

Verification successful

Test coverage verified for ConvertCoinToEvm.

The function ConvertCoinToEvm is covered in multiple test files, indicating that its behavior is being tested. Here are the locations where it is referenced:

  • x/evm/keeper/funtoken_from_erc20_test.go
  • x/evm/keeper/funtoken_from_coin_test.go
  • x/evm/precompile/funtoken_test.go
  • x/evm/evmmodule/genesis_test.go

These references suggest that the function's logic, including error handling, is being exercised in the test suite.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify test coverage for the `ConvertCoinToEvm` function.

# Test: Search for test functions that cover `ConvertCoinToEvm`. Expect: At least one test function.
rg --type go -A 5 $'func TestConvertCoinToEvm'

Length of output: 47


Script:

#!/bin/bash
# Description: Search for any references to `ConvertCoinToEvm` in test files to check for indirect test coverage.

# Search for the usage of `ConvertCoinToEvm` in Go test files.
rg --type go 'ConvertCoinToEvm' --glob '*_test.go'

Length of output: 769

Tools
GitHub Check: codecov/patch

[warning] 559-559: x/evm/keeper/msg_server.go#L559
Added line #L559 was not covered by tests


[warning] 562-562: x/evm/keeper/msg_server.go#L562
Added line #L562 was not covered by tests


622-705: Verify test coverage for convertCoinNativeERC20.

This new function handles the conversion of ERC20 tokens back to their original form. Ensure that the test suite covers this functionality, including error paths.

Tools
GitHub Check: codecov/patch

[warning] 633-633: x/evm/keeper/msg_server.go#L633
Added line #L633 was not covered by tests


[warning] 636-636: x/evm/keeper/msg_server.go#L636
Added line #L636 was not covered by tests


[warning] 657-657: x/evm/keeper/msg_server.go#L657
Added line #L657 was not covered by tests


[warning] 660-660: x/evm/keeper/msg_server.go#L660
Added line #L660 was not covered by tests


[warning] 663-663: x/evm/keeper/msg_server.go#L663
Added line #L663 was not covered by tests


[warning] 675-675: x/evm/keeper/msg_server.go#L675
Added line #L675 was not covered by tests


[warning] 678-678: x/evm/keeper/msg_server.go#L678
Added line #L678 was not covered by tests


[warning] 684-684: x/evm/keeper/msg_server.go#L684
Added line #L684 was not covered by tests


[warning] 687-687: x/evm/keeper/msg_server.go#L687
Added line #L687 was not covered by tests


[warning] 692-692: x/evm/keeper/msg_server.go#L692
Added line #L692 was not covered by tests


[warning] 698-698: x/evm/keeper/msg_server.go#L698
Added line #L698 was not covered by tests


Line range hint 487-523:
Verify test coverage for CreateFunToken.

The function has been updated with improved error handling and pointer usage. Ensure that the test suite covers these changes, especially the error paths.

Verification successful

Test Coverage Verified for CreateFunToken: The CreateFunToken function is adequately covered by tests in multiple files, including funtoken_from_coin_test.go and funtoken_from_erc20_test.go. These tests cover various scenarios, ensuring the function's robustness.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify test coverage for the `CreateFunToken` function.

# Test: Search for test functions that cover `CreateFunToken`. Expect: At least one test function.
rg --type go -A 5 $'func TestCreateFunToken'

Length of output: 45


Script:

#!/bin/bash
# Description: Search for references to `CreateFunToken` in test files to verify indirect test coverage.

# Test: Look for any usage of `CreateFunToken` in test files.
rg --type go 'CreateFunToken' --glob '*_test.go'

Length of output: 3961


Line range hint 576-617:
Verify test coverage for convertCoinNativeCoin.

This new function handles the conversion of native coins to ERC20 tokens. Ensure that the test suite covers this functionality, including error paths.

Tools
GitHub Check: codecov/patch

[warning] 559-559: x/evm/keeper/msg_server.go#L559
Added line #L559 was not covered by tests


[warning] 562-562: x/evm/keeper/msg_server.go#L562
Added line #L562 was not covered by tests

CHANGELOG.md (1)

108-108: Ensure clarity and consistency in changelog entries.

The new entry for the bug fix related to FunToken conversions is clear and concise. Ensure that it follows the project's changelog guidelines and includes all necessary details, such as the GitHub issue reference.

@k-yang k-yang enabled auto-merge (squash) August 15, 2024 14:12
@k-yang k-yang merged commit ccb4763 into main Aug 15, 2024
13 of 14 checks passed
@k-yang k-yang deleted the fix/evm/funtoken-conversions branch August 15, 2024 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x: evm Relates to Nibiru EVM or the EVM Module
Projects
Status: ✅ Completed
Development

Successfully merging this pull request may close these issues.

epic(evm): fungible tokens: Coin to ERC20 and ERC20 to Coin
2 participants