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

break the "module-accounts" Invariant #4795

Closed
4 tasks
whunmr opened this issue Jul 29, 2019 · 2 comments · Fixed by #4797
Closed
4 tasks

break the "module-accounts" Invariant #4795

whunmr opened this issue Jul 29, 2019 · 2 comments · Fixed by #4797
Assignees
Labels

Comments

@whunmr
Copy link

whunmr commented Jul 29, 2019

Summary of Bug

The ModuleAccount bonded_tokens_pool can receive coins,
so we can send some coins to this account to break the invariant.
same problem for not_bonded_tokens_pool

Suggestion

We suggest do not allow send coins into these Module Account through MsgSend and MsgMultiSend

Version

cosmos-sdk v0.36.0-rc1

x/staking/keeper/invariants.go:15

Steps to Reproduce

  1. send some coins to the bonded_tokens_pool ModuleAccount
  2. send tx MsgVerifyInvariant

Actually in our functional test env below, we enabled every block invariant checks.
--inv-check-period=1
so after the send coins tx processed, the invariant will broken and chain will showstop.

Scenario: send coins to bonded_tokens_pool ModuleAccount
  Given CoinEx Chain is started
    And node0 has 90000_0000_0000 sato.CET
    And node0 send 1 sato.CET to bonded_tokens_pool address coinex1fl48vsnmsdzcv85q5d2q4z5ajdha8yu37uc2e4
    And wait at least 3 blocks
    #"module_name": "bonded_tokens_pool",
    #coinex1fl48vsnmsdzcv85q5d2q4z5ajdha8yu37uc2e4

Invariant broken logs:

E[2019-07-29|12:13:31.858] CONSENSUS FAILURE!!!                         module=consensus err="
invariant broken: staking: bonded and not bonded module account coins invariant
\tPool's bonded tokens: 1000000000001
\tsum of bonded tokens: 1000000000000
not bonded token invariance:
\tPool's not bonded tokens: 0
\tsum of not bonded tokens: 0
module accounts total (bonded + not bonded):
\tModule Accounts' tokens: 1000000000001
\tsum tokens:              1000000000000
invariant broken: true

