') + ')', '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('^' + ".*" + ', '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" + ', '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('^' + ".*" + ', '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); } })(); })(); Move forwarding_fee_proportional_millionths, forwarding_fee_base_msat to ChannelHandshakeConfig by matchacactus · Pull Request #1247 · lightningdevkit/rust-lightning · GitHub
Skip to content

Move forwarding_fee_proportional_millionths, forwarding_fee_base_msat to ChannelHandshakeConfig - #1247

Closed
matchacactus wants to merge 2 commits into
lightningdevkit:mainfrom
matchacactus:handshakeconfig
Closed

Move forwarding_fee_proportional_millionths, forwarding_fee_base_msat to ChannelHandshakeConfig#1247
matchacactus wants to merge 2 commits into
lightningdevkit:mainfrom
matchacactus:handshakeconfig

Conversation

@matchacactus

@matchacactusmatchacactus commented Jan 18, 2022

Copy link
Copy Markdown

First step for #216

@matchacactus

Copy link
Copy Markdown
Author

The second commit broke test_priv_forwarding_rejection randomly. I have tried and failed to find out why, another 👀 for debug help?

@codecov-commenter

codecov-commenter commented Jan 18, 2022

Copy link
Copy Markdown

Codecov Report

Merging #1247 (6bccdaa) into main (34cdca9) will increase coverage by 0.01%.
The diff coverage is 92.00%.

❗ Current head 6bccdaa differs from pull request most recent head c49c086. Consider uploading reports for the commit c49c086 to get more accurate results
Impacted file tree graph

@@ Coverage Diff @@## main #1247 +/- ##
==========================================
+ Coverage 90.40% 90.42% +0.01% 
==========================================
Files 70 70 Lines 38087 38098 +11 ==========================================
+ Hits 34434 34449 +15 + Misses 3653 3649 -4 
Impacted FilesCoverage Δ
lightning/src/ln/functional_test_utils.rs95.28% <ø> (ø)
lightning/src/util/config.rs47.82% <0.00%> (+2.17%)⬆️
lightning/src/ln/channel.rs89.44% <100.00%> (+0.06%)⬆️
lightning/src/ln/functional_tests.rs97.28% <100.00%> (+0.01%)⬆️
lightning/src/ln/onion_route_tests.rs96.69% <100.00%> (ø)
lightning/src/ln/payment_tests.rs99.14% <100.00%> (ø)
lightning/src/util/events.rs32.73% <0.00%> (+0.35%)⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 34cdca9...c49c086. Read the comment docs.

@TheBlueMatt

Copy link
Copy Markdown
Collaborator

I'm somewhat confused as to the motivation for this PR - ChannelHandshakeConfig is all configuration options which are set at the beginning of a channel and then can not be changed afterwards - its all stuff in the "handshake". Instead, I think we should move things from ChannelConfig which cannot be updated after the handshake, specifically announced_channel (cannot be updated per lightning spec), commit_upfront_shutdown_pubkey (its literally about whether we include the pubkey in the handshake or not).

}

impl_writeable_tlv_based!(ChannelConfig, {
(0, forwarding_fee_proportional_millionths, required),

@TheBlueMattTheBlueMattJan 18, 2022

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.

Note that we cannot remove required elements, as old clients will refuse to load data serialized by new clients (which we guarantee won't happen at least across a few releases). We instead need to write the default value with something like (0, Some(DEFAULT_VALUE as u64), option)

@matchacactus

Copy link
Copy Markdown
Author

I'm somewhat confused as to the motivation for this PR - ChannelHandshakeConfig is all configuration options which are set at the beginning of a channel and then can not be changed afterwards - its all stuff in the "handshake". Instead, I think we should move things from ChannelConfig which cannot be updated after the handshake, specifically announced_channel (cannot be updated per lightning spec), commit_upfront_shutdown_pubkey (its literally about whether we include the pubkey in the handshake or not).

I misunderstood the issue - am I on the right track with #1270? Closing this PR in favor of #1270

@TheBlueMattTheBlueMatt mentioned this pull request May 16, 2022
@TheBlueMatt

Copy link
Copy Markdown
Collaborator

Eventually landed rebased as #1529.

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.

3 participants

@matchacactus@codecov-commenter@TheBlueMatt