This repository was archived by the owner on Dec 19, 2025. It is now read-only.

Closing Signed Message Update wire - #182

Merged
bmancini55 merged 4 commits into
node-lightning:mainfrom
Vib-UX:wire_closing_signed
Jul 30, 2021
Merged

Closing Signed Message Update wire#182
bmancini55 merged 4 commits into
node-lightning:mainfrom
Vib-UX:wire_closing_signed

Conversation

@Vib-UX

Copy link
Copy Markdown
Contributor

wire: create closing_signed message type #180

BOLT #2

  • Parameters are defined according to closing_signed in BOLT #2
  • Implementation is as per the standards given
  • UT attached for testing

@bmancini55

@Vib-UXVib-UX closed this Jul 24, 2021
@Vib-UXVib-UX reopened this Jul 24, 2021

@bmancini55bmancini55 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code structure looks good! Couple minors on commenting the protocol.

"0027"+ // type
"0000000000000000000000000000000000000000000000000000000000000000" + // Channel ID
"0000000000030d40" + // feeSatoshi
"22222222222222222222222222222222222222222222222222222222222222223333333333333333333333333333333333333333333333333333333333333333" //signature

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: fix whitespace



/**
*/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add comment on the purpose of this message and include when we send it and when we expect to receive it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, but let's add two more things:

  • purpose of fee negotiation
  • how fee rate is convergent

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

  1. Maybe I am wrong, isn't the purpose for fee negotiation is that how fast one of the two side wants the transaction to be included in the block?
  2. As for the Fee rate convergence, On every negotiation step we must give up some amount from our proposal towards the other peer’s proposal. Eg. assuming the peer proposes a closing fee of 3000 satoshi and our estimate shows it must be 4000. “10”: our next proposal will be 4000-10=3990. This exchange continues using the channelID until both agree on the same fee or when one side fails the channel.

Should I update the nxt commit using the above points?

References :

  1. https://github.com/lightningnetwork/lightning-rfc/blob/master/02-peer-protocol.md#closing-negotiation-closing_signed
  2. https://lightning.readthedocs.io/lightning-close.7.html

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re 1: For mutual close both sides want the transaction in with minimal fees. Without anchors, the feerate_per_kw set at open or through update_fee is suggested to be 5x the amount that will get the transaction included in the block, which is extremely expensive. One of the benefits of mutual close is that the fee rate can be negotiated down from the existing fee rate to a more reasonable one.

Re 2: Correct, though you may want to say that it is convergent by requiring the fee rate to be strictly between the last proposed and the counterparty's last proposed value. The back and forth continues until values are equal, at which point both sides can broadcast the closing tx.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

That's very informative. Wasn't aware that initial fee kept or suggested is that high! Thanks

public static type: MessageType = MessageType.ClosingSigned;

/**
* Deserializes the funding_signed message

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

closing_signed message

public channelId: ChannelId;

/**
* fee_satoshis is set according to its estimate of cost of inclusion in a block.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Include comments about the expected value when sending or receiving.

@bmancini55bmancini55 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking good with a couple small changes



/**
*/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, but let's add two more things:

  • purpose of fee negotiation
  • how fee rate is convergent

/**
* Expected value for fee_satoshis could vary how fast either side wants the transaction
* to be included in the block. So changes are made accordingly on both ends and communicated
* between the channel untill both of them comes to an agreement or when one side fails the

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: s/untill/until/

@bmancini55bmancini55 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good!

@bmancini55
bmancini55 marked this pull request as ready for review July 30, 2021 12:44
@bmancini55
bmancini55 merged commit a95e7d5 into node-lightning:mainJul 30, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Vib-UX@bmancini55
, '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
This repository was archived by the owner on Dec 19, 2025. It is now read-only.

Closing Signed Message Update wire - #182

Merged
bmancini55 merged 4 commits into
node-lightning:mainfrom
Vib-UX:wire_closing_signed
Jul 30, 2021
Merged

Closing Signed Message Update wire#182
bmancini55 merged 4 commits into
node-lightning:mainfrom
Vib-UX:wire_closing_signed

Conversation

@Vib-UX

Copy link
Copy Markdown
Contributor

wire: create closing_signed message type #180

BOLT #2

  • Parameters are defined according to closing_signed in BOLT #2
  • Implementation is as per the standards given
  • UT attached for testing

@bmancini55

@Vib-UXVib-UX closed this Jul 24, 2021
@Vib-UXVib-UX reopened this Jul 24, 2021

@bmancini55bmancini55 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code structure looks good! Couple minors on commenting the protocol.

"0027"+ // type
"0000000000000000000000000000000000000000000000000000000000000000" + // Channel ID
"0000000000030d40" + // feeSatoshi
"22222222222222222222222222222222222222222222222222222222222222223333333333333333333333333333333333333333333333333333333333333333" //signature

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: fix whitespace



/**
*/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add comment on the purpose of this message and include when we send it and when we expect to receive it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, but let's add two more things:

  • purpose of fee negotiation
  • how fee rate is convergent

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

  1. Maybe I am wrong, isn't the purpose for fee negotiation is that how fast one of the two side wants the transaction to be included in the block?
  2. As for the Fee rate convergence, On every negotiation step we must give up some amount from our proposal towards the other peer’s proposal. Eg. assuming the peer proposes a closing fee of 3000 satoshi and our estimate shows it must be 4000. “10”: our next proposal will be 4000-10=3990. This exchange continues using the channelID until both agree on the same fee or when one side fails the channel.

Should I update the nxt commit using the above points?

References :

  1. https://github.com/lightningnetwork/lightning-rfc/blob/master/02-peer-protocol.md#closing-negotiation-closing_signed
  2. https://lightning.readthedocs.io/lightning-close.7.html

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re 1: For mutual close both sides want the transaction in with minimal fees. Without anchors, the feerate_per_kw set at open or through update_fee is suggested to be 5x the amount that will get the transaction included in the block, which is extremely expensive. One of the benefits of mutual close is that the fee rate can be negotiated down from the existing fee rate to a more reasonable one.

Re 2: Correct, though you may want to say that it is convergent by requiring the fee rate to be strictly between the last proposed and the counterparty's last proposed value. The back and forth continues until values are equal, at which point both sides can broadcast the closing tx.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

That's very informative. Wasn't aware that initial fee kept or suggested is that high! Thanks

public static type: MessageType = MessageType.ClosingSigned;

/**
* Deserializes the funding_signed message

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

closing_signed message

public channelId: ChannelId;

/**
* fee_satoshis is set according to its estimate of cost of inclusion in a block.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Include comments about the expected value when sending or receiving.

@bmancini55bmancini55 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking good with a couple small changes



/**
*/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, but let's add two more things:

  • purpose of fee negotiation
  • how fee rate is convergent

/**
* Expected value for fee_satoshis could vary how fast either side wants the transaction
* to be included in the block. So changes are made accordingly on both ends and communicated
* between the channel untill both of them comes to an agreement or when one side fails the

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: s/untill/until/

@bmancini55bmancini55 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good!

@bmancini55
bmancini55 marked this pull request as ready for review July 30, 2021 12:44
@bmancini55
bmancini55 merged commit a95e7d5 into node-lightning:mainJul 30, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Vib-UX@bmancini55
, '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
This repository was archived by the owner on Dec 19, 2025. It is now read-only.

Closing Signed Message Update wire - #182

Merged
bmancini55 merged 4 commits into
node-lightning:mainfrom
Vib-UX:wire_closing_signed
Jul 30, 2021
Merged

Closing Signed Message Update wire#182
bmancini55 merged 4 commits into
node-lightning:mainfrom
Vib-UX:wire_closing_signed

Conversation

@Vib-UX

Copy link
Copy Markdown
Contributor

wire: create closing_signed message type #180

BOLT #2

  • Parameters are defined according to closing_signed in BOLT #2
  • Implementation is as per the standards given
  • UT attached for testing

@bmancini55

@Vib-UXVib-UX closed this Jul 24, 2021
@Vib-UXVib-UX reopened this Jul 24, 2021

@bmancini55bmancini55 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code structure looks good! Couple minors on commenting the protocol.

"0027"+ // type
"0000000000000000000000000000000000000000000000000000000000000000" + // Channel ID
"0000000000030d40" + // feeSatoshi
"22222222222222222222222222222222222222222222222222222222222222223333333333333333333333333333333333333333333333333333333333333333" //signature

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: fix whitespace



/**
*/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add comment on the purpose of this message and include when we send it and when we expect to receive it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, but let's add two more things:

  • purpose of fee negotiation
  • how fee rate is convergent

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

  1. Maybe I am wrong, isn't the purpose for fee negotiation is that how fast one of the two side wants the transaction to be included in the block?
  2. As for the Fee rate convergence, On every negotiation step we must give up some amount from our proposal towards the other peer’s proposal. Eg. assuming the peer proposes a closing fee of 3000 satoshi and our estimate shows it must be 4000. “10”: our next proposal will be 4000-10=3990. This exchange continues using the channelID until both agree on the same fee or when one side fails the channel.

