Skip to content

Commit

Permalink
fix: add user memo to ln send metadata (#2311)
Browse files Browse the repository at this point in the history
* test: remove unnecessary imbalanceCalc

* test: add new test for memo below threshold on external send

* refactor: add 'memoPayer' to send metadata types

* chore: add 'memoOfPayer' property to ln/onchain metadata constructors

* chore: pass user memo to ln/onchain metadata in use-cases
  • Loading branch information
vindard authored Feb 9, 2023
1 parent 1586c5a commit be285e5
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 21 deletions.
10 changes: 7 additions & 3 deletions src/app/payments/send-lightning.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export const payInvoiceByWalletId = async ({
senderUsername: senderAccount.username,
memo,
})
: executePaymentViaLn({ decodedInvoice, paymentFlow, senderWallet })
: executePaymentViaLn({ decodedInvoice, paymentFlow, senderWallet, memo })
}

const payNoAmountInvoiceByWalletId = async ({
Expand Down Expand Up @@ -130,7 +130,7 @@ const payNoAmountInvoiceByWalletId = async ({
senderUsername: senderAccount.username,
memo,
})
: executePaymentViaLn({ decodedInvoice, paymentFlow, senderWallet })
: executePaymentViaLn({ decodedInvoice, paymentFlow, senderWallet, memo })
}

export const payNoAmountInvoiceByWalletIdForBtcWallet = async (
Expand Down Expand Up @@ -478,10 +478,12 @@ const executePaymentViaLn = async ({
decodedInvoice,
paymentFlow,
senderWallet,
memo,
}: {
decodedInvoice: LnInvoice
paymentFlow: PaymentFlow<WalletCurrency, WalletCurrency>
senderWallet: Wallet
memo: string | null
}): Promise<PaymentSendStatus | ApplicationError> => {
addAttributesToCurrentSpan({
"payment.settlement_method": SettlementMethod.Lightning,
Expand Down Expand Up @@ -540,10 +542,12 @@ const executePaymentViaLn = async ({
pubkey: outgoingNodePubkey || lndService.defaultPubkey(),
paymentHash,
feeKnownInAdvance: !!rawRoute,

memoOfPayer: memo || paymentFlow.descriptionFromInvoice,
})

const journal = await LedgerFacade.recordSend({
description: paymentFlow.descriptionFromInvoice,
description: paymentFlow.descriptionFromInvoice || memo || "",
amountToDebitSender: {
btc: {
currency: paymentFlow.btcPaymentAmount.currency,
Expand Down
1 change: 1 addition & 0 deletions src/app/wallets/send-on-chain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,7 @@ const executePaymentViaOnChain = async <

payeeAddresses: [paymentFlow.address],
sendAll,
memoOfPayer: memo || undefined,
})

// Record transaction
Expand Down
22 changes: 14 additions & 8 deletions src/services/ledger/index.types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ type LedgerMetadata = {
pending: boolean
}

type LedgerSendMetadata = {
memoPayer?: string
}

type LnReceiveLedgerMetadata = LedgerMetadata &
SendAmountsMetadata & {
hash: PaymentHash
Expand All @@ -72,13 +76,15 @@ type SendAmountsMetadata = {
}

type AddLnSendLedgerMetadata = LedgerMetadata &
LedgerSendMetadata &
SendAmountsMetadata & {
hash: PaymentHash
pubkey: Pubkey
feeKnownInAdvance: boolean
}

type AddOnchainSendLedgerMetadata = LedgerMetadata &
LedgerSendMetadata &
SendAmountsMetadata & {
hash: OnChainTxHash
payee_addresses: OnChainAddress[]
Expand All @@ -101,13 +107,13 @@ type AddColdStorageReceiveLedgerMetadata = AddColdStorageLedgerMetadata

type AddColdStorageSendLedgerMetadata = AddColdStorageLedgerMetadata

type IntraledgerBaseMetadata = LedgerMetadata & {
usd: DisplayCurrencyBaseAmount // to be renamed amountDisplayCurrency
memoPayer?: string
username?: Username
}
type IntraledgerSendBaseMetadata = LedgerMetadata &
LedgerSendMetadata & {
usd: DisplayCurrencyBaseAmount // to be renamed amountDisplayCurrency
username?: Username
}

type AddLnIntraledgerSendLedgerMetadata = IntraledgerBaseMetadata &
type AddLnIntraledgerSendLedgerMetadata = IntraledgerSendBaseMetadata &
SendAmountsMetadata & {
hash: PaymentHash
pubkey: Pubkey
Expand All @@ -118,7 +124,7 @@ type AddLnTradeIntraAccountLedgerMetadata = Omit<
"username"
>

type AddOnChainIntraledgerSendLedgerMetadata = IntraledgerBaseMetadata &
type AddOnChainIntraledgerSendLedgerMetadata = IntraledgerSendBaseMetadata &
SendAmountsMetadata & {
payee_addresses: OnChainAddress[]
sendAll: boolean
Expand All @@ -129,7 +135,7 @@ type AddOnChainTradeIntraAccountLedgerMetadata = Omit<
"username"
>

type AddWalletIdIntraledgerSendLedgerMetadata = IntraledgerBaseMetadata &
type AddWalletIdIntraledgerSendLedgerMetadata = IntraledgerSendBaseMetadata &
SendAmountsMetadata

type AddWalletIdTradeIntraAccountLedgerMetadata = Omit<
Expand Down
6 changes: 6 additions & 0 deletions src/services/ledger/tx-metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export const LnSendLedgerMetadata = ({
amountDisplayCurrency,
displayCurrency,
feeKnownInAdvance,
memoOfPayer,
}: {
paymentHash: PaymentHash
pubkey: Pubkey
Expand All @@ -20,6 +21,7 @@ export const LnSendLedgerMetadata = ({
displayCurrency: DisplayCurrency

feeKnownInAdvance: boolean
memoOfPayer?: string
}) => {
const {
btcPaymentAmount: { amount: satsAmount },
Expand All @@ -34,6 +36,7 @@ export const LnSendLedgerMetadata = ({
hash: paymentHash,
pubkey,
feeKnownInAdvance,
memoPayer: memoOfPayer,

satsFee: toSats(satsFee),
displayFee: feeDisplayCurrency,
Expand All @@ -55,6 +58,7 @@ export const OnChainSendLedgerMetadata = ({
displayCurrency,
payeeAddresses,
sendAll,
memoOfPayer,
}: {
onChainTxHash: OnChainTxHash
paymentAmounts: AmountsAndFees
Expand All @@ -65,6 +69,7 @@ export const OnChainSendLedgerMetadata = ({

payeeAddresses: OnChainAddress[]
sendAll: boolean
memoOfPayer?: string
}) => {
const {
btcPaymentAmount: { amount: satsAmount },
Expand All @@ -79,6 +84,7 @@ export const OnChainSendLedgerMetadata = ({
hash: onChainTxHash,
payee_addresses: payeeAddresses,
sendAll,
memoPayer: memoOfPayer,

satsFee: toSats(satsFee),
displayFee: feeDisplayCurrency,
Expand Down
52 changes: 42 additions & 10 deletions test/integration/02-user-wallet/02-send-lightning.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -620,16 +620,6 @@ describe("UserWallet - Lightning Pay", () => {
})

it("pay zero amount invoice with amount less than 1 cent", async () => {
const imbalanceCalc = ImbalanceCalculator({
method: WithdrawalFeePriceMethod.proportionalOnImbalance,
sinceDaysAgo: ONE_DAY,
volumeLightningFn: LedgerService().lightningTxBaseVolumeSince,
volumeOnChainFn: LedgerService().onChainTxBaseVolumeSince,
})

const imbalanceInit = await imbalanceCalc.getSwapOutImbalanceAmount(walletDescriptorB)
if (imbalanceInit instanceof Error) throw imbalanceInit

const { request } = await createInvoice({ lnd: lndOutside1 })

// Test payment is successful
Expand All @@ -644,6 +634,48 @@ describe("UserWallet - Lightning Pay", () => {
expect(paymentResult).toBe(PaymentSendStatus.Success)
})

it("does not filter spam message for external send", async () => {
const amount = toSats(1)
const memoSpamBelowThreshold = "Memo BELOW spam threshold"
const memoOnInvoice = `${memoSpamBelowThreshold} -- from payment request`
const memoFromUser = `${memoSpamBelowThreshold} -- from user`

const { request } = await createInvoice({
lnd: lndOutside1,
description: memoOnInvoice,
})

// Test probe + payment is successful
const { result: fee, error } =
await Payments.getNoAmountLightningFeeEstimationForBtcWallet({
walletId: walletIdB,
uncheckedPaymentRequest: request,
amount,
})
if (error instanceof Error) throw error
expect(fee).not.toBeNull()

const paymentResult = await Payments.payNoAmountInvoiceByWalletIdForBtcWallet({
uncheckedPaymentRequest: request,
memo: memoFromUser,
amount,
senderWalletId: walletIdB,
senderAccount: accountB,
})
if (paymentResult instanceof Error) throw paymentResult
expect(paymentResult).toBe(PaymentSendStatus.Success)

// Check memo on txns
const txResult = await getTransactionsForWalletId(walletIdB)
if (txResult.error instanceof Error || txResult.result === null) {
throw txResult.error
}
const txns = txResult.result.slice
expect(txns.length).toBeGreaterThan(0)
const txn = txns[0]
expect(txn.memo).toBe(memoFromUser)
})

it("filters spam from send to another Galoy user as push payment", async () => {
// TODO: good candidate for a unit test?

Expand Down

0 comments on commit be285e5

Please sign in to comment.