thesimplekid / cln-nostr-wallet-connect Goto Github PK

Core lighting plugin for NIP47 nostr connect support

License: BSD 3-Clause "New" or "Revised" License

Nix 1.92% Rust 98.08%

cln-nostr-wallet-connect's Introduction

Core Lightning plugin for nostr wallet connect

Warning This is an ALPHA implementation NIP47, that has both privacy implications and risk of funds. You should not run this unless you understand those risks and ideally reviewed the code.

You can add the plugin by copying it to CLN's plugin directory or by adding the following line to your config file:

plugin=/path/to/nostr-wallet-connect

CLN Config

Since zap invoices include only description hash and the description is not currently included in nip47 allow-deprecated-apis=true must be set.

Options

cln-nostr-wallet-connect exposes the following config options that can be included in CLN's config file or as command line flags:

nostr_connect_wallet_nsec: Nostr Key to publish events, optional: will auto generate if not defined
nostr_connect_client_secret: This should not be your main secret key, you should create a new keypair as securely as possible as it can authorize spending funds. optional
nostr_connect_relay: Nostr relay to connect to. You should use a private relay with auth to reduce leaking metadata
nostr_connect_max_invoice: Max amount in msats of an invoice to pay. Defaults to 5000000
nostr_connect_hour_limit: Max msat to spend per hour. Defaults to 10,000,000.
nostr_connect_day_limit: Max msat to spend per day. Defaults to 35000000
nostr_connect_config_path: Path to configuration file. Defaults to data directory

License

Code is under the BSD 3-Clause License

Contribution

All contributions welcome.

Unless you explicitly state otherwise, any contribution intentionally submitted for inclusion in the work by you, shall be licensed as above, without any additional terms or conditions.

Contact

I can be contacted for comments or questions on nostr at [email protected] (npub1qjgcmlpkeyl8mdkvp4s0xls4ytcux6my606tgfx9xttut907h0zs76lgjw) or via email [email protected].

Credit

https://github.com/elsirion/clnurl

https://github.com/jb55/cln-nostr-zapper

https://github.com/0xtrr/nostr-tool

cln-nostr-wallet-connect's People

Contributors

Stargazers

Watchers

Forkers

22388o

cln-nostr-wallet-connect's Issues

Always returns failed yet still pays invoice

Seeing this on Damus... started happening recently but wasn't yesterday... wtf?

Add test for authoritzation and limits

The more tests the better of course, but this like the most security critical part:

cln-nostr-wallet-connect/src/main.rs

Lines 239 to 285 in 6450ad9

 // Check event is valid 

 if event.verify().is_err() { 

 info!("Event {} is invalid", event.id.to_hex()); 

 continue; 

 } 

 // info!("Got event: {:?}", serde_json::to_string_pretty(&event)); 

 // Check event is from correct pubkey 

 if event.pubkey.ne(&connect_client_keys.public_key()) { 

 // TODO: Should respond with unauth 

 info!("Event from incorrect pubkey: {}", event.pubkey.to_string()); 

 continue; 

 } 

 // Decrypt bolt11 from content (NIP04) 

 let content = match decrypt( 

 &keys.secret_key()?, 

 &connect_client_keys.public_key(), 

 &event.content, 

 ) { 

 Ok(content) => content, 

 Err(err) => { 

 info!("Could not decrypt: {err}"); 

 continue; 

 } 

 }; 

 // info!("Decrypted Content: {:?}", content); 

 let request = match nip47::Request::from_json(&content) { 

 Ok(req) => req, 

 Err(err) => { 

 warn!("Could not decode request {:?}", err); 

 continue; 

 } 

 }; 

 // info!("request: {:?}", request); 

 let event_builder = match request.params { 

 nip47::RequestParams::PayInvoice(pay_invoice_param) => { 

 handle_pay_invoice( 

 &event, 

 &keys, 

 pay_invoice_param, 

 cln_client.clone(), 

 limits.clone(),

Maybe these lines can go in a helper function and then the tests can check that it returns true / false depending on some test vectors?

panic when relay gets disconnected

thread 'thread 'thread 'mainmainmain' panicked at '' panicked at '' panicked at 'Error reading message: Protocol(ResetWithoutClosingHandshake)Error
 reading message: Protocol(ResetWithoutClosingHandshake)Error reading message: Protocol(ResetWithoutClosingHandshake)', thread '', ', mainsrc/main.
rssrc/main.rs' panicked at 'Error reading message: Protocol(ResetWithoutClosingHandshake)', src/main.rs:386:::386386::18src/main.rs18
thread '18                                                                                                                                         
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace                                                        
:386:                                                                                                                                              
18                                                                                                                                                 
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace                                                        
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace                                                                
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace                                                                      main' panicked at 'Error reading message: Protocol(ResetWithoutClosingHandshake)', src/main.rs:386:18
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace                                                                
2023-05-16T18:07:00.888Z INFO    plugin-cln-nostr-wallet-connect: Disconnected from ws://127.0.0.1:8080/                                           thread 'main' panicked at 'Error reading message: Protocol(ResetWithoutClosingHandshake)', src/main.rs:424:18
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace                                                        
thread 'thread 'thread 'mainmainmain' panicked at '' panicked at '' panicked at 'Error reading message: Protocol(ResetWithoutClosingHandshake)Error
 reading message: Protocol(ResetWithoutClosingHandshake)Error reading message: Protocol(ResetWithoutClosingHandshake)', ', ', src/main.rssrc/main.r
ssrc/main.rs:::376376376:::1818                                                                                                                    
18                                                                                                                                                 
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace                                                        
                                                                                                                                                   
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Allow multiple relays

cln-nostr-wallet-connect/src/main.rs

Lines 59 to 63 in 6450ad9

 // TODO: Would be better to be a list 

 .option(ConfigOption::new( 

 RELAY, 

 Value::String("ws://localhost:8080".to_string()), 

 "Default nostr relay",

As much as I agree having a private relay is probably a good idea, it seems orthogonal to this plugin.

Kill msat?

I find msat amounts confusing and useless for the purpose of a maximum budget. They just make it easier to add a few too many zeros.

Changing to sat would be a breaking change though, maybe not worth it if too many people already use the plugin.

"why is one msat?"

I noticed this comment:

cln-nostr-wallet-connect/src/main.rs

Lines 208 to 213 in 6450ad9

 // REVIEW: why is one msat? 

 let limits = Arc::new(Mutex::new(Limits::new( 

 Amount::from_sat(max_invoice_amount as u64), 

 Amount::from_msat(hour_limit as u64), 

 Amount::from_sat(day_limit as u64), 

 )));

Can you explain what the problem is?

Should I worry? :-)

Respond with unauth

I'm turning some TODO's into Github issues.

This one seems potentially useful for debugging:

cln-nostr-wallet-connect/src/main.rs

Lines 247 to 252 in 6450ad9

 // Check event is from correct pubkey 

 if event.pubkey.ne(&connect_client_keys.public_key()) { 

 // TODO: Should respond with unauth 

 info!("Event from incorrect pubkey: {}", event.pubkey.to_string()); 

 continue; 

 }

Reconnect to relay

If relay connection fails the plugin has to be restarted to reconnect

Can't connect to my own relay + where does .config disappear to?

Hello!

I wanted to set this up to pay zaps on Snort, however this didn't go very well:

2023-07-27T10:40:28.443Z INFO    plugin-cln-nostr-wallet-connect: Nostr Wallet Connect Config: \"/.config/cln-nostr-wallet-connect/config\"
2023-07-27T10:40:28.444Z INFO    plugin-cln-nostr-wallet-connect: No private key provided, creating new identity
2023-07-27T10:40:28.445Z INFO    plugin-cln-nostr-wallet-connect: nostr+walletconnect://(redacted)
2023-07-27T10:40:28.502Z **BROKEN** plugin-cln-nostr-wallet-connect: Impossible to connect to wss://relay.ingwie.me/: ws error: IO error: received fatal alert: InternalError

This is odd for two reasons:

wss://relay.ingwie.me has a valid certificate and is running. Looking at my logs, I can actually tell that it is quite busy and used by more than myself.
/.config? That path is not writeable by the user running C-Lightning; in fact it should only have access to it's home at /usb/c-lightning.

Can you help me figure this out? I disabled the plugin for now.

Thanks a lot and kind regards,
Ingwie

How to generate nostr_connect_client_secret

The README could a hint on how to generate this.

And what format it should be in. And what it is :-)

Is allow-deprecated-apis=true still needed?

And if so, is there a more narrow config option to enable one specific deprecated API?

Related code:

cln-nostr-wallet-connect/src/main.rs

Lines 457 to 466 in 6450ad9

 // Currently there is some debate over whether the description hash should be known or not before paying. 

 // The change in CLN requiring the description was reverted but it is still deprecated 

 // With NIP47 as of now the description is unknown 

 // This may need to be reviewed in the future 

 // https://github.com/ElementsProject/lightning/pull/6092 

 // https://github.com/ElementsProject/lightning/releases/tag/v23.02.2 

 let _description = match bolt11.description() { 

 Bolt11InvoiceDescription::Direct(des) => des.to_string(), 

 Bolt11InvoiceDescription::Hash(hash) => hash.0.to_string(), 

 };

Allocate a total budget and use fake balance?

In order to not accidentally leak your lightning node wallet when the nsec is compromised, I think the balance should be the minimum of the real balance and some fake value.

cln-nostr-wallet-connect/src/main.rs

Lines 354 to 361 in 6450ad9

 Ok(Response::ListFunds(funds)) => { 

 let balance: u64 = funds 

 .channels 

 .iter() 

 .map(|c| c.our_amount_msat.msat()) 

 .sum(); 

 let balance = Amount::from_msat(balance);

One approach would be to have a command like lightning-cli nostr-wallet-topup which allocates a specific budget. You'd then have to track receives and spends. This could be a nice additional safety above the hourly and daily limits.

The prevent fractional reserve shenanigans, you'd still want to take the minimum of this value and the real balance.

Update NIP47

nostr-protocol/nips#406

	// Check event is valid
	if event.verify().is_err() {
	info!("Event {} is invalid", event.id.to_hex());
	continue;
	}

	// info!("Got event: {:?}", serde_json::to_string_pretty(&event));

	// Check event is from correct pubkey
	if event.pubkey.ne(&connect_client_keys.public_key()) {
	// TODO: Should respond with unauth
	info!("Event from incorrect pubkey: {}", event.pubkey.to_string());
	continue;
	}

	// Decrypt bolt11 from content (NIP04)
	let content = match decrypt(
	&keys.secret_key()?,
	&connect_client_keys.public_key(),
	&event.content,
	) {
	Ok(content) => content,
	Err(err) => {
	info!("Could not decrypt: {err}");
	continue;
	}
	};

	// info!("Decrypted Content: {:?}", content);

	let request = match nip47::Request::from_json(&content) {
	Ok(req) => req,
	Err(err) => {
	warn!("Could not decode request {:?}", err);
	continue;
	}
	};
	// info!("request: {:?}", request);

	let event_builder = match request.params {
	nip47::RequestParams::PayInvoice(pay_invoice_param) => {
	handle_pay_invoice(
	&event,
	&keys,
	pay_invoice_param,
	cln_client.clone(),
	limits.clone(),

	// TODO: Would be better to be a list
	.option(ConfigOption::new(
	RELAY,
	Value::String("ws://localhost:8080".to_string()),
	"Default nostr relay",

	// REVIEW: why is one msat?
	let limits = Arc::new(Mutex::new(Limits::new(
	Amount::from_sat(max_invoice_amount as u64),
	Amount::from_msat(hour_limit as u64),
	Amount::from_sat(day_limit as u64),
	)));

	// Currently there is some debate over whether the description hash should be known or not before paying.
	// The change in CLN requiring the description was reverted but it is still deprecated
	// With NIP47 as of now the description is unknown
	// This may need to be reviewed in the future
	// https://github.com/ElementsProject/lightning/pull/6092
	// https://github.com/ElementsProject/lightning/releases/tag/v23.02.2
	let _description = match bolt11.description() {
	Bolt11InvoiceDescription::Direct(des) => des.to_string(),
	Bolt11InvoiceDescription::Hash(hash) => hash.0.to_string(),
	};

	Ok(Response::ListFunds(funds)) => {
	let balance: u64 = funds
	.channels
	.iter()
	.map(\|c\| c.our_amount_msat.msat())
	.sum();

	let balance = Amount::from_msat(balance);

thesimplekid / cln-nostr-wallet-connect Goto Github PK

cln-nostr-wallet-connect's Introduction

Core Lightning plugin for nostr wallet connect

CLN Config

Options

License

Contribution

Contact

Credit

cln-nostr-wallet-connect's People

Contributors

Stargazers

Watchers

Forkers

cln-nostr-wallet-connect's Issues

Recommend Projects

Recommend Topics

Recommend Org