Should I update the nxt commit using the above points?

References :

  1. https://github.com/lightningnetwork/lightning-rfc/blob/master/02-peer-protocol.md#closing-negotiation-closing_signed
  2. https://lightning.readthedocs.io/lightning-close.7.html

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re 1: For mutual close both sides want the transaction in with minimal fees. Without anchors, the feerate_per_kw set at open or through update_fee is suggested to be 5x the amount that will get the transaction included in the block, which is extremely expensive. One of the benefits of mutual close is that the fee rate can be negotiated down from the existing fee rate to a more reasonable one.

Re 2: Correct, though you may want to say that it is convergent by requiring the fee rate to be strictly between the last proposed and the counterparty's last proposed value. The back and forth continues until values are equal, at which point both sides can broadcast the closing tx.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

That's very informative. Wasn't aware that initial fee kept or suggested is that high! Thanks

public static type: MessageType = MessageType.ClosingSigned;

/**
* Deserializes the funding_signed message

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

closing_signed message

public channelId: ChannelId;

/**
* fee_satoshis is set according to its estimate of cost of inclusion in a block.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Include comments about the expected value when sending or receiving.

@bmancini55bmancini55 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking good with a couple small changes



/**
*/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, but let's add two more things:

  • purpose of fee negotiation
  • how fee rate is convergent

/**
* Expected value for fee_satoshis could vary how fast either side wants the transaction
* to be included in the block. So changes are made accordingly on both ends and communicated
* between the channel untill both of them comes to an agreement or when one side fails the

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: s/untill/until/

@bmancini55bmancini55 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good!

@bmancini55
bmancini55 marked this pull request as ready for review July 30, 2021 12:44
@bmancini55
bmancini55 merged commit a95e7d5 into node-lightning:mainJul 30, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Vib-UX@bmancini55
, '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
This repository was archived by the owner on Dec 19, 2025. It is now read-only.

Closing Signed Message Update wire - #182

Merged
bmancini55 merged 4 commits into
node-lightning:mainfrom
Vib-UX:wire_closing_signed
Jul 30, 2021
Merged

Closing Signed Message Update wire#182
bmancini55 merged 4 commits into
node-lightning:mainfrom
Vib-UX:wire_closing_signed

Conversation

@Vib-UX

Copy link
Copy Markdown
Contributor

wire: create closing_signed message type #180

BOLT #2

  • Parameters are defined according to closing_signed in BOLT #2
  • Implementation is as per the standards given
  • UT attached for testing

@bmancini55

@Vib-UXVib-UX closed this Jul 24, 2021
@Vib-UXVib-UX reopened this Jul 24, 2021

@bmancini55bmancini55 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code structure looks good! Couple minors on commenting the protocol.

"0027"+ // type
"0000000000000000000000000000000000000000000000000000000000000000" + // Channel ID
"0000000000030d40" + // feeSatoshi
"22222222222222222222222222222222222222222222222222222222222222223333333333333333333333333333333333333333333333333333333333333333" //signature

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: fix whitespace



/**
*/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add comment on the purpose of this message and include when we send it and when we expect to receive it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, but let's add two more things:

  • purpose of fee negotiation
  • how fee rate is convergent

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

  1. Maybe I am wrong, isn't the purpose for fee negotiation is that how fast one of the two side wants the transaction to be included in the block?
  2. As for the Fee rate convergence, On every negotiation step we must give up some amount from our proposal towards the other peer’s proposal. Eg. assuming the peer proposes a closing fee of 3000 satoshi and our estimate shows it must be 4000. “10”: our next proposal will be 4000-10=3990. This exchange continues using the channelID until both agree on the same fee or when one side fails the channel.

Should I update the nxt commit using the above points?

References :

  1. https://github.com/lightningnetwork/lightning-rfc/blob/master/02-peer-protocol.md#closing-negotiation-closing_signed
  2. https://lightning.readthedocs.io/lightning-close.7.html

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re 1: For mutual close both sides want the transaction in with minimal fees. Without anchors, the feerate_per_kw set at open or through update_fee is suggested to be 5x the amount that will get the transaction included in the block, which is extremely expensive. One of the benefits of mutual close is that the fee rate can be negotiated down from the existing fee rate to a more reasonable one.

Re 2: Correct, though you may want to say that it is convergent by requiring the fee rate to be strictly between the last proposed and the counterparty's last proposed value. The back and forth continues until values are equal, at which point both sides can broadcast the closing tx.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

That's very informative. Wasn't aware that initial fee kept or suggested is that high! Thanks

public static type: MessageType = MessageType.ClosingSigned;

/**
* Deserializes the funding_signed message

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

closing_signed message

public channelId: ChannelId;

/**
* fee_satoshis is set according to its estimate of cost of inclusion in a block.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Include comments about the expected value when sending or receiving.

@bmancini55bmancini55 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking good with a couple small changes



/**
*/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, but let's add two more things:

  • purpose of fee negotiation
  • how fee rate is convergent

/**
* Expected value for fee_satoshis could vary how fast either side wants the transaction
* to be included in the block. So changes are made accordingly on both ends and communicated
* between the channel untill both of them comes to an agreement or when one side fails the

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: s/untill/until/

@bmancini55bmancini55 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good!

@bmancini55
bmancini55 marked this pull request as ready for review July 30, 2021 12:44
@bmancini55
bmancini55 merged commit a95e7d5 into node-lightning:mainJul 30, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Vib-UX@bmancini55
, '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
This repository was archived by the owner on Dec 19, 2025. It is now read-only.

Closing Signed Message Update wire - #182

Merged
bmancini55 merged 4 commits into
node-lightning:mainfrom
Vib-UX:wire_closing_signed
Jul 30, 2021
Merged

Closing Signed Message Update wire#182
bmancini55 merged 4 commits into
node-lightning:mainfrom
Vib-UX:wire_closing_signed

Conversation

@Vib-UX

Copy link
Copy Markdown
Contributor

wire: create closing_signed message type #180

BOLT #2

  • Parameters are defined according to closing_signed in BOLT #2
  • Implementation is as per the standards given
  • UT attached for testing

@bmancini55

@Vib-UXVib-UX closed this Jul 24, 2021
@Vib-UXVib-UX reopened this Jul 24, 2021

@bmancini55bmancini55 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code structure looks good! Couple minors on commenting the protocol.

"0027"+ // type
"0000000000000000000000000000000000000000000000000000000000000000" + // Channel ID
"0000000000030d40" + // feeSatoshi
"22222222222222222222222222222222222222222222222222222222222222223333333333333333333333333333333333333333333333333333333333333333" //signature

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: fix whitespace



/**
*/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add comment on the purpose of this message and include when we send it and when we expect to receive it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, but let's add two more things:

  • purpose of fee negotiation
  • how fee rate is convergent

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

  1. Maybe I am wrong, isn't the purpose for fee negotiation is that how fast one of the two side wants the transaction to be included in the block?
  2. As for the Fee rate convergence, On every negotiation step we must give up some amount from our proposal towards the other peer’s proposal. Eg. assuming the peer proposes a closing fee of 3000 satoshi and our estimate shows it must be 4000. “10”: our next proposal will be 4000-10=3990. This exchange continues using the channelID until both agree on the same fee or when one side fails the channel.

Should I update the nxt commit using the above points?

