Improve testing across dependencies

[adizere]

One difficulty we're facing in ibc-rs with regards to testing is that we need to instantiate certain types that belong to an external dependency create, namely to tendermint-rs.

An example: we want to test ClientState. This data type includes a member of type Header which in turn includes a member of type tendermint::block::signed_header::SignedHeader including both a Header and a Commit.

To instantiate a ClientState we need to instantiate all these dependencies, which is very tricky because there are tens of them. Just a Header has 14 fields, and correct instantiation of the Header is not exactly relevant for testing ClientState.

As I understand it, an alternative to instantiating this whole chain of dependencies, is for some of these types to implement the trait Default. Then we could say let sh = Default::default() to get a basic (valid) signed header we could use in testing ClientState. In general, adopting this strategy should simplify testing of cross-dependent data types. There are already many types implementing Default so we should derive it for free quite often. At the moment, I counted 20 types in tendermint-rs that derive Default.

I can't think of any cons, except that there may be perhaps more code to maintain. But the burden should not be big, since tests already include these default values (example).

[andrey-kuprianov]

Hi!

I am already working on this; take a look at: https://github.com/informalsystems/tendermint-rs/blob/andrey/mbt-utils/mbt-utils/src/tendermint-produce.rs

The idea is that we will be able to produce the Tendermint datastructures for testing from minimal input, like a validator from a string identifier. These datastructures can be then instantiated both from the CLI, e.g.:

mbt-tendermint-produce validator --id a --voting-power 3

and from the Rust code, e.g.:

let validators = vec![Validator::new("a"), Validator::new("b")];
let header = Header::new(&validators).height(100).time(Time::now());
let commit = Commit::new(&header).round(10);

This is already close to being finished; sorry for not creating an issue for it beforehand...
Any input is of course welcome.

[andrey-kuprianov]

And the real Tendermint datastructures are then produced in Rust from the above as follows:

let block_header = header.produce().unwrap();
let block_commit = commit.produce().unwrap();

[andrey-kuprianov]

also, to answer your original question, and for a bit of motivation: implementing Default won't help here, as the datastructures that are being produced need to be linked to each other via hashes, encryption, etc. E.g., if a Commit is produced without the actual Header it commits, the validation code will fail.

[adizere]

I am already working on this; take a look at: https://github.com/informalsystems/tendermint-rs/blob/andrey/mbt-utils/mbt-utils/src/tendermint-produce.rs
The idea is that we will be able to produce the Tendermint datastructures for testing from minimal input, like a validator from a string identifier.

As far as I'm concerned (for cross-testing) purposes, this is beautiful.

I suppose it would be a more "standardized" approach to build on the trait Default rather than Produce, that's why I suggested it. Note that the implementation of ::default() can be arbitrarily complex, e.g., along the lines of produce to ensure valid data structures are instantiated. But I guess it's also a matter of taste. As long as there is a simple API to instantiate a Header or a Commit and other complex Tendermint types, we should be fine in ibs-rs.

[andrey-kuprianov]

Glad you like it! I hope the API is sufficiently simple (at least I've designed it with this intention)... and thanks for creating the issue:)

Currently this is implemented for model-based testing of the LightClient; but the datastructure encoding part is of course generic, and I will be happy to help setting up MBT for ibc-rs as well.

[ebuchman]

@andrey-kuprianov can you please open an issue describing the work at a high level so we can better track

[andrey-kuprianov]

@ebuchman documented in #393

[ebuchman]

I think we can close this now that the testgen is merged and supports generating these types. Let me know if not!