Skip to content
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

chore: prevent WalletInvoice deletion #3557

Merged
merged 2 commits into from
Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 25 additions & 11 deletions core/api/src/app/wallets/update-pending-invoices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,11 @@ export const declineHeldInvoice = wrapAsyncToRunInSpan({
})

if (lnInvoiceLookup instanceof InvoiceNotFoundError) {
const isDeleted = await walletInvoicesRepo.deleteByPaymentHash(paymentHash)
if (isDeleted instanceof Error) {
pendingInvoiceLogger.error("impossible to delete WalletInvoice entry")
return isDeleted
const processingCompletedInvoice =
await walletInvoicesRepo.markAsProcessingCompleted(paymentHash)
if (processingCompletedInvoice instanceof Error) {
pendingInvoiceLogger.error("Unable to mark invoice as processingCompleted")
return processingCompletedInvoice
}
return false
}
Expand Down Expand Up @@ -148,9 +149,10 @@ export const declineHeldInvoice = wrapAsyncToRunInSpan({
const invoiceSettled = await lndService.cancelInvoice({ pubkey, paymentHash })
if (invoiceSettled instanceof Error) return invoiceSettled

const isDeleted = await walletInvoicesRepo.deleteByPaymentHash(paymentHash)
if (isDeleted instanceof Error) {
pendingInvoiceLogger.error("impossible to delete WalletInvoice entry")
const processingCompletedInvoice =
await walletInvoicesRepo.markAsProcessingCompleted(paymentHash)
if (processingCompletedInvoice instanceof Error) {
pendingInvoiceLogger.error("Unable to mark invoice as processingCompleted")
}

return true
Expand Down Expand Up @@ -211,10 +213,11 @@ const updatePendingInvoiceBeforeFinally = async ({

const lnInvoiceLookup = await lndService.lookupInvoice({ pubkey, paymentHash })
if (lnInvoiceLookup instanceof InvoiceNotFoundError) {
const isDeleted = await walletInvoicesRepo.deleteByPaymentHash(paymentHash)
if (isDeleted instanceof Error) {
pendingInvoiceLogger.error("impossible to delete WalletInvoice entry")
return isDeleted
const processingCompletedInvoice =
await walletInvoicesRepo.markAsProcessingCompleted(paymentHash)
if (processingCompletedInvoice instanceof Error) {
pendingInvoiceLogger.error("Unable to mark invoice as processingCompleted")
return processingCompletedInvoice
}
return false
}
Expand All @@ -230,6 +233,17 @@ const updatePendingInvoiceBeforeFinally = async ({
return true
}

if (lnInvoiceLookup.isCanceled) {
pendingInvoiceLogger.info("invoice has been canceled")
const processingCompletedInvoice =
await walletInvoicesRepo.markAsProcessingCompleted(paymentHash)
if (processingCompletedInvoice instanceof Error) {
pendingInvoiceLogger.error("Unable to mark invoice as processingCompleted")
return processingCompletedInvoice
}
return false
}

if (!lnInvoiceLookup.isHeld && !lnInvoiceLookup.isSettled) {
pendingInvoiceLogger.info("invoice has not been paid yet")
return false
Expand Down
7 changes: 4 additions & 3 deletions core/api/src/domain/wallet-invoices/index.types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ type WalletInvoiceWithOptionalLnInvoice = {
recipientWalletDescriptor: PartialWalletDescriptor<WalletCurrency>
paid: boolean
createdAt: Date
processingCompleted?: boolean
lnInvoice?: LnInvoice // LnInvoice is optional because some older invoices don't have it
}

Expand Down Expand Up @@ -197,7 +198,7 @@ interface IWalletInvoicesRepository {

yieldPending: () => AsyncGenerator<WalletInvoiceWithOptionalLnInvoice> | RepositoryError

deleteByPaymentHash: (paymentHash: PaymentHash) => Promise<boolean | RepositoryError>

deleteUnpaidOlderThan: (before: Date) => Promise<number | RepositoryError>
markAsProcessingCompleted: (
paymentHash: PaymentHash,
) => Promise<WalletInvoiceWithOptionalLnInvoice | RepositoryError>
}
6 changes: 0 additions & 6 deletions core/api/src/servers/cron.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
} from "@/services/tracing"
import {
deleteExpiredLightningPaymentFlows,
deleteExpiredWalletInvoice,
deleteFailedPaymentsAttemptAllLnds,
updateEscrows,
updateRoutingRevenues,
Expand Down Expand Up @@ -40,10 +39,6 @@ const updateLegacyOnChainReceipt = async () => {
if (txNumber instanceof Error) throw txNumber
}

const deleteExpiredInvoices = async () => {
await deleteExpiredWalletInvoice()
}

const deleteExpiredPaymentFlows = async () => {
await deleteExpiredLightningPaymentFlows()
}
Expand Down Expand Up @@ -85,7 +80,6 @@ const main = async () => {
...(cronConfig.rebalanceEnabled ? [rebalance] : []),
...(cronConfig.swapEnabled ? [swapOutJob] : []),
deleteExpiredPaymentFlows,
deleteExpiredInvoices,
deleteLndPaymentsBefore2Months,
deleteFailedPaymentsAttemptAllLnds,
]
Expand Down
20 changes: 1 addition & 19 deletions core/api/src/services/lnd/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,29 +35,11 @@ import {
updateLndEscrow,
} from "@/services/ledger/admin-legacy"
import { baseLogger } from "@/services/logger"
import { PaymentFlowStateRepository, WalletInvoicesRepository } from "@/services/mongoose"
import { PaymentFlowStateRepository } from "@/services/mongoose"
import { DbMetadata } from "@/services/mongoose/schema"

import { timestampDaysAgo } from "@/utils"

export const deleteExpiredWalletInvoice = async (): Promise<number> => {
const walletInvoicesRepo = WalletInvoicesRepository()

// this should be longer than the invoice validity time
const delta = 90 // days

const date = new Date()
date.setDate(date.getDate() - delta)

const result = await walletInvoicesRepo.deleteUnpaidOlderThan(date)
if (result instanceof Error) {
baseLogger.error({ error: result }, "error deleting expired invoices")
return 0
}

return result
}

export const deleteFailedPaymentsAttemptAllLnds = async () => {
const lnds = offchainLnds
for (const { lnd, socket } of lnds) {
Expand Down
5 changes: 5 additions & 0 deletions core/api/src/services/mongoose/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ const walletInvoiceSchema = new Schema<WalletInvoiceRecord>({
default: false,
},

processingCompleted: {
type: Boolean,
default: false,
},

paymentRequest: {
type: String,
},
Expand Down
1 change: 1 addition & 0 deletions core/api/src/services/mongoose/schema.types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ interface WalletInvoiceRecord {
currency: string
timestamp: Date
selfGenerated: boolean
processingCompleted?: boolean
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need to add it as optional? isnt default value working?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't default value only used on write? I have a database migration that will go back and populate this field and then i will make it non optional.

pubkey: string
paid: boolean
paymentRequest?: string // optional because we historically did not store it
Expand Down
62 changes: 30 additions & 32 deletions core/api/src/services/mongoose/wallet-invoices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,27 @@ export const WalletInvoicesRepository = (): IWalletInvoicesRepository => {
try {
const walletInvoice = await WalletInvoice.findOneAndUpdate(
{ _id: paymentHash },
{ paid: true },
{ paid: true, processingCompleted: true },
{
new: true,
},
)
if (!walletInvoice) {
return new CouldNotFindWalletInvoiceError()
}
return walletInvoiceFromRaw(walletInvoice)
} catch (err) {
return parseRepositoryError(err)
}
}

const markAsProcessingCompleted = async (
paymentHash: PaymentHash,
): Promise<WalletInvoiceWithOptionalLnInvoice | RepositoryError> => {
try {
const walletInvoice = await WalletInvoice.findOneAndUpdate(
{ _id: paymentHash },
{ processingCompleted: true },
{
new: true,
},
Expand Down Expand Up @@ -98,7 +118,13 @@ export const WalletInvoicesRepository = (): IWalletInvoicesRepository => {
| RepositoryError {
let pending
try {
pending = WalletInvoice.find({ paid: false }).cursor({
pending = WalletInvoice.find({
paid: false,
$or: [
{ processingCompleted: false },
{ processingCompleted: { $exists: false } }, // TODO remove this after migration
],
}).cursor({
batchSize: 100,
})
} catch (error) {
Expand All @@ -110,34 +136,6 @@ export const WalletInvoicesRepository = (): IWalletInvoicesRepository => {
}
}

const deleteByPaymentHash = async (
paymentHash: PaymentHash,
): Promise<boolean | RepositoryError> => {
try {
const result = await WalletInvoice.deleteOne({ _id: paymentHash })
if (result.deletedCount === 0) {
return new CouldNotFindWalletInvoiceError(paymentHash)
}
return true
} catch (error) {
return new UnknownRepositoryError(error)
}
}

const deleteUnpaidOlderThan = async (
before: Date,
): Promise<number | RepositoryError> => {
try {
const result = await WalletInvoice.deleteMany({
timestamp: { $lt: before },
paid: false,
})
return result.deletedCount
} catch (error) {
return new UnknownRepositoryError(error)
}
}

const findInvoicesForWallets = async ({
walletIds,
paginationArgs,
Expand Down Expand Up @@ -251,11 +249,10 @@ export const WalletInvoicesRepository = (): IWalletInvoicesRepository => {
return {
persistNew,
markAsPaid,
markAsProcessingCompleted,
findByPaymentHash,
findForWalletByPaymentHash,
yieldPending,
deleteByPaymentHash,
deleteUnpaidOlderThan,
findInvoicesForWallets,
}
}
Expand All @@ -281,6 +278,7 @@ const walletInvoiceFromRaw = (
paid: result.paid as boolean,
usdAmount: result.cents ? UsdPaymentAmount(BigInt(result.cents)) : undefined,
createdAt: new Date(result.timestamp.getTime()),
processingCompleted: result.processingCompleted,
lnInvoice,
}
}
Expand Down
1 change: 1 addition & 0 deletions core/api/test/helpers/wallet-invoices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,6 @@ export const createMockWalletInvoice = (recipientWalletDescriptor: {
"lnbc1pjjahwgpp5zzh9s6tkhpk7heu8jt4l7keuzg7v046p0lzx2hvy3jf6a56w50nqdp82pshjgr5dusyymrfde4jq4mpd3kx2apq24ek2uscqzpuxqyz5vqsp5vl4zmuvhl8rzy4rmq0g3j28060pv3gqp22rh8l7u45xwyu27928q9qyyssqn9drylhlth9ee320e4ahz52y9rklujqgw0kj9ce2gcmltqk6uuay5yv8vgks0y5tggndv0kek2m2n02lf43znx50237mglxsfw4au2cqqr6qax",
) as LnInvoice, // Use a real invoice to test decoding
createdAt: new Date(),
processingCompleted: false,
}
}
21 changes: 2 additions & 19 deletions core/api/test/legacy-integration/services/lnd/utils.spec.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { MS_PER_DAY, ONE_DAY } from "@/config"
import { deleteExpiredWalletInvoice, updateRoutingRevenues } from "@/services/lnd/utils"
import { updateRoutingRevenues } from "@/services/lnd/utils"
import { baseLogger } from "@/services/logger"
import { ledgerAdmin } from "@/services/mongodb"
import { DbMetadata, WalletInvoice } from "@/services/mongoose/schema"
import { DbMetadata } from "@/services/mongoose/schema"

import { sleep, timestampDaysAgo } from "@/utils"

Expand Down Expand Up @@ -129,21 +129,4 @@ describe("lndUtils", () => {

expect((endBalance - initBalance) * 1000).toBeCloseTo(totalFees, 0)
})

it("deletes expired WalletInvoice without throw an exception", async () => {
const delta = 90 // days
const mockDate = new Date()
mockDate.setDate(mockDate.getDate() + delta)
jest.spyOn(global.Date, "now").mockImplementation(() => new Date(mockDate).valueOf())

const queryDate = new Date()
queryDate.setDate(queryDate.getDate() - delta)

const invoicesCount = await WalletInvoice.countDocuments({
timestamp: { $lt: queryDate },
paid: false,
})
const result = await deleteExpiredWalletInvoice()
expect(result).toBe(invoicesCount)
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe("WalletInvoices", () => {
expect(lookedUpInvoice).toEqual(invoiceToPersist)
})

it("updates an invoice", async () => {
it("marks an invoice as paid", async () => {
const repo = WalletInvoicesRepository()
const invoiceToPersist = createMockWalletInvoice(walletDescriptor)
const persistResult = await repo.persistNew(invoiceToPersist)
Expand All @@ -46,23 +46,33 @@ describe("WalletInvoices", () => {
const updatedResult = await repo.markAsPaid(invoiceToUpdate.paymentHash)
expect(updatedResult).not.toBeInstanceOf(Error)
expect(updatedResult).toHaveProperty("paid", true)
expect(updatedResult).toHaveProperty("processingCompleted", true)

const { paymentHash } = updatedResult as WalletInvoice
const lookedUpInvoice = await repo.findByPaymentHash(paymentHash)
expect(lookedUpInvoice).not.toBeInstanceOf(Error)
expect(lookedUpInvoice).toEqual(updatedResult)
expect(lookedUpInvoice).toHaveProperty("paid", true)
expect(updatedResult).toHaveProperty("processingCompleted", true)
})

it("deletes an invoice by hash", async () => {
it("marks an invoice as processing completed", async () => {
const repo = WalletInvoicesRepository()
const invoiceToPersist = createMockWalletInvoice(walletDescriptor)
const persistResult = await repo.persistNew(invoiceToPersist)
expect(persistResult).not.toBeInstanceOf(Error)

const { paymentHash } = persistResult as WalletInvoice
const isDeleted = await repo.deleteByPaymentHash(paymentHash)
expect(isDeleted).not.toBeInstanceOf(Error)
expect(isDeleted).toEqual(true)
const invoiceToUpdate = persistResult as WalletInvoice
const updatedResult = await repo.markAsProcessingCompleted(
invoiceToUpdate.paymentHash,
)
expect(updatedResult).not.toBeInstanceOf(Error)
expect(updatedResult).toHaveProperty("processingCompleted", true)

const { paymentHash } = updatedResult as WalletInvoice
const lookedUpInvoice = await repo.findByPaymentHash(paymentHash)
expect(lookedUpInvoice).not.toBeInstanceOf(Error)
expect(lookedUpInvoice).toEqual(updatedResult)
expect(lookedUpInvoice).toHaveProperty("processingCompleted", true)
})
})
Loading