Add ChannelReady event

[tnull]

Closes#1394.

This adds a ChannelReady event that will be emitted as soon as a new
channel becomes usable, i.e., after both sides have sent channel_ready.

[tnull]

Draft for now, since tests are still failing.

[tnull]

Rebased on main and reworked the approach: a bool is now bubbled up from Channel::check_get_channel_ready along side the msgs::ChannelReady and used in ChannelManager to decide whether to emit_channel_ready!.

In the second commit I included the discussed renaming of ChannelState::ChannelFunded to ChannelState::ChannelReady, and since all three align nicely then, I now reconsidered and named the event Event::ChannelReady after all.

Finally, the third commit includes a minor fix for unused import warnings.

Tests currently still failing, looking into it.

[tnull]

Fixed tests and rebased on main.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 84.03361% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.28%. Comparing base (ad7ff0b) to head (49dfcb6).
⚠️ Report is 7080 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/util/events.rs	28.57%	15 Missing ⚠️
lightning-background-processor/src/lib.rs	71.42%	2 Missing ⚠️
lightning/src/ln/channelmanager.rs	91.66%	1 Missing ⚠️
lightning/src/ln/functional_test_utils.rs	92.85%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #1743 +/- ##
==========================================
+ Coverage 90.73% 91.28% +0.55% 
==========================================
Files 87 87 Lines 47336 49835 +2499 Branches 47336 49835 +2499 ==========================================
+ Hits 42950 45494 +2544 + Misses 4386 4341 -45 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[wpaulino]

Feel free to squash.

[tnull]

I now reverted the 'bubble-up' approach after all since all required checks are conducted in the should_emit_channel_ready_event and send_channel_ready!, which makes this much less intrusive.

Squashed changes.

[dunxen]

Looks good. Just noticed we use "0conf" and "0-conf" in comments throughout the project. The hyphenated one looks "more correct". No biggie.

[TheBlueMatt]

I'm not sure I understand why we have to move this up?

[tnull]

Ah, we want to move this because the PaymentForwarded event is emitted by the handle_update_fulfill_htlc already and then the expect_channel_ready_event call in create_announced_chan_between_nodes fails due to 2 events being queued.

[TheBlueMatt]

I think this is good - can you squash the fixups into the appropriate commits? Its somewhat hard to re-review when the fixups are all at the end rather than with the commit they'll eventually get squashed into.

[tnull]

I think this is good - can you squash the fixups into the appropriate commits? Its somewhat hard to re-review when the fixups are all at the end rather than with the commit they'll eventually get squashed into.

Think all fixups belonged to the first commit => Done.

[TheBlueMatt]

Is it worth having a release notes entry for this that says that "no ChannelReady events will be generated for existing channels, including those which become ready on 0.0.113"? I don't think its worth having complicated logic for whether we default channel_ready_event_emitted to true or false based on state, but I also think it may be worth calling out.

[TheBlueMatt]

Either way, current code LGTM, probably fine to squash given the reviewers currently. You can leave a diff-tree of the fixups if you want.

[tnull]

Squashed fixups and included the pending changelog message

> git diff-tree -U2 a8720b2 49dfcb6
diff --git a/pending_changelog/1743.txt b/pending_changelog/1743.txt
new file mode 100644
index 00000000..93e41202
--- /dev/null
+++ b/pending_changelog/1743.txt
@@ -0,0 +1,7 @@
+## API Updates
+- A new `ChannelReady` event is generated whenever a channel becomes ready to
+ be used, i.e., after both sides sent the `channel_ready` message.
+
+## Backwards Compatibilty
+- No `ChannelReady` events will be generated for previously existing channels, including
+ those which become ready after upgrading 0.0.113.

[tnull]

Closes#1394.

This adds a ChannelReady event that will be emitted as soon as a new
channel becomes usable, i.e., after both sides have sent channel_ready.

[tnull]

Draft for now, since tests are still failing.

[tnull]

Rebased on main and reworked the approach: a bool is now bubbled up from Channel::check_get_channel_ready along side the msgs::ChannelReady and used in ChannelManager to decide whether to emit_channel_ready!.

