Skip to content

Simplify UpdateFulfillFetch::NewClaim #3803

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 1 commit into
base: main
Choose a base branch
from

Conversation

joostjager
Copy link
Contributor

When working on #3801, I discovered that this code can be simplified.

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented May 27, 2025

👋 Thanks for assigning @jkczyz as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@joostjager joostjager force-pushed the claim-without-msg branch from cbc4450 to 237259f Compare May 27, 2025 10:08
@@ -1015,7 +1015,7 @@ enum UpdateFulfillFetch {
NewClaim {
monitor_update: ChannelMonitorUpdate,
htlc_value_msat: u64,
msg: Option<msgs::UpdateFulfillHTLC>,
update_non_blocked: bool,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not 100% sure what to call this. Could invert and simply call it blocked?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I was thinking maybe update_blocked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@joostjager joostjager force-pushed the claim-without-msg branch 2 times, most recently from 6f094bb to 5ef74c0 Compare May 27, 2025 10:10
@joostjager joostjager marked this pull request as ready for review May 28, 2025 12:51
@joostjager joostjager requested review from jkczyz and removed request for TheBlueMatt May 28, 2025 12:54
The actual message is not used anywhere.
@joostjager joostjager force-pushed the claim-without-msg branch from 5ef74c0 to d0288cd Compare May 28, 2025 20:49
@ldk-reviews-bot
Copy link

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

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.

3 participants