-
Notifications
You must be signed in to change notification settings - Fork 113
Insert channel funding outputs into Wallet #726
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1458,6 +1458,33 @@ where | |
| counterparty_node_id, | ||
| funding_txo, | ||
| ); | ||
|
|
||
| let chans = | ||
| self.channel_manager.list_channels_with_counterparty(&counterparty_node_id); | ||
| let chan_output = chans | ||
| .iter() | ||
| .find(|c| c.user_channel_id == user_channel_id) | ||
| .and_then(|c| c.get_funding_output()); | ||
| match chan_output { | ||
| None => { | ||
| log_error!( | ||
| self.logger, | ||
| "Failed to find channel info for pending channel {channel_id} with counterparty {counterparty_node_id}" | ||
| ); | ||
| debug_assert!(false, | ||
| "Failed to find channel info for pending channel {channel_id} with counterparty {counterparty_node_id}" | ||
| ); | ||
| }, | ||
| Some(output) => { | ||
| if let Err(e) = self.wallet.insert_txo(funding_txo, output) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmmm... I guess we didn't really need the redeem script in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah sadly I messed up here, i think we needed to add the redeem script along with the new channel size so we had the full utxo information |
||
| log_error!( | ||
| self.logger, | ||
| "Failed to insert funding TXO into wallet: {e}" | ||
| ); | ||
| return Err(ReplayEvent()); | ||
| } | ||
| }, | ||
| } | ||
| } else { | ||
| log_info!( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In what case would we end up in this
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. According to the docs, the |
||
| self.logger, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,7 +20,7 @@ pub use bip39::Mnemonic; | |
| use bitcoin::hashes::sha256::Hash as Sha256; | ||
| use bitcoin::hashes::Hash; | ||
| use bitcoin::secp256k1::PublicKey; | ||
| pub use bitcoin::{Address, BlockHash, FeeRate, Network, OutPoint, Txid}; | ||
| pub use bitcoin::{Address, BlockHash, FeeRate, Network, OutPoint, ScriptBuf, Txid}; | ||
| pub use lightning::chain::channelmonitor::BalanceSource; | ||
| pub use lightning::events::{ClosureReason, PaymentFailureReason}; | ||
| use lightning::ln::channelmanager::PaymentId; | ||
|
|
@@ -106,6 +106,22 @@ impl UniffiCustomTypeConverter for Address { | |
| } | ||
| } | ||
|
|
||
| impl UniffiCustomTypeConverter for ScriptBuf { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: While this should be fine for now, we generally try to get away from the |
||
| type Builtin = String; | ||
|
|
||
| fn into_custom(val: Self::Builtin) -> uniffi::Result<Self> { | ||
| if let Ok(key) = ScriptBuf::from_hex(&val) { | ||
| return Ok(key); | ||
| } | ||
|
|
||
| Err(Error::InvalidScriptPubKey.into()) | ||
| } | ||
|
|
||
| fn from_custom(obj: Self) -> Self::Builtin { | ||
| obj.to_string() | ||
| } | ||
| } | ||
|
|
||
| #[derive(Debug, Clone, PartialEq, Eq)] | ||
| pub enum OfferAmount { | ||
| Bitcoin { amount_msats: u64 }, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -995,7 +995,7 @@ async fn splice_channel() { | |
| // Splice-in funds for Node B so that it has outbound liquidity to make a payment | ||
| node_b.splice_in(&user_channel_id_b, node_a.node_id(), 4_000_000).unwrap(); | ||
|
|
||
| expect_splice_pending_event!(node_a, node_b.node_id()); | ||
| let txo = expect_splice_pending_event!(node_a, node_b.node_id()); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These test changes pass on
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The bug/issue only exists with bitcoind syncing so to do so, we'd have to change the test to sync with bitcoind rpc. Is that wanted? |
||
| expect_splice_pending_event!(node_b, node_a.node_id()); | ||
|
|
||
| generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await; | ||
|
|
@@ -1006,11 +1006,16 @@ async fn splice_channel() { | |
| expect_channel_ready_event!(node_a, node_b.node_id()); | ||
| expect_channel_ready_event!(node_b, node_a.node_id()); | ||
|
|
||
| let splice_in_fee_sat = 252; | ||
| let expected_splice_in_fee_sat = 252; | ||
|
|
||
| let payments = node_b.list_payments(); | ||
| let payment = | ||
| payments.into_iter().find(|p| p.id == PaymentId(txo.txid.to_byte_array())).unwrap(); | ||
| assert_eq!(payment.fee_paid_msat, Some(expected_splice_in_fee_sat * 1_000)); | ||
|
|
||
| assert_eq!( | ||
| node_b.list_balances().total_onchain_balance_sats, | ||
| premine_amount_sat - 4_000_000 - splice_in_fee_sat | ||
| premine_amount_sat - 4_000_000 - expected_splice_in_fee_sat | ||
| ); | ||
| assert_eq!(node_b.list_balances().total_lightning_balance_sats, 4_000_000); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing this in
ChannelReadywouldn't work for splicing preexisting channels. Wonder if we should instead do this when initiating a splice? There we already look up the channel. It would also let us use the realshared_inputwhen selecting UTXOs.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont think this is right, in the docs it says we get a ChannelReady for splices, also my test seems to confirm