-
Notifications
You must be signed in to change notification settings - Fork 3
Conversation
4fef757
to
69a8b12
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for writing this up!
Some general feedback (feel free to ignore 😉 )
- The reader is thrown into the cold water on the whole CFD topic - You mention a previous blog post - Maybe wrapping that into a short introduction would help the reader.
- The rust-lightning lib is mentioned several times, including some implementation challenges around it (custom-output). I think for this blog post its not as important. Probably a higher level would make it more accessible for a broader audience.
- Revisit the evaluation criteria below, a few paragraphs on scale and features could bring us some points 😅
⚡️ Bitcoin Integration & Scalability: Have they used bitcoin/lightning? If so, how many features? How well will this product scale for either local or global adoption?
docs/stories/bitcoin-integration.md
Outdated
It will be the responsibility of the application built on top of `rust-lightning` to spend the custom output. | ||
For the 10101 wallet that will mean publishing a CET according to what the oracle attests. | ||
|
||
If you want to learn more about how a CFD protocol using DLCs works in the context of ItchySats, earlier in the year we wrote a [blog post](https://comit.network/blog/2022/01/11/cfd-protocol-explained/) about it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be helpful if that reference is added somewhere in the beginning (introduction). Here it breaks the flow if I would jump to the blog post 🙈
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it does need a mention here as well, because it is related here, but maybe we don't need a separate sentence for it? I tried to just link it in the sentence above but could not find a good way to do so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be helpful if that reference is added somewhere in the beginning (introduction). Here it breaks the flow if I would jump to the blog post 🙈
I see where you are coming from, but I want to disagree 😛
I think it's okay for them to jump out of the blog post at this stage, because they've just learnt about the "Lightning custom output" protocol. It's encouraging them to dig deeper if they want to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noice work, very concise!
docs/stories/bitcoin-integration.md
Outdated
It will be the responsibility of the application built on top of `rust-lightning` to spend the custom output. | ||
For the 10101 wallet that will mean publishing a CET according to what the oracle attests. | ||
|
||
If you want to learn more about how a CFD protocol using DLCs works in the context of ItchySats, earlier in the year we wrote a [blog post](https://comit.network/blog/2022/01/11/cfd-protocol-explained/) about it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it does need a mention here as well, because it is related here, but maybe we don't need a separate sentence for it? I tried to just link it in the sentence above but could not find a good way to do so.
docs/stories/bitcoin-integration.md
Outdated
For the 10101 wallet that will mean publishing a CET according to what the oracle attests. | ||
|
||
If you want to learn more about how a CFD protocol using DLCs works in the context of ItchySats, earlier in the year we wrote a [blog post](https://comit.network/blog/2022/01/11/cfd-protocol-explained/) about it. | ||
It should still be relevant to what we are building today. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should still be relevant to what we are building today. |
I don't think this sentence helps, maybe it can be incorporated into the sentence(s) above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree and I've already removed it in the next version!
docs/stories/bitcoin-integration.md
Outdated
It will be the responsibility of the application built on top of `rust-lightning` to spend the custom output. | ||
For the 10101 wallet that will mean publishing a CET according to what the oracle attests. | ||
|
||
If you want to learn more about how a CFD protocol using DLCs works in the context of ItchySats, earlier in the year we wrote a [blog post](https://comit.network/blog/2022/01/11/cfd-protocol-explained/) about it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to learn more about how a CFD protocol using DLCs works in the context of ItchySats, earlier in the year we wrote a [blog post](https://comit.network/blog/2022/01/11/cfd-protocol-explained/) about it. | |
Settling a force-close scenario on layer 1 is in line with our original DLC protocol as described in [this blog post](https://comit.network/blog/2022/01/11/cfd-protocol-explained/). Since force-close scenarios are worst-case fallback scenarios it is feasible to handle them on chain rather than within the channel. |
Not super happy about my wording - I'm trying to add some context to make it better understandable :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've used some of this, thank you!
docs/stories/bitcoin-integration.md
Outdated
|
||
- Our changes to `rust-lightning` do not need to be polished for the purpose of upstreaming. | ||
We are taking these weeks as an opportunity to better understand the problem space, but we are building fast. | ||
The code might not be reused in the future, but the ideas surely will. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code might not be reused in the future, but the ideas surely will. | |
The code might not be reused in the future, but the concept surely will. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sentence was removed!
docs/stories/bitcoin-integration.md
Outdated
This is already being [worked on by the community](https://github.com/lightning/bolts/pull/851) and it is out of scope. | ||
There are ways to mitigate this problem (trusting your counterparty, using [submarine swaps](https://docs.lightning.engineering/the-lightning-network/multihop-payments/understanding-submarine-swaps)) and for the tournament we will not spend too much time on this. | ||
- Multi-hop custom outputs (DLCs, CFDs) are not supported. | ||
Given what we said about the Lightning library not even knowing how to spend the custom output directly, it may not even make sense to support multiple hops for these types of outputs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really understand this sentence - given that it is the last sentence and might need more context, maybe just change / remove / reduce it.I would either keep it more high-level or add more context so one can understand what is meant :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made it simpler!
I did have it in mind when writing the blog post! It's fair to say that we have explained how we are using Bitcoin and Lightning. Not sure if it would be very natural to list the features we are using explicitly, but do let me know if we can organically mention (name drop haha) something else. I do find it hard to talk about scaling and adoption. I'm not sure what to say about that in this blog post. |
2ef863f
to
c8734f1
Compare
I've tried to address most of @holzeis' and @da-kami's comments with this force-push. |
bors r+ |
141: Add Bitcoin integration blog post r=luckysori a=luckysori Resolves #14. Sorry it took me so long! Even though I spent a long time today it felt a bit rushed so I'm happy to change almost anything haha. Co-authored-by: Lucas Soriano del Pino <[email protected]>
c8734f1
to
ed6b2af
Compare
Canceled. |
bors retry |
Build succeeded: |
Resolves #14.
Sorry it took me so long! Even though I spent a long time today it felt a bit rushed so I'm happy to change almost anything haha.