-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[6/5]: invoice+rpc: add exit hop InvoiceAcceptor sub-systems and RPC calls #8771
Conversation
Important Review SkippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
91236ab
to
a5dde8a
Compare
12d4694
to
44fe836
Compare
This isn't ready for review yet. And for some reason I can't put it back to draft anymore, sorry. |
44fe836
to
5401a54
Compare
a5dde8a
to
8d716d9
Compare
8d716d9
to
6c31983
Compare
PR is now ready to review. |
c6b7ef7
to
13828b7
Compare
6c31983
to
7158c31
Compare
13828b7
to
f2e71de
Compare
7158c31
to
3d167f3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking good
CI is unhappy
|
||
// MockHtlcModifier is a mock implementation of the HtlcModifier interface. | ||
type MockHtlcModifier struct { | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't the mocks live under some test related pkg?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately in Go we can't import test code from one package into the other. And because we need the mock in multiple packages, it needs to be defined in non-test code.
3d167f3
to
f9d0aa1
Compare
f2e71de
to
d2d5064
Compare
This commit introduces a new invoice htlc interceptor service that intercepts invoice HTLCs during their settlement phase. It forwards HTLCs to a subscribed client to determine their settlement outcomes. This commit also introduces an interface to facilitate integrating the interceptor with other packages.
This commit updates the invoice registry to utilize the settlement interceptor during the invoice settlement routine. It allows the interceptor to capture the invoice, providing interception clients an opportunity to determine the settlement outcome.
This commit initiates the invoice settlement interceptor during the main server startup, assigning it a handle within the server.
This commit integrates the HTLC modifier service into the invoices RPC server.
f9d0aa1
to
d593663
Compare
This commit introduces a singleton invoice HTLC modifier RPC server and an endpoint to activate it. The server interfaces with the internal invoice HTLC modifier interpreter, handling the marshalling between RPC types and internal formats.
This commit enhances the itest LND node harness to include support for the new `HtlcModifier` RPC endpoint. At the same time we move another method to the correct file.
This commit introduces a basic integration test for the invoice HTLC modifier. The test covers scenarios where an invoice is settled with a payment that is less than the invoice amount, facilitated by the invoice HTLC modifier.
d593663
to
db61508
Compare
db61508
to
197b291
Compare
Fixed the CI, ready for final look. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
map<uint64, bytes> exit_htlc_wire_custom_records = 6; | ||
} | ||
|
||
message HtlcModifyResponse { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my only side note in this PR is that we may wanna add more stuff here in the future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I guess we might want to, but it's not clear right now if we actually need to. So leaving off a TODO for now to avoid needing to trigger another CI run. But added this comment to the tracking issue.
Replaces #8669.
Closes #8616
This pull request introduces the following components:
Link to all PRs in the saga: