Skip to content

Commit

Permalink
chore: prevent WalletInvoice deletion (#3557)
Browse files Browse the repository at this point in the history
* chore: prevent WalletInvoice deletion

* chore: apply linting
  • Loading branch information
UncleSamtoshi authored Nov 16, 2023
1 parent 7d624ab commit 6b16663
Show file tree
Hide file tree
Showing 10 changed files with 85 additions and 96 deletions.
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
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)
})
})

0 comments on commit 6b16663

Please sign in to comment.