Item creation with JIT invoices fails with TypeError

[ekzyis]

Description

JIT invoices for item creation currently fail on c267bf9 because serializeInvoiceable returns an empty array if a JIT invoice was used (hash provided).

Steps to Reproduce

1. Post something without enough sats
2. Pay invoice

HODL invoice gets settled because we immediately settle it after the db transaction.

However, the action still fails because of an TypeError:

Field Mutation.upsertComment failed with TypeError: Cannot read properties of undefined (reading 'text')

Expected behavior

JIT invoices work for posting content + HODL invoices don't get settled if action fails

Video

video

2024-01-10.23-22-58.mp4

Additional context

Bug is triggered by results = results.slice(1,-1) since results only contains two elements (see video) and thus the end index of -1 points to the same start index of 1:

stacker.news/api/resolvers/serial.js

Lines 85 to 91 in c267bf9

	let results = await serialize(models, ...trx)
	if (hash) {
	if (invoice?.isHeld) await settleHodlInvoice({ secret: invoice.preimage, lnd })
	results = results.slice(1, -1)
	}

$ node
Welcome to Node.js v21.4.0.
Type ".help" for more information.
> let x = [1,2]
undefined
> x.slice(1,-1)
[]
> let y = [1,2,3]
undefined
> y.slice(1,-1)
[ 2 ]

I wonder if we can simply use results[1] instead of results.slice(1,-1). But not sure yet, don't want to break anything else. Okay, I guess the intention with this code was to simply remove the first element and don't care about the size of the array. So results[1] isn't the same. But something like results.length > 2 ? results.slice(1,-1) : [results[1]] would be. (jeez, is this ugly code but I started with this mess, lol)

I also wonder where the 1 as the first element of the returned array of serialize comes from (see video). confirm_invoice returns 0, not 1.

[ekzyis]

Yep, this fixes it:

diff --git a/api/resolvers/serial.js b/api/resolvers/serial.js
index e160f32..d00b2b5 100644
--- a/api/resolvers/serial.js
+++ b/api/resolvers/serial.js
@@ -86,7 +86,7 @@ export async function serializeInvoicable (query, { models, lnd, hash, hmac, me,

   if (hash) {
     if (invoice?.isHeld) await settleHodlInvoice({ secret: invoice.preimage, lnd })
-    results = results.slice(1, -1)
+    results = results.length > 2 ? results.slice(1, -1) : [results[1]]
   }

   // if there is only one result, return it directly, else the array

But damn, all these array length checks are ugly, lol. Is there no better way?

(I know I started with this when I implemented JIT invoicing here)

Still wondering where the 1 comes from though.