search

lightningdevkit / rust-lightning

A highly modular Bitcoin Lightning library written in Rust. It's rust-lightning, not Rusty's Lightning!
Other
1.11k stars 342 forks source link

Refactor async signing test utils to toggle specific method availability #3115

Closed alecchendev closed 2 weeks ago

alecchendev commented 3 weeks ago

Another pre-requisite PR to help shrink async signing diffs. To test async signing, we want be able to control whether our test channel signer returns a valid signature versus no signature. Previously we just had a catch-all flag that would toggle the signer being available/not across all signing methods. In upcoming PRs we'll want to test individual methods becoming available at different times (e.g. we made multiple requests to a remote signer, and got responses back in various orders). This PR refactors our existing tests and test utils to implement this.

codecov-commenter commented 3 weeks ago

Codecov Report

Attention: Patch coverage is 28.57143% with 40 lines in your changes missing coverage. Please review.

Project coverage is 89.79%. Comparing base (07f3380) to head (7e2789e).

:exclamation: Current head 7e2789e differs from pull request most recent head 3785375

Please upload reports for the commit 3785375 to get more accurate results.

Files Patch % Lines
lightning/src/util/test_channel_signer.rs 30.43% 32 Missing :warning:
lightning/src/ln/functional_test_utils.rs 0.00% 6 Missing :warning:
lightning/src/util/test_utils.rs 50.00% 0 Missing and 2 partials :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3115 +/- ## ========================================== - Coverage 89.91% 89.79% -0.12% ========================================== Files 121 119 -2 Lines 99172 97915 -1257 Branches 99172 97915 -1257 ========================================== - Hits 89167 87922 -1245 - Misses 7405 7428 +23 + Partials 2600 2565 -35 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

alecchendev commented 2 weeks ago

accidentally included previous commits, rebased to fix it. also have some nits/cleanups + a build issue i'll fix soon

alecchendev commented 2 weeks ago

squashed immediately since the changes were getting unreadable. Previously i included a change here to change how we disable a signer upon derive_channel_signer for channels that haven't be created yet, but I decided to move that to a separate PR, so this one is more focused. The other big change is instead of using a mask, I thought it was simpler to just use an enum for the signer ops

TheBlueMatt commented 2 weeks ago

Its just test code, there's no reason to hold this up on another reviewer.