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

Feature request: detect onchain QR codes, and allow to spend from the onchain wallet instead #20

Open
darosior opened this issue Jul 11, 2020 · 6 comments
Labels
enhancement New feature or request

Comments

@darosior
Copy link
Contributor

I'd like to switch to Lamp as my main wallet but too many people are still using onchain payments.

Hint: you can use the withdraw RPC command under the hood to spend from lightningd's onchain wallet.

@lvaccaro
Copy link
Collaborator

lvaccaro commented Jul 13, 2020

Concept ack on #22 to implement a bitcoin validation uri/address from scratch (continue the discussion on the MR about improvements and requirements).

Other approach:

  • use lightning-txprepare to make a fake transaction testing the address
  • extend esplora plugin with a validation address command, but this should call bitcoin-cli to work in bitcoin peer node mode.

@vincenzopalazzo
Copy link
Member

Hi @lvaccaro,

Thanks to give me the lock on this feature.

use lightning-txprepare to make a fake transaction testing the address

Concept ack, I agree with this, we can use the node lightning to make a check on this, I can try to implement another draft with this solution.

extend esplora plugin with a validation address command, but this should call bitcoin-cli to work in bitcoin peer node mode.

This is very cool to have, but now I'm asking a question about the electrum server, there is the possibility to validate the address with the server? such as use bitcoin-cli. If yes, we can try to wrapping this idea inside a real solution.

@lvaccaro
Copy link
Collaborator

Maybe @darosior could give us a bit of wisdom about lightning-txprepare and if he think is a good approach.

On the other hand, Esplora has the https://github.com/Blockstream/esplora/blob/master/API.md#get-addressaddress api but you have to send the address to the explorer for validation that it takes time and maybe it is not always the best case. So I don't like so much.

@darosior
Copy link
Contributor Author

Maybe @darosior could give us a bit of wisdom about lightning-txprepare and if he think is a good approach.

Under the hood txprepare and withdraw use almost the same codepath. In addition, withdraw will check the address (in the same way txprepare would btw):

struct command_result *param_bitcoin_address(struct command *cmd,
					     const char *name,
					     const char *buffer,
					     const jsmntok_t *tok,
					     const u8 **scriptpubkey)
{
	/* Parse address. */
	switch (json_to_address_scriptpubkey(cmd,
					     chainparams,
					     buffer, tok,
					     scriptpubkey)) {
	case ADDRESS_PARSE_UNRECOGNIZED:
		return command_fail(cmd, LIGHTNINGD,
				    "Could not parse destination address, "
				    "%s should be a valid address",
				    name ? name : "address field");
	case ADDRESS_PARSE_WRONG_NETWORK:
		return command_fail(cmd, LIGHTNINGD,
				    "Destination address is not on network %s",
				    chainparams->network_name);
	case ADDRESS_PARSE_SUCCESS:
		return NULL;
	}
	abort();
}

Hence I see no additional benefits to use txprepare and I think using withdraw directly is OK.

On the other hand, Esplora has the https://github.com/Blockstream/esplora/blob/master/API.md#get-addressaddress api but you have to send the address to the explorer for validation that it takes time and maybe it is not always the best case. So I don't like so much.

Strongly agree with this, lightningd is already capable to check an address (and the network btw) let's not encumber ourself with YA roundtrip.

@lvaccaro
Copy link
Collaborator

Reading withdraw from https://lightning.readthedocs.io/lightning-withdraw.7.html?highlight=withdraw, it looks the tx is built and broadcasted, if valid.
In QR code scanning, I think we only need to parse the address without broadcast tx, in order to postpone amount selection and user confirmation, or maybe I am missing something.

@darosior
Copy link
Contributor Author

darosior commented Jul 16, 2020 via email

vincenzopalazzo added a commit to vincenzopalazzo/lamp that referenced this issue Jul 18, 2020
This is a draft solution to discuss with developer is the Validator solution is a good solution or I'm losing somethings important.

If the solution, like at developer I will require the lock on the issue.

Commit built by @vincenzopalazzo [email protected]
vincenzopalazzo added a commit to vincenzopalazzo/lamp that referenced this issue Jul 18, 2020
This is a draft solution to discuss with developer is the Validator solution is a good solution or I'm losing somethings important.

If the solution, like at developer I will require the lock on the issue.

Commit built by @vincenzopalazzo [email protected]
vincenzopalazzo added a commit to vincenzopalazzo/lamp that referenced this issue Jul 19, 2020
lvaccaro pushed a commit that referenced this issue Aug 2, 2020
This is a draft solution to discuss with developer is the Validator solution is a good solution or I'm losing somethings important.

If the solution, like at developer I will require the lock on the issue.

Commit built by @vincenzopalazzo [email protected]
@vincenzopalazzo vincenzopalazzo added the enhancement New feature or request label Aug 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants