[Bug]: Get invoice endpoint sometimes returns stale data if you manually mark invoice

[pirus111]

What is your BTCPay version?

v1.10.0

How did you deploy BTCPay Server?

Docker

What happened?

If you manually mark an invoice through the frontend, and then quickly make a get invoice greenfield API request, sometimes you get stale data back.

How did you encounter this bug?

1. I created an invoice through the greenfield API
2. I manually marked the invoice as complete through the btcpay frontend's invoices list page
3. My configured webhook (on send everything, redelivery on) to my application's endpoint. The received event is not stale (i.e. invoice is complete, and marked)
4. I then call the greenfield API's get invoice endpoint which returns stale data (i.e. invoice is incomplete, and not marked).

Context: My endpoint does not trust the webhook due to a library limitation in my programming language of choice :(

Relevant log output

n/a

What browser do you use?

Firefox latest

Additional information

The bug is presumably in InvoiceRepository#MarkInvoiceStatus.
You can see in the following that the event is fired before the db context is saved.

invoiceData.Status = legacyStatus.ToLowerInvariant();
invoiceData.ExceptionStatus = InvoiceExceptionStatus.Marked.ToString().ToLowerInvariant();
_eventAggregator.Publish(new InvoiceEvent(ToEntity(invoiceData), eventName)); // <-- event sent before save
await context.SaveChangesAsync(); // <-- now we save, but the event could already have been processed by third parties

We cannot blindly reverse the order, we would have to consider its impact on the system wrt error handling. Since in the above example, if publishing the event fails then the db transaction would be rolled back.

But there's another method in the same repository called UpdateInvoiceExpiry which (at a glance) doesn't handle the situation well.
I would double check my findings since this stuff could be a pain to debug if it goes wrong, and I did not look into whether the event publisher delays sending or sends on demand.

Are you sure this is a bug report?

• I confirm this is a bug report