Conversation
|
👋 Thanks for assigning @joostjager as a reviewer! |
d393f3e to
ce0d9cf
Compare
ajaysehwal
left a comment
ajaysehwal
left a comment
There was a problem hiding this comment.
@tnull , one gap in thorough test suite: there’s no test covering a sender that gets rate-limited once and then successfully retries after the bucket refills.
Not required for correctness — the existing tests already show that the rate-limited request is dropped cleanly without affecting other requests. But a small test using a short refill interval or deterministic bucket advancement would make the intended contract explicit: the request is dropped, and the sender is free to retry later rather than being blocked permanently.
Purely a documentation-via-test nice-to-have.
Drop static invoice requests when the rate limit is reached instead of retrying them and delaying later events. Keep retrying storage errors and only acknowledge invoices after saving them. Fixes an issue reported by Ajay Sehwal. Co-Authored-By: HAL 9000
ce0d9cf to
264852e
Compare
| self.channel_manager.static_invoice_persisted(invoice_persisted_path); | ||
| }, | ||
| Err(e) => { | ||
| Err(StaticInvoiceStoreError::RateLimited) => { |
There was a problem hiding this comment.
Don't we want to skip the rate limiter once we are retrying persistence, so that we don't exit the retry loop because new requests came in?
| } | ||
|
|
||
| #[tokio::test(flavor = "multi_thread", worker_threads = 2)] | ||
| async fn static_invoice_read_error_is_replayed() { |
There was a problem hiding this comment.
Quite a bit of test code. Can't AI come up with something more compact with still reasonable coverage?
4 participants
Drop static invoice requests when the rate limit is reached instead of retrying them and delaying later events. Keep retrying storage errors and only acknowledge invoices after saving them.
Fixes an issue reported by @ajaysehwal.