Add e2e testing framework based on Osmosis #247
Conversation
|
|
||
| func (bc *baseConfigurer) connectIBCChains(chainA *chain.Config, chainB *chain.Config) error { | ||
| bc.t.Logf("connecting %s and %s chains via IBC", chainA.ChainMeta.Id, chainB.ChainMeta.Id) | ||
| cmd := []string{"hermes", "create", "channel", chainA.ChainMeta.Id, chainB.ChainMeta.Id, "--port-a=zoneconcierge", "--port-b=zoneconcierge"} |
There was a problem hiding this comment.
@SebastianElvis another ibc question to which maybe you now answer, and spare me some debuggin 😅
If we change port-a and port-b here to transfer the the test will fail, which expected. What is less expected is that two nodes from chain bbn-test-a will be killed with consensus failure looking like:
4:12PM ERR CONSENSUS FAILURE!!! err="module does not own channel capability: sourcePort: transfer, sourceChannel: channel-0: channel capability not found" module=consensus stack="goroutine 175 [running]:\nruntime/debug.Stack()\n\truntime/debug/stack.go:24 +0x65\ngithub.com/tendermint/tendermint/consensus.(*State).receiveRoutine.func2()\n\tgithub.com/tendermint/tendermint@v0.34.24/consensus/state.go:724 +0x4c\npanic({0x1f274a0, 0xc0034559e0})\n\truntime/panic.go:838 +0x207\ngithub.com/babylonchain/babylon/x/zoneconcierge.EndBlocker({{0x2b84e00, 0xc00019e000}, {0x2b93140, 0xc0032d6c40}, {{0xb, 0x0}, {0xc0015ac830, 0xa}, 0x1b, {0x466d8e7, ...}, ...}, ...}, ...)\n\tgithub.com/babylonchain/babylon/x/zoneconcierge/abci.go:27 +0x299\ngithub.com/babylonchain/babylon/x/zoneconcierge.AppModule.EndBlock(...)\n\tgithub.com/babylonchain/babylon/x/zoneconcierge/module.go:161\ngithub.com/cosmos/cosmos-sdk/types/module.(*Manager).EndBlock(_, {{0x2b84e00, 0xc00019e000}, {0x2b93140, 0xc0032d6c40}, {{0xb, 0x0}, {0xc0015ac830, 0xa}, 0x1b, ...}, ...}, ...)\n\tgithub.com/cosmos/cosmos-sdk@v0.46.6/types/module/module.go:505 +0x482\ngithub.com/babylonchain/babylon/app.(*BabylonApp).EndBlocker(_, {{0x2b84e00, 0xc00019e000}, {0x2b93140, 0xc0032d6c40}, {{0xb, 0x0}, {0xc0015ac830, 0xa}, 0x1b, ...}, ...}, ...)\n\tgithub.com/babylonchain/babylon/app/app.go:724 +0x78\ngithub.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).EndBlock(0xc000d768c0, {0x0?})\n\tgithub.com/cosmos/cosmos-sdk@v0.46.6/baseapp/abci.go:206 +0x165\ngithub.com/tendermint/tendermint/abci/client.(*localClient).EndBlockSync(0xc000e1b800, {0xc000e1b800?})\n\tgithub.com/tendermint/tendermint@v0.34.24/abci/client/local_client.go:288 +0xdf\ngithub.com/tendermint/tendermint/proxy.(*appConnConsensus).EndBlockSync(0xc003384d40?, {0xc000f0d200?})\n\tgithub.com/tendermint/tendermint@v0.34.24/proxy/app_conn.go:89 +0x24\ngithub.com/tendermint/tendermint/state.execBlockOnProxyApp({0x2b85fb8?, 0xc0011f4300}, {0x2b8be10, 0xc000e8a750}, 0xc0000012c0, {0x2b92930, 0xc00022c540}, 0x1a?)\n\tgithub.com/tendermint/tendermint@v0.34.24/state/execution.go:327 +0x54e\ngithub.com/tendermint/tendermint/state.(*BlockExecutor).ApplyBlock(_, {{{0xb, 0x0}, {0xc0011e11c0, 0x7}}, {0xc0011e11d0, 0xa}, 0x1, 0x1a, {{0xc00501be80, ...}, ...}, ...}, ...)\n\tgithub.com/tendermint/tendermint@v0.34.24/state/execution.go:140 +0x171\ngithub.com/tendermint/tendermint/consensus.(*State).finalizeCommit(0xc000c84700, 0x1b)\n\tgithub.com/tendermint/tendermint@v0.34.24/consensus/state.go:1654 +0xafd\ngithub.com/tendermint/tendermint/consensus.(*State).tryFinalizeCommit(0xc000c84700, 0x1b)\n\tgithub.com/tendermint/tendermint@v0.34.24/consensus/state.go:1563 +0x2ff\ngithub.com/tendermint/tendermint/consensus.(*State).enterCommit.func1()\n\tgithub.com/tendermint/tendermint@v0.34.24/consensus/state.go:1498 +0x87\ngithub.com/tendermint/tendermint/consensus.(*State).enterCommit(0xc000c84700, 0x1b, 0x0)\n\tgithub.com/tendermint/tendermint@v0.34.24/consensus/state.go:1536 +0xcb7\ngithub.com/tendermint/tendermint/consensus.(*State).addVote(0xc000c84700, 0xc0013f5c20, {0xc000059530, 0x28})\n\tgithub.com/tendermint/tendermint@v0.34.24/consensus/state.go:2150 +0xb7c\ngithub.com/tendermint/tendermint/consensus.(*State).tryAddVote(0xc000c84700, 0xc0013f5c20, {0xc000059530?, 0xc00157e300?})\n\tgithub.com/tendermint/tendermint@v0.34.24/consensus/state.go:1948 +0x2c\ngithub.com/tendermint/tendermint/consensus.(*State).handleMsg(0xc000c84700, {{0x2b67400?, 0xc004b2a6f8?}, {0xc000059530?, 0x0?}})\n\tgithub.com/tendermint/tendermint@v0.34.24/consensus/state.go:853 +0x44b\ngithub.com/tendermint/tendermint/consensus.(*State).receiveRoutine(0xc000c84700, 0x0)\n\tgithub.com/tendermint/tendermint@v0.34.24/consensus/state.go:760 +0x419\ncreated by github.com/tendermint/tendermint/consensus.(*State).OnStart\n\tgithub.com/tendermint/tendermint@v0.34.24/consensus/state.go:379 +0x12d\n"
This imo sounds critical as it means anybody could try to open ibc channel on transfer port and kill our whole network ? But maybe I am missing something so thats why I am asking.
There was a problem hiding this comment.
Looks like you are using main, given the path github.com/babylonchain/babylon/x/zoneconcierge/abci.go:27 in error trace (this file only has 24 lines in dev). In main, this line is sending IBC heartbeat packet. It has been removed in dev.
Also, how did you create a channel between two Babylon chains with port transfer rather than zoneconcierge? Such channel request will be rejected by verification rules here and here. If we somehow worked around these rules, then this panic would happen upon sending IBC packets since the ZoneConcierge module only has capability for zoneconcierge port but not other ones.
I used to try Hermes relayer for integrating Injective, and encountered this issue as well (even in the I will spend some time for debugging this later today. |
|
Right, so the two problems seem to share the same root cause: the genesis does not include BLS PKs. When executing The above error means that the BLS PK is not specified when initialising genesis. Without the BLS PK, the checkpointing keeper will panic when initialising the BLS validator set for a new epoch. To fix this, we will need to manually insert BLS PKs to the validator when initialising genesis, possibly here. An example is https://github.com/babylonchain/babylon/pull/242/files#diff-860afb36e1ac0fcfc5e7680274639a6e3861bf4c9c09856a147a38b65aa4e6eeR47-R55. |
test/e2e/initialization/node.go
Outdated
| mnemonic string | ||
| keyInfo *keyring.Record | ||
| privateKey cryptotypes.PrivKey | ||
| consensusKey privval.FilePVKey |
There was a problem hiding this comment.
We have Babylon private validator type called privval.WrappedFilePVKey which includes BLS keys. Maybe we should use that
test/e2e/initialization/node.go
Outdated
| return err | ||
| } | ||
|
|
||
| filePV := privval.LoadOrGenFilePV(pvKeyFile, pvStateFile) |
There was a problem hiding this comment.
Maybe we can use privval.LoadOrGenWrappedFilePV to generate validator key with BLS keys
| return nil | ||
| } | ||
|
|
||
| func initGenesis(chain *internalChain, votingPeriod, expeditedVotingPeriod time.Duration, forkHeight int) error { |
There was a problem hiding this comment.
To ensure that every genesis validator has a BLS key, we should also insert BLS keys into genesis state of the checkpointing module. Please see an example here
|
@SebastianElvis thanks for debugging this.
Thats a bit weird as when i have babylon image built form main branch, the test passes. Imo it would be great if node would not start if validators do not have public keys specified in genesis. Nevertheless,I will try to add those keys (this should be pretty similar to the staff we currently do in |
Ah sorry I forgot to mention that, this change was introduced in 80c406a, which is indeed not in the main branch. |
|
@SebastianElvis @gitferry I have added bls keys to validators and genesis. I have also needed to add non validator nodes to serve as relayer nodes, as when using validators there were constant failures due to race conditions with bls transactions. Now everything should work smoothly. I would add this setup to CI in separate pr though. |
test/e2e/configurer/factory.go
Outdated
|
|
||
| var ( | ||
| // Last nodes are non validator nodes to serve as the ones using relayer. Out | ||
| // validator are constantly sending bls transactions which make relayer operatrions |
There was a problem hiding this comment.
| // validator are constantly sending bls transactions which make relayer operatrions | |
| // validators are constantly sending bls transactions which make relayer operatrions |
test/e2e/configurer/factory.go
Outdated
| // validator are constantly sending bls transactions which make relayer operatrions | ||
| // fail constantly | ||
|
|
||
| // each started validator containers corresponds to one of |
There was a problem hiding this comment.
| // each started validator containers corresponds to one of | |
| // each started validator container corresponds to one of |
|
|
||
| // TODO currently using image hosted by osmolab we should probably use our own | ||
| // Hermes repo/version for relayer | ||
| relayerRepository = "osmolabs/hermes" |
There was a problem hiding this comment.
Just asking, why does Osmosis use its own fork of Hermes, rather than using the official version?
There was a problem hiding this comment.
tbh I did not investigate it that much, just assumed it is somehow necessary for the starting script to work. It is on my follow up task list to either have our own fork or official hermes docker image used in here .
test/e2e/initialization/config.go
Outdated
| valPubKey, err := cryptocodec.FromTmPubKeyInterface(node.consensusKey.PubKey) | ||
|
|
||
| if err != nil { | ||
| panic("It should be possible retrieve validator public key") |
There was a problem hiding this comment.
| panic("It should be possible retrieve validator public key") | |
| panic("It should be possible to retrieve validator public key") |
test/e2e/initialization/config.go
Outdated
| da, err := sdk.AccAddressFromBech32(node.consensusKey.DelegatorAddress) | ||
|
|
||
| if err != nil { | ||
| panic("is should be possible to get validator address from delegator address") |
There was a problem hiding this comment.
| panic("is should be possible to get validator address from delegator address") | |
| panic("It should be possible to get validator address from delegator address") |
Adds e2e test with ibc based on osmosis e2e test (https://github.com/osmosis-labs/osmosis/tree/main/tests/e2e)
As currently the only thing it does is:
zoneconciergeport.Two run tests:
make localnet-build-envmake test-e2eNot adding to CI yet as, it fails on current babylon
devdocker image but works onmaindocker image.On current dev it is hermes releyer that fails with:
@SebastianElvis any leads what could have changed between current
mainanddevthat could lead to that ?