Use cosmos v47 #320
Use cosmos v47 #320
Conversation
KonradStaniec
force-pushed
the
use-cosmos-v47
branch
from
460561b
to
afe68ce
Compare
| @@ -68,6 +78,8 @@ func TestUpgradeStateOnGenesis(t *testing.T) { | |||
| ctx := app.NewContext(false, tmproto.Header{}) | |||
| vm := app.UpgradeKeeper.GetModuleVersionMap(ctx) | |||
| for v, i := range app.mm.Modules { | |||
| require.Equal(t, vm[v], i.ConsensusVersion()) | |||
| if i, ok := i.(module.HasConsensusVersion); ok { | |||
There was a problem hiding this comment.
What's the case in which a module does not have a ConsensusVersion()?
There was a problem hiding this comment.
then we do not need to test it here. From what I understand , this test mainly checks that upgrade keeper versions are inline with versions declared by modules themselves
| ) (servertypes.ExportedApp, error) { | ||
| forZeroHeight bool, | ||
| jailAllowedAddrs []string, | ||
| modulesToExport []string) (servertypes.ExportedApp, error) { |
There was a problem hiding this comment.
The modulesToExport is not used here.
There was a problem hiding this comment.
good catch! we should use here new method for exporting genesis i.e have app.mm.ExportGenesisForModules(ctx, app.appCodec, modulesToExport) instead of app.mm.ExportGenesis(ctx, app.appCodec)
| @@ -15,7 +16,7 @@ import ( | |||
| "github.com/babylonchain/babylon/x/checkpointing/types" | |||
| ) | |||
|
|
|||
| func AddGenBlsCmd() *cobra.Command { | |||
| func AddGenBlsCmd(validator genutiltypes.MessageValidator) *cobra.Command { | |||
There was a problem hiding this comment.
Why do we add this argument? cc @gitferry
There was a problem hiding this comment.
it is needed as genutiltypes.ValidateAndGetGenTx require new argument of type genutiltypes.MessageValidator. This argument should be consistent across whole app and all commands so we need to pass it from root.go to all necessary call sites.
From what I understand, this new argument makes it possible to customize validator message which is put in genesis file. (at least validations done on that message) So it may come in handy at some point, for example to require custom create validator message with bls key. For now everywhere I am passing just the default implementation DefaultMessageValidator which makes this behave as old implementation without breaking stuff.
There was a problem hiding this comment.
This is a nice feature introduced by v47, which allows us to customize the validation rules against staking.MsgCreateValidator messages. I think for now the DefaultMessageValidator is sufficient, which solely calls the ValidateBasic method of the staking.MsgCreateValidator. I don't think we should add more rules about BLS keys because BLS keys are added after genesis txs are generated.
There was a problem hiding this comment.
Finally we can get rid of the btcd issue! I only have some minor comments. Maybe we can wait for followup comments from @vitsalis before merging this
Makefile
Outdated
|
|
||
| proto-all: proto-gen proto-swagger-gen | ||
| proto-all: proto-format proto-lint proto-gen |
There was a problem hiding this comment.
Why do we ditch proto-swagger-gen here?
There was a problem hiding this comment.
good point, I just got carried away and copied stuff from cosmos-sdk. (we do not even have proto-format and proto-lint)
I will bring it back to how it was and crated task to and linter and formatter for protos- #321
Bump cosmos sdk to 47, ibc-go to v7, and btcd to latest version.
One caveat, for now I disabled e2e test as it seems hermes relayer is not ready for new tendermint version. Long story short, hermes uses events associated with transaction to declare in if some operation succeeded. Format of events was updated with new tendermint version. One easy wasy to confirm that is to run manually some operation on hermes (like
create client) and then checkhermes query tx events hashand and./babylond query tx hash, even though babylon node shows all events are there, hermes does not recoignize them.UPDATE: hermes 1.3 actually returns an error when trying to retrieve events by cli:
ERROR relayer error: RPC error to endpoint http://bbn-test-a-node-babylon-default-a-3:26657/: serde parse error: incomplete utf-8 byte sequence from index 1 at line 22 column 30We can re-enable those after support will be added.