Handle fallible commitment secret - #3152

Merged
TheBlueMatt merged 2 commits into
lightningdevkit:mainfrom
alecchendev:2024-06-async-commit-secret-raa
Jul 18, 2024
Merged

Handle fallible commitment secret#3152
TheBlueMatt merged 2 commits into
lightningdevkit:mainfrom
alecchendev:2024-06-async-commit-secret-raa

Conversation

@alecchendev

@alecchendevalecchendev commented Jul 2, 2024

Copy link
Copy Markdown
Contributor

Builds on #3149, #3150. Most of the logic for this was added in the prerequisite PRs. Here we just set the signer_pending_revoke_and_ack flag if we fail to get the commitment secret.

@codecov-commenter

codecov-commenter commented Jul 2, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 55.55556% with 12 lines in your changes missing coverage. Please review.

Project coverage is 90.08%. Comparing base (6ed398d) to head (3413032).
Report is 28 commits behind head on main.

FilesPatch %Lines
lightning/src/ln/channel.rs31.25%11 Missing ⚠️
lightning/src/util/test_channel_signer.rs80.00%0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #3152 +/- ##
==========================================
+ Coverage 89.83% 90.08% +0.25% 
==========================================
Files 121 121 Lines 100612 102971 +2359 Branches 100612 102971 +2359 ==========================================
+ Hits 90380 92766 +2386 - Misses 7564 7571 +7 + Partials 2668 2634 -34 

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

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch 2 times, most recently from cf058bc to 18acc2aCompareJuly 10, 2024 01:22
@TheBlueMatt

Copy link
Copy Markdown
Collaborator

This needs rebase now 🎉

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch from 18acc2a to e296b74CompareJuly 16, 2024 23:23
@alecchendev

Copy link
Copy Markdown
ContributorAuthor

Rebased! Btw, for the missing test coverage in #3150, I'll get to those in the PR where I handle fallible per commitment point for the channel reestablish case (as opposed to here), since I think those cases will require that - hoping to make that PR this week

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch 2 times, most recently from 50df53a to e4ad15bCompareJuly 16, 2024 23:38
@TheBlueMatt

Copy link
Copy Markdown
Collaborator

CI is very, very sad.

@TheBlueMatt

Copy link
Copy Markdown
Collaborator

Otherwise basically LGTM, nice and easy PR :)

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch from e4ad15b to 3413032CompareJuly 17, 2024 17:58
@alecchendev

Copy link
Copy Markdown
ContributorAuthor

lol yesterday i thought i fixed the CI error but didn't commit anything and did an empty force push 😆

@valentinewallacevalentinewallace left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just nits, feel free to land!

//! Tests for asynchronous signing. These tests verify that the channel state machine behaves
//! properly with a signer implementation that asynchronously derives signatures.

use std::collections::HashSet;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: in most files we have a use crate::prelude::*;, which I believe would avoid this

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah, right, think that means this file will fail to build no-std, though we can also fix that when we drop the cfg flag.

Comment threadlightning/src/ln/channel.rs
Comment on lines +5535 to +5536
log_trace!(logger, "Last revoke-and-ack pending in channel {} for sequence {} because the next per-commitment point is not available",
&self.context.channel_id(), self.context.holder_commitment_point.transaction_number());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: I think this log is redundant with the ones above

@TheBlueMatt

TheBlueMatt commented Jul 18, 2024

Copy link
Copy Markdown
Collaborator

Gonna land, @alecchendev can handle the followups in #3109.

@TheBlueMatt
TheBlueMatt merged commit 9ce3dd5 into lightningdevkit:mainJul 18, 2024
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Jul 19, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Jul 20, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 16, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 16, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 16, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 17, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 18, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Nov 26, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Nov 26, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 6, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 10, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 11, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 11, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 13, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 13, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
valentinewallace pushed a commit to valentinewallace/rust-lightning that referenced this pull request Jan 6, 2025
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
valentinewallace pushed a commit to valentinewallace/rust-lightning that referenced this pull request Jan 6, 2025
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
Sign up for freeto 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.

4 participants

@alecchendev@codecov-commenter@TheBlueMatt@valentinewallace
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Handle fallible commitment secret - #3152

Merged
TheBlueMatt merged 2 commits into
lightningdevkit:mainfrom
alecchendev:2024-06-async-commit-secret-raa
Jul 18, 2024
Merged

Handle fallible commitment secret#3152
TheBlueMatt merged 2 commits into
lightningdevkit:mainfrom
alecchendev:2024-06-async-commit-secret-raa

Conversation

@alecchendev

@alecchendevalecchendev commented Jul 2, 2024

Copy link
Copy Markdown
Contributor

Builds on #3149, #3150. Most of the logic for this was added in the prerequisite PRs. Here we just set the signer_pending_revoke_and_ack flag if we fail to get the commitment secret.

@codecov-commenter

codecov-commenter commented Jul 2, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 55.55556% with 12 lines in your changes missing coverage. Please review.

Project coverage is 90.08%. Comparing base (6ed398d) to head (3413032).
Report is 28 commits behind head on main.

FilesPatch %Lines
lightning/src/ln/channel.rs31.25%11 Missing ⚠️
lightning/src/util/test_channel_signer.rs80.00%0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #3152 +/- ##
==========================================
+ Coverage 89.83% 90.08% +0.25% 
==========================================
Files 121 121 Lines 100612 102971 +2359 Branches 100612 102971 +2359 ==========================================
+ Hits 90380 92766 +2386 - Misses 7564 7571 +7 + Partials 2668 2634 -34 

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

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch 2 times, most recently from cf058bc to 18acc2aCompareJuly 10, 2024 01:22
@TheBlueMatt

Copy link
Copy Markdown
Collaborator

This needs rebase now 🎉

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch from 18acc2a to e296b74CompareJuly 16, 2024 23:23
@alecchendev

Copy link
Copy Markdown
ContributorAuthor

Rebased! Btw, for the missing test coverage in #3150, I'll get to those in the PR where I handle fallible per commitment point for the channel reestablish case (as opposed to here), since I think those cases will require that - hoping to make that PR this week

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch 2 times, most recently from 50df53a to e4ad15bCompareJuly 16, 2024 23:38
@TheBlueMatt

Copy link
Copy Markdown
Collaborator

CI is very, very sad.

@TheBlueMatt

Copy link
Copy Markdown
Collaborator

Otherwise basically LGTM, nice and easy PR :)

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch from e4ad15b to 3413032CompareJuly 17, 2024 17:58
@alecchendev

Copy link
Copy Markdown
ContributorAuthor

lol yesterday i thought i fixed the CI error but didn't commit anything and did an empty force push 😆

@valentinewallacevalentinewallace left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just nits, feel free to land!

//! Tests for asynchronous signing. These tests verify that the channel state machine behaves
//! properly with a signer implementation that asynchronously derives signatures.

use std::collections::HashSet;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: in most files we have a use crate::prelude::*;, which I believe would avoid this

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah, right, think that means this file will fail to build no-std, though we can also fix that when we drop the cfg flag.

Comment threadlightning/src/ln/channel.rs
Comment on lines +5535 to +5536
log_trace!(logger, "Last revoke-and-ack pending in channel {} for sequence {} because the next per-commitment point is not available",
&self.context.channel_id(), self.context.holder_commitment_point.transaction_number());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: I think this log is redundant with the ones above

@TheBlueMatt

TheBlueMatt commented Jul 18, 2024

Copy link
Copy Markdown
Collaborator

Gonna land, @alecchendev can handle the followups in #3109.

@TheBlueMatt
TheBlueMatt merged commit 9ce3dd5 into lightningdevkit:mainJul 18, 2024
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Jul 19, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Jul 20, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 16, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 16, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 16, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 17, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 18, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Nov 26, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Nov 26, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 6, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 10, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 11, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 11, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 13, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 13, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
valentinewallace pushed a commit to valentinewallace/rust-lightning that referenced this pull request Jan 6, 2025
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
valentinewallace pushed a commit to valentinewallace/rust-lightning that referenced this pull request Jan 6, 2025
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
Sign up for freeto 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.

4 participants

@alecchendev@codecov-commenter@TheBlueMatt@valentinewallace
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Handle fallible commitment secret - #3152

Merged
TheBlueMatt merged 2 commits into
lightningdevkit:mainfrom
alecchendev:2024-06-async-commit-secret-raa
Jul 18, 2024
Merged

Handle fallible commitment secret#3152
TheBlueMatt merged 2 commits into
lightningdevkit:mainfrom
alecchendev:2024-06-async-commit-secret-raa

Conversation

@alecchendev

@alecchendevalecchendev commented Jul 2, 2024

Copy link
Copy Markdown
Contributor

Builds on #3149, #3150. Most of the logic for this was added in the prerequisite PRs. Here we just set the signer_pending_revoke_and_ack flag if we fail to get the commitment secret.

@codecov-commenter

codecov-commenter commented Jul 2, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 55.55556% with 12 lines in your changes missing coverage. Please review.

Project coverage is 90.08%. Comparing base (6ed398d) to head (3413032).
Report is 28 commits behind head on main.

FilesPatch %Lines
lightning/src/ln/channel.rs31.25%11 Missing ⚠️
lightning/src/util/test_channel_signer.rs80.00%0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #3152 +/- ##
==========================================
+ Coverage 89.83% 90.08% +0.25% 
==========================================
Files 121 121 Lines 100612 102971 +2359 Branches 100612 102971 +2359 ==========================================
+ Hits 90380 92766 +2386 - Misses 7564 7571 +7 + Partials 2668 2634 -34 

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

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch 2 times, most recently from cf058bc to 18acc2aCompareJuly 10, 2024 01:22
@TheBlueMatt

Copy link
Copy Markdown
Collaborator

This needs rebase now 🎉

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch from 18acc2a to e296b74CompareJuly 16, 2024 23:23
@alecchendev

Copy link
Copy Markdown
ContributorAuthor

Rebased! Btw, for the missing test coverage in #3150, I'll get to those in the PR where I handle fallible per commitment point for the channel reestablish case (as opposed to here), since I think those cases will require that - hoping to make that PR this week

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch 2 times, most recently from 50df53a to e4ad15bCompareJuly 16, 2024 23:38
@TheBlueMatt

Copy link
Copy Markdown
Collaborator

CI is very, very sad.

@TheBlueMatt

Copy link
Copy Markdown
Collaborator

Otherwise basically LGTM, nice and easy PR :)

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch from e4ad15b to 3413032CompareJuly 17, 2024 17:58
@alecchendev

Copy link
Copy Markdown
ContributorAuthor

lol yesterday i thought i fixed the CI error but didn't commit anything and did an empty force push 😆

@valentinewallacevalentinewallace left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just nits, feel free to land!

//! Tests for asynchronous signing. These tests verify that the channel state machine behaves
//! properly with a signer implementation that asynchronously derives signatures.

use std::collections::HashSet;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: in most files we have a use crate::prelude::*;, which I believe would avoid this

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah, right, think that means this file will fail to build no-std, though we can also fix that when we drop the cfg flag.

Comment threadlightning/src/ln/channel.rs
Comment on lines +5535 to +5536
log_trace!(logger, "Last revoke-and-ack pending in channel {} for sequence {} because the next per-commitment point is not available",
&self.context.channel_id(), self.context.holder_commitment_point.transaction_number());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: I think this log is redundant with the ones above

@TheBlueMatt

TheBlueMatt commented Jul 18, 2024

Copy link
Copy Markdown
Collaborator

Gonna land, @alecchendev can handle the followups in #3109.

@TheBlueMatt
TheBlueMatt merged commit 9ce3dd5 into lightningdevkit:mainJul 18, 2024
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Jul 19, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Jul 20, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 16, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 16, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 16, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 17, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 18, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Nov 26, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Nov 26, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 6, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 10, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 11, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 11, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 13, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 13, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
valentinewallace pushed a commit to valentinewallace/rust-lightning that referenced this pull request Jan 6, 2025
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
valentinewallace pushed a commit to valentinewallace/rust-lightning that referenced this pull request Jan 6, 2025
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
Sign up for freeto 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.

4 participants

@alecchendev@codecov-commenter@TheBlueMatt@valentinewallace
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Handle fallible commitment secret - #3152

Merged
TheBlueMatt merged 2 commits into
lightningdevkit:mainfrom
alecchendev:2024-06-async-commit-secret-raa
Jul 18, 2024
Merged

Handle fallible commitment secret#3152
TheBlueMatt merged 2 commits into
lightningdevkit:mainfrom
alecchendev:2024-06-async-commit-secret-raa

Conversation

@alecchendev

@alecchendevalecchendev commented Jul 2, 2024

Copy link
Copy Markdown
Contributor

Builds on #3149, #3150. Most of the logic for this was added in the prerequisite PRs. Here we just set the signer_pending_revoke_and_ack flag if we fail to get the commitment secret.

@codecov-commenter

codecov-commenter commented Jul 2, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 55.55556% with 12 lines in your changes missing coverage. Please review.

Project coverage is 90.08%. Comparing base (6ed398d) to head (3413032).
Report is 28 commits behind head on main.

FilesPatch %Lines
lightning/src/ln/channel.rs31.25%11 Missing ⚠️
lightning/src/util/test_channel_signer.rs80.00%0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #3152 +/- ##
==========================================
+ Coverage 89.83% 90.08% +0.25% 
==========================================
Files 121 121 Lines 100612 102971 +2359 Branches 100612 102971 +2359 ==========================================
+ Hits 90380 92766 +2386 - Misses 7564 7571 +7 + Partials 2668 2634 -34 

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

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch 2 times, most recently from cf058bc to 18acc2aCompareJuly 10, 2024 01:22
@TheBlueMatt

Copy link
Copy Markdown
Collaborator

This needs rebase now 🎉

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch from 18acc2a to e296b74CompareJuly 16, 2024 23:23
@alecchendev

Copy link
Copy Markdown
ContributorAuthor

Rebased! Btw, for the missing test coverage in #3150, I'll get to those in the PR where I handle fallible per commitment point for the channel reestablish case (as opposed to here), since I think those cases will require that - hoping to make that PR this week

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch 2 times, most recently from 50df53a to e4ad15bCompareJuly 16, 2024 23:38
@TheBlueMatt

Copy link
Copy Markdown
Collaborator

CI is very, very sad.

@TheBlueMatt

Copy link
Copy Markdown
Collaborator

Otherwise basically LGTM, nice and easy PR :)

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch from e4ad15b to 3413032CompareJuly 17, 2024 17:58
@alecchendev

Copy link
Copy Markdown
ContributorAuthor

lol yesterday i thought i fixed the CI error but didn't commit anything and did an empty force push 😆

@valentinewallacevalentinewallace left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just nits, feel free to land!

//! Tests for asynchronous signing. These tests verify that the channel state machine behaves
//! properly with a signer implementation that asynchronously derives signatures.

use std::collections::HashSet;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: in most files we have a use crate::prelude::*;, which I believe would avoid this

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah, right, think that means this file will fail to build no-std, though we can also fix that when we drop the cfg flag.

Comment threadlightning/src/ln/channel.rs
Comment on lines +5535 to +5536
log_trace!(logger, "Last revoke-and-ack pending in channel {} for sequence {} because the next per-commitment point is not available",
&self.context.channel_id(), self.context.holder_commitment_point.transaction_number());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: I think this log is redundant with the ones above

@TheBlueMatt

TheBlueMatt commented Jul 18, 2024

Copy link
Copy Markdown
Collaborator

Gonna land, @alecchendev can handle the followups in #3109.

@TheBlueMatt
TheBlueMatt merged commit 9ce3dd5 into lightningdevkit:mainJul 18, 2024
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Jul 19, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Jul 20, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 16, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 16, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 16, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 17, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 18, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Nov 26, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Nov 26, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 6, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 10, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 11, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 11, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 13, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 13, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
valentinewallace pushed a commit to valentinewallace/rust-lightning that referenced this pull request Jan 6, 2025
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
valentinewallace pushed a commit to valentinewallace/rust-lightning that referenced this pull request Jan 6, 2025
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
Sign up for freeto 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.

4 participants

@alecchendev@codecov-commenter@TheBlueMatt@valentinewallace
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Handle fallible commitment secret - #3152

Merged
TheBlueMatt merged 2 commits into
lightningdevkit:mainfrom
alecchendev:2024-06-async-commit-secret-raa
Jul 18, 2024
Merged

Handle fallible commitment secret#3152
TheBlueMatt merged 2 commits into
lightningdevkit:mainfrom
alecchendev:2024-06-async-commit-secret-raa

Conversation

@alecchendev

@alecchendevalecchendev commented Jul 2, 2024

Copy link
Copy Markdown
Contributor

Builds on #3149, #3150. Most of the logic for this was added in the prerequisite PRs. Here we just set the signer_pending_revoke_and_ack flag if we fail to get the commitment secret.

@codecov-commenter

codecov-commenter commented Jul 2, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 55.55556% with 12 lines in your changes missing coverage. Please review.

Project coverage is 90.08%. Comparing base (6ed398d) to head (3413032).
Report is 28 commits behind head on main.

FilesPatch %Lines
lightning/src/ln/channel.rs31.25%11 Missing ⚠️
lightning/src/util/test_channel_signer.rs80.00%0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #3152 +/- ##
==========================================
+ Coverage 89.83% 90.08% +0.25% 
==========================================
Files 121 121 Lines 100612 102971 +2359 Branches 100612 102971 +2359 ==========================================
+ Hits 90380 92766 +2386 - Misses 7564 7571 +7 + Partials 2668 2634 -34 

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

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch 2 times, most recently from cf058bc to 18acc2aCompareJuly 10, 2024 01:22
@TheBlueMatt

Copy link
Copy Markdown
Collaborator

This needs rebase now 🎉

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch from 18acc2a to e296b74CompareJuly 16, 2024 23:23
@alecchendev

Copy link
Copy Markdown
ContributorAuthor

Rebased! Btw, for the missing test coverage in #3150, I'll get to those in the PR where I handle fallible per commitment point for the channel reestablish case (as opposed to here), since I think those cases will require that - hoping to make that PR this week

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch 2 times, most recently from 50df53a to e4ad15bCompareJuly 16, 2024 23:38
@TheBlueMatt

Copy link
Copy Markdown
Collaborator

CI is very, very sad.

@TheBlueMatt

Copy link
Copy Markdown
Collaborator

Otherwise basically LGTM, nice and easy PR :)

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch from e4ad15b to 3413032CompareJuly 17, 2024 17:58
@alecchendev

Copy link
Copy Markdown
ContributorAuthor

lol yesterday i thought i fixed the CI error but didn't commit anything and did an empty force push 😆

@valentinewallacevalentinewallace left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just nits, feel free to land!

//! Tests for asynchronous signing. These tests verify that the channel state machine behaves
//! properly with a signer implementation that asynchronously derives signatures.

use std::collections::HashSet;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: in most files we have a use crate::prelude::*;, which I believe would avoid this

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah, right, think that means this file will fail to build no-std, though we can also fix that when we drop the cfg flag.

Comment threadlightning/src/ln/channel.rs
Comment on lines +5535 to +5536
log_trace!(logger, "Last revoke-and-ack pending in channel {} for sequence {} because the next per-commitment point is not available",
&self.context.channel_id(), self.context.holder_commitment_point.transaction_number());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: I think this log is redundant with the ones above

@TheBlueMatt

TheBlueMatt commented Jul 18, 2024

Copy link
Copy Markdown
Collaborator

Gonna land, @alecchendev can handle the followups in #3109.

@TheBlueMatt
TheBlueMatt merged commit 9ce3dd5 into lightningdevkit:mainJul 18, 2024
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Jul 19, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Jul 20, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 16, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 16, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 16, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 17, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 18, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Nov 26, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Nov 26, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 6, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 10, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 11, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 11, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 13, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 13, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
valentinewallace pushed a commit to valentinewallace/rust-lightning that referenced this pull request Jan 6, 2025
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
valentinewallace pushed a commit to valentinewallace/rust-lightning that referenced this pull request Jan 6, 2025
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
Sign up for freeto 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.

4 participants

@alecchendev@codecov-commenter@TheBlueMatt@valentinewallace
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Handle fallible commitment secret - #3152

Merged
TheBlueMatt merged 2 commits into
lightningdevkit:mainfrom
alecchendev:2024-06-async-commit-secret-raa
Jul 18, 2024
Merged

Handle fallible commitment secret#3152
TheBlueMatt merged 2 commits into
lightningdevkit:mainfrom
alecchendev:2024-06-async-commit-secret-raa

Conversation

@alecchendev

@alecchendevalecchendev commented Jul 2, 2024

Copy link
Copy Markdown
Contributor

Builds on #3149, #3150. Most of the logic for this was added in the prerequisite PRs. Here we just set the signer_pending_revoke_and_ack flag if we fail to get the commitment secret.

@codecov-commenter

codecov-commenter commented Jul 2, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 55.55556% with 12 lines in your changes missing coverage. Please review.

Project coverage is 90.08%. Comparing base (6ed398d) to head (3413032).
Report is 28 commits behind head on main.

FilesPatch %Lines
lightning/src/ln/channel.rs31.25%11 Missing ⚠️
lightning/src/util/test_channel_signer.rs80.00%0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #3152 +/- ##
==========================================
+ Coverage 89.83% 90.08% +0.25% 
==========================================
Files 121 121 Lines 100612 102971 +2359 Branches 100612 102971 +2359 ==========================================
+ Hits 90380 92766 +2386 - Misses 7564 7571 +7 + Partials 2668 2634 -34 

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

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch 2 times, most recently from cf058bc to 18acc2aCompareJuly 10, 2024 01:22
@TheBlueMatt

Copy link
Copy Markdown
Collaborator

This needs rebase now 🎉

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch from 18acc2a to e296b74CompareJuly 16, 2024 23:23
@alecchendev

Copy link
Copy Markdown
ContributorAuthor

Rebased! Btw, for the missing test coverage in #3150, I'll get to those in the PR where I handle fallible per commitment point for the channel reestablish case (as opposed to here), since I think those cases will require that - hoping to make that PR this week

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch 2 times, most recently from 50df53a to e4ad15bCompareJuly 16, 2024 23:38
@TheBlueMatt

Copy link
Copy Markdown
Collaborator

CI is very, very sad.

@TheBlueMatt

Copy link
Copy Markdown
Collaborator

Otherwise basically LGTM, nice and easy PR :)

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch from e4ad15b to 3413032CompareJuly 17, 2024 17:58
@alecchendev

Copy link
Copy Markdown
ContributorAuthor

lol yesterday i thought i fixed the CI error but didn't commit anything and did an empty force push 😆

@valentinewallacevalentinewallace left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just nits, feel free to land!

//! Tests for asynchronous signing. These tests verify that the channel state machine behaves
//! properly with a signer implementation that asynchronously derives signatures.

use std::collections::HashSet;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: in most files we have a use crate::prelude::*;, which I believe would avoid this

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah, right, think that means this file will fail to build no-std, though we can also fix that when we drop the cfg flag.

Comment threadlightning/src/ln/channel.rs
Comment on lines +5535 to +5536
log_trace!(logger, "Last revoke-and-ack pending in channel {} for sequence {} because the next per-commitment point is not available",
&self.context.channel_id(), self.context.holder_commitment_point.transaction_number());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: I think this log is redundant with the ones above

@TheBlueMatt

TheBlueMatt commented Jul 18, 2024

Copy link
Copy Markdown
Collaborator

Gonna land, @alecchendev can handle the followups in #3109.

@TheBlueMatt
TheBlueMatt merged commit 9ce3dd5 into lightningdevkit:mainJul 18, 2024
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Jul 19, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Jul 20, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 16, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 16, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 16, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 17, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 18, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Nov 26, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Nov 26, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 6, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 10, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 11, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 11, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 13, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 13, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
valentinewallace pushed a commit to valentinewallace/rust-lightning that referenced this pull request Jan 6, 2025
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
valentinewallace pushed a commit to valentinewallace/rust-lightning that referenced this pull request Jan 6, 2025
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
Sign up for freeto 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.

4 participants

@alecchendev@codecov-commenter@TheBlueMatt@valentinewallace
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Handle fallible commitment secret - #3152

Merged
TheBlueMatt merged 2 commits into
lightningdevkit:mainfrom
alecchendev:2024-06-async-commit-secret-raa
Jul 18, 2024
Merged

Handle fallible commitment secret#3152
TheBlueMatt merged 2 commits into
lightningdevkit:mainfrom
alecchendev:2024-06-async-commit-secret-raa

Conversation

@alecchendev

@alecchendevalecchendev commented Jul 2, 2024