References :

  1. https://github.com/lightningnetwork/lightning-rfc/blob/master/02-peer-protocol.md#closing-negotiation-closing_signed
  2. https://lightning.readthedocs.io/lightning-close.7.html

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re 1: For mutual close both sides want the transaction in with minimal fees. Without anchors, the feerate_per_kw set at open or through update_fee is suggested to be 5x the amount that will get the transaction included in the block, which is extremely expensive. One of the benefits of mutual close is that the fee rate can be negotiated down from the existing fee rate to a more reasonable one.

Re 2: Correct, though you may want to say that it is convergent by requiring the fee rate to be strictly between the last proposed and the counterparty's last proposed value. The back and forth continues until values are equal, at which point both sides can broadcast the closing tx.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

That's very informative. Wasn't aware that initial fee kept or suggested is that high! Thanks

public static type: MessageType = MessageType.ClosingSigned;

/**
* Deserializes the funding_signed message

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

closing_signed message

public channelId: ChannelId;

/**
* fee_satoshis is set according to its estimate of cost of inclusion in a block.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Include comments about the expected value when sending or receiving.

@bmancini55bmancini55 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking good with a couple small changes



/**
*/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, but let's add two more things:

  • purpose of fee negotiation
  • how fee rate is convergent

/**
* Expected value for fee_satoshis could vary how fast either side wants the transaction
* to be included in the block. So changes are made accordingly on both ends and communicated
* between the channel untill both of them comes to an agreement or when one side fails the

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: s/untill/until/

@bmancini55bmancini55 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good!

@bmancini55
bmancini55 marked this pull request as ready for review July 30, 2021 12:44
@bmancini55
bmancini55 merged commit a95e7d5 into node-lightning:mainJul 30, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Vib-UX@bmancini55
, '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
This repository was archived by the owner on Dec 19, 2025. It is now read-only.

Closing Signed Message Update wire - #182

Merged
bmancini55 merged 4 commits into
node-lightning:mainfrom
Vib-UX:wire_closing_signed
Jul 30, 2021
Merged

Closing Signed Message Update wire#182
bmancini55 merged 4 commits into
node-lightning:mainfrom
Vib-UX:wire_closing_signed

Conversation

@Vib-UX

Copy link
Copy Markdown
Contributor

wire: create closing_signed message type #180

BOLT #2

  • Parameters are defined according to closing_signed in BOLT #2
  • Implementation is as per the standards given
  • UT attached for testing

@bmancini55

@Vib-UXVib-UX closed this Jul 24, 2021
@Vib-UXVib-UX reopened this Jul 24, 2021

@bmancini55bmancini55 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code structure looks good! Couple minors on commenting the protocol.

"0027"+ // type
"0000000000000000000000000000000000000000000000000000000000000000" + // Channel ID
"0000000000030d40" + // feeSatoshi
"22222222222222222222222222222222222222222222222222222222222222223333333333333333333333333333333333333333333333333333333333333333" //signature

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: fix whitespace



/**
*/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add comment on the purpose of this message and include when we send it and when we expect to receive it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, but let's add two more things:

  • purpose of fee negotiation
  • how fee rate is convergent

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

  1. Maybe I am wrong, isn't the purpose for fee negotiation is that how fast one of the two side wants the transaction to be included in the block?
  2. As for the Fee rate convergence, On every negotiation step we must give up some amount from our proposal towards the other peer’s proposal. Eg. assuming the peer proposes a closing fee of 3000 satoshi and our estimate shows it must be 4000. “10”: our next proposal will be 4000-10=3990. This exchange continues using the channelID until both agree on the same fee or when one side fails the channel.

Should I update the nxt commit using the above points?

References :

  1. https://github.com/lightningnetwork/lightning-rfc/blob/master/02-peer-protocol.md#closing-negotiation-closing_signed
  2. https://lightning.readthedocs.io/lightning-close.7.html

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re 1: For mutual close both sides want the transaction in with minimal fees. Without anchors, the feerate_per_kw set at open or through update_fee is suggested to be 5x the amount that will get the transaction included in the block, which is extremely expensive. One of the benefits of mutual close is that the fee rate can be negotiated down from the existing fee rate to a more reasonable one.

Re 2: Correct, though you may want to say that it is convergent by requiring the fee rate to be strictly between the last proposed and the counterparty's last proposed value. The back and forth continues until values are equal, at which point both sides can broadcast the closing tx.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

