Lightning Test Improvements

[m1sterc001guy]

With the development of the gateway (and devimint in parallel) our lightning tests have grown and morphed to a point where they are now difficult to work with. This issue describes the current issues and has a proposal for how to fix these tests going forward.

Issues

• There are multiple different test suites: fedimint-ln-tests, the gateway's integration_tests.rs, and tests that exist in devimint. When adding new tests its unclear where they should go.
• LN Tests have a "fake" and a "real" mode. In the "real" mode, they connect to external daemons that are spawned by devimint. This is difficult to debug, since it requires setting an environment variable that new contributors might not know. It also hampers parallelism, since all rusts tests that run in parallel use the same backing daemon.
• The gateway tests use an ugly hack for verifying the state machine transitions. The test first removes the gateway's federation client so that REST requests to the gateway fail, but the necessary data (i.e the contract) is created correctly. The results in confusing error messages in the logs, hacky code, etc.
• The tests iterate over each gateway sequentially. This makes the tests much slower than normal tests, and because of the above issue, they can't easily be parallelized.

Proposal

At the time most of this test suite was created, we did not have devimint tests like we do today. I believe most of the LN tests should be refactored into standalone devimint tests. These can run in parallel much easier since they use their own daemons.

fedimint-ln-tests: should only contain tests verifying state machine transitions of the LN client. All necessary dependencies should be mocked (i.e gateway)
integration_tests.rs: should only contain tests verifying state machine transitions of the gateway client. All necessary dependencies should be mocked (i.e LN node)

All other tests should be refactored to devimint tests.

GatewayTest should be deleted. fedimint-testing/src/ln/real.rs should be deleted.

Test Breakdown

fedimint-ln-tests

• text_can_attach_extra_meta_to_receive_operation - good
• cannot_pay_same_internal_invoice_twice - good
• gateway_protects_preimage_for_payment - should be devimint test
• cannot_pay_same_external_invoice_twice - gateway needs to be mocked
• makes_internal_payments_within_federation - gateway needs to be mocked
• can_receive_for_other_user - gateway needs to be mocked
• can_receive_for_other_user_tweaked - gateway needs to be mocked
• rejects_wrong_network_invoice - gateway needs to be mocked

integration_tests.rs

• test_gateway_client_pay_valid_invoice - no real LN
• test_can_change_default_routing_fees - should be devimint test
• test_can_change_federation_routing_fees - should be devimint test
• test_gateway_enforces_fees - should be devimint test
• test_gateway_cannot_claim_invalid_preimage - no real LN
• test_gateway_client_pay_unpayable_invoice - no real LN
• test_gateway_client_intercept_valid_htlc - no real LN
• test_gateway_client_intercept_offer_does_not_exist - no real LN
• test_gateway_client_intercept_htlc_no_funds - no real LN
• test_gateway_client_intercept_htlc_invalid_offer - no real LN
• test_gateway_register_with_federation - should be devimint test
• test_gateway_cannot_pay_expired_invoice - no real LN
• test_gateway_filters_route_hints_by_inbound - should be devimint test
• test_cannot_connect_same_federation - should be devimint test
• test_gateway_configuration - should be devimint test
• test_gateway_supports_connecting_multiple_federations - should be devimint test
• test_gateway_shows_info_about_all_connected_federations - should be devimint test
• test_gateway_can_leave_connected_federations - should be devimint test
• test_gateway_shows_balance_for_any_connected_federations - should be devimint test
• test_gateway_executes_swaps_between_connected_federations - remove webserver RPCs

Some of these can probably be combined together. There shouldn't be any RPCs in integration_tests.rs.

[m1sterc001guy]

I've investigated trying to mock out the user client for the gateway tests and it looks like its going to be more complicated than initially hoped. We would need to manually write into each federation database to simulate the creation of a contract, which is error prone and might lead to bugs in the long run. For now I think its best to keep these tests with both the user client and gateway client.

We should still remove any RPCs and webserver requests in these tests though.