Closed feelancer21 closed 2 months ago
[!IMPORTANT]
Review Skipped
Auto reviews are disabled on this repository.
Please 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?
Hi @feelancer21 we'd like to get this pr merged into the next rc asap. Please let us know, if you can turn this around quickly. If not, we can take this over. Thanks
Hello guys,
have made an update. Here are a few comments:
2 I have now included the inbound fees in the edge. However, I did not split up the individual fields because they would have been merged again in UpdatePolicy anyway. But I can still change this if desired.
extract and pack the ExtraOpaqueData
I have not outsourced to a separate method. Since policies have no other ExtraOpaqueData, this should not lead to problems.
I had to integrate itests for the inbound fees into the lnd_channel_policy_test.go
. As part of this, I had to fix another issue with the assert of the policies. The nil case is also tested.
Regards
Hi @feelancer21 would you be able to address the feedback comments today?
Hi @feelancer21 would you be able to address the feedback comments today?
that's the plan
Fixes the problem that inbound base fee and fee rate are overwritten with 0 if they are not specified in PolicyUpdateRequest. This ensures backward compatibility with older rpc clients that do not yet support the inbound feature.
Change Description
Fixes #8614
I had initially considered working with a boolean. However, as the base fee and fee rate are two values that are updated and not just one, I found it more elegant to outsource them to an extra message, which can also be nil. Both implementation variants mean that rpc clients that already implement the inbound fees in the current variant have to update their protos again.
The idea with the fn package comes from @ziggie1984 . Thanks for that .
Steps to Test
Tested with
lncli
as described in #8614.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.