That's very informative. Wasn't aware that initial fee kept or suggested is that high! Thanks

public static type: MessageType = MessageType.ClosingSigned;

/**
* Deserializes the funding_signed message

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

closing_signed message

public channelId: ChannelId;

/**
* fee_satoshis is set according to its estimate of cost of inclusion in a block.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Include comments about the expected value when sending or receiving.

@bmancini55bmancini55 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking good with a couple small changes



/**
*/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, but let's add two more things:

  • purpose of fee negotiation
  • how fee rate is convergent

/**
* Expected value for fee_satoshis could vary how fast either side wants the transaction
* to be included in the block. So changes are made accordingly on both ends and communicated
* between the channel untill both of them comes to an agreement or when one side fails the

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: s/untill/until/

@bmancini55bmancini55 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good!

@bmancini55
bmancini55 marked this pull request as ready for review July 30, 2021 12:44
@bmancini55
bmancini55 merged commit a95e7d5 into node-lightning:mainJul 30, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Vib-UX@bmancini55
, '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
This repository was archived by the owner on Dec 19, 2025. It is now read-only.

Closing Signed Message Update wire - #182

Merged
bmancini55 merged 4 commits into
node-lightning:mainfrom
Vib-UX:wire_closing_signed
Jul 30, 2021
Merged

Closing Signed Message Update wire#182
bmancini55 merged 4 commits into
node-lightning:mainfrom
Vib-UX:wire_closing_signed

Conversation

@Vib-UX

Copy link
Copy Markdown
Contributor

wire: create closing_signed message type #180

BOLT #2

  • Parameters are defined according to closing_signed in BOLT #2
  • Implementation is as per the standards given
  • UT attached for testing

@bmancini55

@Vib-UXVib-UX closed this Jul 24, 2021
@Vib-UXVib-UX reopened this Jul 24, 2021

@bmancini55bmancini55 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code structure looks good! Couple minors on commenting the protocol.

"0027"+ // type
"0000000000000000000000000000000000000000000000000000000000000000" + // Channel ID
"0000000000030d40" + // feeSatoshi
"22222222222222222222222222222222222222222222222222222222222222223333333333333333333333333333333333333333333333333333333333333333" //signature

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: fix whitespace



/**
*/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add comment on the purpose of this message and include when we send it and when we expect to receive it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, but let's add two more things:

  • purpose of fee negotiation
  • how fee rate is convergent

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

  1. Maybe I am wrong, isn't the purpose for fee negotiation is that how fast one of the two side wants the transaction to be included in the block?
  2. As for the Fee rate convergence, On every negotiation step we must give up some amount from our proposal towards the other peer’s proposal. Eg. assuming the peer proposes a closing fee of 3000 satoshi and our estimate shows it must be 4000. “10”: our next proposal will be 4000-10=3990. This exchange continues using the channelID until both agree on the same fee or when one side fails the channel.

Should I update the nxt commit using the above points?

References :

  1. https://github.com/lightningnetwork/lightning-rfc/blob/master/02-peer-protocol.md#closing-negotiation-closing_signed
  2. https://lightning.readthedocs.io/lightning-close.7.html

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re 1: For mutual close both sides want the transaction in with minimal fees. Without anchors, the feerate_per_kw set at open or through update_fee is suggested to be 5x the amount that will get the transaction included in the block, which is extremely expensive. One of the benefits of mutual close is that the fee rate can be negotiated down from the existing fee rate to a more reasonable one.

Re 2: Correct, though you may want to say that it is convergent by requiring the fee rate to be strictly between the last proposed and the counterparty's last proposed value. The back and forth continues until values are equal, at which point both sides can broadcast the closing tx.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

That's very informative. Wasn't aware that initial fee kept or suggested is that high! Thanks

public static type: MessageType = MessageType.ClosingSigned;

/**
* Deserializes the funding_signed message

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

closing_signed message

public channelId: ChannelId;

/**
* fee_satoshis is set according to its estimate of cost of inclusion in a block.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Include comments about the expected value when sending or receiving.

@bmancini55bmancini55 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking good with a couple small changes



/**
*/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, but let's add two more things:

  • purpose of fee negotiation
  • how fee rate is convergent

