-
Notifications
You must be signed in to change notification settings - Fork 156
Include onchain transactions in events #448
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 |
|---|---|---|
|
|
@@ -82,33 +82,43 @@ where | |
| } | ||
|
|
||
| pub(crate) async fn insert_or_update(&self, object: SO) -> Result<bool, Error> { | ||
| self.insert_or_update_with(object, |updated, _| updated).await | ||
| } | ||
|
|
||
| /// Inserts `object` or merges it into an existing object, returning whether the store changed | ||
| /// and the effective object after the merge. | ||
| pub(crate) async fn insert_or_update_and_get(&self, object: SO) -> Result<(bool, SO), Error> { | ||
| self.insert_or_update_with(object, |updated, stored_object| { | ||
| (updated, stored_object.clone()) | ||
| }) | ||
| .await | ||
| } | ||
|
|
||
| async fn insert_or_update_with<R>( | ||
| &self, object: SO, result_fn: impl FnOnce(bool, &SO) -> R, | ||
| ) -> Result<R, Error> { | ||
| let _guard = self.mutation_lock.lock().await; | ||
|
|
||
| let id = object.id(); | ||
| let data_to_persist = { | ||
| let updated_object = { | ||
| let locked_objects = self.objects.lock().expect("lock"); | ||
| if let Some(existing_object) = locked_objects.get(&id) { | ||
| let mut updated_object = existing_object.clone(); | ||
| let updated = updated_object.update(object.to_update()); | ||
| if updated { | ||
| Some(updated_object) | ||
| updated_object | ||
| } else { | ||
| None | ||
| return Ok(result_fn(false, existing_object)); | ||
| } | ||
| } else { | ||
| Some(object) | ||
| object | ||
| } | ||
| }; | ||
|
|
||
| match data_to_persist { | ||
| Some(updated_object) => { | ||
| self.persist(&updated_object).await?; | ||
| let mut locked_objects = self.objects.lock().expect("lock"); | ||
| locked_objects.insert(id, updated_object); | ||
| Ok(true) | ||
| }, | ||
| None => Ok(false), | ||
| } | ||
| self.persist(&updated_object).await?; | ||
| let mut locked_objects = self.objects.lock().expect("lock"); | ||
| let stored_object = locked_objects.entry(id).insert_entry(updated_object).into_mut(); | ||
| Ok(result_fn(true, stored_object)) | ||
| } | ||
|
|
||
| pub(crate) async fn remove(&self, id: &SO::Id) -> Result<(), Error> { | ||
|
|
@@ -287,6 +297,52 @@ mod tests { | |
| (2, data, required), | ||
| }); | ||
|
|
||
| struct MergingTestObjectUpdate { | ||
| id: TestObjectId, | ||
| data: [u8; 3], | ||
| } | ||
|
|
||
| impl StorableObjectUpdate<MergingTestObject> for MergingTestObjectUpdate { | ||
| fn id(&self) -> TestObjectId { | ||
| self.id | ||
| } | ||
| } | ||
|
|
||
| #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] | ||
| struct MergingTestObject { | ||
| id: TestObjectId, | ||
| data: [u8; 3], | ||
| preserved_data: [u8; 3], | ||
| } | ||
|
|
||
| impl StorableObject for MergingTestObject { | ||
| type Id = TestObjectId; | ||
| type Update = MergingTestObjectUpdate; | ||
|
|
||
| fn id(&self) -> Self::Id { | ||
| self.id | ||
| } | ||
|
|
||
| fn update(&mut self, update: Self::Update) -> bool { | ||
| if self.data != update.data { | ||
| self.data = update.data; | ||
| true | ||
| } else { | ||
| false | ||
| } | ||
| } | ||
|
|
||
| fn to_update(&self) -> Self::Update { | ||
| Self::Update { id: self.id, data: self.data } | ||
| } | ||
| } | ||
|
|
||
| impl_writeable_tlv_based!(MergingTestObject, { | ||
| (0, id, required), | ||
| (2, data, required), | ||
| (4, preserved_data, required), | ||
| }); | ||
|
|
||
| struct FailingStore; | ||
|
|
||
| impl KVStore for FailingStore { | ||
|
|
@@ -403,6 +459,28 @@ mod tests { | |
| assert_eq!(Ok(true), data_store.insert_or_update(new_iou_object).await); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn insert_or_update_and_get_returns_merged_object() { | ||
| let store: Arc<DynStore> = Arc::new(DynStoreWrapper(InMemoryStore::new())); | ||
| let logger = Arc::new(TestLogger::new()); | ||
| let id = TestObjectId { id: [42u8; 4] }; | ||
| let existing = MergingTestObject { id, data: [23u8; 3], preserved_data: [24u8; 3] }; | ||
| let data_store = DataStore::new( | ||
| vec![existing], | ||
| "datastore_test_primary".to_string(), | ||
| "datastore_test_secondary".to_string(), | ||
| store, | ||
| logger, | ||
| ); | ||
|
|
||
| let supplied = MergingTestObject { id, data: [25u8; 3], preserved_data: [26u8; 3] }; | ||
| let expected = MergingTestObject { data: supplied.data, ..existing }; | ||
| assert_eq!(Ok((true, expected)), data_store.insert_or_update_and_get(supplied).await); | ||
|
|
||
| let unchanged = MergingTestObject { preserved_data: [27u8; 3], ..expected }; | ||
| assert_eq!(Ok((false, expected)), data_store.insert_or_update_and_get(unchanged).await); | ||
|
Comment on lines
+476
to
+481
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. Add an assertion for when an insert occurs. |
||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn insert_or_update_does_not_mutate_memory_if_persist_fails() { | ||
| let existing_id = TestObjectId { id: [42u8; 4] }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,7 +13,7 @@ use std::sync::{Arc, Mutex}; | |
|
|
||
| use bitcoin::blockdata::locktime::absolute::LockTime; | ||
| use bitcoin::secp256k1::PublicKey; | ||
| use bitcoin::{Amount, OutPoint}; | ||
| use bitcoin::{Amount, BlockHash, OutPoint, Txid}; | ||
| use lightning::blinded_path::message::NextMessageHop; | ||
| use lightning::events::bump_transaction::BumpTransactionEvent; | ||
| #[cfg(not(feature = "uniffi"))] | ||
|
|
@@ -271,6 +271,46 @@ pub enum Event { | |
| /// This will be `None` for events serialized by LDK Node v0.2.1 and prior. | ||
| reason: Option<ClosureReason>, | ||
| }, | ||
| /// A sent on-chain payment was successful. | ||
| /// | ||
| /// This is only emitted for wallet transactions which were not classified as channel | ||
| /// funding, splices, closes, sweeps, or other LDK-driven chain activity. | ||
|
Comment on lines
+276
to
+277
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. Claude: Nit: two narrow cases can still emit these events for LDK-driven transactions and may deserve a doc sentence: (1) classification only happens when our own broadcaster handles a tx, so a closing tx broadcast solely by the counterparty that confirms while this node is offline is discovered by wallet sync unclassified and emits |
||
| /// | ||
| /// It's guaranteed to have reached at least [`ANTI_REORG_DELAY`] confirmations. | ||
| /// | ||
| /// [`ANTI_REORG_DELAY`]: lightning::chain::channelmonitor::ANTI_REORG_DELAY | ||
| OnchainPaymentSuccessful { | ||
| /// A local identifier used to track the payment. | ||
| payment_id: PaymentId, | ||
| /// The transaction identifier. | ||
| txid: Txid, | ||
| /// The value, in thousandths of a satoshi, that was sent. | ||
| amount_msat: u64, | ||
|
Comment on lines
+287
to
+288
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. Should we include a field for the fees paid? |
||
| /// The hash of the block in which the transaction was confirmed. | ||
| block_hash: BlockHash, | ||
| /// The height at which the block was confirmed. | ||
|
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. The height of the block in which the transaction was confirmed. |
||
| block_height: u32, | ||
| }, | ||
| /// An on-chain payment has been received. | ||
| /// | ||
| /// This is only emitted for wallet transactions which were not classified as channel | ||
| /// funding, splices, closes, sweeps, or other LDK-driven chain activity. | ||
| /// | ||
| /// It's guaranteed to have reached at least [`ANTI_REORG_DELAY`] confirmations. | ||
| /// | ||
| /// [`ANTI_REORG_DELAY`]: lightning::chain::channelmonitor::ANTI_REORG_DELAY | ||
| OnchainPaymentReceived { | ||
| /// A local identifier used to track the payment. | ||
| payment_id: PaymentId, | ||
| /// The transaction identifier. | ||
| txid: Txid, | ||
| /// The value, in thousandths of a satoshi, that has been received. | ||
| amount_msat: u64, | ||
| /// The hash of the block in which the transaction was confirmed. | ||
| block_hash: BlockHash, | ||
| /// The height at which the block was confirmed. | ||
|
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. The height of the block in which the transaction was confirmed. |
||
| block_height: u32, | ||
| }, | ||
| /// A channel splice has been negotiated and the funding transaction is pending | ||
| /// confirmation on-chain. | ||
| SpliceNegotiated { | ||
|
|
@@ -374,6 +414,20 @@ impl_writeable_tlv_based_enum!(Event, | |
| (5, user_channel_id, required), | ||
| // TLV 7 (abandoned_funding_txo) may be set for LDK Node v0.7. | ||
| }, | ||
| (10, OnchainPaymentSuccessful) => { | ||
| (0, payment_id, required), | ||
| (2, txid, required), | ||
| (4, amount_msat, required), | ||
| (6, block_hash, required), | ||
| (8, block_height, required), | ||
| }, | ||
| (11, OnchainPaymentReceived) => { | ||
| (0, payment_id, required), | ||
| (2, txid, required), | ||
| (4, amount_msat, required), | ||
| (6, block_hash, required), | ||
| (8, block_height, required), | ||
| }, | ||
| ); | ||
|
|
||
| pub struct EventQueue<L: Deref> | ||
|
|
||
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.
How about testing
OnchainPaymentSuccessful?