Testgen: LightBlock generation without cyclic dependency

[adizere]

Discussion ground for reviewing the fix for closing #605

EDIT: Also contains a minimal implementation of LightChain (which is eventually expected to support ibc-rs testing).

• Referenced an issue explaining the need for the change
• Updated all relevant documentation in docs
• Updated all code comments where relevant
• Wrote tests
• Updated CHANGELOG.md

[codecov-commenter]

Codecov Report

Merging #606 into master will increase coverage by 0.2%.
The diff coverage is 75.6%.

@@           Coverage Diff            @@
##           master    #606     +/-   ##
========================================
+ Coverage    41.3%   41.6%   +0.2%     
========================================
  Files         163     164      +1     
  Lines       11311   11393     +82     
  Branches     2559    2600     +41     
========================================
+ Hits         4682    4748     +66     
- Misses       6258    6274     +16     
  Partials      371     371             

Impacted Files	Coverage Δ
testgen/src/light_block.rs	60.8% <60.8%> (ø)
testgen/src/validator.rs	80.0% <90.9%> (+1.0%)	⬆️
light-client/src/predicates.rs	87.7% <96.0%> (+1.8%)	⬆️
testgen/src/header.rs	78.7% <0.0%> (+0.9%)	⬆️
tendermint/src/time.rs	65.3% <0.0%> (+6.1%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ec241f...7c9aa3c. Read the comment docs.

[Shivani912]

Converted to draft PR because it needs some improvements in the way it is coded like LightBlock and ValidatorSet needs to implement the Generator trait and I think it's also good to add the chain generation code here.

[Shivani912]

Test / nightly-coverage (pull_request) fails on master as well and the error seems to be coming from outside the scope of our code

[romac]

@Shivani912 @adizere Is this ready for review? If not, what's missing? Can I be of any help?

[adizere]

@Shivani912 @adizere Is this ready for review? If not, what's missing? Can I be of any help?

I need to bring some input to help progress on this, namely, to answer the question: which components of a light block are necessary (i.e., read and verified) as part of chain mocking to test the relayer. I'm still chewing on this question.

As far as I know, both Shivani and Andrey temporarily shifted focus away from this to work on fuzzing instead -- and generally on testing towards a 0.17 release. Looking forward to hear back from them when they get the time to jump back on this.

[adizere]

I need to bring some input to help progress on this, namely, to answer the question: which components of a light block are necessary (i.e., read and verified) as part of chain mocking to test the relayer. I'm still chewing on this question.

Coming back to this. Initially, the properties/interface we require from a light chain are:

1. Heights in blocks should be monotonically increasing when we call advance_chain. So each light block that is subsequently created upon calling this function, should have the field .signed_header.header.height larger by 1 compared to the header of the previous block.

2. We need the ability to create the first block (at Height 1). The function default_with_length() in this PR looks good for this purpose.

3. We need to fetch a block at a certain height, e.g., a function LightChain::block(&self, target_height: u64) -> Option<LightBlock> that may return none if the target height does not exist.

4. We need a getter for the latest light block, e.g., a function called latest_block(&self) -> LightBlock.

5. We need to be able to convert between a LightBlock and a ConsensusState type. This seems to be possible given the signed_header field inside the block.

[Shivani912]

Thanks for a detailed description of requirements @adizere! I'm on it :))

[adizere]

Scratch my prior comment. We can simplify if we forget about LightChain and instead stick to the lower-level LighBlock interface. What would be great is the following interface:

impl LightBlock {
    /// Constructs a default light block for a certain height: field `.signed_header.header.height`
    /// will correspond to input `height`.
    pub fn default_with_height(height: u64) -> Self {}

    /// Produces a light block "chained" to the input block (i.e., having height larger by 1). All
    /// other details of the block are currently ignored.
    pub fn next(&self) -> Self {}
}

LE: we'll go with the original requirements above.

[codecov-io]

Codecov Report

Merging #606 into master will increase coverage by 0.2%.
The diff coverage is 55.9%.

@@           Coverage Diff            @@
##           master    #606     +/-   ##
========================================
+ Coverage    42.6%   42.8%   +0.2%     
========================================
  Files         167     169      +2     
  Lines       11629   11816    +187     
  Branches     2625    2703     +78     
========================================
+ Hits         4961    5068    +107     
- Misses       6391    6471     +80     
  Partials      277     277             

Impacted Files	Coverage Δ
testgen/src/validator.rs	77.5% <ø> (ø)
testgen/src/light_block.rs	34.1% <34.1%> (ø)
testgen/src/light_chain.rs	92.7% <92.7%> (ø)
testgen/src/header.rs	77.6% <92.8%> (+1.9%)	⬆️
testgen/src/helpers.rs	77.7% <0.0%> (-2.3%)	⬇️
testgen/src/commit.rs	78.1% <0.0%> (+2.3%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6be5ebf...450997e. Read the comment docs.

[adizere]

Some parts are not clear to me, maybe we can simplify?

[Shivani912]

@adizere I've addressed your comments above. I won't be able to add you as a reviewer here, so please review this PR at your convenience and let me know if this is okay!

[adizere]

From the IBC-side this looks good. Thanks @Shivani912, much appreciated!

@andrey-kuprianov can you do a last pass to review this PR towards getting it merged?

[andrey-kuprianov]

Great work, @Shivani912! Glad to see the Testgen's capabilities growing:)