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

Token Transfer receiving denom bug #8120

Closed
4 tasks
andrey-kuprianov opened this issue Dec 9, 2020 · 0 comments · Fixed by #8119
Closed
4 tasks

Token Transfer receiving denom bug #8120

andrey-kuprianov opened this issue Dec 9, 2020 · 0 comments · Fixed by #8119
Assignees
Labels

Comments

@andrey-kuprianov
Copy link
Contributor

Surfaced from Informal Systems IBC Audit

Summary of a Bug

I have implemented some model-based tests for token transfer, and I have a test that fails like this:

=== RUN   TestKeeperTestSuite
=== RUN   TestKeeperTestSuite/TestGenesis
=== RUN   TestKeeperTestSuite/TestGetTransferAccount
=== RUN   TestKeeperTestSuite/TestModelBasedRelay
=== RUN   TestKeeperTestSuite/TestModelBasedRelay/Case_relay-test.json_#_1
=== RUN   TestKeeperTestSuite/TestModelBasedRelay/Case_relay-test.json_#_2
=== RUN   TestKeeperTestSuite/TestModelBasedRelay/Case_relay-test.json_#_3
--- FAIL: TestKeeperTestSuite (0.35s)
    --- PASS: TestKeeperTestSuite/TestGenesis (0.02s)
    --- PASS: TestKeeperTestSuite/TestGetTransferAccount (0.02s)
    --- FAIL: TestKeeperTestSuite/TestModelBasedRelay (0.30s)
        --- PASS: TestKeeperTestSuite/TestModelBasedRelay/Case_relay-test.json_#_1 (0.00s)
        --- PASS: TestKeeperTestSuite/TestModelBasedRelay/Case_relay-test.json_#_2 (0.00s)
        --- FAIL: TestKeeperTestSuite/TestModelBasedRelay/Case_relay-test.json_#_3 (0.00s)
panic: invalid denom: transfer/channel-0/eth [recovered]
	panic: invalid denom: transfer/channel-0/eth

goroutine 4350 [running]:
testing.tRunner.func1.1(0x16fbfe0, 0xc0004fda50)
	/snap/go/6745/src/testing/testing.go:1072 +0x30d
testing.tRunner.func1(0xc000582900)
	/snap/go/6745/src/testing/testing.go:1075 +0x41a
panic(0x16fbfe0, 0xc0004fda50)
	/snap/go/6745/src/runtime/panic.go:969 +0x1b9
github.com/cosmos/cosmos-sdk/types.NewCoin(0xc00149bde3, 0x16, 0xc00139a3c0, 0x2, 0x2, 0xc0010976e0)
	/home/andrey/work/cosmos-sdk/types/coin.go:23 +0x8a
github.com/cosmos/cosmos-sdk/x/ibc/applications/transfer/keeper.Keeper.OnRecvPacket(0x1ce5bc0, 0xc001135750, 0x1d14760, 0xc0011340c0, 0x1d14760, 0xc0011340c0, 0xc0005198f0, 0x1ce5bc0, 0xc001135710, 0x1ce5c40, ...)
	/home/andrey/work/cosmos-sdk/x/ibc/applications/transfer/keeper/relay.go:221 +0x5c6
github.com/cosmos/cosmos-sdk/x/ibc/applications/transfer/keeper_test.(*KeeperTestSuite).TestModelBasedRelay.func1()
	/home/andrey/work/cosmos-sdk/x/ibc/applications/transfer/keeper/mbt_relay_test.go:345 +0xe38

Notice panic: invalid denom: transfer/channel-0/eth in relay.go:OnRecvPacket(), and in coin.Validate(). I believe this is due to the fact that coin.go:reDnmString doesn't allow for the - character, which was introduced in #7993 .

Originally surfaced in #7993 (comment)


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants