Add On-Chain Send/Receive to NWC

[barrydeen]

Introduces new chain methods:

• pay_chain
• make_chain_address
• list_utxos
• sign_psbt
• list_chain_transactions
• get_chain_balance

[benthecarman]

A sign psbt and list utxos cmd would make sense as well.

Also the balance command might want to include an onchain and unconfirmed balance now as well

[barrydeen]

A sign psbt and list utxos cmd would make sense as well.

Also the balance command might want to include an onchain and unconfirmed balance now as well

Agreed on all fronts.

[barrydeen]

@benthecarman

would you do sign psbt like this?

sign_psbt

Request:

{
  "method": "sign_psbt",
  "params": {
    "psbt": "string" // base64 encoded psbt
  }
}

Response:

{
  "result_type": "sign_psbt",
  "result": {
    "psbt": "string" // base64 encoded signed psbt
  }
}

[benthecarman]

Yeah, could add an optional input index too

[supertestnet]

ACK

[vitorpamplona]

Very cool!

But please don't change formatting with new ideas.

[barrydeen]

Very cool!

But please don't change formatting with new ideas.

Ah yes. Fixed.

[barrydeen]

One thing I'm a bit unsure about is if chain amounts should be expressed as msats or sats. For now an msat can never settle on chain, but if you're building an app or wallet service I bet it will be extremely annoying to treat the amounts differently

[janniks]

For signPsbt maybe take a look at Unisats wallet API. I believe it's the best one out there that's being used. Leather wallet has done more research in terms of what can be done directly in PSBT info (fingerprints encoding which input should be signed), so technically a lot of the API isn't needed, but developers like the convenience.

We iterated and improved the available options to this signPsbt spec for WBIPs. And have 4 wallets using the standard or changing to it.

[kumulynja]

Would it make sense to add an ALREADY_PAID error and recommend wallets to not pay the same amount to the same address twice?

This to reduce the risk of making the same payment twice in case the client for whatever reason didn't receive the response and retries a request? Or should this just be kept track of and be handled by any wallet how it seems fit and not really be mentioned in the specs?

[barrydeen]

For signPsbt maybe take a look at Unisats wallet API. I believe it's the best one out there that's being used. Leather wallet has done more research in terms of what can be done directly in PSBT info (fingerprints encoding which input should be signed), so technically a lot of the API isn't needed, but developers like the convenience.

We iterated and improved the available options to this signPsbt spec for WBIPs. And have 4 wallets using the standard or changing to it.

If you specify the input index, wouldn't the wallet service already know the pubkey and address?

[barrydeen]

Would it make sense to add an ALREADY_PAID error and recommend wallets to not pay the same amount to the same address twice?

This to reduce the risk of making the same payment twice in case the client for whatever reason didn't receive the response and retries a request? Or should this just be kept track of and be handled by any wallet how it seems fit and not really be mentioned in the specs?

Lightning nodes already reply with an error if you try to pay the same payment request twice, I don't see how NWC would open up a new opportunity for double payments. You can put a failure reason in the message of the response already.

[kumulynja]

Would it make sense to add an ALREADY_PAID error and recommend wallets to not pay the same amount to the same address twice? This to reduce the risk of making the same payment twice in case the client for whatever reason didn't receive the response and retries a request? Or should this just be kept track of and be handled by any wallet how it seems fit and not really be mentioned in the specs?

Lightning nodes already reply with an error if you try to pay the same payment request twice, I don't see how NWC would open up a new opportunity for double payments. You can put a failure reason in the message of the response already.

Lightning nodes with bolt11 invoices indeed do, but when adding onchain payments, they can be made twice to the same address. But I agree that it can be put in a failure reason and just use an internal or other error...

[supertestnet]

I agree that it can be put in a failure reason and just use an internal or other error...

Still, an error message should be specified so that implementations don't diverge on this point. Also, I think there should be an override option. A user might want to send the same amount to the same address twice. E.g. what if it's a subscription service? Or what if a user decided to send 10k sats to an exchange once per week and sell it as a kind of "allowance"? If that exchange has a static deposit address, as some do, then if the wallet says "you're not supposed to send the same amount to the same address twice," the user should be able to override that and say "do it anyway."

[benthecarman]

Would it make sense to add an ALREADY_PAID error and recommend wallets to not pay the same amount to the same address twice?

This to reduce the risk of making the same payment twice in case the client for whatever reason didn't receive the response and retries a request? Or should this just be kept track of and be handled by any wallet how it seems fit and not really be mentioned in the specs?

I don't love this, you can already have idempotentcy from the event id, adding it redundantly like this just inhibits use cases and can be confusing

[kumulynja]

Would it make sense to add an ALREADY_PAID error and recommend wallets to not pay the same amount to the same address twice? This to reduce the risk of making the same payment twice in case the client for whatever reason didn't receive the response and retries a request? Or should this just be kept track of and be handled by any wallet how it seems fit and not really be mentioned in the specs?

I don't love this, you can already have idempotentcy from the event id, adding it redundantly like this just inhibits use cases and can be confusing

