Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce bLIP numbers in lightning-liquidity documentation #3510

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tnull
Copy link
Contributor

@tnull tnull commented Jan 8, 2025

Recently, LSPS0, 1, and 2 were upstreamed as bLIP-50, 51, and 52, respectively.

Here, we

  1. start linking to the bLIPs instead of the LSP spec repository, which is likely going to be deprecated.
  2. start consistently citing the specs as LSPSX / bLIP-5X to avoid any confusions and to potentially initiate a process in which the LSP specs will over time get referred to by their bLIP number rather than their LSPS number (especially given that any upcoming specs by the LSP spec group will directly be drafted as bLIPs going forward).

@tnull tnull requested a review from TheBlueMatt January 16, 2025 09:32
TheBlueMatt
TheBlueMatt previously approved these changes Jan 20, 2025
Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

Thanks.

lightning-liquidity/src/lsps2/client.rs Outdated Show resolved Hide resolved
@TheBlueMatt
Copy link
Collaborator

Feel free to squash.

Recently, LSPS0, 1, and 2 were upstreamed as bLIP-50, 51, and 52,
respectively. Here, we

1. start linking to the bLIPs instead of the LSP spec repository, which
   is likely going to be deprecated.
2. start consistently citing the specs as `LSPSX / bLIP-5X` to avoid any
   confusions and to potentially initiate a process in which the LSP
   specs will be referred to by their bLIP number rather than their LSPS
   number (especially given that any upcoming specs by the LSP
   spec group will directly be drafted as bLIPs going forward).
@tnull tnull force-pushed the 2025-01-start-referring-to-liquidity-blips branch from 0fd3c19 to 4f077b9 Compare January 20, 2025 15:43
@tnull
Copy link
Contributor Author

tnull commented Jan 20, 2025

Feel free to squash.

Done

diff --git a/lightning-liquidity/src/lsps2/client.rs b/lightning-liquidity/src/lsps2/client.rs
index 70881de6f..9f56f98a2 100644
--- a/lightning-liquidity/src/lsps2/client.rs
+++ b/lightning-liquidity/src/lsps2/client.rs
@@ -62,5 +62,5 @@ impl PeerState {
 /// Note that currently only the 'client-trusts-LSP' trust model is supported, i.e., we don't
 /// provide any additional API guidance to allow withholding the preimage until the channel is
-/// opened. Please refer to the [`LSPS2 / blIP-52 specification`] for more information.
+/// opened. Please refer to the [`LSPS2 / bLIP-52 specification`] for more information.
 ///
 /// [`LSPS2 / bLIP-52 specification`]: https://github.com/lightning/blips/blob/master/blip-0052.md#trust-models

Copy link

codecov bot commented Jan 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.60%. Comparing base (2c0066e) to head (4f077b9).
Report is 60 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3510      +/-   ##
==========================================
+ Coverage   88.30%   89.60%   +1.30%     
==========================================
  Files         149      149              
  Lines      112921   121476    +8555     
  Branches   112921   121476    +8555     
==========================================
+ Hits        99718   108854    +9136     
+ Misses      10722    10218     -504     
+ Partials     2481     2404      -77     

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

Sign up for free to 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.

2 participants