Adds new architecture diagram and update copy #152

ConorOkus · 2022-08-02T10:20:26Z

Screenshots & Preview

netlify · 2022-08-02T10:20:34Z

✅ Deploy Preview for lightningdevkit ready!

Name	Link
🔨 Latest commit	`9c2eabc`
🔍 Latest deploy log	https://app.netlify.com/sites/lightningdevkit/deploys/62fce87ab2fa3500097d748e
😎 Deploy Preview	https://deploy-preview-152--lightningdevkit.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

docs/overview/architecture.md

G8XSU · 2022-08-02T20:07:55Z

docs/overview/architecture.md

-## LDK Architecture
-![LDK Architecture](../assets/ldk-architecture.svg)
+## Architecture
+![Architecture](../assets/architecture.png)


Diagram:

doubt: what is wallet tooling ?

instead of direct arrow to database, it should be to Storage Module(dotted) and then to database.

"LDK creates events that users can respond to" -> "LDK emits events that users can act upon"

"Net I/O" -> "Network I/O"

Not 100% sure what we mean by tooling, I think it can refer to different types of integrations, HWI, remote signing, encryption, testing in different environments etc.

instead of direct arrow to database, it should be to Storage Module(dotted) and then to database.

We don't really have a Storage Module AFAIK, rather LDK provides the raw bytes for the network graph, channel state etc and the user defines the schema they want to use to back that data up. Correct me if I'm wrong @jkczyz @arik-so

The other changes to the diagram seem reasonable, thanks.

instead of direct arrow to database, it should be to Storage Module(dotted) and then to database.

We don't really have a Storage Module AFAIK, rather LDK provides the raw bytes for the network graph, channel state etc and the user defines the schema they want to use to back that data up. Correct me if I'm wrong @jkczyz @arik-so

The other changes to the diagram seem reasonable, thanks.

Arguably, there could be a "Persistence" box that sits between LDK and "Data storage" just as "Chain Interactions" sits between LDK and the depiction of "Bitcoin Network". The diagram was meant to be a bit more high-level, thus the use of depictions rather than simple boxes in places. And the term "module" is a bit nebulous. e.g., Is fee estimation a module? It's mentioned in a later slide but isn't depicted anywhere.

Similarly, when making payments someone can bring their own channel scoring or router -- though we provide our own -- but those aren't depicted anywhere. At a higher-level, these are part of making payments. Perhaps there should have been a depiction of (and/or box for?) this in the diagram.

So maybe a dotted box is really either a module or set of related modules. @arik-so Thoughts?

Yeah I think that's fair, maybe these dotted boxes are better being termed as components as you had before. It's tricky some have default implementations others do not. I like the higher-level approach though as it's easier to drill down from there e.g I know to get started I'll need:

A PeerManager

A ChannelManager

A way to route payments, generate invoices etc

Then I can drill down into what "modules" are needed to create these objects.

So maybe a dotted box is really either a module or set of related modules.

If my memory is correct from when we built this family of diagrams at BTC++, the dotted line was meant to indicate that the developer could swap this out with whatever they wanted. For example, Net I/O could be the pre-built Tokio example, or they could swap that out with their own network I/O stuff. I'm not sure if it makes sense to make that distinction explicit with a key/legend, but I think that was the intent. Many of the other diagrams don't have as many dotted line boxes.

Here is an updated SVG with revised text and a Persistence box.

Should we remove the arrow from the dotted line going form LDK to Persistence for consistency with the other dotted lines connected to LDK?

Good catch, I think so.

@sbddesign one last nit 🚧

docs/overview/architecture.md

G8XSU · 2022-08-02T20:19:21Z

docs/overview/architecture.md

+This makes it environment agnostic, allowing you to choose your own networking stack, data storage, wallet and blockchain monitoring. In the diagram above the boxes with dotted borders are LDK's modules — these must be configured with either off-the-shelf or custom implementations that you provide.
+
+We also use an event driven archictecture that will ansychronously prompt you to do an number of things. For example LDK will generate an event when a payment is received (`PaymentReceived`). More on this in a later section. 
+


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.

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.

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.

docs/overview/architecture.md

G8XSU · 2022-08-09T18:09:11Z

docs/overview/architecture.md

