Bank and Supply are not pruning zero coins causing invariants to break

[colin-axner]

While updating to the latest SDK commit 045c45f5507315834415c45b6eae5b1755cf002a on a branch in ibc-go, one of our ICS20 transfer tests broke:

--- FAIL: TestTransferTestSuite (1.92s)
    --- FAIL: TestTransferTestSuite/TestHandleMsgTransfer (0.41s)
        suite.go:63: test panicked: invariant broken: bank: total supply invariant
            	sum of accounts coins: 100000279066286stake
            	supply.Total:          0ibc/F48A74D4F2A8B3D0CA0572F682A334C4F9595827BFE030BFE112863A2BC928C0,100000279066286stake
            
            
            	CRITICAL please submit the following transaction:
            		 tx crisis invariant-broken bank total-supply
            goroutine 99 [running]:
            runtime/debug.Stack(0xc000535b70, 0x17be5e0, 0xc001c20b20)
            	/usr/lib/go/src/runtime/debug/stack.go:24 +0x9f
            github.com/stretchr/testify/suite.failOnPanic(0xc000ed4300)
            	/home/bartleby/work/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:63 +0x5b
            panic(0x17be5e0, 0xc001c20b20)
            	/usr/lib/go/src/runtime/panic.go:965 +0x1b9
            github.com/cosmos/cosmos-sdk/x/crisis/keeper.Keeper.AssertInvariants(0xc000f83900, 0xb, 0x10, 0x1e1c8e8, 0xc001362130, 0xc0011ceb78, 0x1df1aa0, 0xc001363460, 0x1df1af0, 0xc0013634e0, ...)
            	/home/bartleby/work/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.42.0-alpha1.0.20210421132932-045c45f55073/x/crisis/keeper/keeper.go:83 +0x7b7
            github.com/cosmos/cosmos-sdk/x/crisis.EndBlocker(0x1e0a3a8, 0xc000040150, 0x1e1eae0, 0xc001709800, 0x0, 0x0, 0xc0013330f0, 0xa, 0x1e, 0x0, ...)
            	/home/bartleby/work/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.42.0-alpha1.0.20210421132932-045c45f55073/x/crisis/abci.go:20 +0x27b
            github.com/cosmos/cosmos-sdk/x/crisis.AppModule.EndBlock(...)
            	/home/bartleby/work/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.42.0-alpha1.0.20210421132932-045c45f55073/x/crisis/module.go:168
            github.com/cosmos/cosmos-sdk/types/module.(*Manager).EndBlock(0xc001372540, 0x1e0a3a8, 0xc000040150, 0x1e1eae0, 0xc001709800, 0x0, 0x0, 0xc0013330f0, 0xa, 0x1e, ...)
            	/home/bartleby/work/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.42.0-alpha1.0.20210421132932-045c45f55073/types/module/module.go:454 +0x1e5
            github.com/cosmos/ibc-go/testing/simapp.(*SimApp).EndBlocker(...)
            	/home/bartleby/work/go/src/github.com/cosmos/ibc-go/testing/simapp/app.go:486
            github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).EndBlock(0xc00131f860, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
            	/home/bartleby/work/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.42.0-alpha1.0.20210421132932-045c45f55073/baseapp/abci.go:198 +0x2c6
            github.com/cosmos/ibc-go/testing/simapp.SignCheckDeliver(0xc000ed4300, 0x1e19858, 0xc001064080, 0xc00131f860, 0x0, 0x0, 0xc0013330f0, 0xa, 0x1e, 0x0, ...)
            	/home/bartleby/work/go/src/github.com/cosmos/ibc-go/testing/simapp/test_helpers.go:365 +0x589
            github.com/cosmos/ibc-go/testing.(*TestChain).SendMsgs(0xc001041440, 0xc001056730, 0x1, 0x1, 0x4103b8, 0x10, 0x177f280)
            	/home/bartleby/work/go/src/github.com/cosmos/ibc-go/testing/chain.go:283 +0x3a5
            github.com/cosmos/ibc-go/testing.(*TestChain).sendMsgs(...)
            	/home/bartleby/work/go/src/github.com/cosmos/ibc-go/testing/chain.go:275
            github.com/cosmos/ibc-go/testing.(*Coordinator).SendMsgs(0xc0005ae290, 0xc001041440, 0xc001322900, 0xc001dac5f0, 0xf, 0xc001056730, 0x1, 0x1, 0xa, 0x3b)
            	/home/bartleby/work/go/src/github.com/cosmos/ibc-go/testing/coordinator.go:346 +0x56
            github.com/cosmos/ibc-go/testing.(*Coordinator).SendMsg(...)
            	/home/bartleby/work/go/src/github.com/cosmos/ibc-go/testing/coordinator.go:340
            github.com/cosmos/ibc-go/modules/apps/transfer_test.(*TransferTestSuite).TestHandleMsgTransfer(0xc0000eed40)
            	/home/bartleby/work/go/src/github.com/cosmos/ibc-go/modules/apps/transfer/handler_test.go:96 +0x2417
            reflect.Value.call(0xc000f22240, 0xc000023080, 0x13, 0x19d8613, 0x4, 0xc000f6fe30, 0x1, 0x1, 0xc000f6fcf8, 0x40db8a, ...)
            	/usr/lib/go/src/reflect/value.go:476 +0x8e7
            reflect.Value.Call(0xc000f22240, 0xc000023080, 0x13, 0xc000f6fe30, 0x1, 0x1, 0x21535f9, 0x22, 0x64cb00000439)
            	/usr/lib/go/src/reflect/value.go:337 +0xb9
            github.com/stretchr/testify/suite.Run.func1(0xc000ed4300)
            	/home/bartleby/work/go/pkg/mod/github.com/stretchr/testify@v1.7.0/suite/suite.go:158 +0x379
            testing.tRunner(0xc000ed4300, 0xc000eae2d0)
            	/usr/lib/go/src/testing/testing.go:1194 +0xef
            created by testing.(*T).Run
            	/usr/lib/go/src/testing/testing.go:1239 +0x2b3
FAIL

I have just run into this issue and have not spent time looking into it. Based on the error message, it appears the bank balance is not being pruned after it hits 0 causing the invariant to trigger. This error occurs when we send from A -> B and then B -> A causing the tokens to be fully sent out of the account

cc @AdityaSripal

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[colin-axner]

It is possible I didn't configure something correctly when updating, but the error message seems to clearly indicate there exists a 0 coin

The branch I'm using can be found here

[colin-axner]

I figured it out. Burning coins is currently on master subtracting from the supply coin by coin. Previous behaviour used Coins to subtract from the total supply.

Coin and Coins do not perform the same operations

Coin will end up here clearly capable of returning a 0coin

Coins appears to remove zero coins after subtraction. I can also see this in the code

Thus the old behaviour pruned zero coins from the total supply.

The issue is two fold

• supply isn't pruning zero coins (new bug)
• bank is continuing to store zero coins (existing bug - have not verified, may be wrong)

[colin-axner]

cc @AmauryM @clevinson

I added to v0.43 milestone as it needs to be fixed for the next release

[aaronc]

This should be pretty straightforward to fix in bank.

It also strikes me as strange that Coins itself doesn't handle zero coins correctly. First, the zero coin should have never gotten added to Coins to begin with and second IsEqual should have properly dealt with zero values.

[colin-axner]

It also strikes me as strange that Coins itself doesn't handle zero coins correctly. First, the zero coin should have never gotten added to Coins to begin with and second IsEqual should have properly dealt with zero values.

Yes I completely agree. The mechanics of coin need to be revisited/audited/documented imo. There's a lot of subtle details that can easily lead to misuse. There's other oddities like bank allowing empty coin sets as valid no-ops