In the second commit I included the discussed renaming of ChannelState::ChannelFunded to ChannelState::ChannelReady, and since all three align nicely then, I now reconsidered and named the event Event::ChannelReady after all.

Finally, the third commit includes a minor fix for unused import warnings.

Tests currently still failing, looking into it.

[tnull]

Fixed tests and rebased on main.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 84.03361% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.28%. Comparing base (ad7ff0b) to head (49dfcb6).
⚠️ Report is 7080 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/util/events.rs	28.57%	15 Missing ⚠️
lightning-background-processor/src/lib.rs	71.42%	2 Missing ⚠️
lightning/src/ln/channelmanager.rs	91.66%	1 Missing ⚠️
lightning/src/ln/functional_test_utils.rs	92.85%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #1743 +/- ##
==========================================
+ Coverage 90.73% 91.28% +0.55% 
==========================================
Files 87 87 Lines 47336 49835 +2499 Branches 47336 49835 +2499 ==========================================
+ Hits 42950 45494 +2544 + Misses 4386 4341 -45 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[wpaulino]

Feel free to squash.

[tnull]

I now reverted the 'bubble-up' approach after all since all required checks are conducted in the should_emit_channel_ready_event and send_channel_ready!, which makes this much less intrusive.

Squashed changes.

[dunxen]

Looks good. Just noticed we use "0conf" and "0-conf" in comments throughout the project. The hyphenated one looks "more correct". No biggie.

[TheBlueMatt]

I'm not sure I understand why we have to move this up?

[tnull]

Ah, we want to move this because the PaymentForwarded event is emitted by the handle_update_fulfill_htlc already and then the expect_channel_ready_event call in create_announced_chan_between_nodes fails due to 2 events being queued.

[TheBlueMatt]

I think this is good - can you squash the fixups into the appropriate commits? Its somewhat hard to re-review when the fixups are all at the end rather than with the commit they'll eventually get squashed into.

[tnull]

I think this is good - can you squash the fixups into the appropriate commits? Its somewhat hard to re-review when the fixups are all at the end rather than with the commit they'll eventually get squashed into.

Think all fixups belonged to the first commit => Done.

[TheBlueMatt]

Is it worth having a release notes entry for this that says that "no ChannelReady events will be generated for existing channels, including those which become ready on 0.0.113"? I don't think its worth having complicated logic for whether we default channel_ready_event_emitted to true or false based on state, but I also think it may be worth calling out.

[TheBlueMatt]

Either way, current code LGTM, probably fine to squash given the reviewers currently. You can leave a diff-tree of the fixups if you want.

[tnull]

Squashed fixups and included the pending changelog message

> git diff-tree -U2 a8720b2 49dfcb6
diff --git a/pending_changelog/1743.txt b/pending_changelog/1743.txt
new file mode 100644
index 00000000..93e41202
--- /dev/null
+++ b/pending_changelog/1743.txt
@@ -0,0 +1,7 @@
+## API Updates
+- A new `ChannelReady` event is generated whenever a channel becomes ready to
+ be used, i.e., after both sides sent the `channel_ready` message.
+
+## Backwards Compatibilty
+- No `ChannelReady` events will be generated for previously existing channels, including
+ those which become ready after upgrading 0.0.113.

[tnull]

Closes#1394.

This adds a ChannelReady event that will be emitted as soon as a new
channel becomes usable, i.e., after both sides have sent channel_ready.

[tnull]

Draft for now, since tests are still failing.

[tnull]

Rebased on main and reworked the approach: a bool is now bubbled up from Channel::check_get_channel_ready along side the msgs::ChannelReady and used in ChannelManager to decide whether to emit_channel_ready!.

In the second commit I included the discussed renaming of ChannelState::ChannelFunded to ChannelState::ChannelReady, and since all three align nicely then, I now reconsidered and named the event Event::ChannelReady after all.

Finally, the third commit includes a minor fix for unused import warnings.

Tests currently still failing, looking into it.

[tnull]

Fixed tests and rebased on main.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 84.03361% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.28%. Comparing base (ad7ff0b) to head (49dfcb6).
⚠️ Report is 7080 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/util/events.rs	28.57%	15 Missing ⚠️
lightning-background-processor/src/lib.rs	71.42%	2 Missing ⚠️
lightning/src/ln/channelmanager.rs	91.66%	1 Missing ⚠️
lightning/src/ln/functional_test_utils.rs	92.85%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #1743 +/- ##
==========================================
+ Coverage 90.73% 91.28% +0.55% 
==========================================
Files 87 87 Lines 47336 49835 +2499 Branches 47336 49835 +2499 ==========================================
+ Hits 42950 45494 +2544 + Misses 4386 4341 -45 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[wpaulino]

Feel free to squash.

[tnull]

I now reverted the 'bubble-up' approach after all since all required checks are conducted in the should_emit_channel_ready_event and send_channel_ready!, which makes this much less intrusive.

Squashed changes.

[dunxen]

Looks good. Just noticed we use "0conf" and "0-conf" in comments throughout the project. The hyphenated one looks "more correct". No biggie.

[TheBlueMatt]

I'm not sure I understand why we have to move this up?

[tnull]

Ah, we want to move this because the PaymentForwarded event is emitted by the handle_update_fulfill_htlc already and then the expect_channel_ready_event call in create_announced_chan_between_nodes fails due to 2 events being queued.

[TheBlueMatt]

I think this is good - can you squash the fixups into the appropriate commits? Its somewhat hard to re-review when the fixups are all at the end rather than with the commit they'll eventually get squashed into.

[tnull]

I think this is good - can you squash the fixups into the appropriate commits? Its somewhat hard to re-review when the fixups are all at the end rather than with the commit they'll eventually get squashed into.

Think all fixups belonged to the first commit => Done.

[TheBlueMatt]

Is it worth having a release notes entry for this that says that "no ChannelReady events will be generated for existing channels, including those which become ready on 0.0.113"? I don't think its worth having complicated logic for whether we default channel_ready_event_emitted to true or false based on state, but I also think it may be worth calling out.

[TheBlueMatt]

Either way, current code LGTM, probably fine to squash given the reviewers currently. You can leave a diff-tree of the fixups if you want.

[tnull]

Squashed fixups and included the pending changelog message

> git diff-tree -U2 a8720b2 49dfcb6
diff --git a/pending_changelog/1743.txt b/pending_changelog/1743.txt
new file mode 100644
index 00000000..93e41202
--- /dev/null
+++ b/pending_changelog/1743.txt
@@ -0,0 +1,7 @@
+## API Updates
+- A new `ChannelReady` event is generated whenever a channel becomes ready to
+ be used, i.e., after both sides sent the `channel_ready` message.
+
+## Backwards Compatibilty
+- No `ChannelReady` events will be generated for previously existing channels, including
+ those which become ready after upgrading 0.0.113.

[tnull]

Closes#1394.

This adds a ChannelReady event that will be emitted as soon as a new
channel becomes usable, i.e., after both sides have sent channel_ready.

[tnull]

Draft for now, since tests are still failing.

[tnull]

Rebased on main and reworked the approach: a bool is now bubbled up from Channel::check_get_channel_ready along side the msgs::ChannelReady and used in ChannelManager to decide whether to emit_channel_ready!.

In the second commit I included the discussed renaming of ChannelState::ChannelFunded to ChannelState::ChannelReady, and since all three align nicely then, I now reconsidered and named the event Event::ChannelReady after all.

Finally, the third commit includes a minor fix for unused import warnings.

Tests currently still failing, looking into it.

[tnull]

Fixed tests and rebased on main.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 84.03361% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.28%. Comparing base (ad7ff0b) to head (49dfcb6).
⚠️ Report is 7080 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/util/events.rs	28.57%	15 Missing ⚠️
lightning-background-processor/src/lib.rs	71.42%	2 Missing ⚠️
lightning/src/ln/channelmanager.rs	91.66%	1 Missing ⚠️
lightning/src/ln/functional_test_utils.rs	92.85%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #1743 +/- ##
==========================================
+ Coverage 90.73% 91.28% +0.55% 
==========================================
Files 87 87 Lines 47336 49835 +2499 Branches 47336 49835 +2499 ==========================================
+ Hits 42950 45494 +2544 + Misses 4386 4341 -45 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[wpaulino]

Feel free to squash.

[tnull]

I now reverted the 'bubble-up' approach after all since all required checks are conducted in the should_emit_channel_ready_event and send_channel_ready!, which makes this much less intrusive.

Squashed changes.

[dunxen]

Looks good. Just noticed we use "0conf" and "0-conf" in comments throughout the project. The hyphenated one looks "more correct". No biggie.

[TheBlueMatt]

I'm not sure I understand why we have to move this up?

[tnull]

Ah, we want to move this because the PaymentForwarded event is emitted by the handle_update_fulfill_htlc already and then the expect_channel_ready_event call in create_announced_chan_between_nodes fails due to 2 events being queued.

[TheBlueMatt]

I think this is good - can you squash the fixups into the appropriate commits? Its somewhat hard to re-review when the fixups are all at the end rather than with the commit they'll eventually get squashed into.

[tnull]

I think this is good - can you squash the fixups into the appropriate commits? Its somewhat hard to re-review when the fixups are all at the end rather than with the commit they'll eventually get squashed into.

Think all fixups belonged to the first commit => Done.

[TheBlueMatt]

Is it worth having a release notes entry for this that says that "no ChannelReady events will be generated for existing channels, including those which become ready on 0.0.113"? I don't think its worth having complicated logic for whether we default channel_ready_event_emitted to true or false based on state, but I also think it may be worth calling out.

[TheBlueMatt]

Either way, current code LGTM, probably fine to squash given the reviewers currently. You can leave a diff-tree of the fixups if you want.

[tnull]

Squashed fixups and included the pending changelog message

> git diff-tree -U2 a8720b2 49dfcb6
diff --git a/pending_changelog/1743.txt b/pending_changelog/1743.txt
new file mode 100644
index 00000000..93e41202
--- /dev/null
+++ b/pending_changelog/1743.txt
@@ -0,0 +1,7 @@
+## API Updates
+- A new `ChannelReady` event is generated whenever a channel becomes ready to
+ be used, i.e., after both sides sent the `channel_ready` message.
+
+## Backwards Compatibilty
+- No `ChannelReady` events will be generated for previously existing channels, including
+ those which become ready after upgrading 0.0.113.

[tnull]

Closes#1394.

This adds a ChannelReady event that will be emitted as soon as a new
channel becomes usable, i.e., after both sides have sent channel_ready.

[tnull]

Draft for now, since tests are still failing.

[tnull]

Rebased on main and reworked the approach: a bool is now bubbled up from Channel::check_get_channel_ready along side the msgs::ChannelReady and used in ChannelManager to decide whether to emit_channel_ready!.

In the second commit I included the discussed renaming of ChannelState::ChannelFunded to ChannelState::ChannelReady, and since all three align nicely then, I now reconsidered and named the event Event::ChannelReady after all.

Finally, the third commit includes a minor fix for unused import warnings.

Tests currently still failing, looking into it.

[tnull]

Fixed tests and rebased on main.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 84.03361% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.28%. Comparing base (ad7ff0b) to head (49dfcb6).
⚠️ Report is 7080 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/util/events.rs	28.57%	15 Missing ⚠️
lightning-background-processor/src/lib.rs	71.42%	2 Missing ⚠️
lightning/src/ln/channelmanager.rs	91.66%	1 Missing ⚠️
lightning/src/ln/functional_test_utils.rs	92.85%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #1743 +/- ##
==========================================
+ Coverage 90.73% 91.28% +0.55% 
==========================================
Files 87 87 Lines 47336 49835 +2499 Branches 47336 49835 +2499 ==========================================
+ Hits 42950 45494 +2544 + Misses 4386 4341 -45 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[wpaulino]

Feel free to squash.

[tnull]

I now reverted the 'bubble-up' approach after all since all required checks are conducted in the should_emit_channel_ready_event and send_channel_ready!, which makes this much less intrusive.

Squashed changes.

[dunxen]

Looks good. Just noticed we use "0conf" and "0-conf" in comments throughout the project. The hyphenated one looks "more correct". No biggie.

[TheBlueMatt]

I'm not sure I understand why we have to move this up?

[tnull]

Ah, we want to move this because the PaymentForwarded event is emitted by the handle_update_fulfill_htlc already and then the expect_channel_ready_event call in create_announced_chan_between_nodes fails due to 2 events being queued.

[TheBlueMatt]

I think this is good - can you squash the fixups into the appropriate commits? Its somewhat hard to re-review when the fixups are all at the end rather than with the commit they'll eventually get squashed into.

[tnull]

I think this is good - can you squash the fixups into the appropriate commits? Its somewhat hard to re-review when the fixups are all at the end rather than with the commit they'll eventually get squashed into.

Think all fixups belonged to the first commit => Done.

[TheBlueMatt]

Is it worth having a release notes entry for this that says that "no ChannelReady events will be generated for existing channels, including those which become ready on 0.0.113"? I don't think its worth having complicated logic for whether we default channel_ready_event_emitted to true or false based on state, but I also think it may be worth calling out.

[TheBlueMatt]

Either way, current code LGTM, probably fine to squash given the reviewers currently. You can leave a diff-tree of the fixups if you want.

[tnull]

Squashed fixups and included the pending changelog message

> git diff-tree -U2 a8720b2 49dfcb6
diff --git a/pending_changelog/1743.txt b/pending_changelog/1743.txt
new file mode 100644
index 00000000..93e41202
--- /dev/null
+++ b/pending_changelog/1743.txt
@@ -0,0 +1,7 @@
+## API Updates
+- A new `ChannelReady` event is generated whenever a channel becomes ready to
+ be used, i.e., after both sides sent the `channel_ready` message.
+
+## Backwards Compatibilty
+- No `ChannelReady` events will be generated for previously existing channels, including
+ those which become ready after upgrading 0.0.113.

[tnull]

Closes#1394.

This adds a ChannelReady event that will be emitted as soon as a new
channel becomes usable, i.e., after both sides have sent channel_ready.

[tnull]

Draft for now, since tests are still failing.

[tnull]

Rebased on main and reworked the approach: a bool is now bubbled up from Channel::check_get_channel_ready along side the msgs::ChannelReady and used in ChannelManager to decide whether to emit_channel_ready!.

In the second commit I included the discussed renaming of ChannelState::ChannelFunded to ChannelState::ChannelReady, and since all three align nicely then, I now reconsidered and named the event Event::ChannelReady after all.

Finally, the third commit includes a minor fix for unused import warnings.

Tests currently still failing, looking into it.

[tnull]

Fixed tests and rebased on main.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 84.03361% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.28%. Comparing base (ad7ff0b) to head (49dfcb6).
⚠️ Report is 7080 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/util/events.rs	28.57%	15 Missing ⚠️
lightning-background-processor/src/lib.rs	71.42%	2 Missing ⚠️
lightning/src/ln/channelmanager.rs	91.66%	1 Missing ⚠️
lightning/src/ln/functional_test_utils.rs	92.85%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #1743 +/- ##
==========================================
+ Coverage 90.73% 91.28% +0.55% 
==========================================
Files 87 87 Lines 47336 49835 +2499 Branches 47336 49835 +2499 ==========================================
+ Hits 42950 45494 +2544 + Misses 4386 4341 -45 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[wpaulino]

Feel free to squash.

[tnull]

I now reverted the 'bubble-up' approach after all since all required checks are conducted in the should_emit_channel_ready_event and send_channel_ready!, which makes this much less intrusive.

Squashed changes.

[dunxen]

Looks good. Just noticed we use "0conf" and "0-conf" in comments throughout the project. The hyphenated one looks "more correct". No biggie.

[TheBlueMatt]

I'm not sure I understand why we have to move this up?

[tnull]

Ah, we want to move this because the PaymentForwarded event is emitted by the handle_update_fulfill_htlc already and then the expect_channel_ready_event call in create_announced_chan_between_nodes fails due to 2 events being queued.

[TheBlueMatt]

I think this is good - can you squash the fixups into the appropriate commits? Its somewhat hard to re-review when the fixups are all at the end rather than with the commit they'll eventually get squashed into.

[tnull]

I think this is good - can you squash the fixups into the appropriate commits? Its somewhat hard to re-review when the fixups are all at the end rather than with the commit they'll eventually get squashed into.

Think all fixups belonged to the first commit => Done.

[TheBlueMatt]

Is it worth having a release notes entry for this that says that "no ChannelReady events will be generated for existing channels, including those which become ready on 0.0.113"? I don't think its worth having complicated logic for whether we default channel_ready_event_emitted to true or false based on state, but I also think it may be worth calling out.

[TheBlueMatt]

Either way, current code LGTM, probably fine to squash given the reviewers currently. You can leave a diff-tree of the fixups if you want.

[tnull]

Squashed fixups and included the pending changelog message

> git diff-tree -U2 a8720b2 49dfcb6
diff --git a/pending_changelog/1743.txt b/pending_changelog/1743.txt
new file mode 100644
index 00000000..93e41202
--- /dev/null
+++ b/pending_changelog/1743.txt
@@ -0,0 +1,7 @@
+## API Updates
+- A new `ChannelReady` event is generated whenever a channel becomes ready to
+ be used, i.e., after both sides sent the `channel_ready` message.
+
+## Backwards Compatibilty
+- No `ChannelReady` events will be generated for previously existing channels, including
+ those which become ready after upgrading 0.0.113.

[tnull]

Closes#1394.

This adds a ChannelReady event that will be emitted as soon as a new
channel becomes usable, i.e., after both sides have sent channel_ready.

[tnull]

Draft for now, since tests are still failing.

[tnull]

Rebased on main and reworked the approach: a bool is now bubbled up from Channel::check_get_channel_ready along side the msgs::ChannelReady and used in ChannelManager to decide whether to emit_channel_ready!.

In the second commit I included the discussed renaming of ChannelState::ChannelFunded to ChannelState::ChannelReady, and since all three align nicely then, I now reconsidered and named the event Event::ChannelReady after all.

Finally, the third commit includes a minor fix for unused import warnings.

Tests currently still failing, looking into it.

[tnull]

Fixed tests and rebased on main.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 84.03361% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.28%. Comparing base (ad7ff0b) to head (49dfcb6).
⚠️ Report is 7080 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/util/events.rs	28.57%	15 Missing ⚠️
lightning-background-processor/src/lib.rs	71.42%	2 Missing ⚠️
lightning/src/ln/channelmanager.rs	91.66%	1 Missing ⚠️
lightning/src/ln/functional_test_utils.rs	92.85%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #1743 +/- ##
==========================================
+ Coverage 90.73% 91.28% +0.55% 
==========================================
Files 87 87 Lines 47336 49835 +2499 Branches 47336 49835 +2499 ==========================================
+ Hits 42950 45494 +2544 + Misses 4386 4341 -45 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[wpaulino]

Feel free to squash.

[tnull]

I now reverted the 'bubble-up' approach after all since all required checks are conducted in the should_emit_channel_ready_event and send_channel_ready!, which makes this much less intrusive.

Squashed changes.

[dunxen]

Looks good. Just noticed we use "0conf" and "0-conf" in comments throughout the project. The hyphenated one looks "more correct". No biggie.

[TheBlueMatt]

I'm not sure I understand why we have to move this up?

[tnull]

Ah, we want to move this because the PaymentForwarded event is emitted by the handle_update_fulfill_htlc already and then the expect_channel_ready_event call in create_announced_chan_between_nodes fails due to 2 events being queued.

[TheBlueMatt]

I think this is good - can you squash the fixups into the appropriate commits? Its somewhat hard to re-review when the fixups are all at the end rather than with the commit they'll eventually get squashed into.

[tnull]

I think this is good - can you squash the fixups into the appropriate commits? Its somewhat hard to re-review when the fixups are all at the end rather than with the commit they'll eventually get squashed into.

Think all fixups belonged to the first commit => Done.

[TheBlueMatt]

Is it worth having a release notes entry for this that says that "no ChannelReady events will be generated for existing channels, including those which become ready on 0.0.113"? I don't think its worth having complicated logic for whether we default channel_ready_event_emitted to true or false based on state, but I also think it may be worth calling out.

[TheBlueMatt]

Either way, current code LGTM, probably fine to squash given the reviewers currently. You can leave a diff-tree of the fixups if you want.

[tnull]

Squashed fixups and included the pending changelog message

