Closed ProofOfKeags closed 3 days ago
[!IMPORTANT]
Review skipped
Auto reviews are limited to specific labels.
Labels to auto review (1)
* llm-reviewPlease check the settings in the CodeRabbit UI or the
.coderabbit.yaml
file in this repository. To trigger a single review, invoke the@coderabbitai review
command.You can disable this status message by setting the
reviews.review_status
tofalse
in the CodeRabbit configuration file.
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
Did a quick review - have a mixed feeling now that we are pushing towards a symmetric design with the help of Dual, and sometimes the usage gives me a "too smart" vibe, maybe I'm being too cautious.
The main reason I am doing this is so we can select the appropriate field with the knowledge of which party we are looking for. This eliminates the following pattern:
thing := superStructure.localThing
if party == lntypes.Remote {
thing = superStructure.remoteThing
}
and replaces it with a single-assignment expression:
thing := superStructure.things.GetForParty(party)
@proofofkeags, remember to re-request review from reviewers when ready
Completed a high level pass on this, great refactor with really nice commit structure 🙏 Noticed that a lot of this is pulled out into #9097 - is that what we should review?
Closing this as it is superseded by #9158 and #9097
Change Description
Depends on #8270.Depends on #8981This is a series of single responsibility commits that improve the state of affairs within the channel state machine. The ultimate goal here in service of the Dynamic Commitments project is to make it such that we no longer have the
updateLog
s containpaymentDescriptor
s as the fundamentally tracked unit of update. This is unfortunately a long road which requires a number of changes which on their face do not seem necessary. I assure you that every one of these commits is ultimately in service of that goal, though.It is highly recommended that you review commits one by one and in order. One of the consequences of having atomic, semantics-invariant commits is that some subsequent commits may ultimately make prior commits partially or fully obsolete. This is the price of making commit-by-commit review easy.
There is no specific requirement that every commit in this PR go in at once. This PR is mergeable at every commit and will be semantics-invariant.
Godspeed 🫡
Steps to Test
make unit pkg=lnwallet
Pull Request Checklist
Testing
Code Style and Documentation
[skip ci]
in the commit message for small changes.📝 Please see our Contribution Guidelines for further guidance.