\tCRITICAL please submit the following transaction:
\t\t tx crisis invariant-broken staking module-accounts" stack="goroutine 102 [running]:
runtime/debug.Stack(0x0, 0x0, 0x0)
\t/usr/local/go/src/runtime/debug/stack.go:24 +0xa1
github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine.func2(0xc000ab2380, 0x18b6100)
\t/home/j/lab/go_linux/pkg/mod/github.com/tendermint/tendermint@v0.32.1/consensus/state.go:602 +0x63
panic(0x1536880, 0xc0027894d0)
\t/usr/local/go/src/runtime/panic.go:522 +0x1e0
github.com/cosmos/cosmos-sdk/x/crisis/internal/keeper.Keeper.AssertInvariants(0xc000b74f00, 0xc, 0x10, 0xc0000edea0, 0x1aa02e0, 0xc0009ac190, 0x1aa0320, 0xc0009ac1a0, 0xc0009a6a40, 0x6, ...)
\t/home/j/lab/go_linux/pkg/mod/github.com/cosmos/cosmos-sdk@v0.36.0-rc1/x/crisis/internal/keeper/keeper.go:74 +0x76a
github.com/cosmos/cosmos-sdk/x/crisis.EndBlocker(0x1aaea20, 0xc00003a068, 0x1abe980, 0xc00262b9c0, 0xa, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
\t/home/j/lab/go_linux/pkg/mod/github.com/cosmos/cosmos-sdk@v0.36.0-rc1/x/crisis/abci.go:13 +0xfb
github.com/cosmos/cosmos-sdk/x/crisis.AppModule.EndBlock(0xc000b74f00, 0xc, 0x10, 0xc0000edea0, 0x1aa02e0, 0xc0009ac190, 0x1aa0320, 0xc0009ac1a0, 0xc0009a6a40, 0x6, ...)
\t/home/j/lab/go_linux/pkg/mod/github.com/cosmos/cosmos-sdk@v0.36.0-rc1/x/crisis/module.go:125 +0x9f
github.com/cosmos/cosmos-sdk/types/module.(*Manager).EndBlock(0xc0001abd50, 0x1aaea20, 0xc00003a068, 0x1abe980, 0xc00262b9c0, 0xa, 0x0, 0x0, 0x0, 0x0, ...)
\t/home/j/lab/go_linux/pkg/mod/github.com/cosmos/cosmos-sdk@v0.36.0-rc1/types/module/module.go:306 +0x249
github.com/coinexchain/dex/app.(*CetChainApp).EndBlocker(0xc0000d9900, 0x1aaea20, 0xc00003a068, 0x1abe980, 0xc00262b9c0, 0xa, 0x0, 0x0, 0x0, 0x0, ...)
\t/home/j/lab/dex/app/app.go:496 +0xd2
github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).EndBlock(0xc0008f9200, 0x6, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
\t/home/j/lab/go_linux/pkg/mod/github.com/cosmos/cosmos-sdk@v0.36.0-rc1/baseapp/baseapp.go:928 +0x212
github.com/tendermint/tendermint/abci/client.(*localClient).EndBlockSync(0xc0008e02a0, 0x6, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
\t/home/j/lab/go_linux/pkg/mod/github.com/tendermint/tendermint@v0.32.1/abci/client/local_client.go:239 +0x104
github.com/tendermint/tendermint/proxy.(*appConnConsensus).EndBlockSync(0xc000b6f680, 0x6, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
\t/home/j/lab/go_linux/pkg/mod/github.com/tendermint/tendermint@v0.32.1/proxy/app_conn.go:77 +0x90
github.com/tendermint/tendermint/state.execBlockOnProxyApp(0x1aaf520, 0xc00091c860, 0x1ab8c40, 0xc000b6f680, 0xc0025ff4a0, 0x1ac0300, 0xc00000ea20, 0x0, 0x0, 0x0)
\t/home/j/lab/go_linux/pkg/mod/github.com/tendermint/tendermint@v0.32.1/state/execution.go:294 +0x9e3
github.com/tendermint/tendermint/state.(*BlockExecutor).ApplyBlock(0xc0001abf80, 0xa, 0x0, 0xc0008e4490, 0x6, 0xc000252d60, 0x1c, 0x5, 0x0, 0xc00256ace0, ...)
\t/home/j/lab/go_linux/pkg/mod/github.com/tendermint/tendermint@v0.32.1/state/execution.go:124 +0x29b
github.com/tendermint/tendermint/consensus.(*ConsensusState).finalizeCommit(0xc000ab2380, 0x6)
\t/home/j/lab/go_linux/pkg/mod/github.com/tendermint/tendermint@v0.32.1/consensus/state.go:1326 +0xe5f
github.com/tendermint/tendermint/consensus.(*ConsensusState).tryFinalizeCommit(0xc000ab2380, 0x6)
\t/home/j/lab/go_linux/pkg/mod/github.com/tendermint/tendermint@v0.32.1/consensus/state.go:1257 +0x441
github.com/tendermint/tendermint/consensus.(*ConsensusState).enterCommit.func1(0xc000ab2380, 0x0, 0x6)
\t/home/j/lab/go_linux/pkg/mod/github.com/tendermint/tendermint@v0.32.1/consensus/state.go:1203 +0xd7
github.com/tendermint/tendermint/consensus.(*ConsensusState).enterCommit(0xc000ab2380, 0x6, 0x0)
\t/home/j/lab/go_linux/pkg/mod/github.com/tendermint/tendermint@v0.32.1/consensus/state.go:1234 +0xae6
github.com/tendermint/tendermint/consensus.(*ConsensusState).addVote(0xc000ab2380, 0xc0026a85a0, 0x0, 0x0, 0x1, 0x0, 0x0)
\t/home/j/lab/go_linux/pkg/mod/github.com/tendermint/tendermint@v0.32.1/consensus/state.go:1659 +0x2212
github.com/tendermint/tendermint/consensus.(*ConsensusState).tryAddVote(0xc000ab2380, 0xc0026a85a0, 0x0, 0x0, 0x18b7300, 0x0, 0x0)
\t/home/j/lab/go_linux/pkg/mod/github.com/tendermint/tendermint@v0.32.1/consensus/state.go:1505 +0x88
github.com/tendermint/tendermint/consensus.(*ConsensusState).handleMsg(0xc000ab2380, 0x1a92340, 0xc0024da9a8, 0x0, 0x0)
\t/home/j/lab/go_linux/pkg/mod/github.com/tendermint/tendermint@v0.32.1/consensus/state.go:688 +0x6cc
github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine(0xc000ab2380, 0x0)
\t/home/j/lab/go_linux/pkg/mod/github.com/tendermint/tendermint@v0.32.1/consensus/state.go:646 +0x6d7
created by github.com/tendermint/tendermint/consensus.(*ConsensusState).OnStart
\t/home/j/lab/go_linux/pkg/mod/github.com/tendermint/tendermint@v0.32.1/consensus/state.go:334 +0x846
"
  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@alexanderbez
Copy link
Contributor

Good catch @whunmr. During the construction of the bank keeper in the app, we can pass it app.ModuleAccountAddrs(). The bank keeper will prohibit any sends to an account in this list.

/cc @fedekunze

@AdityaSripal
Copy link
Member

@alexanderbez This will work for now. But it won't work when Colin's MultiAccount PR #4732 gets merged

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.

4 participants