Skip to content

Adds new architecture diagram and update copy #152

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Aug 17, 2022
Merged

Adds new architecture diagram and update copy #152

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2183fb0
Adds new architecture diagram and update copy
Aug 2, 2022
c2c8b1a
Update copy
Aug 3, 2022
70db1ca
Fixes typo
Aug 3, 2022
96f8110
Reorder languages
Aug 3, 2022
ff93b52
Update copy
Aug 3, 2022
8940c49
Fix typos and update readiness of bindings
Aug 11, 2022
65bd98f
Add updated diagram
Aug 13, 2022
1e6bccb
Minor copy updates
Aug 16, 2022
9c2eabc
Update image
Aug 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/.vuepress/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const docsSidebar = [
title: 'Overview',
collapsable: true,
children: [
['/overview/architecture', 'LDK Architecture'],
['/overview/architecture', 'Architecture'],
['/overview/persistent_storage', 'Persistent Storage'],
['/overview/blockchain_data', 'Blockchain Data'],
['/overview/wallet_management', 'Wallet Management'],
Expand Down Expand Up @@ -184,7 +184,7 @@ module.exports = {
link: '/running-a-sample-ldk-node/'
},
{
text: 'LDK Architecture',
text: 'Architecture',
link: '/overview/architecture/'
},
{
Expand Down
71 changes: 70 additions & 1 deletion docs/assets/ldk-architecture.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion docs/introduction/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ provide a useful overview of Rust Lightning in the context of language bindings.
The sample serves as a complete reference for constructing a Lightning node with
the LDK. This is a good starting point if you want a self-guided tour!

### [LDK Architecture](../overview/architecture)
### [Architecture](../overview/architecture)

Gives a high-level organization of LDK and how the pieces fit together. Variations of this diagram
are used throughout the site. This is the primary source and is still a work in progress.
18 changes: 10 additions & 8 deletions docs/overview/architecture.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
## LDK Architecture
![LDK Architecture](../assets/ldk-architecture.svg)
## Architecture
![Architecture](../assets/ldk-architecture.svg)
Copy link
Contributor

Choose a reason for hiding this comment

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

earlier this picture was .png in previous revision, its back to .svg i guess.
svg are generally not considered standard and are security concern because they are executing code.

"The danger of an SVG file comes from the fact that it’s an XML that can have embedded CSS and JavaScript. The web browser will automatically run any JavaScript embedded in an SVG file. Therefore, if the script contains malicious code, it will place the user’s computer at risk."
from: https://blog.online-convert.com/svg-file-and-its-danger/

Copy link
Contributor

Choose a reason for hiding this comment

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

Although we created this image and its not a security concern, but would prefer if its just png or jpeg for simplicity.

Copy link

Choose a reason for hiding this comment

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

LDK_architecture_revised_2022-08-17_opt

There is a slight issue with the dashed strokes in that image, so I fixed and optimized it. See above 73kb SVG.

Although we created this image and its not a security concern, but would prefer if its just png or jpeg for simplicity.

While there is certainly the possibility that malicious code can be hidden inside an SVG file, the same can be said for any other pieces of code on the site.

Also, the existing architecture page contains 20 SVG tags in its current form. Including the current architecture image.

Therefore, I don't buy that argument at all @G8XSU.

Now having said that, if we want to have a discussion about switching to PNG or JPG for images like these, that conversation should be focused on usability and load time, IMO.

I ran this through a PNG optimizer and got it down to around 35kb. And we can get similarly good results with a 2x export of the same image.

LDK_architecture_revised_2022-08-17_opt

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mostly agree with @sbddesign here. Even if we want to have the performance discussion a 73kb optimized SVG is pretty good for this example and usage. I'll update to use the latest SVG provided for now and log an issue to discuss the overall strategy for image formats etc

Copy link
Contributor

Choose a reason for hiding this comment

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

Well i was just pointing out that in previous rev it was png and now it changed to svg.
As long as it is intentional and not a miss, go ahead since we are uploading these to server 👍


LDK's core components are shown in the center box labeled `lightning`. The boxes
with dotted borders are LDK's modules — these must be configured with either
off-the-shelf or custom implementations that you provide.
Firstly, LDK is not a daemon, but rather its own implementation of the Lightning protocol written as an SDK and designed to be compiled and executed in your application.

`EventHandler` in the diagram is not so much a necessary LDK module, but instead
refers to the fact that LDK generates events that the user should handle (e.g.
the `PaymentReceived` event).
This makes it environment agnostic, allowing you to choose your own data storage, wallet, networking stack and blockchain monitoring. In the diagram above the boxes with dotted borders are LDK's modules — these must be configured with either default or custom implementations that you provide.

LDK also uses an event-driven architecture which allows for asynchronous result notification. For example you perform actions like making payments without waiting for the result and are later made aware via an event if the payment was successful or not.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ignore if i am preempting your next step.
It could be helpful if we add a small subsection with 2-liner about each of the modules and their functionality
(Can tackle in next PR)

2-liner should probably be an answer to:

  • Storage: what does LDK use this for ?
  • Broadcast transactions: what does LDK use this for ?
    etc.

Need not be too detailed in overview section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah the plan is to provide examples for both of these in the following PR's.
I plan to tackle Peer management, Channel management, Payments & Routing and their dependencies.

Copy link
Contributor

Choose a reason for hiding this comment

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

That was going to be my feedback after seeing the preview...I liked the organization of Peer management, Channel management, and Payments & Routing from the btc++ prezo.

::: tip Langauge Bindings

Although the core SDK is written in Rust, LDK supports many other programming languages. These include Java/Kotlin, Swift, JavaScript/TypeScript (Beta), C++ (Alpha). Check out [examples](../examples.md) to see some of the implementations out in the wild!