Totally agree, and that was where I was going to go, should we add a recommendation to the specs now that 'pay' retries should be done with the same event/event id? Because I can imagine naive client implementations may have their retry logic generate a completely new event...

[kumulynja]

I agree that it can be put in a failure reason and just use an internal or other error...

Still, an error message should be specified so that implementations don't diverge on this point. Also, I think there should be an override option. A user might want to send the same amount to the same address twice. E.g. what if it's a subscription service? Or what if a user decided to send 10k sats to an exchange once per week and sell it as a kind of "allowance"? If that exchange has a static deposit address, as some do, then if the wallet says "you're not supposed to send the same amount to the same address twice," the user should be able to override that and say "do it anyway."

Yes, I agree that should all be possible. I think the most straightforward way to handle it and permit those use cases is as @benthecarman mentions, wallet services should keep track of already processed events and not process them twice. Clients that have some retry mechanisms, where they try to get the same payment done in a short amount of time, should reuse the same event.

My worry is just that both wallet services as clients may not think about this initially, especially since the "risk" of accidentally making the same payment twice didn't exist with lightning invoices only. Since with on-chain payments it does exist, I thought we could at least add a recommendation on what to check in a wallet service and how best to retry (on-chain) payments from the client side to reduce this risk.

[benthecarman]

Yeah I think putting some text about the potential issue in the nip is fine. But shaping the protocol around that doesn't make sense.

[lnbc1QWFyb24]

Stoked to see this @barrydeen. We will add this to the Clams Accounting app. A couple of thoughts / questions:

• For the list_transactions method, would it be more robust for each transaction object to have a spentUtxos and receivedUtxos field which lists the spent inputs and or received outputs for the wallet to be more explicit about the wallet balance changes? I think a single address and amount for a onchain transaction could be ambiguous and or limiting since there can be multiple inputs and multiple outputs per transaction compared to a Lightning invoice which can only be a simple receive or send. Defining the spent inputs and received outputs would also remove the need for the type parameter since the direction of the payment can be derived from the inputs and outputs. spentUtxos and receivedUtxos could be an array of utxo objects: [{ txid: "<TXID>", vout: u16, address: "<ADDRESS>", amount: u64 }] where the array is just empty when needed.

• Will the list_utxos method return historically owned (spent) utxos as well as current unspent? Maybe a spent boolean field could be added to the utxo object?

[barrydeen]

Sorry for late reply, you raise a lot of good points that make sense. I'll make some changes to the PR soon to reflect. Only thing is I think it makes sense for these output lists to be in the list_chain_transactions method and not mash it with the lightning txns

[lnbc1QWFyb24]

Oh yes, my bad, list_chain_transactions is what I meant 👍

[stackingsaunter]

Just highlighting I was on spaces with Cove wallet creator that was looking a way to connect a node to his mobile wallet and mentioned this may work with NWC. He's main request was "getting txn information for addresses"

[benthecarman]

Just highlighting I was on spaces with Cove wallet creator that was looking a way to connect a node to his mobile wallet and mentioned this may work with NWC. He's main request was "getting txn information for addresses"

That is completely separate and should be separate from nwc

[f321x]

A method to get the current mempool state might be useful to have in this context, so the client can get fee estimates from its own backend and doesn't have to rely on other sources: e.g. get_mempool_info

{
    "method": "get_mempool_info",
    "params": {}
}

Response:

{
    "result_type": "get_mempool_info",
    "result": {
        "estimates": {    # blocks -> sat/vb
          "1": "24",
          "2": "20",
          "5": "18",
          "10": "10",
          "25": "5",
          "144": "2",
          "1008": "1",
      }
    }
}

the backend should be free to specify the available block values, the client shouldn't rely on these specific values. or some NO_FEE_ESTIMATES_AVAILABLE_ERROR in case no single value is available.

Edit: I guess estimates could as well be an optional field of the existing get_info request

[barrydeen]

Gentleman, major update, look forward to your comments:

• changed pay_chain_address to just pay_chain given that we can do multiple recipients
• list_chain_transactions now shows information about the spent and received utxos
• added feerate param to pay_chain

[barrydeen]

A method to get the current mempool state might be useful to have in this context, so the client can get fee estimates from its own backend and doesn't have to rely on other sources: e.g. get_mempool_info

{
    "method": "get_mempool_info",
    "params": {}
}

Response:

{
    "result_type": "get_mempool_info",
    "result": {
        "estimates": {    # blocks -> sat/vb
          "1": "24",
          "2": "20",
          "5": "18",
          "10": "10",
          "25": "5",
          "144": "2",
          "1008": "1",
      }
    }
}

the backend should be free to specify the available block values, the client shouldn't rely on these specific values. or some NO_FEE_ESTIMATES_AVAILABLE_ERROR in case no single value is available.

i just wonder how far we go down this route, in theory why not just expose the entire bitcoin rpc to nwc methods? (maybe we should!) - I don't disagree with your logic that it would make life easier for a client, I'm just not sure.

[vitorpamplona]

How is this going? Have any implementations supported it?

[lnbc1QWFyb24]

LGTM, nice work @barrydeen