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

Add CLN send via CLNRest #1981

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
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
38 changes: 38 additions & 0 deletions wallets/cln/client.js
Original file line number Diff line number Diff line change
@@ -1 +1,39 @@
import { fetchWithTimeout } from '@/lib/fetch'
import { assertContentTypeJson, assertResponseOk } from '@/lib/url'

export * from '@/wallets/cln'

export async function testSendPayment ({ socket, runePay }, { logger, signal }) {
// TODO: check rune
Copy link
Member

Choose a reason for hiding this comment

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

Missing TODO.

We usually do "integration tests" here that involve the network unlike the formik/yup validation that triggers no network requests.

So for example, you could try to use the rune in some way that wouldn't work if the rune didn't exist but since we don't actually want to pay something (this was planned in #1341 but I got blocked by an issue), you can't actually use the pay method. Maybe it helps to look at testSendPayment from existing wallets.

}

export async function sendPayment (bolt11, { socket, runePay }, { logger, signal }) {
const url = `https://${socket}/v1/pay`
Copy link
Member

Choose a reason for hiding this comment

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

You can't hardcode the protocol since we allow HTTP over Tor.

This also breaks testing in local dev so I couldn't test this.

So this will definitely require testing with a real CLNRest server.

We have a CLN node in our docker-compose setup that you should be able to test with.


const headers = new Headers()
headers.set('Content-Type', 'application/json')
headers.set('Rune', runePay)
// see nodeId in lib/cln.js
headers.set('nodeId', '02cb2e2d5a6c5b17fa67b1a883e2973c82e328fb9bd08b2b156a9e23820c87a490')

const body = new URLSearchParams()
body.append('bolt11', bolt11)

const res = await fetchWithTimeout(url, {
method: 'POST',
headers,
body,
signal
})

assertResponseOk(res)
assertContentTypeJson(res)

const payment = await res.json()
const preimage = payment.data.payment_preimage
if (!preimage) {
throw new Error(payment.reason)
}

return preimage
}
25 changes: 24 additions & 1 deletion wallets/cln/index.js
Original file line number Diff line number Diff line change
@@ -14,7 +14,6 @@ export const fields = [
placeholder: '55.5.555.55:3010',
hint: 'tor or clearnet',
clear: true,
serverOnly: true,
validate: string().socket()
},
{
@@ -33,10 +32,13 @@ export const fields = [
hint: 'must be restricted to method=invoice',
clear: true,
serverOnly: true,
optional: 'for receiving',
requiredWithout: 'runePay',
validate: string().matches(B64_URL_REGEX, { message: 'invalid rune' })
.test({
name: 'rune',
test: (v, context) => {
if (!v) return true
const decoded = decodeRune(v)
if (!decoded) return context.createError({ message: 'invalid rune' })
if (decoded.restrictions.length === 0) {
@@ -52,6 +54,27 @@ export const fields = [
}
})
},
{
name: 'runePay',
label: 'pay rune',
type: 'text',
placeholder: 'S34KtUW-6gqS_hD_9cc_PNhfF-NinZyBOCgr1aIrark9NCZtZXRob2Q9aW52b2ljZQ==',
hint: 'must allow method=pay',
clear: true,
clientOnly: true,
optional: 'for sending',
requiredWithout: 'rune',
validate: string().matches(B64_URL_REGEX, { message: 'invalid rune' })
.test({
name: 'runePay',
test: (v, context) => {
if (!v) return true
const decoded = decodeRune(v)
if (!decoded) return context.createError({ message: 'invalid rune' })
return true
}
Comment on lines +70 to +75
Copy link
Member

Choose a reason for hiding this comment

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

this doesn't test that the rune can actually pay.

})
},
Comment on lines +57 to +77
Copy link
Member

Choose a reason for hiding this comment

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

Please add help like we have for the invoice only rune.

{
name: 'cert',
label: 'cert',