/**
* Expected value for fee_satoshis could vary how fast either side wants the transaction
* to be included in the block. So changes are made accordingly on both ends and communicated
* between the channel untill both of them comes to an agreement or when one side fails the

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: s/untill/until/

@bmancini55bmancini55 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good!

@bmancini55
bmancini55 marked this pull request as ready for review July 30, 2021 12:44
@bmancini55
bmancini55 merged commit a95e7d5 into node-lightning:mainJul 30, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Vib-UX@bmancini55
, '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
This repository was archived by the owner on Dec 19, 2025. It is now read-only.

Closing Signed Message Update wire - #182

Merged
bmancini55 merged 4 commits into
node-lightning:mainfrom
Vib-UX:wire_closing_signed
Jul 30, 2021
Merged

Closing Signed Message Update wire#182
bmancini55 merged 4 commits into
node-lightning:mainfrom
Vib-UX:wire_closing_signed

Conversation

@Vib-UX

Copy link
Copy Markdown
Contributor

wire: create closing_signed message type #180

BOLT #2

  • Parameters are defined according to closing_signed in BOLT #2
  • Implementation is as per the standards given
  • UT attached for testing

@bmancini55

@Vib-UXVib-UX closed this Jul 24, 2021
@Vib-UXVib-UX reopened this Jul 24, 2021

@bmancini55bmancini55 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code structure looks good! Couple minors on commenting the protocol.

"0027"+ // type
"0000000000000000000000000000000000000000000000000000000000000000" + // Channel ID
"0000000000030d40" + // feeSatoshi
"22222222222222222222222222222222222222222222222222222222222222223333333333333333333333333333333333333333333333333333333333333333" //signature

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: fix whitespace



/**
*/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add comment on the purpose of this message and include when we send it and when we expect to receive it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, but let's add two more things:

  • purpose of fee negotiation
  • how fee rate is convergent

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

  1. Maybe I am wrong, isn't the purpose for fee negotiation is that how fast one of the two side wants the transaction to be included in the block?
  2. As for the Fee rate convergence, On every negotiation step we must give up some amount from our proposal towards the other peer’s proposal. Eg. assuming the peer proposes a closing fee of 3000 satoshi and our estimate shows it must be 4000. “10”: our next proposal will be 4000-10=3990. This exchange continues using the channelID until both agree on the same fee or when one side fails the channel.

Should I update the nxt commit using the above points?

References :

  1. https://github.com/lightningnetwork/lightning-rfc/blob/master/02-peer-protocol.md#closing-negotiation-closing_signed
  2. https://lightning.readthedocs.io/lightning-close.7.html

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re 1: For mutual close both sides want the transaction in with minimal fees. Without anchors, the feerate_per_kw set at open or through update_fee is suggested to be 5x the amount that will get the transaction included in the block, which is extremely expensive. One of the benefits of mutual close is that the fee rate can be negotiated down from the existing fee rate to a more reasonable one.

Re 2: Correct, though you may want to say that it is convergent by requiring the fee rate to be strictly between the last proposed and the counterparty's last proposed value. The back and forth continues until values are equal, at which point both sides can broadcast the closing tx.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

That's very informative. Wasn't aware that initial fee kept or suggested is that high! Thanks

public static type: MessageType = MessageType.ClosingSigned;

/**
* Deserializes the funding_signed message

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

closing_signed message

public channelId: ChannelId;

/**
* fee_satoshis is set according to its estimate of cost of inclusion in a block.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Include comments about the expected value when sending or receiving.

@bmancini55bmancini55 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking good with a couple small changes



/**
*/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, but let's add two more things:

  • purpose of fee negotiation
  • how fee rate is convergent

/**
* Expected value for fee_satoshis could vary how fast either side wants the transaction
* to be included in the block. So changes are made accordingly on both ends and communicated
* between the channel untill both of them comes to an agreement or when one side fails the

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: s/untill/until/

@bmancini55bmancini55 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good!

@bmancini55
bmancini55 marked this pull request as ready for review July 30, 2021 12:44
@bmancini55
bmancini55 merged commit a95e7d5 into node-lightning:mainJul 30, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Vib-UX@bmancini55