Skip to content

Commit 7e1f259

Browse files
dongsamdongsam
committed
fix: apply review comments, add testcases, error handling
1 parent b9645b1 commit 7e1f259

18 files changed

+206
-36
lines changed

‎evmd/config_testing.go

+10
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,16 @@ var ChainsCoinInfo = map[string]evmtypes.EvmCoinInfo{
2929
DisplayDenom: testconstants.ExampleDisplayDenom,
3030
Decimals: evmtypes.SixDecimals,
3131
},
32+
TestChainID1: {
33+
Denom: ExampleChainDenom,
34+
DisplayDenom: ExampleChainDenom,
35+
Decimals: evmtypes.EighteenDecimals,
36+
},
37+
TestChainID2: {
38+
Denom: ExampleChainDenom,
39+
DisplayDenom: ExampleChainDenom,
40+
Decimals: evmtypes.EighteenDecimals,
41+
},
3242
}
3343

3444
// EVMOptionsFn defines a function type for setting app options specifically for

‎evmd/constants.go

+5
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,9 @@ const (
1414
SixDecimalsChainID = "ossix_9002"
1515

1616
CosmosChainID = "cosmos_262144"
17+
18+
// TestChainID1 is test chain IDs for IBC E2E test
19+
TestChainID1 = "testchain_9001"
20+
// TestChainID2 is test chain IDs for IBC E2E test
21+
TestChainID2 = "testchain_9002"
1722
)

‎ibc/testing/coordinator.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,10 @@ func NewCoordinator(t *testing.T, nEVMChains, mCosmosChains int) *Coordinator {
3838
CurrentTime: globalStartTime,
3939
}
4040

