-
Notifications
You must be signed in to change notification settings - Fork 145
Newsletters: add #20 (2018-11-06) #81
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
Conversation
jnewbery
left a comment
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.
Two comments. Otherwise looks great!
| for multiple people to be able to pay the same invoice, such as a | ||
| static donation invoice or a monthly recurrent payment. | ||
|
|
||
| 2. The protocol can't provide proof of payment. You can prove that |
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 the first sentence here is confusing, since the next sentence seems to contradict it. How about replacing the first sentence with something like "It is not possible to prove that a particular party paid the invoice." or similar?
| sent to that address. | ||
|
|
||
| - [LND #2027][] adds a configuration option that allows a node to reject | ||
| new channels being opened with an initial "push" of funds. This is |
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.
Where did you get this motivation? The initial issue was opened because customers were accidentally pushing payment to bitrefill when opening a channel: (lightningnetwork/lnd#1884)
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 didn't mean to suggest that as a motivation for this change. I myself didn't see a clearly-stated motivation for the change and invented that example as a plausible use for this feature. I'd be happy to learn of a different motivation or to remove the suggested benefit from the text.
-
I saw that issue in my research and, double checking it now, I seem to be consistently reading it differently than you. It sounds like you're saying the problem is "Alice accidentally paid Bob when opening a channel to him", but I think the problem is "Alice accidentally opened a channel to Bob when paying him". I think the issue (and the PR fixing it) were about finding a way to keep payments in an existing channel rather than having users open a new channel; I think the documentation in the PR supports this: "
rejectpush: If true, lnd will not accept channel opening requests with non-zero push amounts. This should prevent accidental pushes to merchant nodes."I think this option makes sense for a merchant that only wants users to have an experience of fast payments, as pay-in-a-new-channel payments have to wait for the configured number of confirmations like any onchain transaction.
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.
@jamesob and I have discussed this, and I still think my interpretation is correct. Here's what I guess is happening at bitrefill:
- customer goes through the shopping process on bitrefill. Selects lightning payment and is presented with a screen saying "send 100000 satoshis. Here's a lightning invoice: lnbcabc123"
- the customer doesn't have any channels open, so they decide to open a channel with bitrefill with an initial push of 100000 to pay for the order
- that push is not a payment for the invoice. The customer thinks they've paid, but the merchant doesn't see a paid invoice.
A merchant would want all payments over lightning to be associated with an invoice, so they would never want customers opening funds with an initial push.
This is just speculation though. It'd be great if @juscamarena or @mrwhythat would confirm for us!
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.
@jnewbery is correct with his interpretation.
I opened the issue to have the option to prevent users or even routing nodes from being able to push an amount when creating a channel as it's almost always done mistakenly.
I and other node operators have seen users mistakenly push funds, for a public service that means having to refund the user, for nodes without a public identity it's free money!
At bitrefill did want to have some sort of push and pay while opening a channel but it's a bit trickier without an invoice, unless we have an out of bound acknowledgment that it's for paying an order. Maybe in the future!
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.
Awesome. Thanks for the confirmation @juscamarena !
I went through the bitrefill shopping process today to check my thesis. It looks really good, so great job 👍
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.
@jnewbery thanks for defending your thesis!
@juscamarena thank you very much for commenting! I'll update the text.
| 12.56368175 | ||
| ``` | ||
|
|
||
| As you can see, a miner who enabled segwit would've earned more |
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 the paragraph as written is fine and does not need to change, but just to point out yet another reason .02 btc is more meaningful than it may seem (as a % of 12.5), a miner will be considering their profit margin, and .02 btc is a far higher % of their profit margin than .02/12.5.
|
tACK. Another great newsletter! A made a trivial commit to fix a misspelling, and I left another comment that is largely FYI unless you feel compelling to incorporate it, but please don't feel the need to. |
|
Commit 7a8e6a4 should resolve all feedback. Thank you all! @moneyball thanks for fixing that mispelling! I'd have been quite embarrassed if that went to print. |
|
ACK 7a8e6a4. I'll squash commits (including updated segwit block plot if we have one) before merge. |
d5e3ef2 to
d1f9d84
Compare
|
I've squashed all commits. ACK d1f9d84 |
|
Great work team! 🙌 |
* newsletter273zh * Apply suggestions from code review Co-authored-by: freeyao <[email protected]> * fix style --------- Co-authored-by: Zhiwei(Jeffrey) Hu <[email protected]> Co-authored-by: freeyao <[email protected]>
* newsletter273zh * Apply suggestions from code review Co-authored-by: freeyao <[email protected]> * fix style --------- Co-authored-by: Zhiwei(Jeffrey) Hu <[email protected]> Co-authored-by: freeyao <[email protected]>
Todo: