Skip to content
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

WIP: add LogEntry interface to replace monolithic PaymenDescriptor type #8

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

calvinrzachman
Copy link
Owner

@calvinrzachman calvinrzachman commented Feb 15, 2023

NOTE: This is not review ready.

The motivation for this PR comes via a request from Laolu to consider removing the PaymentDescriptor type and replacing it with a LogEntry interface for our in-memory HTLC update log.

We separate the relevant LogEntry interface bits from Laolu's Dynamic Commitment PR, fix a couple bugs, and...

TODO:

  • add in local changes which fix all unit tests.
  • make Laolu co-author on more or less cherry-picked first commit

Change Description

Description of change / link to associated issue.

Steps to Test

Steps for reviewers to follow to test the change.

Pull Request Checklist

Testing

  • Your PR passes all CI checks.
  • Tests covering the positive and negative (error paths) are included.
  • Bug fixes contain tests triggering the bug to prevent regressions.

Code Style and Documentation

📝 Please see our Contribution Guidelines for further guidance.

@github-actions
Copy link

Pull reviewers stats

Stats of the last 30 days for lnd:

User Total reviews Time to review Total comments

fix all build errors.
unit tests failing, some with incorrect amounts
Ran into a tricky to debug issue where forwarding nodes in a multi-hop
payment incorrectly compute the balance for the remote party when handling
a ChildLogEntry (Settle/Fail), end up missing an output on the commitment
transaction, and then send a CommitSig that our peer fails to validate, as
it does not match the view they were expecting given the updates they've sent.
use AddLogEntry before we create Settle/FailLogEntry
remove all traces of PaymentDescriptor from test cases as well
@calvinrzachman calvinrzachman changed the title multi: add LogEntry interface to replace monolithic PaymenDescriptor type WIP: add LogEntry interface to replace monolithic PaymenDescriptor type Mar 19, 2023
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.

1 participant