zoneconcierge: test infra for IBC and vanilla tests #193
Conversation
| @@ -53,6 +53,9 @@ func (e Epoch) IsFirstBlock(ctx sdk.Context) bool { | |||
| } | |||
|
|
|||
| func (e Epoch) IsSecondBlock(ctx sdk.Context) bool { | |||
| if e.EpochNumber == 0 { | |||
There was a problem hiding this comment.
@gitferry Some of the IBC tests will panic without this newly added if condition. It seems that those tests execute this function in epoch 0. While this modification does not break other things, do you have any idea on when will this function be invoked in epoch 0?
There was a problem hiding this comment.
It will be called at epoch ends. See here.
There was a problem hiding this comment.
Right, then how did we skip invoking IsSecondBlock for the first (0-th) epoch? 😢
There was a problem hiding this comment.
Why would ibc tests invoke IsSecondBlock?
There was a problem hiding this comment.
Actually I don't know 🤣 Without these three lines the new tests in this PR will fail. You can have a try if you are interested. Nvm I feel that returning false in the 0-th epoch for this function should not introduce any bug.
There was a problem hiding this comment.
Not sure about that. If it simply returns false for epoch 0, no BLS-sig txs will be sent for epoch 0. That would mean that we don't checkpoint epoch 0
There was a problem hiding this comment.
After a quick look, the root is likely caused here. Some helper functions of ibc tests still use old simapp functionalities. We might need to fork that.
There was a problem hiding this comment.
Not sure about that. If it simply returns false for epoch 0, no BLS-sig txs will be sent for epoch 0. That would mean that we don't checkpoint epoch 0
Aren't BLS-sig txs for epoch 0 are signed and sent in epoch 1? I was under the impression that in epoch 0 no BLS-sig txs should be signed since there is no epoch -1.
After a quick look, the root is likely caused here. Some helper functions of ibc tests still use old simapp functionalities. We might need to fork that.
Thanks for digging into it. Modifying IBC tests seems to make many tests fail. I feel that the current modification is more straightforward. Do you have any objection on this change?
There was a problem hiding this comment.
Aren't BLS-sig txs for epoch 0 are signed and sent in epoch 1? I was under the impression that in epoch 0 no BLS-sig txs should be signed since there is no epoch -1.
Oh, right. Ignore me. No blocker at this point
| @@ -53,6 +53,9 @@ func (e Epoch) IsFirstBlock(ctx sdk.Context) bool { | |||
| } | |||
|
|
|||
| func (e Epoch) IsSecondBlock(ctx sdk.Context) bool { | |||
| if e.EpochNumber == 0 { | |||
There was a problem hiding this comment.
It will be called at epoch ends. See here.
x/zoneconcierge/module_test.go
Outdated
| ) | ||
|
|
||
| // ZoneConciergeTestSuite provides a test suite for IBC functionalities in ZoneConcierge | ||
| // (adapted from https://github.com/cosmos/ibc-go/blob/main/modules/core/02-client/keeper/keeper_test.go) |
There was a problem hiding this comment.
This is not a permanent link. When the main branch of ibc-go is updated, then the content might not be consistent here. We can get a permalink like this: https://github.com/cosmos/ibc-go/blob/b1f494c643280c73f3696461abbb11a39303b190/modules/core/02-client/keeper/keeper_test.go. I get it from a specific line of keeper_test.go and remove the line number
x/zoneconcierge/module_test.go
Outdated
| } | ||
|
|
||
| // TestUpdateClientTendermint provides tests on verifying different headers from the IBC light client | ||
| // (adapted from https://github.com/cosmos/ibc-go/blob/main/modules/core/02-client/keeper/client_test.go) |
Partially fixes BM-275
This PR provides infra for testing IBC functionalities in ZoneConcierge, and adds basic tests of IBC light clients.
The test infra is largely adopted from https://github.com/cosmos/ibc-go/blob/main/modules/core/02-client/keeper/keeper_test.go. It allows to create two chains with an IBC connection, and simulate all behaviours defined in IBC. This PR replaces one of the chain with Babylon chain. The test below indicates that Babylon has supported establishing IBC connections with other CZs.
The vanilla tests are copied from https://github.com/cosmos/ibc-go/blob/main/modules/core/02-client/keeper/client_test.go. It tests the verification of different types of headers in the IBC light client. Later we will modify these tests w.r.t. Babylon's IBC light client verification rules.