Skip to content

Inline ignore_error macro #3093

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

G8XSU
Copy link
Contributor

@G8XSU G8XSU commented Jun 4, 2024

  • Inline ignore_error macro

Based on #3081

G8XSU added 2 commits June 3, 2024 11:53
This removes dependency of watched_outputs from
per_commitment_claimable_outpoints, it is required since we will
no longer have direct access to per_commitment_claimable_outpoints
once we start publishing PersistClaimInfo as part of lightningdevkit#3049.
arik-so
arik-so previously approved these changes Jun 4, 2024
@TheBlueMatt TheBlueMatt dismissed arik-so’s stale review June 4, 2024 21:02

The merge-base changed after approval.

let per_commitment_key = ignore_error!(SecretKey::from_slice(&secret));
let per_commitment_key = match SecretKey::from_slice(&secret) {
Ok(per_commitment_key) => per_commitment_key,
Err(_) => return (claimable_outpoints, to_counterparty_output_info)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, blindly ignoring this is a bit of a holdover from when we thought we could use the same ChannelMonitor as a watchtower. We should definitely be logging here, at a minimum, and maybe panicking.

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 90.90909% with 2 lines in your changes missing coverage. Please review.

Project coverage is 91.16%. Comparing base (7326334) to head (42597bc).
Report is 22 commits behind head on main.

Files Patch % Lines
lightning/src/chain/channelmonitor.rs 90.90% 2 Missing ⚠️

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

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3093      +/-   ##
==========================================
+ Coverage   89.90%   91.16%   +1.25%     
==========================================
  Files         117      119       +2     
  Lines       97415   106993    +9578     
  Branches    97415   106993    +9578     
==========================================
+ Hits        87581    97535    +9954     
+ Misses       7272     7050     -222     
+ Partials     2562     2408     -154     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants