Expose onchain transactions in store by tnull · Pull Request #432 · lightningdevkit/ldk-node

tnull · 2025-01-16T13:28:13Z

Closes#67

Previously, onchain-transactions where not tracked in our PaymentStore.
Now, that the upgrade to BDK 1.0 is behind us, we take a stab at finally exposing them in our interface via PaymentStore.

In the future we're looking to add dedicated PaymentKinds for channel fundings/channel closes (see #447), but for now all are exposed under the Onchain umbrella kind.

tnull · 2025-01-27T15:57:10Z

As we currently don't have a good wait to discern closing transactions, I now split out the event emission to #448 which will land at a later date. Going ahead and undrafting this.

jkczyz · 2025-01-28T02:53:02Z

+ // TODO: It would be great to introduce additional variants for
+ // `ChannelFunding` and `ChannelClosing`. For the former, we could just
+ // take a reference to `ChannelManager` here and check against
+ // `list_channels`. But for the latter the best approach is much less
+ // clear: for force-closes/HTLC spends we should be good querying
+ // `OutputSweeper::tracked_spendable_outputs`, but regular channel closes
+ // (i.e., `SpendableOutputDescriptor::StaticOutput` variants) are directly
+ // spent to a wallet address. The only solution I can come up with is to
+ // create and persist a list of 'static pending outputs' that we could use
+ // here to determine the `PaymentKind`, but that's not really satisfactory, so
+ // we're punting on it until we can come up with a better solution.


I wonder if this gets more complicated with splicing in/out or more exotic transactions paying us and opening a channel, for instance.

Hmm, good point, now opened the more lightningdevkit/rust-lightning#3566 to supersede lightningdevkit/rust-lightning#3548
Would be great if an API allowing to query the type of a given transaction (Txid) could be added as part of the splicing work.

jkczyz · 2025-01-28T02:53:58Z

+ let (direction, amount_msat) = if sent > received {
+ let direction = PaymentDirection::Outbound;
+ let amount_msat = Some(sent.to_sat().saturating_sub(received.to_sat()) * 1000);
+ (direction, amount_msat)
+ } else {
+ let direction = PaymentDirection::Inbound;
+ let amount_msat = Some(received.to_sat().saturating_sub(sent.to_sat()) * 1000);
+ (direction, amount_msat)
+ };


Should we consider these two separate payments? I guess we could't use the txid as the payment id, though.

Mhh, given that (IIUC) received includes the change, I'd rather not add each change output as an inbound payment?

jkczyz · 2025-01-28T03:02:40Z

+ let payment_status = if cur_height >= confirmation_height + ANTI_REORG_DELAY - 1
+ {
+ PaymentStatus::Succeeded
+ } else {
+ PaymentStatus::Pending
+ };


Say the transaction was re-orged out and was then RBF'ed. Is that possible? Would we have payment stuck in pending?

Hmm, that's a very valid concern. Tbh. I'm not entirely sure how to handle that: for one, the RBF'd transaction is technically pending as there could be another reorg that makes it part of the best chain again. So essentially any signed & broadcasted transaction remains 'pending' forever.
I think the only way I can come up with to handle this would be to never add Pending transactions but jump to Succeeded directly once they reach ANTI_REORG_DELAY. Or we just keep the current approach and add docs noting that transactions in pending might never succeed for one reason or another?
I think at least for BDK's own RBF's we could see if we find a better solution when we do #367.

Could we possibly detect that the UTXO has been spent and remove the pending payment?

I guess, but that would require us to keep track and persist a list of all the UTXOs, write an adapter around lightning-transaction-sync's Filter implementation to also register spends for them, and then implement Confirm on Wallet, just to be able to remove any Pending entries that have been replaced.
For now that seems like a lot of overhead to just account for this rare edgecase (reorg + RBF)? But I wouldn't completely rule it out to do something like it in if we find we need to do additional RBF tracking anyways as part of #367.

jkczyz · 2025-01-28T03:04:42Z

+ // here to determine the `PaymentKind`, but that's not really satisfactory, so
+ // we're punting on it until we can come up with a better solution.
+ let kind = crate::payment::PaymentKind::Onchain { txid, status: confirmation_status };
+ let (sent, received) = locked_wallet.sent_and_received(&wtx.tx_node.tx);


Is change abstracted away from this API?

I don't think so, the docs state:
/// This method returns a tuple `(sent, received)`. Sent is the sum of the txin amounts /// that spend from previous txouts tracked by this wallet. Received is the summation /// of this tx's outputs that send to script pubkeys tracked by this wallet.
so IIUC received would include the change.

Couldn't the change be more than the amount sent, making it look like an inbound payment when it was really an outbound? Or am I misunderstanding?

Couldn't the change be more than the amount sent, making it look like an inbound payment when it was really an outbound? Or am I misunderstanding?
IIUC, any change amount would be accounted for both in sent and received, so you can essentially ignore it in terms of comparisons (think: subtract on both sides).
Say you send a payment of 10 sats, and add a txin spending an wallet output with 100 sats. This would result in two txouts, one to the recipient with 10 sats, one change back to a wallet address with 85sats or so, leaving 5 for mining fees. IIUC, this would result in sent = 100, received = 85, resulting in sent > received => PaymentDirection::Outbound.

jkczyz

Feel free to squash

jkczyz · 2025-01-29T22:46:41Z

+ let payment_status = if cur_height >= confirmation_height + ANTI_REORG_DELAY - 1
+ {
+ PaymentStatus::Succeeded
+ } else {
+ PaymentStatus::Pending
+ };


Could we possibly detect that the UTXO has been spent and remove the pending payment?

Here, we move updating fields via `PaymentDetailsUpdate` to `PaymentDetails::update`. This allows us to only update the fields that changed, keeping track *whether* something changed, and only updating the timestamp and persisting the entry *if* something changed. This is a nice improvement in general (as we want to reduce persist calls anyways), but we'll also use this for batch updates in the next commits.

We implement an inserting/updating method that will only persist entries that have been changed.

tnull · 2025-01-30T12:57:37Z

Rebased to resolve minor conflicts

Previously, `PaymentKind::Onchain` was simply a placeholder entry we never actually used. Here, we extend it to include fields that are actually useful.

We update the payment store whenever syncing the wallet state finished.

tnull · 2025-01-30T16:03:54Z

Squashed the fixup without further changes.

danielnordh · 2025-01-30T17:10:41Z

Yes, thank you for getting this done!!

tnull · 2025-01-16T13:28:13Z

Closes#67

Previously, onchain-transactions where not tracked in our PaymentStore.
Now, that the upgrade to BDK 1.0 is behind us, we take a stab at finally exposing them in our interface via PaymentStore.

In the future we're looking to add dedicated PaymentKinds for channel fundings/channel closes (see #447), but for now all are exposed under the Onchain umbrella kind.

tnull · 2025-01-27T15:57:10Z

As we currently don't have a good wait to discern closing transactions, I now split out the event emission to #448 which will land at a later date. Going ahead and undrafting this.

jkczyz · 2025-01-28T02:53:02Z

+ // TODO: It would be great to introduce additional variants for
+ // `ChannelFunding` and `ChannelClosing`. For the former, we could just
+ // take a reference to `ChannelManager` here and check against
+ // `list_channels`. But for the latter the best approach is much less
+ // clear: for force-closes/HTLC spends we should be good querying
+ // `OutputSweeper::tracked_spendable_outputs`, but regular channel closes
+ // (i.e., `SpendableOutputDescriptor::StaticOutput` variants) are directly
+ // spent to a wallet address. The only solution I can come up with is to
+ // create and persist a list of 'static pending outputs' that we could use
+ // here to determine the `PaymentKind`, but that's not really satisfactory, so
+ // we're punting on it until we can come up with a better solution.


I wonder if this gets more complicated with splicing in/out or more exotic transactions paying us and opening a channel, for instance.

Hmm, good point, now opened the more lightningdevkit/rust-lightning#3566 to supersede lightningdevkit/rust-lightning#3548
Would be great if an API allowing to query the type of a given transaction (Txid) could be added as part of the splicing work.

jkczyz · 2025-01-28T02:53:58Z

+ let (direction, amount_msat) = if sent > received {
+ let direction = PaymentDirection::Outbound;
+ let amount_msat = Some(sent.to_sat().saturating_sub(received.to_sat()) * 1000);
+ (direction, amount_msat)
+ } else {
+ let direction = PaymentDirection::Inbound;
+ let amount_msat = Some(received.to_sat().saturating_sub(sent.to_sat()) * 1000);
+ (direction, amount_msat)
+ };


Should we consider these two separate payments? I guess we could't use the txid as the payment id, though.

Mhh, given that (IIUC) received includes the change, I'd rather not add each change output as an inbound payment?

jkczyz · 2025-01-28T03:02:40Z

+ let payment_status = if cur_height >= confirmation_height + ANTI_REORG_DELAY - 1
+ {
+ PaymentStatus::Succeeded
+ } else {
+ PaymentStatus::Pending
+ };


Say the transaction was re-orged out and was then RBF'ed. Is that possible? Would we have payment stuck in pending?

Hmm, that's a very valid concern. Tbh. I'm not entirely sure how to handle that: for one, the RBF'd transaction is technically pending as there could be another reorg that makes it part of the best chain again. So essentially any signed & broadcasted transaction remains 'pending' forever.
I think the only way I can come up with to handle this would be to never add Pending transactions but jump to Succeeded directly once they reach ANTI_REORG_DELAY. Or we just keep the current approach and add docs noting that transactions in pending might never succeed for one reason or another?
I think at least for BDK's own RBF's we could see if we find a better solution when we do #367.

Could we possibly detect that the UTXO has been spent and remove the pending payment?

I guess, but that would require us to keep track and persist a list of all the UTXOs, write an adapter around lightning-transaction-sync's Filter implementation to also register spends for them, and then implement Confirm on Wallet, just to be able to remove any Pending entries that have been replaced.
For now that seems like a lot of overhead to just account for this rare edgecase (reorg + RBF)? But I wouldn't completely rule it out to do something like it in if we find we need to do additional RBF tracking anyways as part of #367.

jkczyz · 2025-01-28T03:04:42Z

+ // here to determine the `PaymentKind`, but that's not really satisfactory, so
+ // we're punting on it until we can come up with a better solution.
+ let kind = crate::payment::PaymentKind::Onchain { txid, status: confirmation_status };
+ let (sent, received) = locked_wallet.sent_and_received(&wtx.tx_node.tx);


Is change abstracted away from this API?

I don't think so, the docs state:
/// This method returns a tuple `(sent, received)`. Sent is the sum of the txin amounts /// that spend from previous txouts tracked by this wallet. Received is the summation /// of this tx's outputs that send to script pubkeys tracked by this wallet.
so IIUC received would include the change.

Couldn't the change be more than the amount sent, making it look like an inbound payment when it was really an outbound? Or am I misunderstanding?

Couldn't the change be more than the amount sent, making it look like an inbound payment when it was really an outbound? Or am I misunderstanding?
IIUC, any change amount would be accounted for both in sent and received, so you can essentially ignore it in terms of comparisons (think: subtract on both sides).
Say you send a payment of 10 sats, and add a txin spending an wallet output with 100 sats. This would result in two txouts, one to the recipient with 10 sats, one change back to a wallet address with 85sats or so, leaving 5 for mining fees. IIUC, this would result in sent = 100, received = 85, resulting in sent > received => PaymentDirection::Outbound.

jkczyz

Feel free to squash

jkczyz · 2025-01-29T22:46:41Z

+ let payment_status = if cur_height >= confirmation_height + ANTI_REORG_DELAY - 1
+ {
+ PaymentStatus::Succeeded
+ } else {
+ PaymentStatus::Pending
+ };


Could we possibly detect that the UTXO has been spent and remove the pending payment?

Here, we move updating fields via `PaymentDetailsUpdate` to `PaymentDetails::update`. This allows us to only update the fields that changed, keeping track *whether* something changed, and only updating the timestamp and persisting the entry *if* something changed. This is a nice improvement in general (as we want to reduce persist calls anyways), but we'll also use this for batch updates in the next commits.

We implement an inserting/updating method that will only persist entries that have been changed.

tnull · 2025-01-30T12:57:37Z

Rebased to resolve minor conflicts

Previously, `PaymentKind::Onchain` was simply a placeholder entry we never actually used. Here, we extend it to include fields that are actually useful.

We update the payment store whenever syncing the wallet state finished.

tnull · 2025-01-30T16:03:54Z

Squashed the fixup without further changes.

danielnordh · 2025-01-30T17:10:41Z

Yes, thank you for getting this done!!

tnull · 2025-01-16T13:28:13Z

Closes#67

Previously, onchain-transactions where not tracked in our PaymentStore.
Now, that the upgrade to BDK 1.0 is behind us, we take a stab at finally exposing them in our interface via PaymentStore.

In the future we're looking to add dedicated PaymentKinds for channel fundings/channel closes (see #447), but for now all are exposed under the Onchain umbrella kind.

tnull · 2025-01-27T15:57:10Z

As we currently don't have a good wait to discern closing transactions, I now split out the event emission to #448 which will land at a later date. Going ahead and undrafting this.

jkczyz · 2025-01-28T02:53:02Z

+ // TODO: It would be great to introduce additional variants for
+ // `ChannelFunding` and `ChannelClosing`. For the former, we could just
+ // take a reference to `ChannelManager` here and check against
+ // `list_channels`. But for the latter the best approach is much less
+ // clear: for force-closes/HTLC spends we should be good querying
+ // `OutputSweeper::tracked_spendable_outputs`, but regular channel closes
+ // (i.e., `SpendableOutputDescriptor::StaticOutput` variants) are directly
+ // spent to a wallet address. The only solution I can come up with is to
+ // create and persist a list of 'static pending outputs' that we could use
+ // here to determine the `PaymentKind`, but that's not really satisfactory, so
+ // we're punting on it until we can come up with a better solution.


I wonder if this gets more complicated with splicing in/out or more exotic transactions paying us and opening a channel, for instance.

Hmm, good point, now opened the more lightningdevkit/rust-lightning#3566 to supersede lightningdevkit/rust-lightning#3548
Would be great if an API allowing to query the type of a given transaction (Txid) could be added as part of the splicing work.

jkczyz · 2025-01-28T02:53:58Z

+ let (direction, amount_msat) = if sent > received {
+ let direction = PaymentDirection::Outbound;
+ let amount_msat = Some(sent.to_sat().saturating_sub(received.to_sat()) * 1000);
+ (direction, amount_msat)
+ } else {
+ let direction = PaymentDirection::Inbound;
+ let amount_msat = Some(received.to_sat().saturating_sub(sent.to_sat()) * 1000);
+ (direction, amount_msat)
+ };


Should we consider these two separate payments? I guess we could't use the txid as the payment id, though.

Mhh, given that (IIUC) received includes the change, I'd rather not add each change output as an inbound payment?

jkczyz · 2025-01-28T03:02:40Z

+ let payment_status = if cur_height >= confirmation_height + ANTI_REORG_DELAY - 1
+ {
+ PaymentStatus::Succeeded
+ } else {
+ PaymentStatus::Pending
+ };


Say the transaction was re-orged out and was then RBF'ed. Is that possible? Would we have payment stuck in pending?

Hmm, that's a very valid concern. Tbh. I'm not entirely sure how to handle that: for one, the RBF'd transaction is technically pending as there could be another reorg that makes it part of the best chain again. So essentially any signed & broadcasted transaction remains 'pending' forever.
I think the only way I can come up with to handle this would be to never add Pending transactions but jump to Succeeded directly once they reach ANTI_REORG_DELAY. Or we just keep the current approach and add docs noting that transactions in pending might never succeed for one reason or another?
I think at least for BDK's own RBF's we could see if we find a better solution when we do #367.

Could we possibly detect that the UTXO has been spent and remove the pending payment?

I guess, but that would require us to keep track and persist a list of all the UTXOs, write an adapter around lightning-transaction-sync's Filter implementation to also register spends for them, and then implement Confirm on Wallet, just to be able to remove any Pending entries that have been replaced.
For now that seems like a lot of overhead to just account for this rare edgecase (reorg + RBF)? But I wouldn't completely rule it out to do something like it in if we find we need to do additional RBF tracking anyways as part of #367.

jkczyz · 2025-01-28T03:04:42Z

+ // here to determine the `PaymentKind`, but that's not really satisfactory, so
+ // we're punting on it until we can come up with a better solution.
+ let kind = crate::payment::PaymentKind::Onchain { txid, status: confirmation_status };
+ let (sent, received) = locked_wallet.sent_and_received(&wtx.tx_node.tx);


Is change abstracted away from this API?

I don't think so, the docs state:
/// This method returns a tuple `(sent, received)`. Sent is the sum of the txin amounts /// that spend from previous txouts tracked by this wallet. Received is the summation /// of this tx's outputs that send to script pubkeys tracked by this wallet.
so IIUC received would include the change.

Couldn't the change be more than the amount sent, making it look like an inbound payment when it was really an outbound? Or am I misunderstanding?

Couldn't the change be more than the amount sent, making it look like an inbound payment when it was really an outbound? Or am I misunderstanding?
IIUC, any change amount would be accounted for both in sent and received, so you can essentially ignore it in terms of comparisons (think: subtract on both sides).
Say you send a payment of 10 sats, and add a txin spending an wallet output with 100 sats. This would result in two txouts, one to the recipient with 10 sats, one change back to a wallet address with 85sats or so, leaving 5 for mining fees. IIUC, this would result in sent = 100, received = 85, resulting in sent > received => PaymentDirection::Outbound.

jkczyz

Feel free to squash

jkczyz · 2025-01-29T22:46:41Z

+ let payment_status = if cur_height >= confirmation_height + ANTI_REORG_DELAY - 1
+ {
+ PaymentStatus::Succeeded
+ } else {
+ PaymentStatus::Pending
+ };


Could we possibly detect that the UTXO has been spent and remove the pending payment?

Here, we move updating fields via `PaymentDetailsUpdate` to `PaymentDetails::update`. This allows us to only update the fields that changed, keeping track *whether* something changed, and only updating the timestamp and persisting the entry *if* something changed. This is a nice improvement in general (as we want to reduce persist calls anyways), but we'll also use this for batch updates in the next commits.

We implement an inserting/updating method that will only persist entries that have been changed.

tnull · 2025-01-30T12:57:37Z

Rebased to resolve minor conflicts

Previously, `PaymentKind::Onchain` was simply a placeholder entry we never actually used. Here, we extend it to include fields that are actually useful.

We update the payment store whenever syncing the wallet state finished.

tnull · 2025-01-30T16:03:54Z

Squashed the fixup without further changes.

danielnordh · 2025-01-30T17:10:41Z

Yes, thank you for getting this done!!

tnull · 2025-01-16T13:28:13Z

Closes#67

Previously, onchain-transactions where not tracked in our PaymentStore.
Now, that the upgrade to BDK 1.0 is behind us, we take a stab at finally exposing them in our interface via PaymentStore.

In the future we're looking to add dedicated PaymentKinds for channel fundings/channel closes (see #447), but for now all are exposed under the Onchain umbrella kind.

tnull · 2025-01-27T15:57:10Z

As we currently don't have a good wait to discern closing transactions, I now split out the event emission to #448 which will land at a later date. Going ahead and undrafting this.

jkczyz · 2025-01-28T02:53:02Z

+ // TODO: It would be great to introduce additional variants for
+ // `ChannelFunding` and `ChannelClosing`. For the former, we could just
+ // take a reference to `ChannelManager` here and check against
+ // `list_channels`. But for the latter the best approach is much less
+ // clear: for force-closes/HTLC spends we should be good querying
+ // `OutputSweeper::tracked_spendable_outputs`, but regular channel closes
+ // (i.e., `SpendableOutputDescriptor::StaticOutput` variants) are directly
+ // spent to a wallet address. The only solution I can come up with is to
+ // create and persist a list of 'static pending outputs' that we could use
+ // here to determine the `PaymentKind`, but that's not really satisfactory, so
+ // we're punting on it until we can come up with a better solution.


I wonder if this gets more complicated with splicing in/out or more exotic transactions paying us and opening a channel, for instance.

Hmm, good point, now opened the more lightningdevkit/rust-lightning#3566 to supersede lightningdevkit/rust-lightning#3548
Would be great if an API allowing to query the type of a given transaction (Txid) could be added as part of the splicing work.

jkczyz · 2025-01-28T02:53:58Z

+ let (direction, amount_msat) = if sent > received {
+ let direction = PaymentDirection::Outbound;
+ let amount_msat = Some(sent.to_sat().saturating_sub(received.to_sat()) * 1000);
+ (direction, amount_msat)
+ } else {
+ let direction = PaymentDirection::Inbound;
+ let amount_msat = Some(received.to_sat().saturating_sub(sent.to_sat()) * 1000);
+ (direction, amount_msat)
+ };


Should we consider these two separate payments? I guess we could't use the txid as the payment id, though.

Mhh, given that (IIUC) received includes the change, I'd rather not add each change output as an inbound payment?

jkczyz · 2025-01-28T03:02:40Z

+ let payment_status = if cur_height >= confirmation_height + ANTI_REORG_DELAY - 1
+ {
+ PaymentStatus::Succeeded
+ } else {
+ PaymentStatus::Pending
+ };


Say the transaction was re-orged out and was then RBF'ed. Is that possible? Would we have payment stuck in pending?

Hmm, that's a very valid concern. Tbh. I'm not entirely sure how to handle that: for one, the RBF'd transaction is technically pending as there could be another reorg that makes it part of the best chain again. So essentially any signed & broadcasted transaction remains 'pending' forever.
I think the only way I can come up with to handle this would be to never add Pending transactions but jump to Succeeded directly once they reach ANTI_REORG_DELAY. Or we just keep the current approach and add docs noting that transactions in pending might never succeed for one reason or another?
I think at least for BDK's own RBF's we could see if we find a better solution when we do #367.

Could we possibly detect that the UTXO has been spent and remove the pending payment?

I guess, but that would require us to keep track and persist a list of all the UTXOs, write an adapter around lightning-transaction-sync's Filter implementation to also register spends for them, and then implement Confirm on Wallet, just to be able to remove any Pending entries that have been replaced.
For now that seems like a lot of overhead to just account for this rare edgecase (reorg + RBF)? But I wouldn't completely rule it out to do something like it in if we find we need to do additional RBF tracking anyways as part of #367.

jkczyz · 2025-01-28T03:04:42Z

+ // here to determine the `PaymentKind`, but that's not really satisfactory, so
+ // we're punting on it until we can come up with a better solution.
+ let kind = crate::payment::PaymentKind::Onchain { txid, status: confirmation_status };
+ let (sent, received) = locked_wallet.sent_and_received(&wtx.tx_node.tx);


Is change abstracted away from this API?

I don't think so, the docs state:
/// This method returns a tuple `(sent, received)`. Sent is the sum of the txin amounts /// that spend from previous txouts tracked by this wallet. Received is the summation /// of this tx's outputs that send to script pubkeys tracked by this wallet.
so IIUC received would include the change.

Couldn't the change be more than the amount sent, making it look like an inbound payment when it was really an outbound? Or am I misunderstanding?

Couldn't the change be more than the amount sent, making it look like an inbound payment when it was really an outbound? Or am I misunderstanding?
IIUC, any change amount would be accounted for both in sent and received, so you can essentially ignore it in terms of comparisons (think: subtract on both sides).
Say you send a payment of 10 sats, and add a txin spending an wallet output with 100 sats. This would result in two txouts, one to the recipient with 10 sats, one change back to a wallet address with 85sats or so, leaving 5 for mining fees. IIUC, this would result in sent = 100, received = 85, resulting in sent > received => PaymentDirection::Outbound.

jkczyz

Feel free to squash

jkczyz · 2025-01-29T22:46:41Z

+ let payment_status = if cur_height >= confirmation_height + ANTI_REORG_DELAY - 1
+ {
+ PaymentStatus::Succeeded
+ } else {
+ PaymentStatus::Pending
+ };


Could we possibly detect that the UTXO has been spent and remove the pending payment?

Here, we move updating fields via `PaymentDetailsUpdate` to `PaymentDetails::update`. This allows us to only update the fields that changed, keeping track *whether* something changed, and only updating the timestamp and persisting the entry *if* something changed. This is a nice improvement in general (as we want to reduce persist calls anyways), but we'll also use this for batch updates in the next commits.

We implement an inserting/updating method that will only persist entries that have been changed.

tnull · 2025-01-30T12:57:37Z

Rebased to resolve minor conflicts

Previously, `PaymentKind::Onchain` was simply a placeholder entry we never actually used. Here, we extend it to include fields that are actually useful.

We update the payment store whenever syncing the wallet state finished.

tnull · 2025-01-30T16:03:54Z

Squashed the fixup without further changes.

danielnordh · 2025-01-30T17:10:41Z

Yes, thank you for getting this done!!

tnull · 2025-01-16T13:28:13Z

Closes#67

Previously, onchain-transactions where not tracked in our PaymentStore.
Now, that the upgrade to BDK 1.0 is behind us, we take a stab at finally exposing them in our interface via PaymentStore.

In the future we're looking to add dedicated PaymentKinds for channel fundings/channel closes (see #447), but for now all are exposed under the Onchain umbrella kind.

tnull · 2025-01-27T15:57:10Z

As we currently don't have a good wait to discern closing transactions, I now split out the event emission to #448 which will land at a later date. Going ahead and undrafting this.

jkczyz · 2025-01-28T02:53:02Z

+ // TODO: It would be great to introduce additional variants for
+ // `ChannelFunding` and `ChannelClosing`. For the former, we could just
+ // take a reference to `ChannelManager` here and check against
+ // `list_channels`. But for the latter the best approach is much less
+ // clear: for force-closes/HTLC spends we should be good querying
+ // `OutputSweeper::tracked_spendable_outputs`, but regular channel closes
+ // (i.e., `SpendableOutputDescriptor::StaticOutput` variants) are directly
+ // spent to a wallet address. The only solution I can come up with is to
+ // create and persist a list of 'static pending outputs' that we could use
+ // here to determine the `PaymentKind`, but that's not really satisfactory, so
+ // we're punting on it until we can come up with a better solution.


I wonder if this gets more complicated with splicing in/out or more exotic transactions paying us and opening a channel, for instance.

Hmm, good point, now opened the more lightningdevkit/rust-lightning#3566 to supersede lightningdevkit/rust-lightning#3548
Would be great if an API allowing to query the type of a given transaction (Txid) could be added as part of the splicing work.

jkczyz · 2025-01-28T02:53:58Z

+ let (direction, amount_msat) = if sent > received {
+ let direction = PaymentDirection::Outbound;
+ let amount_msat = Some(sent.to_sat().saturating_sub(received.to_sat()) * 1000);
+ (direction, amount_msat)
+ } else {
+ let direction = PaymentDirection::Inbound;
+ let amount_msat = Some(received.to_sat().saturating_sub(sent.to_sat()) * 1000);
+ (direction, amount_msat)
+ };


Should we consider these two separate payments? I guess we could't use the txid as the payment id, though.

Mhh, given that (IIUC) received includes the change, I'd rather not add each change output as an inbound payment?

jkczyz · 2025-01-28T03:02:40Z

+ let payment_status = if cur_height >= confirmation_height + ANTI_REORG_DELAY - 1
+ {
+ PaymentStatus::Succeeded
+ } else {
+ PaymentStatus::Pending
+ };


Say the transaction was re-orged out and was then RBF'ed. Is that possible? Would we have payment stuck in pending?

Hmm, that's a very valid concern. Tbh. I'm not entirely sure how to handle that: for one, the RBF'd transaction is technically pending as there could be another reorg that makes it part of the best chain again. So essentially any signed & broadcasted transaction remains 'pending' forever.
I think the only way I can come up with to handle this would be to never add Pending transactions but jump to Succeeded directly once they reach ANTI_REORG_DELAY. Or we just keep the current approach and add docs noting that transactions in pending might never succeed for one reason or another?
I think at least for BDK's own RBF's we could see if we find a better solution when we do #367.

Could we possibly detect that the UTXO has been spent and remove the pending payment?

I guess, but that would require us to keep track and persist a list of all the UTXOs, write an adapter around lightning-transaction-sync's Filter implementation to also register spends for them, and then implement Confirm on Wallet, just to be able to remove any Pending entries that have been replaced.
For now that seems like a lot of overhead to just account for this rare edgecase (reorg + RBF)? But I wouldn't completely rule it out to do something like it in if we find we need to do additional RBF tracking anyways as part of #367.

jkczyz · 2025-01-28T03:04:42Z

+ // here to determine the `PaymentKind`, but that's not really satisfactory, so
+ // we're punting on it until we can come up with a better solution.
+ let kind = crate::payment::PaymentKind::Onchain { txid, status: confirmation_status };
+ let (sent, received) = locked_wallet.sent_and_received(&wtx.tx_node.tx);


Is change abstracted away from this API?

I don't think so, the docs state:
/// This method returns a tuple `(sent, received)`. Sent is the sum of the txin amounts /// that spend from previous txouts tracked by this wallet. Received is the summation /// of this tx's outputs that send to script pubkeys tracked by this wallet.
so IIUC received would include the change.

Couldn't the change be more than the amount sent, making it look like an inbound payment when it was really an outbound? Or am I misunderstanding?

Couldn't the change be more than the amount sent, making it look like an inbound payment when it was really an outbound? Or am I misunderstanding?
IIUC, any change amount would be accounted for both in sent and received, so you can essentially ignore it in terms of comparisons (think: subtract on both sides).
Say you send a payment of 10 sats, and add a txin spending an wallet output with 100 sats. This would result in two txouts, one to the recipient with 10 sats, one change back to a wallet address with 85sats or so, leaving 5 for mining fees. IIUC, this would result in sent = 100, received = 85, resulting in sent > received => PaymentDirection::Outbound.

jkczyz

Feel free to squash

jkczyz · 2025-01-29T22:46:41Z

+ let payment_status = if cur_height >= confirmation_height + ANTI_REORG_DELAY - 1
+ {
+ PaymentStatus::Succeeded
+ } else {
+ PaymentStatus::Pending
+ };


Could we possibly detect that the UTXO has been spent and remove the pending payment?

Here, we move updating fields via `PaymentDetailsUpdate` to `PaymentDetails::update`. This allows us to only update the fields that changed, keeping track *whether* something changed, and only updating the timestamp and persisting the entry *if* something changed. This is a nice improvement in general (as we want to reduce persist calls anyways), but we'll also use this for batch updates in the next commits.

We implement an inserting/updating method that will only persist entries that have been changed.

tnull · 2025-01-30T12:57:37Z

Rebased to resolve minor conflicts

Previously, `PaymentKind::Onchain` was simply a placeholder entry we never actually used. Here, we extend it to include fields that are actually useful.

We update the payment store whenever syncing the wallet state finished.

tnull · 2025-01-30T16:03:54Z

Squashed the fixup without further changes.

danielnordh · 2025-01-30T17:10:41Z

Yes, thank you for getting this done!!

tnull · 2025-01-16T13:28:13Z

Closes#67

Previously, onchain-transactions where not tracked in our PaymentStore.
Now, that the upgrade to BDK 1.0 is behind us, we take a stab at finally exposing them in our interface via PaymentStore.

In the future we're looking to add dedicated PaymentKinds for channel fundings/channel closes (see #447), but for now all are exposed under the Onchain umbrella kind.

tnull · 2025-01-27T15:57:10Z

As we currently don't have a good wait to discern closing transactions, I now split out the event emission to #448 which will land at a later date. Going ahead and undrafting this.

jkczyz · 2025-01-28T02:53:02Z

+ // TODO: It would be great to introduce additional variants for
+ // `ChannelFunding` and `ChannelClosing`. For the former, we could just
+ // take a reference to `ChannelManager` here and check against
+ // `list_channels`. But for the latter the best approach is much less
+ // clear: for force-closes/HTLC spends we should be good querying
+ // `OutputSweeper::tracked_spendable_outputs`, but regular channel closes
+ // (i.e., `SpendableOutputDescriptor::StaticOutput` variants) are directly
+ // spent to a wallet address. The only solution I can come up with is to
+ // create and persist a list of 'static pending outputs' that we could use
+ // here to determine the `PaymentKind`, but that's not really satisfactory, so
+ // we're punting on it until we can come up with a better solution.


I wonder if this gets more complicated with splicing in/out or more exotic transactions paying us and opening a channel, for instance.

Hmm, good point, now opened the more lightningdevkit/rust-lightning#3566 to supersede lightningdevkit/rust-lightning#3548
Would be great if an API allowing to query the type of a given transaction (Txid) could be added as part of the splicing work.

jkczyz · 2025-01-28T02:53:58Z

+ let (direction, amount_msat) = if sent > received {
+ let direction = PaymentDirection::Outbound;
+ let amount_msat = Some(sent.to_sat().saturating_sub(received.to_sat()) * 1000);
+ (direction, amount_msat)
+ } else {
+ let direction = PaymentDirection::Inbound;
+ let amount_msat = Some(received.to_sat().saturating_sub(sent.to_sat()) * 1000);
+ (direction, amount_msat)
+ };


Should we consider these two separate payments? I guess we could't use the txid as the payment id, though.

Mhh, given that (IIUC) received includes the change, I'd rather not add each change output as an inbound payment?

jkczyz · 2025-01-28T03:02:40Z

+ let payment_status = if cur_height >= confirmation_height + ANTI_REORG_DELAY - 1
+ {
+ PaymentStatus::Succeeded
+ } else {
+ PaymentStatus::Pending
+ };


Say the transaction was re-orged out and was then RBF'ed. Is that possible? Would we have payment stuck in pending?

Hmm, that's a very valid concern. Tbh. I'm not entirely sure how to handle that: for one, the RBF'd transaction is technically pending as there could be another reorg that makes it part of the best chain again. So essentially any signed & broadcasted transaction remains 'pending' forever.
I think the only way I can come up with to handle this would be to never add Pending transactions but jump to Succeeded directly once they reach ANTI_REORG_DELAY. Or we just keep the current approach and add docs noting that transactions in pending might never succeed for one reason or another?
I think at least for BDK's own RBF's we could see if we find a better solution when we do #367.

Could we possibly detect that the UTXO has been spent and remove the pending payment?

I guess, but that would require us to keep track and persist a list of all the UTXOs, write an adapter around lightning-transaction-sync's Filter implementation to also register spends for them, and then implement Confirm on Wallet, just to be able to remove any Pending entries that have been replaced.
For now that seems like a lot of overhead to just account for this rare edgecase (reorg + RBF)? But I wouldn't completely rule it out to do something like it in if we find we need to do additional RBF tracking anyways as part of #367.

jkczyz · 2025-01-28T03:04:42Z

+ // here to determine the `PaymentKind`, but that's not really satisfactory, so
+ // we're punting on it until we can come up with a better solution.
+ let kind = crate::payment::PaymentKind::Onchain { txid, status: confirmation_status };
+ let (sent, received) = locked_wallet.sent_and_received(&wtx.tx_node.tx);


Is change abstracted away from this API?

I don't think so, the docs state:
/// This method returns a tuple `(sent, received)`. Sent is the sum of the txin amounts /// that spend from previous txouts tracked by this wallet. Received is the summation /// of this tx's outputs that send to script pubkeys tracked by this wallet.
so IIUC received would include the change.

Couldn't the change be more than the amount sent, making it look like an inbound payment when it was really an outbound? Or am I misunderstanding?

Couldn't the change be more than the amount sent, making it look like an inbound payment when it was really an outbound? Or am I misunderstanding?
IIUC, any change amount would be accounted for both in sent and received, so you can essentially ignore it in terms of comparisons (think: subtract on both sides).
Say you send a payment of 10 sats, and add a txin spending an wallet output with 100 sats. This would result in two txouts, one to the recipient with 10 sats, one change back to a wallet address with 85sats or so, leaving 5 for mining fees. IIUC, this would result in sent = 100, received = 85, resulting in sent > received => PaymentDirection::Outbound.

jkczyz

Feel free to squash

jkczyz · 2025-01-29T22:46:41Z

+ let payment_status = if cur_height >= confirmation_height + ANTI_REORG_DELAY - 1
+ {
+ PaymentStatus::Succeeded
+ } else {
+ PaymentStatus::Pending
+ };


Could we possibly detect that the UTXO has been spent and remove the pending payment?

Here, we move updating fields via `PaymentDetailsUpdate` to `PaymentDetails::update`. This allows us to only update the fields that changed, keeping track *whether* something changed, and only updating the timestamp and persisting the entry *if* something changed. This is a nice improvement in general (as we want to reduce persist calls anyways), but we'll also use this for batch updates in the next commits.

We implement an inserting/updating method that will only persist entries that have been changed.

tnull · 2025-01-30T12:57:37Z

Rebased to resolve minor conflicts

Previously, `PaymentKind::Onchain` was simply a placeholder entry we never actually used. Here, we extend it to include fields that are actually useful.

We update the payment store whenever syncing the wallet state finished.

tnull · 2025-01-30T16:03:54Z

Squashed the fixup without further changes.

danielnordh · 2025-01-30T17:10:41Z

Yes, thank you for getting this done!!

tnull · 2025-01-16T13:28:13Z

Closes#67

Previously, onchain-transactions where not tracked in our PaymentStore.
Now, that the upgrade to BDK 1.0 is behind us, we take a stab at finally exposing them in our interface via PaymentStore.

In the future we're looking to add dedicated PaymentKinds for channel fundings/channel closes (see #447), but for now all are exposed under the Onchain umbrella kind.

tnull · 2025-01-27T15:57:10Z

As we currently don't have a good wait to discern closing transactions, I now split out the event emission to #448 which will land at a later date. Going ahead and undrafting this.

jkczyz · 2025-01-28T02:53:02Z

+ // TODO: It would be great to introduce additional variants for
+ // `ChannelFunding` and `ChannelClosing`. For the former, we could just
+ // take a reference to `ChannelManager` here and check against
+ // `list_channels`. But for the latter the best approach is much less
+ // clear: for force-closes/HTLC spends we should be good querying
+ // `OutputSweeper::tracked_spendable_outputs`, but regular channel closes
+ // (i.e., `SpendableOutputDescriptor::StaticOutput` variants) are directly
+ // spent to a wallet address. The only solution I can come up with is to
+ // create and persist a list of 'static pending outputs' that we could use
+ // here to determine the `PaymentKind`, but that's not really satisfactory, so
+ // we're punting on it until we can come up with a better solution.


I wonder if this gets more complicated with splicing in/out or more exotic transactions paying us and opening a channel, for instance.

Hmm, good point, now opened the more lightningdevkit/rust-lightning#3566 to supersede lightningdevkit/rust-lightning#3548
Would be great if an API allowing to query the type of a given transaction (Txid) could be added as part of the splicing work.

jkczyz · 2025-01-28T02:53:58Z

+ let (direction, amount_msat) = if sent > received {
+ let direction = PaymentDirection::Outbound;
+ let amount_msat = Some(sent.to_sat().saturating_sub(received.to_sat()) * 1000);
+ (direction, amount_msat)
+ } else {
+ let direction = PaymentDirection::Inbound;
+ let amount_msat = Some(received.to_sat().saturating_sub(sent.to_sat()) * 1000);
+ (direction, amount_msat)
+ };


Should we consider these two separate payments? I guess we could't use the txid as the payment id, though.

Mhh, given that (IIUC) received includes the change, I'd rather not add each change output as an inbound payment?

jkczyz · 2025-01-28T03:02:40Z

+ let payment_status = if cur_height >= confirmation_height + ANTI_REORG_DELAY - 1
+ {
+ PaymentStatus::Succeeded
+ } else {
+ PaymentStatus::Pending
+ };


Say the transaction was re-orged out and was then RBF'ed. Is that possible? Would we have payment stuck in pending?

Hmm, that's a very valid concern. Tbh. I'm not entirely sure how to handle that: for one, the RBF'd transaction is technically pending as there could be another reorg that makes it part of the best chain again. So essentially any signed & broadcasted transaction remains 'pending' forever.
I think the only way I can come up with to handle this would be to never add Pending transactions but jump to Succeeded directly once they reach ANTI_REORG_DELAY. Or we just keep the current approach and add docs noting that transactions in pending might never succeed for one reason or another?
I think at least for BDK's own RBF's we could see if we find a better solution when we do #367.

Could we possibly detect that the UTXO has been spent and remove the pending payment?

I guess, but that would require us to keep track and persist a list of all the UTXOs, write an adapter around lightning-transaction-sync's Filter implementation to also register spends for them, and then implement Confirm on Wallet, just to be able to remove any Pending entries that have been replaced.
For now that seems like a lot of overhead to just account for this rare edgecase (reorg + RBF)? But I wouldn't completely rule it out to do something like it in if we find we need to do additional RBF tracking anyways as part of #367.

jkczyz · 2025-01-28T03:04:42Z

+ // here to determine the `PaymentKind`, but that's not really satisfactory, so
+ // we're punting on it until we can come up with a better solution.
+ let kind = crate::payment::PaymentKind::Onchain { txid, status: confirmation_status };
+ let (sent, received) = locked_wallet.sent_and_received(&wtx.tx_node.tx);


Is change abstracted away from this API?

I don't think so, the docs state:
/// This method returns a tuple `(sent, received)`. Sent is the sum of the txin amounts /// that spend from previous txouts tracked by this wallet. Received is the summation /// of this tx's outputs that send to script pubkeys tracked by this wallet.
so IIUC received would include the change.

Couldn't the change be more than the amount sent, making it look like an inbound payment when it was really an outbound? Or am I misunderstanding?

Couldn't the change be more than the amount sent, making it look like an inbound payment when it was really an outbound? Or am I misunderstanding?
IIUC, any change amount would be accounted for both in sent and received, so you can essentially ignore it in terms of comparisons (think: subtract on both sides).
Say you send a payment of 10 sats, and add a txin spending an wallet output with 100 sats. This would result in two txouts, one to the recipient with 10 sats, one change back to a wallet address with 85sats or so, leaving 5 for mining fees. IIUC, this would result in sent = 100, received = 85, resulting in sent > received => PaymentDirection::Outbound.

jkczyz

Feel free to squash

jkczyz · 2025-01-29T22:46:41Z

+ let payment_status = if cur_height >= confirmation_height + ANTI_REORG_DELAY - 1
+ {
+ PaymentStatus::Succeeded
+ } else {
+ PaymentStatus::Pending
+ };


Could we possibly detect that the UTXO has been spent and remove the pending payment?

Here, we move updating fields via `PaymentDetailsUpdate` to `PaymentDetails::update`. This allows us to only update the fields that changed, keeping track *whether* something changed, and only updating the timestamp and persisting the entry *if* something changed. This is a nice improvement in general (as we want to reduce persist calls anyways), but we'll also use this for batch updates in the next commits.

We implement an inserting/updating method that will only persist entries that have been changed.

tnull · 2025-01-30T12:57:37Z

Rebased to resolve minor conflicts

Previously, `PaymentKind::Onchain` was simply a placeholder entry we never actually used. Here, we extend it to include fields that are actually useful.

We update the payment store whenever syncing the wallet state finished.

tnull · 2025-01-30T16:03:54Z

Squashed the fixup without further changes.

danielnordh · 2025-01-30T17:10:41Z

Yes, thank you for getting this done!!

tnull · 2025-01-16T13:28:13Z

Closes#67

Previously, onchain-transactions where not tracked in our PaymentStore.
Now, that the upgrade to BDK 1.0 is behind us, we take a stab at finally exposing them in our interface via PaymentStore.

In the future we're looking to add dedicated PaymentKinds for channel fundings/channel closes (see #447), but for now all are exposed under the Onchain umbrella kind.

tnull · 2025-01-27T15:57:10Z

As we currently don't have a good wait to discern closing transactions, I now split out the event emission to #448 which will land at a later date. Going ahead and undrafting this.

jkczyz · 2025-01-28T02:53:02Z

+ // TODO: It would be great to introduce additional variants for
+ // `ChannelFunding` and `ChannelClosing`. For the former, we could just
+ // take a reference to `ChannelManager` here and check against
+ // `list_channels`. But for the latter the best approach is much less
+ // clear: for force-closes/HTLC spends we should be good querying
+ // `OutputSweeper::tracked_spendable_outputs`, but regular channel closes
+ // (i.e., `SpendableOutputDescriptor::StaticOutput` variants) are directly
+ // spent to a wallet address. The only solution I can come up with is to
+ // create and persist a list of 'static pending outputs' that we could use
+ // here to determine the `PaymentKind`, but that's not really satisfactory, so
+ // we're punting on it until we can come up with a better solution.


I wonder if this gets more complicated with splicing in/out or more exotic transactions paying us and opening a channel, for instance.

Hmm, good point, now opened the more lightningdevkit/rust-lightning#3566 to supersede lightningdevkit/rust-lightning#3548
Would be great if an API allowing to query the type of a given transaction (Txid) could be added as part of the splicing work.

jkczyz · 2025-01-28T02:53:58Z

+ let (direction, amount_msat) = if sent > received {
+ let direction = PaymentDirection::Outbound;
+ let amount_msat = Some(sent.to_sat().saturating_sub(received.to_sat()) * 1000);
+ (direction, amount_msat)
+ } else {
+ let direction = PaymentDirection::Inbound;
+ let amount_msat = Some(received.to_sat().saturating_sub(sent.to_sat()) * 1000);
+ (direction, amount_msat)
+ };


Should we consider these two separate payments? I guess we could't use the txid as the payment id, though.

Mhh, given that (IIUC) received includes the change, I'd rather not add each change output as an inbound payment?

jkczyz · 2025-01-28T03:02:40Z

+ let payment_status = if cur_height >= confirmation_height + ANTI_REORG_DELAY - 1
+ {
+ PaymentStatus::Succeeded
+ } else {
+ PaymentStatus::Pending
+ };


Say the transaction was re-orged out and was then RBF'ed. Is that possible? Would we have payment stuck in pending?

Hmm, that's a very valid concern. Tbh. I'm not entirely sure how to handle that: for one, the RBF'd transaction is technically pending as there could be another reorg that makes it part of the best chain again. So essentially any signed & broadcasted transaction remains 'pending' forever.
I think the only way I can come up with to handle this would be to never add Pending transactions but jump to Succeeded directly once they reach ANTI_REORG_DELAY. Or we just keep the current approach and add docs noting that transactions in pending might never succeed for one reason or another?
I think at least for BDK's own RBF's we could see if we find a better solution when we do #367.

Could we possibly detect that the UTXO has been spent and remove the pending payment?

I guess, but that would require us to keep track and persist a list of all the UTXOs, write an adapter around lightning-transaction-sync's Filter implementation to also register spends for them, and then implement Confirm on Wallet, just to be able to remove any Pending entries that have been replaced.
For now that seems like a lot of overhead to just account for this rare edgecase (reorg + RBF)? But I wouldn't completely rule it out to do something like it in if we find we need to do additional RBF tracking anyways as part of #367.

jkczyz · 2025-01-28T03:04:42Z

+ // here to determine the `PaymentKind`, but that's not really satisfactory, so
+ // we're punting on it until we can come up with a better solution.
+ let kind = crate::payment::PaymentKind::Onchain { txid, status: confirmation_status };
+ let (sent, received) = locked_wallet.sent_and_received(&wtx.tx_node.tx);


Is change abstracted away from this API?

I don't think so, the docs state:
/// This method returns a tuple `(sent, received)`. Sent is the sum of the txin amounts /// that spend from previous txouts tracked by this wallet. Received is the summation /// of this tx's outputs that send to script pubkeys tracked by this wallet.
so IIUC received would include the change.

Couldn't the change be more than the amount sent, making it look like an inbound payment when it was really an outbound? Or am I misunderstanding?

Couldn't the change be more than the amount sent, making it look like an inbound payment when it was really an outbound? Or am I misunderstanding?
IIUC, any change amount would be accounted for both in sent and received, so you can essentially ignore it in terms of comparisons (think: subtract on both sides).
Say you send a payment of 10 sats, and add a txin spending an wallet output with 100 sats. This would result in two txouts, one to the recipient with 10 sats, one change back to a wallet address with 85sats or so, leaving 5 for mining fees. IIUC, this would result in sent = 100, received = 85, resulting in sent > received => PaymentDirection::Outbound.

jkczyz

Feel free to squash

jkczyz · 2025-01-29T22:46:41Z

+ let payment_status = if cur_height >= confirmation_height + ANTI_REORG_DELAY - 1
+ {
+ PaymentStatus::Succeeded
+ } else {
+ PaymentStatus::Pending
+ };


Could we possibly detect that the UTXO has been spent and remove the pending payment?

Here, we move updating fields via `PaymentDetailsUpdate` to `PaymentDetails::update`. This allows us to only update the fields that changed, keeping track *whether* something changed, and only updating the timestamp and persisting the entry *if* something changed. This is a nice improvement in general (as we want to reduce persist calls anyways), but we'll also use this for batch updates in the next commits.

We implement an inserting/updating method that will only persist entries that have been changed.

tnull · 2025-01-30T12:57:37Z

Rebased to resolve minor conflicts

Previously, `PaymentKind::Onchain` was simply a placeholder entry we never actually used. Here, we extend it to include fields that are actually useful.

We update the payment store whenever syncing the wallet state finished.

tnull · 2025-01-30T16:03:54Z

Squashed the fixup without further changes.

danielnordh · 2025-01-30T17:10:41Z

Yes, thank you for getting this done!!

tnull marked this pull request as draft January 16, 2025 13:28

tnull added this to the 0.5 milestone Jan 16, 2025

tnull mentioned this pull request Jan 23, 2025
LDK Node v0.5 Tracking Issue #360
Closed
10 tasks

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch 2 times, most recently from b959830 to 306a78eCompare January 27, 2025 15:45

tnull changed the title ~~Expose onchain transactions in store and events~~Expose onchain transactions in storeJan 27, 2025

This was referenced Jan 27, 2025
Expose on-chain transactions in events #446
Open
Create new PaymentKind variants for channel funding/closing on-chain transactions #447
Closed
Include onchain transactions in events #448
Open

tnull marked this pull request as ready for review January 27, 2025 15:57

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch 2 times, most recently from 585e14f to 6437515Compare January 27, 2025 16:45

tnull requested a review from jkczyz January 27, 2025 16:46

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch from 6437515 to 403be03Compare January 27, 2025 17:03

jkczyz reviewed Jan 28, 2025
View reviewed changes

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch 2 times, most recently from 1d05f9c to 993bf0bCompare January 28, 2025 10:50

jkczyz reviewed Jan 29, 2025
View reviewed changes

tnull added 3 commits January 30, 2025 13:55

Add more debug_asserts for updating BOLT12 payment hashes
d92a568

Prefactor: Implement PaymentStore::insert_or_update
c9ddfae
We implement an inserting/updating method that will only persist entries that have been changed.

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch from 993bf0b to da45da3Compare January 30, 2025 12:56

jkczyz approved these changes Jan 30, 2025
View reviewed changes

tnull added 3 commits January 30, 2025 17:03

Add required fields to PaymentKind::Onchain
62ff60c
Previously, `PaymentKind::Onchain` was simply a placeholder entry we never actually used. Here, we extend it to include fields that are actually useful.

Have Wallet take PaymentStore ref
e46e407

Update PaymentStore after each Wallet sync
0710434
We update the payment store whenever syncing the wallet state finished.

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch from da45da3 to 0710434Compare January 30, 2025 16:03

tnull merged commit aaf74cc into lightningdevkit:mainJan 30, 2025

tnull mentioned this pull request Jan 30, 2025
Find way to drop RBF'd Pending payments #452
Closed

reez mentioned this pull request Feb 3, 2025
(draft) feat: onchain reez/Monday#161
Closed
7 tasks

tnull marked this pull request as draft January 16, 2025 13:28

tnull added this to the 0.5 milestone Jan 16, 2025

tnull mentioned this pull request Jan 23, 2025
LDK Node v0.5 Tracking Issue #360
Closed
10 tasks

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch 2 times, most recently from b959830 to 306a78eCompare January 27, 2025 15:45

tnull changed the title ~~Expose onchain transactions in store and events~~Expose onchain transactions in storeJan 27, 2025

This was referenced Jan 27, 2025
Expose on-chain transactions in events #446
Open
Create new PaymentKind variants for channel funding/closing on-chain transactions #447
Closed
Include onchain transactions in events #448
Open

tnull marked this pull request as ready for review January 27, 2025 15:57

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch 2 times, most recently from 585e14f to 6437515Compare January 27, 2025 16:45

tnull requested a review from jkczyz January 27, 2025 16:46

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch from 6437515 to 403be03Compare January 27, 2025 17:03

jkczyz reviewed Jan 28, 2025
View reviewed changes

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch 2 times, most recently from 1d05f9c to 993bf0bCompare January 28, 2025 10:50

jkczyz reviewed Jan 29, 2025
View reviewed changes

tnull added 3 commits January 30, 2025 13:55

Add more debug_asserts for updating BOLT12 payment hashes
d92a568

Prefactor: Implement PaymentStore::insert_or_update
c9ddfae
We implement an inserting/updating method that will only persist entries that have been changed.

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch from 993bf0b to da45da3Compare January 30, 2025 12:56

jkczyz approved these changes Jan 30, 2025
View reviewed changes

tnull added 3 commits January 30, 2025 17:03

Add required fields to PaymentKind::Onchain
62ff60c
Previously, `PaymentKind::Onchain` was simply a placeholder entry we never actually used. Here, we extend it to include fields that are actually useful.

Have Wallet take PaymentStore ref
e46e407

Update PaymentStore after each Wallet sync
0710434
We update the payment store whenever syncing the wallet state finished.

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch from da45da3 to 0710434Compare January 30, 2025 16:03

tnull merged commit aaf74cc into lightningdevkit:mainJan 30, 2025

tnull mentioned this pull request Jan 30, 2025
Find way to drop RBF'd Pending payments #452
Closed

reez mentioned this pull request Feb 3, 2025
(draft) feat: onchain reez/Monday#161
Closed
7 tasks

tnull marked this pull request as draft January 16, 2025 13:28

tnull added this to the 0.5 milestone Jan 16, 2025

tnull mentioned this pull request Jan 23, 2025
LDK Node v0.5 Tracking Issue #360
Closed
10 tasks

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch 2 times, most recently from b959830 to 306a78eCompare January 27, 2025 15:45

tnull changed the title ~~Expose onchain transactions in store and events~~Expose onchain transactions in storeJan 27, 2025

This was referenced Jan 27, 2025
Expose on-chain transactions in events #446
Open
Create new PaymentKind variants for channel funding/closing on-chain transactions #447
Closed
Include onchain transactions in events #448
Open

tnull marked this pull request as ready for review January 27, 2025 15:57

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch 2 times, most recently from 585e14f to 6437515Compare January 27, 2025 16:45

tnull requested a review from jkczyz January 27, 2025 16:46

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch from 6437515 to 403be03Compare January 27, 2025 17:03

jkczyz reviewed Jan 28, 2025
View reviewed changes

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch 2 times, most recently from 1d05f9c to 993bf0bCompare January 28, 2025 10:50

jkczyz reviewed Jan 29, 2025
View reviewed changes

tnull added 3 commits January 30, 2025 13:55

Add more debug_asserts for updating BOLT12 payment hashes
d92a568

Prefactor: Implement PaymentStore::insert_or_update
c9ddfae
We implement an inserting/updating method that will only persist entries that have been changed.

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch from 993bf0b to da45da3Compare January 30, 2025 12:56

jkczyz approved these changes Jan 30, 2025
View reviewed changes

tnull added 3 commits January 30, 2025 17:03

Add required fields to PaymentKind::Onchain
62ff60c
Previously, `PaymentKind::Onchain` was simply a placeholder entry we never actually used. Here, we extend it to include fields that are actually useful.

Have Wallet take PaymentStore ref
e46e407

Update PaymentStore after each Wallet sync
0710434
We update the payment store whenever syncing the wallet state finished.

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch from da45da3 to 0710434Compare January 30, 2025 16:03

tnull merged commit aaf74cc into lightningdevkit:mainJan 30, 2025

tnull mentioned this pull request Jan 30, 2025
Find way to drop RBF'd Pending payments #452
Closed

reez mentioned this pull request Feb 3, 2025
(draft) feat: onchain reez/Monday#161
Closed
7 tasks

tnull marked this pull request as draft January 16, 2025 13:28

tnull added this to the 0.5 milestone Jan 16, 2025

tnull mentioned this pull request Jan 23, 2025
LDK Node v0.5 Tracking Issue #360
Closed
10 tasks

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch 2 times, most recently from b959830 to 306a78eCompare January 27, 2025 15:45

tnull changed the title ~~Expose onchain transactions in store and events~~Expose onchain transactions in storeJan 27, 2025

This was referenced Jan 27, 2025
Expose on-chain transactions in events #446
Open
Create new PaymentKind variants for channel funding/closing on-chain transactions #447
Closed
Include onchain transactions in events #448
Open

tnull marked this pull request as ready for review January 27, 2025 15:57

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch 2 times, most recently from 585e14f to 6437515Compare January 27, 2025 16:45

tnull requested a review from jkczyz January 27, 2025 16:46

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch from 6437515 to 403be03Compare January 27, 2025 17:03

jkczyz reviewed Jan 28, 2025
View reviewed changes

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch 2 times, most recently from 1d05f9c to 993bf0bCompare January 28, 2025 10:50

jkczyz reviewed Jan 29, 2025
View reviewed changes

tnull added 3 commits January 30, 2025 13:55

Add more debug_asserts for updating BOLT12 payment hashes
d92a568

Prefactor: Implement PaymentStore::insert_or_update
c9ddfae
We implement an inserting/updating method that will only persist entries that have been changed.

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch from 993bf0b to da45da3Compare January 30, 2025 12:56

jkczyz approved these changes Jan 30, 2025
View reviewed changes

tnull added 3 commits January 30, 2025 17:03

Add required fields to PaymentKind::Onchain
62ff60c
Previously, `PaymentKind::Onchain` was simply a placeholder entry we never actually used. Here, we extend it to include fields that are actually useful.

Have Wallet take PaymentStore ref
e46e407

Update PaymentStore after each Wallet sync
0710434
We update the payment store whenever syncing the wallet state finished.

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch from da45da3 to 0710434Compare January 30, 2025 16:03

tnull merged commit aaf74cc into lightningdevkit:mainJan 30, 2025

tnull mentioned this pull request Jan 30, 2025
Find way to drop RBF'd Pending payments #452
Closed

reez mentioned this pull request Feb 3, 2025
(draft) feat: onchain reez/Monday#161
Closed
7 tasks

tnull marked this pull request as draft January 16, 2025 13:28

tnull added this to the 0.5 milestone Jan 16, 2025

tnull mentioned this pull request Jan 23, 2025
LDK Node v0.5 Tracking Issue #360
Closed
10 tasks

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch 2 times, most recently from b959830 to 306a78eCompare January 27, 2025 15:45

tnull changed the title ~~Expose onchain transactions in store and events~~Expose onchain transactions in storeJan 27, 2025

This was referenced Jan 27, 2025
Expose on-chain transactions in events #446
Open
Create new PaymentKind variants for channel funding/closing on-chain transactions #447
Closed
Include onchain transactions in events #448
Open

tnull marked this pull request as ready for review January 27, 2025 15:57

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch 2 times, most recently from 585e14f to 6437515Compare January 27, 2025 16:45

tnull requested a review from jkczyz January 27, 2025 16:46

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch from 6437515 to 403be03Compare January 27, 2025 17:03

jkczyz reviewed Jan 28, 2025
View reviewed changes

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch 2 times, most recently from 1d05f9c to 993bf0bCompare January 28, 2025 10:50

jkczyz reviewed Jan 29, 2025
View reviewed changes

tnull added 3 commits January 30, 2025 13:55

Add more debug_asserts for updating BOLT12 payment hashes
d92a568

