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

tests(erc20): Add ERC20 metadata query integration tests #2086

Merged
merged 3 commits into from Nov 28, 2023

Conversation

MalteHerrmann
Copy link
Contributor

Description

This PR adds integration tests for the ERC20 EVM extension metadata queries, which is part of merging #2044 in smaller chunks.

@MalteHerrmann MalteHerrmann requested a review from a team as a code owner November 27, 2023 17:00
@MalteHerrmann MalteHerrmann requested review from Vvaradinov and GAtom22 and removed request for a team November 27, 2023 17:00
Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

ACK but what about tests for deriving the name from the IBC voucher? Do we have those?

Copy link
Contributor

@Vvaradinov Vvaradinov left a comment

Choose a reason for hiding this comment

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

LGTM!

@MalteHerrmann
Copy link
Contributor Author

ACK but what about tests for deriving the name from the IBC voucher? Do we have those?

I had those included originally, but we had to manually store the IBC denom in the transfer keeper and Freddy suggested not to access any keepers in the integration tests, which is why I removed those tests in the integration setup.

Those cases are fully covered by unit tests though (and it also worked when I had those included in integration tests), check here:

{
name: "fail - invalid denom (too short < 3 chars)",
denom: tooShortTrace.IBCDenom(),
malleate: func(ctx sdk.Context, app *app.Evmos) {
app.TransferKeeper.SetDenomTrace(ctx, tooShortTrace)
},
errContains: vm.ErrExecutionReverted.Error(),
},
{
name: "fail - denom without metadata and not an IBC voucher",
denom: "noIBCvoucher",
errContains: vm.ErrExecutionReverted.Error(),
},
{
name: "pass - valid ibc denom without metadata and neither atto nor micro prefix",
denom: validTraceDenomNoMicroAtto.IBCDenom(),
malleate: func(ctx sdk.Context, app *app.Evmos) {
app.TransferKeeper.SetDenomTrace(ctx, validTraceDenomNoMicroAtto)
},
expPass: true,
expName: "Evmos",
expSymbol: "EVMOS",
},
{
name: "pass - valid denom with metadata",
denom: validMetadataDenom,
malleate: func(ctx sdk.Context, app *app.Evmos) {
// NOTE: we mint some coins to the inflation module address to be able to set denom metadata
err := app.BankKeeper.MintCoins(ctx, inflationtypes.ModuleName, sdk.Coins{sdk.NewInt64Coin(validMetadata.Base, 1)})
s.Require().NoError(err)
// NOTE: we set the denom metadata for the coin
app.BankKeeper.SetDenomMetaData(ctx, validMetadata)
},
expPass: true,
expName: "Atom",
expSymbol: "ATOM",
},
{
name: "pass - valid ibc denom without metadata",
denom: validTraceDenom.IBCDenom(),
malleate: func(ctx sdk.Context, app *app.Evmos) {
app.TransferKeeper.SetDenomTrace(ctx, validTraceDenom)
},
expPass: true,
expName: "Osmo",
expSymbol: "OSMO",
},

Copy link

codecov bot commented Nov 28, 2023

Codecov Report

Merging #2086 (e96d0fb) into main (f909c22) will increase coverage by 0.07%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2086      +/-   ##
==========================================
+ Coverage   70.06%   70.14%   +0.07%     
==========================================
  Files         339      339              
  Lines       25499    25499              
==========================================
+ Hits        17867    17887      +20     
+ Misses       6697     6675      -22     
- Partials      935      937       +2     

see 1 file with indirect coverage changes

@fedekunze fedekunze merged commit 97307ca into main Nov 28, 2023
34 of 37 checks passed
@fedekunze fedekunze deleted the malte/add-erc20-metadata-integration-tests branch November 28, 2023 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants