Refactor PaymentRequest entity to align with patterns in Invoice entity
During discussions on the implementation of https://github.com/btcpayserver/btcpayserver/pull/6642, @NicolasDorier and I reached several actionable conclusions:
- PaymentRequest: Move the Amount and Currency from Blob2 into separate columns. We may also consider moving the Title and Description.
- PaymentRequest: Remove StoreId from Blob2 and rely exclusively on the StoreDataId column.
- Reevaluate the naming of
ReferenceNumberin PaymentRequest to see if we want to change it toOrderIdin order to align with the naming pattern established for the Invoice entity. -
Invoice: Change the Status field from
textin the PostgreSQL database to the smallest possible data type. It could even be a byte, as it will only represent values from theInvoiceStatusenum. - Invoice: Reassess the ExceptionStatus field type, as it is currently set to unlimited text. Additionally, rename it, since it's more of a Flag that gives additional info on why Status is in certain state. The supported exception statuses are: Marked, PaidLate, PaidPartial, PaidOver, or an empty string. Unfortunately, there is no enum defined for these statuses, so they are matched directly through strings. For more information, see https://github.com/btcpayserver/btcpayserver/blob/65629f40b388212f1209dd5c75448e6c64fd43f9/BTCPayServer.Data/Data/InvoiceData.Migration.cs#L342.
Notes
- We discussed adding Blob.Metadata to PaymentRequests to create a pattern similar to that used for the Invoice. We would then move
ReferenceNumberin theMetadata. But we decided it would be excessive at this time. You can learn more about the approach we took in that context by visiting https://docs.btcpayserver.org/Development/InvoiceMetadata/#well-known-properties. - The Blob column is being retained for backward compatibility and migration purposes.
Current database structure and example data row
Invoice
Example data:
Id: AGNRVQgpiCqfvK1V52s6co
Blob: binary data
Created: 2025-03-06 07:55:33+00
ExceptionStatus: Marked
Status: Settled
StoreDataId: DoBgQhyvaJQCsk2ugfALWGp6ttwaZ6LkBpbci7TGao8s
Archived: false
Blob2:
{
"rates": {
"BTC": "5000"
},
"prompts": {
"BTC-CHAIN": {
"details": {
"keyPath": "0/0",
"accountDerivation": "tpubD6NzVbkrYhZ4XxNXjYTcRujMc8z8734diCthtFGgDMimbG5hUsKBuSTCuUyxWL7YwP7R4A5StMTRQiZnb6vE4pdHWPgy9hbiHuVJfBMumUu-[legacy]",
"recommendedFeeRate": 20.023,
"paymentMethodFeeRate": 20.023
},
"currency": "BTC",
"destination": "my8NcFLvD6PVyYvigm9iVqseudA5Zc24ce",
"divisibility": 8
}
},
"version": 3,
"metadata": {},
"serverUrl": "http://127.0.0.1:8001",
"speedPolicy": 1,
"internalTags": [],
"expirationTime": 1741248633,
"receiptOptions": {},
"fullNotifications": true,
"monitoringExpiration": 1741335033
}
Amount: 100
Currency: USD
Payment Request
Id: 08c0db8e-f3b1-42ef-8e1f-83fad9c26794
StoreDataId: 4oZbwCxZ43XeskaJ7k8RQLEv9oz9wbvfJfwxfEu72ccy
Status: 0
Blob: binary data
Created: 2025-03-29 01:15:59.52819+00
Archived: false
Blob2:
{
"email": null,
"title": "Something",
"amount": 500.0,
"formId": null,
"storeId": null,
"currency": "USD",
"expiryDate": null,
"description": null,
"formResponse": null,
"referenceNumber": "1234",
"allowCustomPaymentAmounts": false
}
Invoice: Change the Status field from text in the PostgreSQL database to the smallest possible data type. It could even be a byte, as it will only represent values from the InvoiceStatus enum.
NACK. Pain in the ass to query manually, pain in the ass to migrate (if you have lot's of invoice you'll have a big downtime)... there is no upside to this one.
Invoice: Reassess the ExceptionStatus field type, as it is currently set to unlimited text.
NACK. All those exceptions have specific logic associated with it, it isn't meant to be user specified. Huge breaking change without any upside.
The upside on both is that you get faster lookups and smaller database.
For ExceptionStatus, I'm also saying that the naming of that variable is very unfortunate... it's like you're saving some kind of Exception, while it's really a flag giving more info on why Status is certain way.
Yeah... it's coming from the naming of Bitpay's API