Copy link
Copy Markdown
Contributor

Builds on #3149, #3150. Most of the logic for this was added in the prerequisite PRs. Here we just set the signer_pending_revoke_and_ack flag if we fail to get the commitment secret.

@codecov-commenter

codecov-commenter commented Jul 2, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 55.55556% with 12 lines in your changes missing coverage. Please review.

Project coverage is 90.08%. Comparing base (6ed398d) to head (3413032).
Report is 28 commits behind head on main.

FilesPatch %Lines
lightning/src/ln/channel.rs31.25%11 Missing ⚠️
lightning/src/util/test_channel_signer.rs80.00%0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #3152 +/- ##
==========================================
+ Coverage 89.83% 90.08% +0.25% 
==========================================
Files 121 121 Lines 100612 102971 +2359 Branches 100612 102971 +2359 ==========================================
+ Hits 90380 92766 +2386 - Misses 7564 7571 +7 + Partials 2668 2634 -34 

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

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch 2 times, most recently from cf058bc to 18acc2aCompareJuly 10, 2024 01:22
@TheBlueMatt

Copy link
Copy Markdown
Collaborator

This needs rebase now 🎉

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch from 18acc2a to e296b74CompareJuly 16, 2024 23:23
@alecchendev

Copy link
Copy Markdown
ContributorAuthor

Rebased! Btw, for the missing test coverage in #3150, I'll get to those in the PR where I handle fallible per commitment point for the channel reestablish case (as opposed to here), since I think those cases will require that - hoping to make that PR this week

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch 2 times, most recently from 50df53a to e4ad15bCompareJuly 16, 2024 23:38
@TheBlueMatt

Copy link
Copy Markdown
Collaborator

CI is very, very sad.

@TheBlueMatt

Copy link
Copy Markdown
Collaborator

Otherwise basically LGTM, nice and easy PR :)

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch from e4ad15b to 3413032CompareJuly 17, 2024 17:58
@alecchendev

Copy link
Copy Markdown
ContributorAuthor

lol yesterday i thought i fixed the CI error but didn't commit anything and did an empty force push 😆

@valentinewallacevalentinewallace left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just nits, feel free to land!

//! Tests for asynchronous signing. These tests verify that the channel state machine behaves
//! properly with a signer implementation that asynchronously derives signatures.

use std::collections::HashSet;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: in most files we have a use crate::prelude::*;, which I believe would avoid this

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah, right, think that means this file will fail to build no-std, though we can also fix that when we drop the cfg flag.

Comment threadlightning/src/ln/channel.rs
Comment on lines +5535 to +5536
log_trace!(logger, "Last revoke-and-ack pending in channel {} for sequence {} because the next per-commitment point is not available",
&self.context.channel_id(), self.context.holder_commitment_point.transaction_number());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: I think this log is redundant with the ones above

@TheBlueMatt

TheBlueMatt commented Jul 18, 2024

Copy link
Copy Markdown
Collaborator

Gonna land, @alecchendev can handle the followups in #3109.

@TheBlueMatt
TheBlueMatt merged commit 9ce3dd5 into lightningdevkit:mainJul 18, 2024
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Jul 19, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Jul 20, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 16, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 16, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 16, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 17, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 18, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Nov 26, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Nov 26, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 6, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 10, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 11, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 11, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 13, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 13, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
valentinewallace pushed a commit to valentinewallace/rust-lightning that referenced this pull request Jan 6, 2025
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
valentinewallace pushed a commit to valentinewallace/rust-lightning that referenced this pull request Jan 6, 2025
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
Sign up for freeto 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.

4 participants

@alecchendev@codecov-commenter@TheBlueMatt@valentinewallace
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Handle fallible commitment secret - #3152

Merged
TheBlueMatt merged 2 commits into
lightningdevkit:mainfrom
alecchendev:2024-06-async-commit-secret-raa
Jul 18, 2024
Merged

Handle fallible commitment secret#3152
TheBlueMatt merged 2 commits into
lightningdevkit:mainfrom
alecchendev:2024-06-async-commit-secret-raa

Conversation

@alecchendev

@alecchendevalecchendev commented Jul 2, 2024

Copy link
Copy Markdown
Contributor

Builds on #3149, #3150. Most of the logic for this was added in the prerequisite PRs. Here we just set the signer_pending_revoke_and_ack flag if we fail to get the commitment secret.

@codecov-commenter

codecov-commenter commented Jul 2, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 55.55556% with 12 lines in your changes missing coverage. Please review.

Project coverage is 90.08%. Comparing base (6ed398d) to head (3413032).
Report is 28 commits behind head on main.

FilesPatch %Lines
lightning/src/ln/channel.rs31.25%11 Missing ⚠️
lightning/src/util/test_channel_signer.rs80.00%0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #3152 +/- ##
==========================================
+ Coverage 89.83% 90.08% +0.25% 
==========================================
Files 121 121 Lines 100612 102971 +2359 Branches 100612 102971 +2359 ==========================================
+ Hits 90380 92766 +2386 - Misses 7564 7571 +7 + Partials 2668 2634 -34 

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

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch 2 times, most recently from cf058bc to 18acc2aCompareJuly 10, 2024 01:22
@TheBlueMatt

Copy link
Copy Markdown
Collaborator

This needs rebase now 🎉

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch from 18acc2a to e296b74CompareJuly 16, 2024 23:23
@alecchendev

Copy link
Copy Markdown
ContributorAuthor

Rebased! Btw, for the missing test coverage in #3150, I'll get to those in the PR where I handle fallible per commitment point for the channel reestablish case (as opposed to here), since I think those cases will require that - hoping to make that PR this week

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch 2 times, most recently from 50df53a to e4ad15bCompareJuly 16, 2024 23:38
@TheBlueMatt

Copy link
Copy Markdown
Collaborator

CI is very, very sad.

@TheBlueMatt

Copy link
Copy Markdown
Collaborator

Otherwise basically LGTM, nice and easy PR :)

@alecchendev
alecchendevforce-pushed the 2024-06-async-commit-secret-raa branch from e4ad15b to 3413032CompareJuly 17, 2024 17:58
@alecchendev

Copy link
Copy Markdown
ContributorAuthor

lol yesterday i thought i fixed the CI error but didn't commit anything and did an empty force push 😆

@valentinewallacevalentinewallace left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just nits, feel free to land!

//! Tests for asynchronous signing. These tests verify that the channel state machine behaves
//! properly with a signer implementation that asynchronously derives signatures.

use std::collections::HashSet;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: in most files we have a use crate::prelude::*;, which I believe would avoid this

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah, right, think that means this file will fail to build no-std, though we can also fix that when we drop the cfg flag.

Comment threadlightning/src/ln/channel.rs
Comment on lines +5535 to +5536
log_trace!(logger, "Last revoke-and-ack pending in channel {} for sequence {} because the next per-commitment point is not available",
&self.context.channel_id(), self.context.holder_commitment_point.transaction_number());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: I think this log is redundant with the ones above

@TheBlueMatt

TheBlueMatt commented Jul 18, 2024

Copy link
Copy Markdown
Collaborator

Gonna land, @alecchendev can handle the followups in #3109.

@TheBlueMatt
TheBlueMatt merged commit 9ce3dd5 into lightningdevkit:mainJul 18, 2024
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Jul 19, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Jul 20, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 16, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 16, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 16, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 17, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Sep 18, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Nov 26, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Nov 26, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 6, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 10, 2024
This also slightly refactors get_funding_signed_msg to match the logic
more similarly, as well as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 11, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 11, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 13, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
alecchendev added a commit to alecchendev/rust-lightning that referenced this pull request Dec 13, 2024
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
valentinewallace pushed a commit to valentinewallace/rust-lightning that referenced this pull request Jan 6, 2025
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
valentinewallace pushed a commit to valentinewallace/rust-lightning that referenced this pull request Jan 6, 2025
For all of our async signing logic in channel establishment v1, we set
signer flags in the method where we create the raw lightning message
object. To keep things consistent, this commit moves setting the signer
flags to where we create funding_created, since this was being set
elsewhere before.
While we're doing this cleanup, this also slightly refactors our
funding_signed method to move some code out of an indent, as well
as removes a log to fix a nit from lightningdevkit#3152.
Sign up for freeto 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.

4 participants

@alecchendev@codecov-commenter@TheBlueMatt@valentinewallace