> git diff-tree -U2 a8720b2 49dfcb6
diff --git a/pending_changelog/1743.txt b/pending_changelog/1743.txt
new file mode 100644
index 00000000..93e41202
--- /dev/null
+++ b/pending_changelog/1743.txt
@@ -0,0 +1,7 @@
+## API Updates
+- A new `ChannelReady` event is generated whenever a channel becomes ready to
+ be used, i.e., after both sides sent the `channel_ready` message.
+
+## Backwards Compatibilty
+- No `ChannelReady` events will be generated for previously existing channels, including
+ those which become ready after upgrading 0.0.113.

[tnull]

Closes#1394.

This adds a ChannelReady event that will be emitted as soon as a new
channel becomes usable, i.e., after both sides have sent channel_ready.

[tnull]

Draft for now, since tests are still failing.

[tnull]

Rebased on main and reworked the approach: a bool is now bubbled up from Channel::check_get_channel_ready along side the msgs::ChannelReady and used in ChannelManager to decide whether to emit_channel_ready!.

In the second commit I included the discussed renaming of ChannelState::ChannelFunded to ChannelState::ChannelReady, and since all three align nicely then, I now reconsidered and named the event Event::ChannelReady after all.

Finally, the third commit includes a minor fix for unused import warnings.

Tests currently still failing, looking into it.

[tnull]

Fixed tests and rebased on main.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 84.03361% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.28%. Comparing base (ad7ff0b) to head (49dfcb6).
⚠️ Report is 7080 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/util/events.rs	28.57%	15 Missing ⚠️
lightning-background-processor/src/lib.rs	71.42%	2 Missing ⚠️
lightning/src/ln/channelmanager.rs	91.66%	1 Missing ⚠️
lightning/src/ln/functional_test_utils.rs	92.85%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #1743 +/- ##
==========================================
+ Coverage 90.73% 91.28% +0.55% 
==========================================
Files 87 87 Lines 47336 49835 +2499 Branches 47336 49835 +2499 ==========================================
+ Hits 42950 45494 +2544 + Misses 4386 4341 -45 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[wpaulino]

Feel free to squash.

[tnull]

I now reverted the 'bubble-up' approach after all since all required checks are conducted in the should_emit_channel_ready_event and send_channel_ready!, which makes this much less intrusive.

Squashed changes.

[dunxen]

Looks good. Just noticed we use "0conf" and "0-conf" in comments throughout the project. The hyphenated one looks "more correct". No biggie.

[TheBlueMatt]

I'm not sure I understand why we have to move this up?

[tnull]

Ah, we want to move this because the PaymentForwarded event is emitted by the handle_update_fulfill_htlc already and then the expect_channel_ready_event call in create_announced_chan_between_nodes fails due to 2 events being queued.

[TheBlueMatt]

I think this is good - can you squash the fixups into the appropriate commits? Its somewhat hard to re-review when the fixups are all at the end rather than with the commit they'll eventually get squashed into.

[tnull]

I think this is good - can you squash the fixups into the appropriate commits? Its somewhat hard to re-review when the fixups are all at the end rather than with the commit they'll eventually get squashed into.

Think all fixups belonged to the first commit => Done.

[TheBlueMatt]

Is it worth having a release notes entry for this that says that "no ChannelReady events will be generated for existing channels, including those which become ready on 0.0.113"? I don't think its worth having complicated logic for whether we default channel_ready_event_emitted to true or false based on state, but I also think it may be worth calling out.

[TheBlueMatt]

Either way, current code LGTM, probably fine to squash given the reviewers currently. You can leave a diff-tree of the fixups if you want.

[tnull]

Squashed fixups and included the pending changelog message

> git diff-tree -U2 a8720b2 49dfcb6
diff --git a/pending_changelog/1743.txt b/pending_changelog/1743.txt
new file mode 100644
index 00000000..93e41202
--- /dev/null
+++ b/pending_changelog/1743.txt
@@ -0,0 +1,7 @@
+## API Updates
+- A new `ChannelReady` event is generated whenever a channel becomes ready to
+ be used, i.e., after both sides sent the `channel_ready` message.
+
+## Backwards Compatibilty
+- No `ChannelReady` events will be generated for previously existing channels, including
+ those which become ready after upgrading 0.0.113.