fix: Watch for specific r_hash #2574
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is not needed as no change notifier is interested in this event at the moment.
This patch fixes two issues. 1. Any paid invoice that has been created by the coordinator will produce a `LnPaymentReceived` event. Before that change we would have simply assumed that this was the payment we've been waiting for. But it could have happened that the users went back and forth multiple times and thus created multiple invoices. So if an old invoice would have been paid, we would have triggered the order creation which would have failed, because the shared pre-image would have been incorrect. We do now verify that the received r_hash is equal to the watched r_hash. 2. On every creation of a hodl invoice we subscribed the `InvoiceWatcher` for `LnPaymentReceived` events. These subscribers lived forever and never got cancelled, producing various error messages since the receiver has already been gone. The invoice watcher is now subscribed at the app startup and the broadcast::Sender is stored in the apps state - so that only a single Subscriber can do the job.
holzeis
force-pushed
the
fix/check-r-hash-on-payment-received
branch
from
c511305 to
c02a736
Compare
Allows the mock to dynamically respond with the correct hash.
luckysori
approved these changes
May 29, 2024
| let message = serde_json::to_string(&InvoiceResult { | ||
| result: Invoice { | ||
| memo: "".to_string(), | ||
| expiry: 0, | ||
| amt_paid_sat: 0, | ||
| state: InvoiceState::Accepted, | ||
| payment_request: "".to_string(), | ||
| r_hash: "".to_string(), | ||
| r_hash: state.read().expect("").to_string(), |
There was a problem hiding this comment.
🙃 Just allow unwrap in this crate or in this file if you don't want to deal with expect.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes two issues.
Any paid invoice that has been created by the coordinator will produce a
LnPaymentReceivedevent. Before that change we would have simply assumed that this was the payment we've been waiting for. But it could have happened that the users went back and forth multiple times and thus created multiple invoices. So if an old invoice would have been paid, we would have triggered the order creation which would have failed, because the shared pre-image would have been incorrect. We do now verify that the received r_hash is equal to the watched r_hash.On every creation of a hodl invoice we subscribed the
InvoiceWatcherforLnPaymentReceivedevents. These subscribers lived forever and never got cancelled, producing various error messages since the receiver has already been gone. The invoice watcher is now subscribed at the app startup and the broadcast::Sender is stored in the apps state - so that only a single Subscriber can do the job.