Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port back changes from lightning experiment branch #227

Merged
merged 1 commit into from
Jul 11, 2024

Conversation

Tibo-lg
Copy link
Contributor

@Tibo-lg Tibo-lg commented Jun 7, 2024

This PR brings back many fixes and changes that were made for the lightning experimental branch to master.

@Tibo-lg Tibo-lg requested a review from luckysori June 7, 2024 04:45
@Tibo-lg Tibo-lg force-pushed the chore/port-back-lightning-changes branch 2 times, most recently from 84ce95b to a280669 Compare June 7, 2024 05:03
Comment on lines -22 to -24
watched_tx: HashMap<Txid, ChannelInfo>,
pub(crate) watched_tx: HashMap<Txid, WatchState>,
pub(crate) watched_txo: HashMap<OutPoint, WatchState>,
pub(crate) last_height: u64,
pub(crate) last_block_hashes: Vec<BlockHash>,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A reminder that this is a breaking change in case you want to communicate it somehow.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess there are a bunch of breaking changes with this PR anyway.

Comment on lines +126 to +131
/// Check if any watched transactions are part of the block, confirming them if so.
///
/// # Panics
///
/// Panics if the new block's height is not exactly one more than the last processed height.
pub(crate) fn process_block(&mut self, block: &Block, height: u64) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, in our fork we went a little bit further:

  1. c3f8850.
  2. 1010f95.
  3. e0588e8.

But I'm not sure we can easily apply these patches because the Blockchain trait could no longer be implemented for ElectrsBlockchainProvider and BitcoinCoreProvider.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevertheless, it might be worth looking at those changes after merging. I know you wanted to change how we monitor the blockchain anyway, so maybe only 1010f95 is relevant.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think better to take that later, but thanks for the pointers!

Comment on lines 1734 to 1735
counter_buffer_adaptor_signature: *accept_buffer_adaptor_signature,
own_buffer_adaptor_signature: *offer_buffer_adaptor_signature,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We fixed this bug in our branch: 0191dc4.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks fixed!

@Tibo-lg Tibo-lg force-pushed the chore/port-back-lightning-changes branch from a280669 to 4ff8528 Compare July 10, 2024 06:37
Copy link
Collaborator

@luckysori luckysori left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@Tibo-lg Tibo-lg force-pushed the chore/port-back-lightning-changes branch from 4ff8528 to a88ea41 Compare July 11, 2024 00:09
@Tibo-lg Tibo-lg merged commit 8667a13 into master Jul 11, 2024
73 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants