Message Generator as a Requirement for PRs

[plebhash]

This is a discussion to debate the proposal of Message Generator tests as a Requirement for PRs that fix bugs coming from message ordering.

This is strategically important to ensure the robustness of SRI, as well as an important foundation for future implementations.

MG is a powerful to detect and mitigate protocol edge cases and make sure roles are properly following the specs.

basically, every bug we see on our prototype roles is a big opportunity to add value to MG and make it the community gold standard tool for building robust SV2 implementations in the future.

also, not following this practice means that bugfixing is a very suboptimal game of engineering whack-a-mole.

[plebhash]

we now have some video tutorials teaching how to write MG tests:

• https://www.youtube.com/playlist?list=PL4okm1s0ZpT9YveJBiU51gaaOmNJviugi
• https://www.youtube.com/playlist?list=PL4okm1s0ZpT8gWyM0UNKNbFoy5-djIheg

These are multi-episode playlists with participation of SRI community members:

• @lorbax
• @GitGab19
• @xyephy

[pavlenex]

Good work putting these together guys!

[plebhash]

another suggestion for convention:

both PR contributor + reviewer should make sure the new added MG test is passing locally

[plebhash]

recently @jbesraa proposed an alternative path inspired by LDK:

https://github.com/lightningdevkit/ldk-node/tree/main/tests

the idea would be to leverage integration tests to enforce SRI follows the spec

MG tests would be restricted for interoperability tests only (which is the original purpose of MG anyways)

[plebhash]

I like this idea given how verbose and complex MG tests are (lengthy and error-prone JSON files). That imposes a barrier that is very hard to go around.

While something like a MG DSL (#943) could help mitigate JSON verbosity, integration tests could potentially be a better long-term solution.

[GitGab19]

recently @jbesraa proposed an alternative path inspired by LDK:

https://github.com/lightningdevkit/ldk-node/tree/main/tests

the idea would be to leverage integration tests to enforce SRI follows the spec

MG tests would be restricted for interoperability tests only (which is the original purpose of MG anyways)

I like this idea, also because MG was born as a tool to check interoperability in the first place (@lorbax correct me if I'm wrong). And then it started to be used internally as a sort of integration tests tool whenever we identified bugs and fixed them.
But as I already mentioned in the past, I don't think it's the best approach to test new fixes we introduce in our codebase. In the current state, it's not trivial to write proper tests, especially when scenarios to test require many roles and messages with a specific ordering (see #923), and I don't think it adds some relevant scale to the process.
Our MG is a very powerful tool, and I'm 100% convinced it should be used to test alternative future (and present) implementations, putting its focus there. Also, I think that many tests we currently have under MG are a bit trivial with respect to the potential of the tool itself. For example:

• bad-pool-config-test just checks that pool manages bad configurations -> it could be easily tested without the need for MG imo
• jds-do-not-fail-on-wrong-tsdatasucc just sends a random RequestTransactionDataSuccess message and check that JDS doesn't crash/panic -> same as above
• jds-do-not-panic-if-jdc-close-connection -> same

These are just some examples, but my point is that I think we could cover those simple cases with some standard integration tests (which I think it's the approach followed by LDK) instead of adding all the MG complexity to them.

[jbesraa]

Sounds good, I can try and play around with the code to see how things can work

[Shourya742]

Having worked on integration tests for CoinSwap, I suggest starting with integration tests for general known flows to strictly adhere to our specifications. We need to set up a test framework infrastructure that can mock some of the current behaviors to ensure the flow works as we intend. Although the exact path forward is still somewhat unclear, we can develop a robust end-to-end integration test suite. Count me in for this initiative.

[jbesraa]

Thats the plan. Ill try to come up with an initial PR soonish and then maybe we can split up some of the work