Skip to content

Remove anchors config flag - #2367

Merged
wpaulino merged 1 commit into
lightningdevkit:mainfrom
wpaulino:remove-anchors-flag
Jun 23, 2023
Merged

Remove anchors config flag#2367
wpaulino merged 1 commit into
lightningdevkit:mainfrom
wpaulino:remove-anchors-flag

Conversation

@wpaulino

@wpaulinowpaulino commented Jun 20, 2023

Copy link
Copy Markdown
Contributor

Now that all of the core functionality for anchor outputs has landed, we're ready to remove the config flag that was temporarily hiding it from our API.

Depends on #2361.

@wpaulinowpaulino mentioned this pull request Jun 20, 2023
Merged
@wpaulinowpaulino added this to the 0.0.116 milestone Jun 20, 2023
@wpaulinowpaulino mentioned this pull request Jun 20, 2023
Open
10 tasks
@TheBlueMattTheBlueMatt modified the milestones: 0.0.116, 0.0.116alphaJun 20, 2023
@TheBlueMattTheBlueMatt mentioned this pull request Jun 20, 2023
Merged
@wpaulino
wpaulinoforce-pushed the remove-anchors-flag branch from f46a8ae to 1a3bad8CompareJune 23, 2023 19:16
@TheBlueMatt

TheBlueMatt commented Jun 23, 2023

Copy link
Copy Markdown
Collaborator

There's a few spurious anchors flags in the CI script, though they'll be ignored now.

TheBlueMatt
TheBlueMatt reviewed Jun 23, 2023
View reviewed changes
Comment threadlightning/src/ln/channelmanager.rs Outdated
TheBlueMatt
TheBlueMatt previously approved these changes Jun 23, 2023
View reviewed changes

@TheBlueMattTheBlueMatt left a comment

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.

Two nits but LGTM.

@wpaulino
wpaulinoforce-pushed the remove-anchors-flag branch from 1a3bad8 to d0d6490CompareJune 23, 2023 19:32
@wpaulino
wpaulinoforce-pushed the remove-anchors-flag branch from d0d6490 to 5cddb23CompareJune 23, 2023 19:33
TheBlueMatt
TheBlueMatt previously approved these changes Jun 23, 2023
View reviewed changes
valentinewallace
valentinewallace previously approved these changes Jun 23, 2023
View reviewed changes

@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.

12 warnings introduced

Comment threadlightning/src/events/bump_transaction.rs Outdated
Comment threadlightning/src/events/bump_transaction.rs
Comment threadlightning/src/chain/chainmonitor.rs
/// For channels featuring anchor outputs, this method will also process [`BumpTransaction`]
/// events produced from each [`ChannelMonitor`] while there is a balance to claim onchain
/// within each channel. As the confirmation of a commitment transaction may be critical to the
/// safety of funds, this method must be invoked frequently, ideally once for every chain tip

@valentinewallacevalentinewallaceJun 23, 2023

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.

Generally I'd prefer if the docs updates were in their own commit, don't care much here though

@wpaulino
Remove anchors config flag
82b646c
Now that all of the core functionality for anchor outputs has landed,
we're ready to remove the config flag that was temporarily hiding it
from our API.
@wpaulino
wpaulinoforce-pushed the remove-anchors-flag branch from 5cddb23 to 82b646cCompareJune 23, 2023 20:32
@valentinewallace

valentinewallace commented Jun 23, 2023

Copy link
Copy Markdown
Contributor

Would it be difficult to fix the warnings? Leaving them can make it easy to miss relevant warnings in local development when working off of main IMO

@wpaulino

wpaulino commented Jun 23, 2023

Copy link
Copy Markdown
ContributorAuthor

They weren't introduced by this PR, but rather by #2361.

@codecov-commenter

codecov-commenter commented Jun 23, 2023

Copy link
Copy Markdown

Codecov Report

Patch coverage: 91.11% and project coverage change: +0.01 🎉

Comparison is base (3973865) 90.30% compared to head (82b646c) 90.32%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@ Coverage Diff @@## main #2367 +/- ##
==========================================
+ Coverage 90.30% 90.32% +0.01% 
==========================================
Files 106 106 Lines 54900 54888 -12 Branches 54900 54888 -12 ==========================================
- Hits 49576 49575 -1 + Misses 5324 5313 -11
Impacted FilesCoverage Δ
lightning/src/chain/chainmonitor.rs94.50% <ø> (-0.10%)⬇️
lightning/src/chain/channelmonitor.rs94.72% <ø> (ø)
lightning/src/chain/package.rs91.43% <ø> (ø)
lightning/src/events/bump_transaction.rs41.55% <ø> (ø)
lightning/src/events/mod.rs41.86% <ø> (ø)
lightning/src/ln/chan_utils.rs94.51% <ø> (ø)
lightning/src/sign/mod.rs84.38% <ø> (+0.02%)⬆️
lightning/src/util/config.rs58.13% <ø> (+0.66%)⬆️
lightning/src/util/enforcing_trait_impls.rs88.00% <ø> (ø)
lightning/src/chain/onchaintx.rs92.72% <77.77%> (ø)
... and 3 more

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@wpaulino
wpaulino merged commit 6fd13d7 into lightningdevkit:mainJun 23, 2023
@wpaulino
wpaulino deleted the remove-anchors-flag branch June 23, 2023 23:17
@TheBlueMattTheBlueMatt modified the milestones: 0.0.116alpha, 0.0.116Jun 24, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TheBlueMattTheBlueMattTheBlueMatt approved these changes
@valentinewallacevalentinewallacevalentinewallace approved these changes

Assignees

No one assigned

Labels

Anchor Support

Projects

None yet

Milestone

0.0.116

Development

Successfully merging this pull request may close these issues.

4 participants

@wpaulino@TheBlueMatt@valentinewallace@codecov-commenter
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all 
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
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;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Remove anchors config flag by wpaulino · Pull Request #2367 · lightningdevkit/rust-lightning · GitHub
Skip to content
Remove anchors config flag - #2367
Merged
wpaulino merged 1 commit into
lightningdevkit:mainfrom 
wpaulino:remove-anchors-flag
Jun 23, 2023
Merged
Remove anchors config flag#2367
wpaulino merged 1 commit into
lightningdevkit:mainfrom 
wpaulino:remove-anchors-flag
Conversation
@wpaulino
@wpaulinowpaulino
commented
Jun 20, 2023

Copy link


Copy Markdown

Contributor
Now that all of the core functionality for anchor outputs has landed, we're ready to remove the config flag that was temporarily hiding it from our API.
Depends on #2361.
@wpaulinowpaulino
mentioned this pull request
Jun 20, 2023
 Merged

@wpaulinowpaulino
added this to the 0.0.116 milestone
Jun 20, 2023
@wpaulinowpaulino
mentioned this pull request
Jun 20, 2023

Open

10 tasks
@TheBlueMattTheBlueMatt
modified the milestones:
0.0.116, 0.0.116alphaJun 20, 2023
@TheBlueMattTheBlueMatt
mentioned this pull request
Jun 20, 2023
 Merged

@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
f46a8ae to
1a3bad8CompareJune 23, 2023 19:16
@TheBlueMatt
TheBlueMatt
commented
Jun 23, 2023

Copy link


Copy Markdown

Collaborator
There's a few spurious anchors flags in the CI script, though they'll be ignored now.
TheBlueMatt
TheBlueMatt
reviewed
Jun 23, 2023
View reviewed changes
Comment threadlightning/src/ln/channelmanager.rs
Outdated

TheBlueMatt
TheBlueMatt
previously approved these changes
Jun 23, 2023
View reviewed changes
@TheBlueMattTheBlueMatt
left a comment


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.

Two nits but LGTM.
@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
1a3bad8 to
d0d6490CompareJune 23, 2023 19:32
@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
d0d6490 to
5cddb23CompareJune 23, 2023 19:33
TheBlueMatt
TheBlueMatt
previously approved these changes
Jun 23, 2023
View reviewed changes
valentinewallace
valentinewallace
previously approved these changes
Jun 23, 2023
View reviewed changes
@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.

12 warnings introduced
Comment threadlightning/src/events/bump_transaction.rs
Outdated

Comment threadlightning/src/events/bump_transaction.rs
Comment threadlightning/src/chain/chainmonitor.rs
	/// For channels featuring anchor outputs, this method will also process [`BumpTransaction`]
	/// events produced from each [`ChannelMonitor`] while there is a balance to claim onchain
	/// within each channel. As the confirmation of a commitment transaction may be critical to the
	/// safety of funds, this method must be invoked frequently, ideally once for every chain tip
@valentinewallacevalentinewallaceJun 23, 2023

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.

Generally I'd prefer if the docs updates were in their own commit, don't care much here though
@wpaulino
Remove anchors config flag
82b646c
Now that all of the core functionality for anchor outputs has landed,
we're ready to remove the config flag that was temporarily hiding it
from our API.
@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
5cddb23 to
82b646cCompareJune 23, 2023 20:32
@valentinewallace
valentinewallace
commented
Jun 23, 2023

Copy link


Copy Markdown

Contributor
Would it be difficult to fix the warnings? Leaving them can make it easy to miss relevant warnings in local development when working off of main IMO
@wpaulino
wpaulino
commented
Jun 23, 2023

Copy link


Copy Markdown

ContributorAuthor
They weren't introduced by this PR, but rather by #2361.
@codecov-commenter
codecov-commenter
commented
Jun 23, 2023

Copy link


Copy Markdown

Codecov Report
Patch coverage: 91.11% and project coverage change: +0.01 🎉
Comparison is base (3973865) 90.30% compared to head (82b646c) 90.32%.
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.
Additional details and impacted files
@@ Coverage Diff @@## main #2367 +/- ##
==========================================
+ Coverage 90.30% 90.32% +0.01% 
==========================================
Files 106 106 Lines 54900 54888 -12 Branches 54900 54888 -12 ==========================================
- Hits 49576 49575 -1 + Misses 5324 5313 -11 
Impacted FilesCoverage Δ
lightning/src/chain/chainmonitor.rs94.50% <ø> (-0.10%)⬇️
lightning/src/chain/channelmonitor.rs94.72% <ø> (ø)
lightning/src/chain/package.rs91.43% <ø> (ø)
lightning/src/events/bump_transaction.rs41.55% <ø> (ø)
lightning/src/events/mod.rs41.86% <ø> (ø)
lightning/src/ln/chan_utils.rs94.51% <ø> (ø)
lightning/src/sign/mod.rs84.38% <ø> (+0.02%)⬆️
lightning/src/util/config.rs58.13% <ø> (+0.66%)⬆️
lightning/src/util/enforcing_trait_impls.rs88.00% <ø> (ø)
lightning/src/chain/onchaintx.rs92.72% <77.77%> (ø)
... and 3 more
... and 3 files with indirect coverage changes
☔ View full report in Codecov by Sentry.

📢 Do you have feedback about the report comment? Let us know in this issue.
@wpaulino
wpaulino
merged commit 6fd13d7
into
lightningdevkit:mainJun 23, 2023
@wpaulino
wpaulino
deleted the

remove-anchors-flag

branch
June 23, 2023 23:17
@TheBlueMattTheBlueMatt
modified the milestones:
0.0.116alpha, 0.0.116Jun 24, 2023
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@TheBlueMattTheBlueMattTheBlueMatt approved these changes
@valentinewallacevalentinewallacevalentinewallace approved these changes

Assignees


No one assigned


Labels

Anchor Support

Projects


None yet


Milestone

0.0.116

Development

Successfully merging this pull request may close these issues.

4 participants

@wpaulino@TheBlueMatt@valentinewallace@codecov-commenter

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Force GitHub README to respect dark mode
(function() {
var style = document.createElement('style');
style.textContent = '
.markdown-body {
color-scheme: dark light;
}
.markdown-body pre { background: #161b22 !important; }
.markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }
.markdown-body table th, .markdown-body table td { border-color: #30363d !important; }
.markdown-body img { background: #0d1117; }
.markdown-body blockquote { border-left-color: #8b949e; }
.markdown-body hr { border-color: #30363d; }
';
document.head.appendChild(style);
})();
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Remove anchors config flag by wpaulino · Pull Request #2367 · lightningdevkit/rust-lightning · GitHub
Skip to content
Remove anchors config flag - #2367
Merged
wpaulino merged 1 commit into
lightningdevkit:mainfrom 
wpaulino:remove-anchors-flag
Jun 23, 2023
Merged
Remove anchors config flag#2367
wpaulino merged 1 commit into
lightningdevkit:mainfrom 
wpaulino:remove-anchors-flag
Conversation
@wpaulino
@wpaulinowpaulino
commented
Jun 20, 2023

Copy link


Copy Markdown

Contributor
Now that all of the core functionality for anchor outputs has landed, we're ready to remove the config flag that was temporarily hiding it from our API.
Depends on #2361.
@wpaulinowpaulino
mentioned this pull request
Jun 20, 2023
 Merged

@wpaulinowpaulino
added this to the 0.0.116 milestone
Jun 20, 2023
@wpaulinowpaulino
mentioned this pull request
Jun 20, 2023

Open

10 tasks
@TheBlueMattTheBlueMatt
modified the milestones:
0.0.116, 0.0.116alphaJun 20, 2023
@TheBlueMattTheBlueMatt
mentioned this pull request
Jun 20, 2023
 Merged

@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
f46a8ae to
1a3bad8CompareJune 23, 2023 19:16
@TheBlueMatt
TheBlueMatt
commented
Jun 23, 2023

Copy link


Copy Markdown

Collaborator
There's a few spurious anchors flags in the CI script, though they'll be ignored now.
TheBlueMatt
TheBlueMatt
reviewed
Jun 23, 2023
View reviewed changes
Comment threadlightning/src/ln/channelmanager.rs
Outdated

TheBlueMatt
TheBlueMatt
previously approved these changes
Jun 23, 2023
View reviewed changes
@TheBlueMattTheBlueMatt
left a comment


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.

Two nits but LGTM.
@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
1a3bad8 to
d0d6490CompareJune 23, 2023 19:32
@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
d0d6490 to
5cddb23CompareJune 23, 2023 19:33
TheBlueMatt
TheBlueMatt
previously approved these changes
Jun 23, 2023
View reviewed changes
valentinewallace
valentinewallace
previously approved these changes
Jun 23, 2023
View reviewed changes
@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.

12 warnings introduced
Comment threadlightning/src/events/bump_transaction.rs
Outdated

Comment threadlightning/src/events/bump_transaction.rs
Comment threadlightning/src/chain/chainmonitor.rs
	/// For channels featuring anchor outputs, this method will also process [`BumpTransaction`]
	/// events produced from each [`ChannelMonitor`] while there is a balance to claim onchain
	/// within each channel. As the confirmation of a commitment transaction may be critical to the
	/// safety of funds, this method must be invoked frequently, ideally once for every chain tip
@valentinewallacevalentinewallaceJun 23, 2023

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.

Generally I'd prefer if the docs updates were in their own commit, don't care much here though
@wpaulino
Remove anchors config flag
82b646c
Now that all of the core functionality for anchor outputs has landed,
we're ready to remove the config flag that was temporarily hiding it
from our API.
@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
5cddb23 to
82b646cCompareJune 23, 2023 20:32
@valentinewallace
valentinewallace
commented
Jun 23, 2023

Copy link


Copy Markdown

Contributor
Would it be difficult to fix the warnings? Leaving them can make it easy to miss relevant warnings in local development when working off of main IMO
@wpaulino
wpaulino
commented
Jun 23, 2023

Copy link


Copy Markdown

ContributorAuthor
They weren't introduced by this PR, but rather by #2361.
@codecov-commenter
codecov-commenter
commented
Jun 23, 2023

Copy link


Copy Markdown

Codecov Report
Patch coverage: 91.11% and project coverage change: +0.01 🎉
Comparison is base (3973865) 90.30% compared to head (82b646c) 90.32%.
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.
Additional details and impacted files
@@ Coverage Diff @@## main #2367 +/- ##
==========================================
+ Coverage 90.30% 90.32% +0.01% 
==========================================
Files 106 106 Lines 54900 54888 -12 Branches 54900 54888 -12 ==========================================
- Hits 49576 49575 -1 + Misses 5324 5313 -11 
Impacted FilesCoverage Δ
lightning/src/chain/chainmonitor.rs94.50% <ø> (-0.10%)⬇️
lightning/src/chain/channelmonitor.rs94.72% <ø> (ø)
lightning/src/chain/package.rs91.43% <ø> (ø)
lightning/src/events/bump_transaction.rs41.55% <ø> (ø)
lightning/src/events/mod.rs41.86% <ø> (ø)
lightning/src/ln/chan_utils.rs94.51% <ø> (ø)
lightning/src/sign/mod.rs84.38% <ø> (+0.02%)⬆️
lightning/src/util/config.rs58.13% <ø> (+0.66%)⬆️
lightning/src/util/enforcing_trait_impls.rs88.00% <ø> (ø)
lightning/src/chain/onchaintx.rs92.72% <77.77%> (ø)
... and 3 more
... and 3 files with indirect coverage changes
☔ View full report in Codecov by Sentry.

📢 Do you have feedback about the report comment? Let us know in this issue.
@wpaulino
wpaulino
merged commit 6fd13d7
into
lightningdevkit:mainJun 23, 2023
@wpaulino
wpaulino
deleted the

remove-anchors-flag

branch
June 23, 2023 23:17
@TheBlueMattTheBlueMatt
modified the milestones:
0.0.116alpha, 0.0.116Jun 24, 2023
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@TheBlueMattTheBlueMattTheBlueMatt approved these changes
@valentinewallacevalentinewallacevalentinewallace approved these changes

Assignees


No one assigned


Labels

Anchor Support

Projects


None yet


Milestone

0.0.116

Development

Successfully merging this pull request may close these issues.

4 participants

@wpaulino@TheBlueMatt@valentinewallace@codecov-commenter

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

Copy link


Copy Markdown

Contributor
Now that all of the core functionality for anchor outputs has landed, we're ready to remove the config flag that was temporarily hiding it from our API.
Depends on #2361.
@wpaulinowpaulino
mentioned this pull request
Jun 20, 2023
 Merged

@wpaulinowpaulino
added this to the 0.0.116 milestone
Jun 20, 2023
@wpaulinowpaulino
mentioned this pull request
Jun 20, 2023

Open

10 tasks
@TheBlueMattTheBlueMatt
modified the milestones:
0.0.116, 0.0.116alphaJun 20, 2023
@TheBlueMattTheBlueMatt
mentioned this pull request
Jun 20, 2023
 Merged

@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
f46a8ae to
1a3bad8CompareJune 23, 2023 19:16
@TheBlueMatt
TheBlueMatt
commented
Jun 23, 2023

Copy link


Copy Markdown

Collaborator
There's a few spurious anchors flags in the CI script, though they'll be ignored now.
TheBlueMatt
TheBlueMatt
reviewed
Jun 23, 2023
View reviewed changes
Comment threadlightning/src/ln/channelmanager.rs
Outdated

TheBlueMatt
TheBlueMatt
previously approved these changes
Jun 23, 2023
View reviewed changes
@TheBlueMattTheBlueMatt
left a comment


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.

Two nits but LGTM.
@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
1a3bad8 to
d0d6490CompareJune 23, 2023 19:32
@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
d0d6490 to
5cddb23CompareJune 23, 2023 19:33
TheBlueMatt
TheBlueMatt
previously approved these changes
Jun 23, 2023
View reviewed changes
valentinewallace
valentinewallace
previously approved these changes
Jun 23, 2023
View reviewed changes
@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.

12 warnings introduced
Comment threadlightning/src/events/bump_transaction.rs
Outdated

Comment threadlightning/src/events/bump_transaction.rs
Comment threadlightning/src/chain/chainmonitor.rs
	/// For channels featuring anchor outputs, this method will also process [`BumpTransaction`]
	/// events produced from each [`ChannelMonitor`] while there is a balance to claim onchain
	/// within each channel. As the confirmation of a commitment transaction may be critical to the
	/// safety of funds, this method must be invoked frequently, ideally once for every chain tip
@valentinewallacevalentinewallaceJun 23, 2023

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.

Generally I'd prefer if the docs updates were in their own commit, don't care much here though
@wpaulino
Remove anchors config flag
82b646c
Now that all of the core functionality for anchor outputs has landed,
we're ready to remove the config flag that was temporarily hiding it
from our API.
@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
5cddb23 to
82b646cCompareJune 23, 2023 20:32
@valentinewallace
valentinewallace
commented
Jun 23, 2023

Copy link


Copy Markdown

Contributor
Would it be difficult to fix the warnings? Leaving them can make it easy to miss relevant warnings in local development when working off of main IMO
@wpaulino
wpaulino
commented
Jun 23, 2023

Copy link


Copy Markdown

ContributorAuthor
They weren't introduced by this PR, but rather by #2361.
@codecov-commenter
codecov-commenter
commented
Jun 23, 2023

Copy link


Copy Markdown

Codecov Report
Patch coverage: 91.11% and project coverage change: +0.01 🎉
Comparison is base (3973865) 90.30% compared to head (82b646c) 90.32%.
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.
Additional details and impacted files
@@ Coverage Diff @@## main #2367 +/- ##
==========================================
+ Coverage 90.30% 90.32% +0.01% 
==========================================
Files 106 106 Lines 54900 54888 -12 Branches 54900 54888 -12 ==========================================
- Hits 49576 49575 -1 + Misses 5324 5313 -11 
Impacted FilesCoverage Δ
lightning/src/chain/chainmonitor.rs94.50% <ø> (-0.10%)⬇️
lightning/src/chain/channelmonitor.rs94.72% <ø> (ø)
lightning/src/chain/package.rs91.43% <ø> (ø)
lightning/src/events/bump_transaction.rs41.55% <ø> (ø)
lightning/src/events/mod.rs41.86% <ø> (ø)
lightning/src/ln/chan_utils.rs94.51% <ø> (ø)
lightning/src/sign/mod.rs84.38% <ø> (+0.02%)⬆️
lightning/src/util/config.rs58.13% <ø> (+0.66%)⬆️
lightning/src/util/enforcing_trait_impls.rs88.00% <ø> (ø)
lightning/src/chain/onchaintx.rs92.72% <77.77%> (ø)
... and 3 more
... and 3 files with indirect coverage changes
☔ View full report in Codecov by Sentry.

📢 Do you have feedback about the report comment? Let us know in this issue.
@wpaulino
wpaulino
merged commit 6fd13d7
into
lightningdevkit:mainJun 23, 2023
@wpaulino
wpaulino
deleted the

remove-anchors-flag

branch
June 23, 2023 23:17
@TheBlueMattTheBlueMatt
modified the milestones:
0.0.116alpha, 0.0.116Jun 24, 2023
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@TheBlueMattTheBlueMattTheBlueMatt approved these changes
@valentinewallacevalentinewallacevalentinewallace approved these changes

Assignees


No one assigned


Labels

Anchor Support

Projects


None yet


Milestone

0.0.116

Development

Successfully merging this pull request may close these issues.

4 participants

@wpaulino@TheBlueMatt@valentinewallace@codecov-commenter

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Strip utm_, fbclid, gclid, etc. from all links on page
(function() {
var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',
'ref', 'ref_src', 'source', 'medium', 'campaign'];
function cleanUrl(url) {
try {
var u = new URL(url, window.location.origin);
var changed = false;
trackingParams.forEach(function(p) {
if (u.searchParams.has(p)) {
u.searchParams.delete(p);
changed = true;
}
});
return changed ? u.toString() : url;
} catch (e) {
return url;
}
}
function cleanLinks() {
document.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
cleanLinks();
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1) {
if (node.tagName === 'A') cleanLinks();
node.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '
Remove anchors config flag by wpaulino · Pull Request #2367 · lightningdevkit/rust-lightning · GitHub
Skip to content
Remove anchors config flag - #2367
Merged
wpaulino merged 1 commit into
lightningdevkit:mainfrom 
wpaulino:remove-anchors-flag
Jun 23, 2023
Merged
Remove anchors config flag#2367
wpaulino merged 1 commit into
lightningdevkit:mainfrom 
wpaulino:remove-anchors-flag
Conversation
@wpaulino
@wpaulinowpaulino
commented
Jun 20, 2023

Copy link


Copy Markdown

Contributor
Now that all of the core functionality for anchor outputs has landed, we're ready to remove the config flag that was temporarily hiding it from our API.
Depends on #2361.
@wpaulinowpaulino
mentioned this pull request
Jun 20, 2023
 Merged

@wpaulinowpaulino
added this to the 0.0.116 milestone
Jun 20, 2023
@wpaulinowpaulino
mentioned this pull request
Jun 20, 2023

Open

10 tasks
@TheBlueMattTheBlueMatt
modified the milestones:
0.0.116, 0.0.116alphaJun 20, 2023
@TheBlueMattTheBlueMatt
mentioned this pull request
Jun 20, 2023
 Merged

@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
f46a8ae to
1a3bad8CompareJune 23, 2023 19:16
@TheBlueMatt
TheBlueMatt
commented
Jun 23, 2023

Copy link


Copy Markdown

Collaborator
There's a few spurious anchors flags in the CI script, though they'll be ignored now.
TheBlueMatt
TheBlueMatt
reviewed
Jun 23, 2023
View reviewed changes
Comment threadlightning/src/ln/channelmanager.rs
Outdated

TheBlueMatt
TheBlueMatt
previously approved these changes
Jun 23, 2023
View reviewed changes
@TheBlueMattTheBlueMatt
left a comment


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.

Two nits but LGTM.
@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
1a3bad8 to
d0d6490CompareJune 23, 2023 19:32
@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
d0d6490 to
5cddb23CompareJune 23, 2023 19:33
TheBlueMatt
TheBlueMatt
previously approved these changes
Jun 23, 2023
View reviewed changes
valentinewallace
valentinewallace
previously approved these changes
Jun 23, 2023
View reviewed changes
@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.

12 warnings introduced
Comment threadlightning/src/events/bump_transaction.rs
Outdated

Comment threadlightning/src/events/bump_transaction.rs
Comment threadlightning/src/chain/chainmonitor.rs
	/// For channels featuring anchor outputs, this method will also process [`BumpTransaction`]
	/// events produced from each [`ChannelMonitor`] while there is a balance to claim onchain
	/// within each channel. As the confirmation of a commitment transaction may be critical to the
	/// safety of funds, this method must be invoked frequently, ideally once for every chain tip
@valentinewallacevalentinewallaceJun 23, 2023

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.

Generally I'd prefer if the docs updates were in their own commit, don't care much here though
@wpaulino
Remove anchors config flag
82b646c
Now that all of the core functionality for anchor outputs has landed,
we're ready to remove the config flag that was temporarily hiding it
from our API.
@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
5cddb23 to
82b646cCompareJune 23, 2023 20:32
@valentinewallace
valentinewallace
commented
Jun 23, 2023

Copy link


Copy Markdown

Contributor
Would it be difficult to fix the warnings? Leaving them can make it easy to miss relevant warnings in local development when working off of main IMO
@wpaulino
wpaulino
commented
Jun 23, 2023

Copy link


Copy Markdown

ContributorAuthor
They weren't introduced by this PR, but rather by #2361.
@codecov-commenter
codecov-commenter
commented
Jun 23, 2023

Copy link


Copy Markdown

Codecov Report
Patch coverage: 91.11% and project coverage change: +0.01 🎉
Comparison is base (3973865) 90.30% compared to head (82b646c) 90.32%.
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.
Additional details and impacted files
@@ Coverage Diff @@## main #2367 +/- ##
==========================================
+ Coverage 90.30% 90.32% +0.01% 
==========================================
Files 106 106 Lines 54900 54888 -12 Branches 54900 54888 -12 ==========================================
- Hits 49576 49575 -1 + Misses 5324 5313 -11 
Impacted FilesCoverage Δ
lightning/src/chain/chainmonitor.rs94.50% <ø> (-0.10%)⬇️
lightning/src/chain/channelmonitor.rs94.72% <ø> (ø)
lightning/src/chain/package.rs91.43% <ø> (ø)
lightning/src/events/bump_transaction.rs41.55% <ø> (ø)
lightning/src/events/mod.rs41.86% <ø> (ø)
lightning/src/ln/chan_utils.rs94.51% <ø> (ø)
lightning/src/sign/mod.rs84.38% <ø> (+0.02%)⬆️
lightning/src/util/config.rs58.13% <ø> (+0.66%)⬆️
lightning/src/util/enforcing_trait_impls.rs88.00% <ø> (ø)
lightning/src/chain/onchaintx.rs92.72% <77.77%> (ø)
... and 3 more
... and 3 files with indirect coverage changes
☔ View full report in Codecov by Sentry.

📢 Do you have feedback about the report comment? Let us know in this issue.
@wpaulino
wpaulino
merged commit 6fd13d7
into
lightningdevkit:mainJun 23, 2023
@wpaulino
wpaulino
deleted the

remove-anchors-flag

branch
June 23, 2023 23:17
@TheBlueMattTheBlueMatt
modified the milestones:
0.0.116alpha, 0.0.116Jun 24, 2023
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@TheBlueMattTheBlueMattTheBlueMatt approved these changes
@valentinewallacevalentinewallacevalentinewallace approved these changes

Assignees


No one assigned


Labels

Anchor Support

Projects


None yet


Milestone

0.0.116

Development

Successfully merging this pull request may close these issues.

4 participants

@wpaulino@TheBlueMatt@valentinewallace@codecov-commenter

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Auto-enable theater mode on YouTube
(function() {
function tryTheater() {
var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]');
if (btn && !btn.classList.contains('activated')) {
btn.click();
}
}
// Try immediately
tryTheater();
// Try after navigation (SPA)
var lastUrl = location.href;
setInterval(function() {
if (location.href !== lastUrl) {
lastUrl = location.href;
setTimeout(tryTheater, 500);
}
}, 1000);
// Also try on player load
var observer = new MutationObserver(tryTheater);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Remove anchors config flag by wpaulino · Pull Request #2367 · lightningdevkit/rust-lightning · GitHub
Skip to content
Remove anchors config flag - #2367
Merged
wpaulino merged 1 commit into
lightningdevkit:mainfrom 
wpaulino:remove-anchors-flag
Jun 23, 2023
Merged
Remove anchors config flag#2367
wpaulino merged 1 commit into
lightningdevkit:mainfrom 
wpaulino:remove-anchors-flag
Conversation
@wpaulino
@wpaulinowpaulino
commented
Jun 20, 2023

Copy link


Copy Markdown

Contributor
Now that all of the core functionality for anchor outputs has landed, we're ready to remove the config flag that was temporarily hiding it from our API.
Depends on #2361.
@wpaulinowpaulino
mentioned this pull request
Jun 20, 2023
 Merged

@wpaulinowpaulino
added this to the 0.0.116 milestone
Jun 20, 2023
@wpaulinowpaulino
mentioned this pull request
Jun 20, 2023

Open

10 tasks
@TheBlueMattTheBlueMatt
modified the milestones:
0.0.116, 0.0.116alphaJun 20, 2023
@TheBlueMattTheBlueMatt
mentioned this pull request
Jun 20, 2023
 Merged

@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
f46a8ae to
1a3bad8CompareJune 23, 2023 19:16
@TheBlueMatt
TheBlueMatt
commented
Jun 23, 2023

Copy link


Copy Markdown

Collaborator
There's a few spurious anchors flags in the CI script, though they'll be ignored now.
TheBlueMatt
TheBlueMatt
reviewed
Jun 23, 2023
View reviewed changes
Comment threadlightning/src/ln/channelmanager.rs
Outdated

TheBlueMatt
TheBlueMatt
previously approved these changes
Jun 23, 2023
View reviewed changes
@TheBlueMattTheBlueMatt
left a comment


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.

Two nits but LGTM.
@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
1a3bad8 to
d0d6490CompareJune 23, 2023 19:32
@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
d0d6490 to
5cddb23CompareJune 23, 2023 19:33
TheBlueMatt
TheBlueMatt
previously approved these changes
Jun 23, 2023
View reviewed changes
valentinewallace
valentinewallace
previously approved these changes
Jun 23, 2023
View reviewed changes
@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.

12 warnings introduced
Comment threadlightning/src/events/bump_transaction.rs
Outdated

Comment threadlightning/src/events/bump_transaction.rs
Comment threadlightning/src/chain/chainmonitor.rs
	/// For channels featuring anchor outputs, this method will also process [`BumpTransaction`]
	/// events produced from each [`ChannelMonitor`] while there is a balance to claim onchain
	/// within each channel. As the confirmation of a commitment transaction may be critical to the
	/// safety of funds, this method must be invoked frequently, ideally once for every chain tip
@valentinewallacevalentinewallaceJun 23, 2023

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.

Generally I'd prefer if the docs updates were in their own commit, don't care much here though
@wpaulino
Remove anchors config flag
82b646c
Now that all of the core functionality for anchor outputs has landed,
we're ready to remove the config flag that was temporarily hiding it
from our API.
@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
5cddb23 to
82b646cCompareJune 23, 2023 20:32
@valentinewallace
valentinewallace
commented
Jun 23, 2023

Copy link


Copy Markdown

Contributor
Would it be difficult to fix the warnings? Leaving them can make it easy to miss relevant warnings in local development when working off of main IMO
@wpaulino
wpaulino
commented
Jun 23, 2023

Copy link


Copy Markdown

ContributorAuthor
They weren't introduced by this PR, but rather by #2361.
@codecov-commenter
codecov-commenter
commented
Jun 23, 2023

Copy link


Copy Markdown

Codecov Report
Patch coverage: 91.11% and project coverage change: +0.01 🎉
Comparison is base (3973865) 90.30% compared to head (82b646c) 90.32%.
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.
Additional details and impacted files
@@ Coverage Diff @@## main #2367 +/- ##
==========================================
+ Coverage 90.30% 90.32% +0.01% 
==========================================
Files 106 106 Lines 54900 54888 -12 Branches 54900 54888 -12 ==========================================
- Hits 49576 49575 -1 + Misses 5324 5313 -11 
Impacted FilesCoverage Δ
lightning/src/chain/chainmonitor.rs94.50% <ø> (-0.10%)⬇️
lightning/src/chain/channelmonitor.rs94.72% <ø> (ø)
lightning/src/chain/package.rs91.43% <ø> (ø)
lightning/src/events/bump_transaction.rs41.55% <ø> (ø)
lightning/src/events/mod.rs41.86% <ø> (ø)
lightning/src/ln/chan_utils.rs94.51% <ø> (ø)
lightning/src/sign/mod.rs84.38% <ø> (+0.02%)⬆️
lightning/src/util/config.rs58.13% <ø> (+0.66%)⬆️
lightning/src/util/enforcing_trait_impls.rs88.00% <ø> (ø)
lightning/src/chain/onchaintx.rs92.72% <77.77%> (ø)
... and 3 more
... and 3 files with indirect coverage changes
☔ View full report in Codecov by Sentry.

📢 Do you have feedback about the report comment? Let us know in this issue.
@wpaulino
wpaulino
merged commit 6fd13d7
into
lightningdevkit:mainJun 23, 2023
@wpaulino
wpaulino
deleted the

remove-anchors-flag

branch
June 23, 2023 23:17
@TheBlueMattTheBlueMatt
modified the milestones:
0.0.116alpha, 0.0.116Jun 24, 2023
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@TheBlueMattTheBlueMattTheBlueMatt approved these changes
@valentinewallacevalentinewallacevalentinewallace approved these changes

Assignees


No one assigned


Labels

Anchor Support

Projects


None yet


Milestone

0.0.116

Development

Successfully merging this pull request may close these issues.

4 participants

@wpaulino@TheBlueMatt@valentinewallace@codecov-commenter

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Remove or un-stick sticky/fixed headers that block content
(function() {
function unstick() {
document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) {
if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {
el.style.position = 'static';
el.style.top = 'auto';
el.style.zIndex = 'auto';
}
});
}
unstick();
var observer = new MutationObserver(unstick);
observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });
})();
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Remove anchors config flag by wpaulino · Pull Request #2367 · lightningdevkit/rust-lightning · GitHub
Skip to content
Remove anchors config flag - #2367
Merged
wpaulino merged 1 commit into
lightningdevkit:mainfrom 
wpaulino:remove-anchors-flag
Jun 23, 2023
Merged
Remove anchors config flag#2367
wpaulino merged 1 commit into
lightningdevkit:mainfrom 
wpaulino:remove-anchors-flag
Conversation
@wpaulino
@wpaulinowpaulino
commented
Jun 20, 2023

Copy link


Copy Markdown

Contributor
Now that all of the core functionality for anchor outputs has landed, we're ready to remove the config flag that was temporarily hiding it from our API.
Depends on #2361.
@wpaulinowpaulino
mentioned this pull request
Jun 20, 2023
 Merged

@wpaulinowpaulino
added this to the 0.0.116 milestone
Jun 20, 2023
@wpaulinowpaulino
mentioned this pull request
Jun 20, 2023

Open

10 tasks
@TheBlueMattTheBlueMatt
modified the milestones:
0.0.116, 0.0.116alphaJun 20, 2023
@TheBlueMattTheBlueMatt
mentioned this pull request
Jun 20, 2023
 Merged

@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
f46a8ae to
1a3bad8CompareJune 23, 2023 19:16
@TheBlueMatt
TheBlueMatt
commented
Jun 23, 2023

Copy link


Copy Markdown

Collaborator
There's a few spurious anchors flags in the CI script, though they'll be ignored now.
TheBlueMatt
TheBlueMatt
reviewed
Jun 23, 2023
View reviewed changes
Comment threadlightning/src/ln/channelmanager.rs
Outdated

TheBlueMatt
TheBlueMatt
previously approved these changes
Jun 23, 2023
View reviewed changes
@TheBlueMattTheBlueMatt
left a comment


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.

Two nits but LGTM.
@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
1a3bad8 to
d0d6490CompareJune 23, 2023 19:32
@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
d0d6490 to
5cddb23CompareJune 23, 2023 19:33
TheBlueMatt
TheBlueMatt
previously approved these changes
Jun 23, 2023
View reviewed changes
valentinewallace
valentinewallace
previously approved these changes
Jun 23, 2023
View reviewed changes
@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.

12 warnings introduced
Comment threadlightning/src/events/bump_transaction.rs
Outdated

Comment threadlightning/src/events/bump_transaction.rs
Comment threadlightning/src/chain/chainmonitor.rs
	/// For channels featuring anchor outputs, this method will also process [`BumpTransaction`]
	/// events produced from each [`ChannelMonitor`] while there is a balance to claim onchain
	/// within each channel. As the confirmation of a commitment transaction may be critical to the
	/// safety of funds, this method must be invoked frequently, ideally once for every chain tip
@valentinewallacevalentinewallaceJun 23, 2023

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.

Generally I'd prefer if the docs updates were in their own commit, don't care much here though
@wpaulino
Remove anchors config flag
82b646c
Now that all of the core functionality for anchor outputs has landed,
we're ready to remove the config flag that was temporarily hiding it
from our API.
@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
5cddb23 to
82b646cCompareJune 23, 2023 20:32
@valentinewallace
valentinewallace
commented
Jun 23, 2023

Copy link


Copy Markdown

Contributor
Would it be difficult to fix the warnings? Leaving them can make it easy to miss relevant warnings in local development when working off of main IMO
@wpaulino
wpaulino
commented
Jun 23, 2023

Copy link


Copy Markdown

ContributorAuthor
They weren't introduced by this PR, but rather by #2361.
@codecov-commenter
codecov-commenter
commented
Jun 23, 2023

Copy link


Copy Markdown

Codecov Report
Patch coverage: 91.11% and project coverage change: +0.01 🎉
Comparison is base (3973865) 90.30% compared to head (82b646c) 90.32%.
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.
Additional details and impacted files
@@ Coverage Diff @@## main #2367 +/- ##
==========================================
+ Coverage 90.30% 90.32% +0.01% 
==========================================
Files 106 106 Lines 54900 54888 -12 Branches 54900 54888 -12 ==========================================
- Hits 49576 49575 -1 + Misses 5324 5313 -11 
Impacted FilesCoverage Δ
lightning/src/chain/chainmonitor.rs94.50% <ø> (-0.10%)⬇️
lightning/src/chain/channelmonitor.rs94.72% <ø> (ø)
lightning/src/chain/package.rs91.43% <ø> (ø)
lightning/src/events/bump_transaction.rs41.55% <ø> (ø)
lightning/src/events/mod.rs41.86% <ø> (ø)
lightning/src/ln/chan_utils.rs94.51% <ø> (ø)
lightning/src/sign/mod.rs84.38% <ø> (+0.02%)⬆️
lightning/src/util/config.rs58.13% <ø> (+0.66%)⬆️
lightning/src/util/enforcing_trait_impls.rs88.00% <ø> (ø)
lightning/src/chain/onchaintx.rs92.72% <77.77%> (ø)
... and 3 more
... and 3 files with indirect coverage changes
☔ View full report in Codecov by Sentry.

📢 Do you have feedback about the report comment? Let us know in this issue.
@wpaulino
wpaulino
merged commit 6fd13d7
into
lightningdevkit:mainJun 23, 2023
@wpaulino
wpaulino
deleted the

remove-anchors-flag

branch
June 23, 2023 23:17
@TheBlueMattTheBlueMatt
modified the milestones:
0.0.116alpha, 0.0.116Jun 24, 2023
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@TheBlueMattTheBlueMattTheBlueMatt approved these changes
@valentinewallacevalentinewallacevalentinewallace approved these changes

Assignees


No one assigned


Labels

Anchor Support

Projects


None yet


Milestone

0.0.116

Development

Successfully merging this pull request may close these issues.

4 participants

@wpaulino@TheBlueMatt@valentinewallace@codecov-commenter

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

Copy link


Copy Markdown

Contributor
Now that all of the core functionality for anchor outputs has landed, we're ready to remove the config flag that was temporarily hiding it from our API.
Depends on #2361.
@wpaulinowpaulino
mentioned this pull request
Jun 20, 2023
 Merged

@wpaulinowpaulino
added this to the 0.0.116 milestone
Jun 20, 2023
@wpaulinowpaulino
mentioned this pull request
Jun 20, 2023

Open

10 tasks
@TheBlueMattTheBlueMatt
modified the milestones:
0.0.116, 0.0.116alphaJun 20, 2023
@TheBlueMattTheBlueMatt
mentioned this pull request
Jun 20, 2023
 Merged

@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
f46a8ae to
1a3bad8CompareJune 23, 2023 19:16
@TheBlueMatt
TheBlueMatt
commented
Jun 23, 2023

Copy link


Copy Markdown

Collaborator
There's a few spurious anchors flags in the CI script, though they'll be ignored now.
TheBlueMatt
TheBlueMatt
reviewed
Jun 23, 2023
View reviewed changes
Comment threadlightning/src/ln/channelmanager.rs
Outdated

TheBlueMatt
TheBlueMatt
previously approved these changes
Jun 23, 2023
View reviewed changes
@TheBlueMattTheBlueMatt
left a comment


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.

Two nits but LGTM.
@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
1a3bad8 to
d0d6490CompareJune 23, 2023 19:32
@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
d0d6490 to
5cddb23CompareJune 23, 2023 19:33
TheBlueMatt
TheBlueMatt
previously approved these changes
Jun 23, 2023
View reviewed changes
valentinewallace
valentinewallace
previously approved these changes
Jun 23, 2023
View reviewed changes
@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.

12 warnings introduced
Comment threadlightning/src/events/bump_transaction.rs
Outdated

Comment threadlightning/src/events/bump_transaction.rs
Comment threadlightning/src/chain/chainmonitor.rs
	/// For channels featuring anchor outputs, this method will also process [`BumpTransaction`]
	/// events produced from each [`ChannelMonitor`] while there is a balance to claim onchain
	/// within each channel. As the confirmation of a commitment transaction may be critical to the
	/// safety of funds, this method must be invoked frequently, ideally once for every chain tip
@valentinewallacevalentinewallaceJun 23, 2023

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.

Generally I'd prefer if the docs updates were in their own commit, don't care much here though
@wpaulino
Remove anchors config flag
82b646c
Now that all of the core functionality for anchor outputs has landed,
we're ready to remove the config flag that was temporarily hiding it
from our API.
@wpaulino
wpaulinoforce-pushed
the
remove-anchors-flag
branch
from
5cddb23 to
82b646cCompareJune 23, 2023 20:32
@valentinewallace
valentinewallace
commented
Jun 23, 2023

Copy link


Copy Markdown

Contributor
Would it be difficult to fix the warnings? Leaving them can make it easy to miss relevant warnings in local development when working off of main IMO
@wpaulino
wpaulino
commented
Jun 23, 2023

Copy link


Copy Markdown

ContributorAuthor
They weren't introduced by this PR, but rather by #2361.
@codecov-commenter
codecov-commenter
commented
Jun 23, 2023

Copy link


Copy Markdown

Codecov Report
Patch coverage: 91.11% and project coverage change: +0.01 🎉
Comparison is base (3973865) 90.30% compared to head (82b646c) 90.32%.
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.
Additional details and impacted files
@@ Coverage Diff @@## main #2367 +/- ##
==========================================
+ Coverage 90.30% 90.32% +0.01% 
==========================================
Files 106 106 Lines 54900 54888 -12 Branches 54900 54888 -12 ==========================================
- Hits 49576 49575 -1 + Misses 5324 5313 -11 
Impacted FilesCoverage Δ
lightning/src/chain/chainmonitor.rs94.50% <ø> (-0.10%)⬇️
lightning/src/chain/channelmonitor.rs94.72% <ø> (ø)
lightning/src/chain/package.rs91.43% <ø> (ø)
lightning/src/events/bump_transaction.rs41.55% <ø> (ø)
lightning/src/events/mod.rs41.86% <ø> (ø)
lightning/src/ln/chan_utils.rs94.51% <ø> (ø)
lightning/src/sign/mod.rs84.38% <ø> (+0.02%)⬆️
lightning/src/util/config.rs58.13% <ø> (+0.66%)⬆️
lightning/src/util/enforcing_trait_impls.rs88.00% <ø> (ø)
lightning/src/chain/onchaintx.rs92.72% <77.77%> (ø)
... and 3 more
... and 3 files with indirect coverage changes
☔ View full report in Codecov by Sentry.

📢 Do you have feedback about the report comment? Let us know in this issue.
@wpaulino
wpaulino
merged commit 6fd13d7
into
lightningdevkit:mainJun 23, 2023
@wpaulino
wpaulino
deleted the

remove-anchors-flag

branch
June 23, 2023 23:17
@TheBlueMattTheBlueMatt
modified the milestones:
0.0.116alpha, 0.0.116Jun 24, 2023
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@TheBlueMattTheBlueMattTheBlueMatt approved these changes
@valentinewallacevalentinewallacevalentinewallace approved these changes

Assignees


No one assigned


Labels

Anchor Support

Projects


None yet


Milestone

0.0.116

Development

Successfully merging this pull request may close these issues.

4 participants

@wpaulino@TheBlueMatt@valentinewallace@codecov-commenter