rust-lightning icon indicating copy to clipboard operation
rust-lightning copied to clipboard

Convert `check_htlc_valid_counterparty` and `log_claim` macros to fns

Open G8XSU opened this issue 1 year ago • 4 comments

  • Convert check_htlc_valid_counterparty and log_claim macros to fns

Branch-off from https://github.com/lightningdevkit/rust-lightning/pull/3057

Re: macro removal

  1. Most of the macros need a immut reference to self for a argument and a mutable reference to self to add onchain-event entry. (which works easily in macro but doesn't work when extracted to a function)

  2. I was able to remove check_htlc_valid_counterparty macro and log_claim macros but they need a long list of arguments to function (writing it as a macro was hiding this fact.). So either we have to be fine with long list of arguments in this case, as it was already using it or this will need more refactoring. In any case, this was getting messy real quick and is best handled out-of-scope of previous PR.

Can be indpendently reviewed but depends on previous PR.

G8XSU avatar May 22 '24 00:05 G8XSU

Codecov Report

:x: Patch coverage is 85.45455% with 8 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 91.03%. Comparing base (1890e80) to head (f7b5b65). :warning: Report is 3133 commits behind head on main.

Files with missing lines Patch % Lines
lightning/src/chain/channelmonitor.rs 85.45% 6 Missing and 2 partials :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3076      +/-   ##
==========================================
+ Coverage   89.82%   91.03%   +1.20%     
==========================================
  Files         116      117       +1     
  Lines       96466   104993    +8527     
  Branches    96466   104993    +8527     
==========================================
+ Hits        86655    95577    +8922     
+ Misses       7264     6938     -326     
+ Partials     2547     2478      -69     

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

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov-commenter avatar May 22 '24 00:05 codecov-commenter

Looks good to me, though obviously pending #3057 getting merged and its macro refactoring commits getting dropped.

arik-so avatar May 22 '24 05:05 arik-so

@arik-so Yes 3057 needs to be merged before this.

its macro refactoring commits getting dropped.

but that doesn't have any macro refactoring commits.

G8XSU avatar May 22 '24 18:05 G8XSU

Seems this unfortunately needs a rebase by now.

tnull avatar Jun 12 '24 09:06 tnull