Prefactor: Implement PaymentStore::insert_or_update
c9ddfae
We implement an inserting/updating method that will only persist entries that have been changed.

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch from 993bf0b to da45da3Compare January 30, 2025 12:56

jkczyz approved these changes Jan 30, 2025
View reviewed changes

tnull added 3 commits January 30, 2025 17:03

Add required fields to PaymentKind::Onchain
62ff60c
Previously, `PaymentKind::Onchain` was simply a placeholder entry we never actually used. Here, we extend it to include fields that are actually useful.

Have Wallet take PaymentStore ref
e46e407

Update PaymentStore after each Wallet sync
0710434
We update the payment store whenever syncing the wallet state finished.

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch from da45da3 to 0710434Compare January 30, 2025 16:03

tnull merged commit aaf74cc into lightningdevkit:mainJan 30, 2025

tnull mentioned this pull request Jan 30, 2025
Find way to drop RBF'd Pending payments #452
Closed

reez mentioned this pull request Feb 3, 2025
(draft) feat: onchain reez/Monday#161
Closed
7 tasks

tnull marked this pull request as draft January 16, 2025 13:28

tnull added this to the 0.5 milestone Jan 16, 2025

tnull mentioned this pull request Jan 23, 2025
LDK Node v0.5 Tracking Issue #360
Closed
10 tasks

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch 2 times, most recently from b959830 to 306a78eCompare January 27, 2025 15:45

tnull changed the title ~~Expose onchain transactions in store and events~~Expose onchain transactions in storeJan 27, 2025

This was referenced Jan 27, 2025
Expose on-chain transactions in events #446
Open
Create new PaymentKind variants for channel funding/closing on-chain transactions #447
Closed
Include onchain transactions in events #448
Open

tnull marked this pull request as ready for review January 27, 2025 15:57

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch 2 times, most recently from 585e14f to 6437515Compare January 27, 2025 16:45

tnull requested a review from jkczyz January 27, 2025 16:46

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch from 6437515 to 403be03Compare January 27, 2025 17:03

jkczyz reviewed Jan 28, 2025
View reviewed changes

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch 2 times, most recently from 1d05f9c to 993bf0bCompare January 28, 2025 10:50

jkczyz reviewed Jan 29, 2025
View reviewed changes

tnull added 3 commits January 30, 2025 13:55

Add more debug_asserts for updating BOLT12 payment hashes
d92a568

Prefactor: Implement PaymentStore::insert_or_update
c9ddfae
We implement an inserting/updating method that will only persist entries that have been changed.

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch from 993bf0b to da45da3Compare January 30, 2025 12:56

jkczyz approved these changes Jan 30, 2025
View reviewed changes

tnull added 3 commits January 30, 2025 17:03

Add required fields to PaymentKind::Onchain
62ff60c
Previously, `PaymentKind::Onchain` was simply a placeholder entry we never actually used. Here, we extend it to include fields that are actually useful.

Have Wallet take PaymentStore ref
e46e407

Update PaymentStore after each Wallet sync
0710434
We update the payment store whenever syncing the wallet state finished.

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch from da45da3 to 0710434Compare January 30, 2025 16:03

tnull merged commit aaf74cc into lightningdevkit:mainJan 30, 2025

tnull mentioned this pull request Jan 30, 2025
Find way to drop RBF'd Pending payments #452
Closed

reez mentioned this pull request Feb 3, 2025
(draft) feat: onchain reez/Monday#161
Closed
7 tasks

tnull marked this pull request as draft January 16, 2025 13:28

tnull added this to the 0.5 milestone Jan 16, 2025

tnull mentioned this pull request Jan 23, 2025
LDK Node v0.5 Tracking Issue #360
Closed
10 tasks

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch 2 times, most recently from b959830 to 306a78eCompare January 27, 2025 15:45

tnull changed the title ~~Expose onchain transactions in store and events~~Expose onchain transactions in storeJan 27, 2025

This was referenced Jan 27, 2025
Expose on-chain transactions in events #446
Open
Create new PaymentKind variants for channel funding/closing on-chain transactions #447
Closed
Include onchain transactions in events #448
Open

tnull marked this pull request as ready for review January 27, 2025 15:57

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch 2 times, most recently from 585e14f to 6437515Compare January 27, 2025 16:45

tnull requested a review from jkczyz January 27, 2025 16:46

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch from 6437515 to 403be03Compare January 27, 2025 17:03

jkczyz reviewed Jan 28, 2025
View reviewed changes

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch 2 times, most recently from 1d05f9c to 993bf0bCompare January 28, 2025 10:50

jkczyz reviewed Jan 29, 2025
View reviewed changes

tnull added 3 commits January 30, 2025 13:55

Add more debug_asserts for updating BOLT12 payment hashes
d92a568

Prefactor: Implement PaymentStore::insert_or_update
c9ddfae
We implement an inserting/updating method that will only persist entries that have been changed.

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch from 993bf0b to da45da3Compare January 30, 2025 12:56

jkczyz approved these changes Jan 30, 2025
View reviewed changes

tnull added 3 commits January 30, 2025 17:03

Add required fields to PaymentKind::Onchain
62ff60c
Previously, `PaymentKind::Onchain` was simply a placeholder entry we never actually used. Here, we extend it to include fields that are actually useful.

Have Wallet take PaymentStore ref
e46e407

Update PaymentStore after each Wallet sync
0710434
We update the payment store whenever syncing the wallet state finished.

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch from da45da3 to 0710434Compare January 30, 2025 16:03

tnull merged commit aaf74cc into lightningdevkit:mainJan 30, 2025

tnull mentioned this pull request Jan 30, 2025
Find way to drop RBF'd Pending payments #452
Closed

reez mentioned this pull request Feb 3, 2025
(draft) feat: onchain reez/Monday#161
Closed
7 tasks

tnull marked this pull request as draft January 16, 2025 13:28

tnull added this to the 0.5 milestone Jan 16, 2025

tnull mentioned this pull request Jan 23, 2025
LDK Node v0.5 Tracking Issue #360
Closed
10 tasks

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch 2 times, most recently from b959830 to 306a78eCompare January 27, 2025 15:45

tnull changed the title ~~Expose onchain transactions in store and events~~Expose onchain transactions in storeJan 27, 2025

This was referenced Jan 27, 2025
Expose on-chain transactions in events #446
Open
Create new PaymentKind variants for channel funding/closing on-chain transactions #447
Closed
Include onchain transactions in events #448
Open

tnull marked this pull request as ready for review January 27, 2025 15:57

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch 2 times, most recently from 585e14f to 6437515Compare January 27, 2025 16:45

tnull requested a review from jkczyz January 27, 2025 16:46

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch from 6437515 to 403be03Compare January 27, 2025 17:03

jkczyz reviewed Jan 28, 2025
View reviewed changes

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch 2 times, most recently from 1d05f9c to 993bf0bCompare January 28, 2025 10:50

jkczyz reviewed Jan 29, 2025
View reviewed changes

tnull added 3 commits January 30, 2025 13:55

Add more debug_asserts for updating BOLT12 payment hashes
d92a568

Prefactor: Implement PaymentStore::insert_or_update
c9ddfae
We implement an inserting/updating method that will only persist entries that have been changed.

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch from 993bf0b to da45da3Compare January 30, 2025 12:56

jkczyz approved these changes Jan 30, 2025
View reviewed changes

tnull added 3 commits January 30, 2025 17:03

Add required fields to PaymentKind::Onchain
62ff60c
Previously, `PaymentKind::Onchain` was simply a placeholder entry we never actually used. Here, we extend it to include fields that are actually useful.

Have Wallet take PaymentStore ref
e46e407

Update PaymentStore after each Wallet sync
0710434
We update the payment store whenever syncing the wallet state finished.

tnull force-pushed the 2025-01-include-onchain-transactions-in-store branch from da45da3 to 0710434Compare January 30, 2025 16:03

tnull merged commit aaf74cc into lightningdevkit:mainJan 30, 2025

tnull mentioned this pull request Jan 30, 2025
Find way to drop RBF'd Pending payments #452
Closed

reez mentioned this pull request Feb 3, 2025
(draft) feat: onchain reez/Monday#161
Closed
7 tasks

Uh oh!

Conversation

tnull commented Jan 16, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tnull commented Jan 27, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tnullJan 30, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkczyz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tnull commented Jan 30, 2025

Uh oh!

tnull commented Jan 30, 2025

Uh oh!

danielnordh commented Jan 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

tnull commented Jan 16, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tnull commented Jan 27, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tnullJan 30, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

tnull commented Jan 16, 2025•
edited
Loading

tnullJan 30, 2025•
edited
Loading

tnull commented Jan 16, 2025•
edited
Loading

tnullJan 30, 2025•
edited
Loading

tnull commented Jan 16, 2025•
edited
Loading