+
+::: tip Langauge Bindings
+
+Although the core SDK is written in Rust, we support a number of other programming langauges. These include C/C++, Java, Kotlin, Swift, JavaScript/Typescript/WASM (Alpha). Check out [examples](../examples.md) to see some of the implementions out in the wild!


Synced on ldk-dev group regarding the same, also about readiness of bindings.

"Java/Kotlin, Swift, TypeScript/JavaScript(Beta), C++(Alpha)"

We don't want to list C, because its unusable.

Thanks @G8XSU, fixed in - b066d58

jkczyz · 2022-08-12T18:45:07Z

@ConorOkus Will need the new diagram in the branch before merging.

G8XSU · 2022-08-12T19:01:10Z

docs/overview/architecture.md

+
+::: tip Langauge Bindings
+
+Although the core SDK is written in Rust, we support many other programming languages. These include Java/Kotlin, Swift, JavaScript/Typescript/WASM (Beta), C++ (Alpha). Check out [examples](../examples.md) to see some of the implementations out in the wild!


Should also drop WASM from the list

Probably "TypeScript" instead of "Typescript" while you're at it lol

ConorOkus · 2022-08-13T09:26:10Z

Thanks @jkczyz @G8XSU @sbddesign. Could do with one more look-through before I squash.

docs/overview/architecture.md

ConorOkus · 2022-08-16T18:15:55Z

Addressed latest comments in - 1e6bccb

Thanks @jkczyz

jkczyz

LGTM. Feel free to squash.

G8XSU · 2022-08-17T06:00:04Z

docs/overview/architecture.md

-## LDK Architecture
-![LDK Architecture](../assets/ldk-architecture.svg)
+## Architecture
+![Architecture](../assets/ldk-architecture.svg)


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/

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

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.

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

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 👍

G8XSU reviewed Aug 2, 2022

View reviewed changes

G8XSU reviewed Aug 9, 2022

View reviewed changes

G8XSU approved these changes Aug 12, 2022

View reviewed changes

G8XSU reviewed Aug 12, 2022

View reviewed changes

jkczyz reviewed Aug 15, 2022

View reviewed changes

Conor Okus added 7 commits August 16, 2022 19:06

Adds new architecture diagram and update copy

2183fb0

Update copy

c2c8b1a

Fixes typo

70db1ca

Reorder languages

96f8110

Update copy

ff93b52

Fix typos and update readiness of bindings

8940c49

Add updated diagram

65bd98f

ConorOkus force-pushed the architecture-diagram branch from 64e5da6 to 65bd98f Compare August 16, 2022 18:06

Minor copy updates

1e6bccb

jkczyz reviewed Aug 17, 2022

View reviewed changes

G8XSU reviewed Aug 17, 2022

View reviewed changes

Update image

9c2eabc

ConorOkus force-pushed the architecture-diagram branch from 09a8083 to 9c2eabc Compare August 17, 2022 13:09

ConorOkus merged commit a8bbd1b into lightningdevkit:main Aug 17, 2022

ConorOkus deleted the architecture-diagram branch August 17, 2022 13:14

		This makes it environment agnostic, allowing you to choose your own networking stack, data storage, wallet and blockchain monitoring. In the diagram above the boxes with dotted borders are LDK's modules — these must be configured with either off-the-shelf or custom implementations that you provide.

		We also use an event driven archictecture that will ansychronously prompt you to do an number of things. For example LDK will generate an event when a payment is received (`PaymentReceived`). More on this in a later section.


		::: tip Langauge Bindings

		Although the core SDK is written in Rust, we support a number of other programming langauges. These include C/C++, Java, Kotlin, Swift, JavaScript/Typescript/WASM (Alpha). Check out [examples](../examples.md) to see some of the implementions out in the wild!

Adds new architecture diagram and update copy #152

Adds new architecture diagram and update copy #152

Uh oh!

Conversation

ConorOkus commented Aug 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify bot commented Aug 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for lightningdevkit ready!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ConorOkus Aug 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkczyz Aug 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbddesign Aug 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkczyz commented Aug 12, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ConorOkus commented Aug 13, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ConorOkus commented Aug 16, 2022

Uh oh!

jkczyz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ConorOkus commented Aug 2, 2022 •

edited

Loading

netlify bot commented Aug 2, 2022 •

edited

Loading

ConorOkus Aug 3, 2022 •

edited

Loading

jkczyz Aug 3, 2022 •

edited

Loading

sbddesign Aug 8, 2022 •

edited

Loading