41-
require.NoError(t, evmd.EvmAppOptions("cosmos_9001-1"))
4241
ibctesting.DefaultTestingAppInit = SetupExampleApp
4342
for i := 1; i <= nEVMChains; i++ {
4443
chainID := GetEvmChainID(i)
44+
require.NoError(t, evmd.EvmAppOptions(chainID))
4545
// setup EVM chains
4646
chains[chainID] = NewTestChain(t, true, coord, chainID)
4747
}
@@ -160,7 +160,7 @@ func GetChainID(index int) string {
160160

161161
// GetEvmChainID returns the EIP-155 chainID used for the provided index.
162162
func GetEvmChainID(index int) string {
163-
return fmt.Sprintf("cosmos_900%d-1", index)
163+
return fmt.Sprintf("%s_%d-1", ChainIDPrefix, 9000+index)
164164
}
165165

166166
// CommitBlock commits a block on the provided indexes and then increments the global time.

‎ibc/utils.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ func IsBaseDenomFromSourceChain(rawDenom string) bool {
123123
return len(denom.GetTrace()) == 0 && len(denomComponents) == 1
124124
}
125125

126-
// GetDenom returns the denomination trace from the corresponding IBC denomination. If the
126+
// GetDenom returns the denomination from the corresponding IBC denomination. If the
127127
// denomination is not an IBC voucher or the trace is not found, it returns an error.
128128
func GetDenom(
129129
transferKeeper transferkeeper.Keeper,

‎precompiles/erc20/query_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,13 @@ import (
2222

2323
// Define useful variables for tests here.
2424
var (
25-
// tooShort is a denomination trace with a name that will raise the "denom too short" error
25+
// tooShort is a denomination with a name that will raise the "denom too short" error
2626
tooShort = types.NewDenom("ab", types.NewHop(types.PortID, "channel-0"))
27-
// validDenom is a denomination trace with a valid IBC voucher name
27+
// validDenom is a denomination with a valid IBC voucher name
2828
validDenom = types.NewDenom("uosmo", types.NewHop(types.PortID, "channel-0"))
29-
// validAttoDenom is a denomination trace with a valid IBC voucher name and 18 decimals
29+
// validAttoDenom is a denomination with a valid IBC voucher name and 18 decimals
3030
validAttoDenom = types.NewDenom("aatom", types.NewHop(types.PortID, "channel-0"))
31-
// validDenomNoMicroAtto is a denomination trace with a valid IBC voucher name but no micro or atto prefix
31+
// validDenomNoMicroAtto is a denomination with a valid IBC voucher name but no micro or atto prefix
3232
validDenomNoMicroAtto = types.NewDenom("matom", types.NewHop(types.PortID, "channel-0"))
3333

3434
// --------------------

‎precompiles/ics20/ICS20I.sol

+3-3
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ interface ICS20I is IICS20Authorization {
7474
string memory memo
7575
) external returns (uint64 nextSequence);
7676

77-
/// @dev denoms Defines a method for returning all denom traces.
77+
/// @dev denoms Defines a method for returning all denoms.
7878
/// @param pageRequest Defines the pagination parameters to for the request.
7979
function denoms(
8080
PageRequest memory pageRequest
@@ -86,12 +86,12 @@ interface ICS20I is IICS20Authorization {
8686
PageResponse memory pageResponse
8787
);
8888

89-
/// @dev DenomTrace defines a method for returning a denom trace.
89+
/// @dev Denom defines a method for returning a denom.
9090
function denom(
9191
string memory hash
9292
) external view returns (Denom memory denom);
9393

94-
/// @dev DenomHash defines a method for returning a hash of the denomination trace info.
94+
/// @dev DenomHash defines a method for returning a hash of the denomination info.
9595
function denomHash(
9696
string memory trace
9797
) external view returns (string memory hash);

‎precompiles/ics20/errors.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,6 @@ const (
1919
ErrNoMatchingAllocation = "no matching allocation found for source port: %s, source channel: %s, and denom: %s"
2020
// ErrDifferentOriginFromSender is raised when the origin address is not the same as the sender address.
2121
ErrDifferentOriginFromSender = "origin address %s is not the same as sender address %s"
22-
// ErrTraceNotFound is raised when the denom trace for the specified request does not exist.
23-
ErrTraceNotFound = "denomination trace not found"
22+
// ErrTraceFound is raised when the denom for the specified request does not exist.
23+
ErrTraceFound = "denomination not found"
2424
)

‎precompiles/ics20/query.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ const (
2626
DenomHashMethod = "denomHash"
2727
)
2828

29-
// Denom returns the requested denomination trace information.
29+
// Denom returns the requested denomination information.
3030
func (p Precompile) Denom(
3131
ctx sdk.Context,
3232
_ *vm.Contract,
@@ -41,7 +41,7 @@ func (p Precompile) Denom(
4141
res, err := p.transferKeeper.Denom(ctx, req)
4242
if err != nil {
4343
// if the trace does not exist, return empty array
44-
if strings.Contains(err.Error(), ErrTraceNotFound) {
44+
if strings.Contains(err.Error(), ErrTraceFound) {
4545
return method.Outputs.Pack(transfertypes.Denom{})
4646
}
4747
return nil, err
@@ -50,7 +50,7 @@ func (p Precompile) Denom(
5050
return method.Outputs.Pack(*res.Denom)
5151
}
5252

53-
// Denoms returns the requested denomination traces information.
53+
// Denoms returns the requested denomination information.
5454
func (p Precompile) Denoms(
5555
ctx sdk.Context,
5656
_ *vm.Contract,
@@ -70,7 +70,7 @@ func (p Precompile) Denoms(
7070
return method.Outputs.Pack(res.Denoms, res.Pagination)
7171
}
7272

73-
// DenomHash returns the denom hash (in hex format) of the denomination trace information.
73+
// DenomHash returns the denom hash (in hex format) of the denomination information.
7474
func (p Precompile) DenomHash(
7575
ctx sdk.Context,
7676
_ *vm.Contract,
@@ -85,7 +85,7 @@ func (p Precompile) DenomHash(
8585
res, err := p.transferKeeper.DenomHash(ctx, req)
8686
if err != nil {
8787
// if the denom hash does not exist, return empty string
88-
if strings.Contains(err.Error(), ErrTraceNotFound) {
88+
if strings.Contains(err.Error(), ErrTraceFound) {
8989
return method.Outputs.Pack("")
9090
}
9191
return nil, err

‎precompiles/ics20/types.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ func NewDenomHashRequest(args []interface{}) (*transfertypes.QueryDenomHashReque
235235

236236
trace, ok := args[0].(string)
237237
if !ok {
238-
return nil, fmt.Errorf("invalid denom trace")
238+
return nil, fmt.Errorf("invalid trace")
239239
}
240240

241241
req := &transfertypes.QueryDenomHashRequest{

‎tests/ibc/ibc_middleware_test.go

+25-1
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,8 @@ func (suite *MiddlewareTestSuite) TestOnRecvPacketNativeErc20() {
301301
suite.Require().True(escrowedBal.IsZero(), "escrowed balance should be un-escrowed after receiving the packet")
302302
balAfterUnescrow := evmApp.Erc20Keeper.BalanceOf(evmCtx, nativeErc20.ContractAbi, nativeErc20.ContractAddr, senderEthAddr)
303303
suite.Require().Equal(nativeErc20.InitialBal.String(), balAfterUnescrow.String())
304+
bankBalAfterUnescrow := evmApp.BankKeeper.GetBalance(evmCtx, sender, nativeErc20.Denom)
305+
suite.Require().True(bankBalAfterUnescrow.IsZero(), "no duplicate state in the bank balance")
304306
}
305307

306308
func (suite *MiddlewareTestSuite) TestOnAcknowledgementPacket() {
@@ -361,6 +363,7 @@ func (suite *MiddlewareTestSuite) TestOnAcknowledgementPacket() {
361363
sendAmt := ibctesting.DefaultCoinAmount
362364
sender := suite.evmChainA.SenderAccount.GetAddress()
363365
receiver := suite.chainB.SenderAccount.GetAddress()
366+
balBeforeTransfer := evmApp.BankKeeper.GetBalance(ctxA, sender, bondDenom)
364367

365368
packetData := transfertypes.NewFungibleTokenPacketData(
366369
bondDenom,
@@ -419,6 +422,16 @@ func (suite *MiddlewareTestSuite) TestOnAcknowledgementPacket() {
419422
// relay the sent packet
420423
err = path.RelayPacket(packet)
421424
suite.Require().NoError(err) // relay committed
425+
426+
// ensure the ibc token is escrowed.
427+
balAfterTransfer := evmApp.BankKeeper.GetBalance(ctxA, sender, bondDenom)
428+
suite.Require().Equal(
429+
balBeforeTransfer.Amount.Sub(sendAmt).String(),
430+
balAfterTransfer.Amount.String(),
431+
)
432+
escrowAddr := transfertypes.GetEscrowAddress(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
433+
escrowedBal := evmApp.BankKeeper.GetBalance(ctxA, escrowAddr, bondDenom)
434+
suite.Require().Equal(sendAmt.String(), escrowedBal.Amount.String())
422435
}
423436

424437
err = onAck()
@@ -620,6 +633,7 @@ func (suite *MiddlewareTestSuite) TestOnTimeoutPacket() {
620633
sendAmt := ibctesting.DefaultCoinAmount
621634
sender := suite.evmChainA.SenderAccount.GetAddress()
622635
receiver := suite.chainB.SenderAccount.GetAddress()
636+
balBeforeTransfer := evmApp.BankKeeper.GetBalance(ctxA, sender, bondDenom)
623637

624638
packetData := transfertypes.NewFungibleTokenPacketData(
625639
bondDenom,
@@ -677,9 +691,19 @@ func (suite *MiddlewareTestSuite) TestOnTimeoutPacket() {
677691

678692
err = path.RelayPacket(packet)
679693
suite.Require().NoError(err) // relay committed
680-
}
681694

695+
}
682696
err = onTimeout()
697+
// ensure that the escrowed coins were refunded on timeout.
698+
balAfterTransfer := evmApp.BankKeeper.GetBalance(ctxA, sender, bondDenom)
699+
escrowAddr := transfertypes.GetEscrowAddress(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
700+
escrowedBal := evmApp.BankKeeper.GetBalance(ctxA, escrowAddr, bondDenom)
701+
suite.Require().Equal(
702+
balBeforeTransfer.Amount.String(),
703+
balAfterTransfer.Amount.String(),
704+
)
705+
suite.Require().Equal(escrowedBal.Amount.String(), math.ZeroInt().String())
706+
683707
if tc.expError == "" {
684708
suite.Require().NoError(err)
685709
} else {

‎tests/ibc/ics20_precompile_transfer_test.go

+58-10
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ func (suite *ICS20TransferTestSuite) TestHandleMsgTransfer() {
7171
erc20 bool
7272
)
7373

74+
// originally a basic test case from the IBC testing package, and it has been added as-is to ensure that
75+
// it still works properly when invoked through the ics20 precompile.
7476
testCases := []struct {
7577
name string
7678
malleate func()
@@ -110,15 +112,6 @@ func (suite *ICS20TransferTestSuite) TestHandleMsgTransfer() {
110112
erc20 = true
111113
},
112114
},
113-
{
114-
"native erc20 case",
115-
func() {
116-
nativeErc20 = SetupNativeErc20(suite.T(), suite.chainA)
117-
sourceDenomToTransfer = nativeErc20.Denom
118-
msgAmount = sdkmath.NewIntFromBigInt(nativeErc20.InitialBal)
119-
erc20 = true
120-
},
121-
},
122115
}
123116

124117
for _, tc := range testCases {
@@ -248,7 +241,7 @@ func (suite *ICS20TransferTestSuite) TestHandleMsgTransfer() {
248241
suite.Require().NoError(err)
249242
suite.Require().Equal(chainBDenom, denomsResponse.Denoms[0])
250243

251-
// denom query method
244+
// denom query method with result
252245
evmRes, err = evmAppB.EVMKeeper.CallEVM(
253246
ctxB,
254247
suite.chainBPrecompile.ABI,
@@ -264,6 +257,34 @@ func (suite *ICS20TransferTestSuite) TestHandleMsgTransfer() {
264257
suite.Require().NoError(err)
265258
suite.Require().Equal(chainBDenom, denomResponse.Denom)
266259

260+
// denom query method not exists case
261+
evmRes, err = evmAppB.EVMKeeper.CallEVM(
262+
ctxB,
263+
suite.chainBPrecompile.ABI,
264+
chainBAddr,
265+
suite.chainBPrecompile.Address(),
266+
false,
267+
ics20.DenomMethod,
268+
"0000000000000000000000000000000000000000000000000000000000000000",
269+
)
270+
suite.Require().NoError(err)
271+
err = suite.chainBPrecompile.UnpackIntoInterface(&denomResponse, ics20.DenomMethod, evmRes.Ret)
272+
suite.Require().NoError(err)
273+
// ensure empty denom struct when not exist
274+
suite.Require().Equal(denomResponse.Denom, transfertypes.Denom{Base: "", Trace: []transfertypes.Hop{}})
275+
276+
// denom query method invalid error case
277+
_, err = evmAppB.EVMKeeper.CallEVM(
278+
ctxB,
279+
suite.chainBPrecompile.ABI,
280+
chainBAddr,
281+
suite.chainBPrecompile.Address(),
282+
false,
283+
ics20.DenomMethod,
284+
"INVALID-DENOM-HASH",
285+
)
286+
suite.Require().ErrorContains(err, "invalid denom trace hash")
287+
267288
// denomHash query method
268289
evmRes, err = evmAppB.EVMKeeper.CallEVM(
269290
ctxB,
@@ -279,6 +300,33 @@ func (suite *ICS20TransferTestSuite) TestHandleMsgTransfer() {
279300
err = suite.chainBPrecompile.UnpackIntoInterface(&denomHashResponse, ics20.DenomHashMethod, evmRes.Ret)
280301
suite.Require().NoError(err)
281302
suite.Require().Equal(chainBDenom.Hash().String(), denomHashResponse.Hash)
303+
304+
// denomHash query method not exists case
305+
evmRes, err = evmAppB.EVMKeeper.CallEVM(
306+
ctxB,
307+
suite.chainBPrecompile.ABI,
308+
chainBAddr,
309+
suite.chainBPrecompile.Address(),
310+
false,
311+
ics20.DenomHashMethod,
312+
"transfer/channel-0/erc20:not-exists-case",
313+
)
314+
suite.Require().NoError(err)
315+
err = suite.chainBPrecompile.UnpackIntoInterface(&denomHashResponse, ics20.DenomHashMethod, evmRes.Ret)
316+
suite.Require().NoError(err)
317+
suite.Require().Equal(denomHashResponse.Hash, "")
318+
319+
// denomHash query method invalid error case
320+
_, err = evmAppB.EVMKeeper.CallEVM(
321+
ctxB,
322+
suite.chainBPrecompile.ABI,
323+
chainBAddr,
324+
suite.chainBPrecompile.Address(),
325+
false,
326+
ics20.DenomHashMethod,
327+
"",
328+
)
329+
suite.Require().ErrorContains(err, "invalid denomination for cross-chain transfer")
282330
})
283331
}
284332
}

‎tests/ibc/transfer_test.go

+2
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ func (suite *TransferTestSuite) TestHandleMsgTransfer() {
4949
err error
5050
)
5151

52+
// originally a basic test case from the IBC testing package, and it has been added as-is to ensure that
53+
// it still works properly when invoked by evmd app.
5254
testCases := []struct {
5355
name string
5456
malleate func()

‎tests/ibc/v2_ibc_middleware_test.go

+17-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package ibc
22

33
import (
4+
"bytes"
45
"errors"
56
"math/big"
67
"testing"
@@ -412,6 +413,8 @@ func (suite *MiddlewareV2TestSuite) TestOnRecvPacketNativeERC20() {
412413
suite.Require().True(escrowedBal.IsZero(), "escrowed balance should be un-escrowed after receiving the packet")
413414
balAfterUnescrow := evmApp.Erc20Keeper.BalanceOf(evmCtx, nativeErc20.ContractAbi, nativeErc20.ContractAddr, senderEthAddr)
414415
suite.Require().Equal(nativeErc20.InitialBal.String(), balAfterUnescrow.String())
416+
bankBalAfterUnescrow := evmApp.BankKeeper.GetBalance(evmCtx, sender, nativeErc20.Denom)
417+
suite.Require().True(bankBalAfterUnescrow.IsZero(), "no duplicate state in the bank balance")
415418
}
416419
})
417420
}
@@ -479,9 +482,14 @@ func (suite *MiddlewareV2TestSuite) TestOnAcknowledgementPacket() {
479482
evmApp := suite.evmChainA.App.(*evmd.EVMD)
480483
bondDenom, err := evmApp.StakingKeeper.BondDenom(ctx)
481484
suite.Require().NoError(err)
485+
sendAmt := ibctesting.DefaultCoinAmount
486+
escrowAddress := transfertypes.GetEscrowAddress(
487+
transfertypes.PortID,
488+
suite.pathAToB.EndpointA.ClientID,
489+
)
482490
packetData = transfertypes.NewFungibleTokenPacketData(
483491
bondDenom,
484-
ibctesting.DefaultCoinAmount.String(),
492+
sendAmt.String(),
485493
suite.evmChainA.SenderAccount.GetAddress().String(),
486494
suite.chainB.SenderAccount.GetAddress().String(),
487495
"",
@@ -510,6 +518,9 @@ func (suite *MiddlewareV2TestSuite) TestOnAcknowledgementPacket() {
510518
payload,
511519
suite.evmChainA.SenderAccount.GetAddress(),
512520
))
521+
// check that the escrowed coin is escrowed
522+
escrowedCoin := evmApp.BankKeeper.GetBalance(ctx, escrowAddress, bondDenom)
523+
suite.Require().Equal(escrowedCoin.Amount, sendAmt)
513524
}
514525
onAckPacket := func() error {
515526
return transferStack.OnAcknowledgementPacket(
@@ -530,6 +541,11 @@ func (suite *MiddlewareV2TestSuite) TestOnAcknowledgementPacket() {
530541
} else {
531542
suite.Require().NoError(err)
532543
}
544+
// check that the escrowed coins are un-escrowed
545+
if tc.onSendRequired && bytes.Equal(ack, channeltypesv2.ErrorAcknowledgement[:]) {
546+
escrowedCoins := evmApp.BankKeeper.GetAllBalances(ctx, escrowAddress)
547+
suite.Require().Equal(0, len(escrowedCoins))
548+
}
533549
})
534550
}
535551
}

0 commit comments

Comments
 (0)