Repository navigation
Conversation
| - MAY send a `warning`. | ||
| - MUST fail the connection. | ||
| - if both nodes advertised the `option_upfront_shutdown_script` feature, and the receiving node received a non-zero-length `shutdown_scriptpubkey` in `open_channel` or `accept_channel`, and that `shutdown_scriptpubkey` is not equal to `scriptpubkey` | ||
| - if not both nodes advertised `option_simple_close` and the `shutdown_scriptpubkey` is `OP_RETURN`: |
There was a problem hiding this comment.
A negation at the beginning of a statement is hard to parse without the right parenthesis 🤓
Is this not ( (both nodes advertised option_simple_close) AND (shutdown_scriptpubkey is OP_RETURN) ) or not ( both nodes advertised option_simple_close ) AND (shutdown_scriptpubkey is OP_RETURN)?
I'd rather rewrite it without boolean gates, explicitly separating simple close from legacy close (it's more verbose but easier to parse IMHO):
- if both nodes advertised `option_simple_close`:
- if the `shutdown_scriptpubkey` received is not using `OP_RETURN`:
- MAY send a `warning`.
- MUST fail the connection.
- otherwise:
- MAY send a `warning`.
- MUST fail the connection.
Does that correctly capture what you meant?
There was a problem hiding this comment.
Hmm, agree it could be formulated better. The wording you propose doesn't mention option_upfront_shutdown_script though, so it would make more sense in the section on option_upfront_shutdown_script where this context is implied, no?
The main edge case that I want to clarify is the case where both nodes negotiate option_upfront_shutdown_script and option_simple_close. In that case, we should permit counterparties to send a shutdown script that is different from the pre-negotiated shutdown script if the new one is OP_RETURN etc. Otherwise option_upfront_shutdown_script and option_simple_close would simply be incompatible IIUC.
There was a problem hiding this comment.
The main edge case that I want to clarify is the case where both nodes negotiate option_upfront_shutdown_script and option_simple_close. In that case, we should permit counterparties to send a shutdown script that is different from the pre-negotiated shutdown script if the new one is OP_RETURN etc. Otherwise option_upfront_shutdown_script and option_simple_close would simply be incompatible IIUC.
Yes, I agree that this can be clarified, I was just pointing out that the current clarification is a bit hard to read and may be misinterpreted.
The wording you propose doesn't mention option_upfront_shutdown_script though, so it would make more sense in the section on option_upfront_shutdown_script where this context is implied, no?
True, feel free to move this somewhere else or change it however you think makes sense!
The `option_upfront_shutdown_script` feature requires the shutdown script to match the script agreed during channel opening. However, `option_simple_close` allows a zero-value `OP_RETURN` output when a node considers its output uneconomical. Allow this exception in both the sender and receiver requirements so the features can be used together. Other script mismatches still fail the connection, and the zero-value output prevents this exception from redirecting funds to another address. Co-Authored-By: HAL 9000
20cf8a7 to
2b2aa9d
Compare
|
Excuse the delay here. Amended with some changes. @t-bast let me know if that reads better. |
There was a problem hiding this comment.
Concept ACK, the nested structure reads much better than the original boolean chain. Two things before this lands.
The exception also needs to cover closing_complete
This PR adds the OP_RETURN exception to shutdown only, but the case the simple close section actually describes happens later: the closer "MAY send a closer_scriptpubkey that is a valid OP_RETURN script" in closing_complete once it considers its output uneconomical. That's often only known once the fee is picked, which is after shutdown has been exchanged.
closing_complete also has no requirement tying closer_scriptpubkey to the upfront script today. The closee only checks that the script is valid "as detailed in the shutdown requirements", and the rationale mentions in passing that the output address can change "if upfront_shutdown_script was not negotiated". lnd treats a new closer_scriptpubkey like a new shutdown script and checks it against the upfront script, and I'd guess others do the same. Without a rule here, implementations will keep diverging on whether closing_complete can change the script under upfront at all, with or without OP_RETURN.
I'd suggest adding the matching closee requirement next to the existing closer_scriptpubkey check, something like:
- if both nodes advertised the `option_upfront_shutdown_script` feature,
and it sent a non-zero-length `shutdown_scriptpubkey` in `open_channel`
or `accept_channel`, and `closer_scriptpubkey` is not equal to that
`shutdown_scriptpubkey`:
- if `closer_scriptpubkey` is not a valid `OP_RETURN` script:
- SHOULD ignore `closing_complete`.
- SHOULD send a `warning`.
- SHOULD close the connection.
(option_simple_close is implied in that section.) The rationale sentence about changing the output address could then point at it.
The rationale overstates the guarantee
so this exception does not allow funds to be redirected to another address.
That's true for addresses, but the funds still go somewhere: to fees, and so to whoever mines the closing transaction. The threat option_upfront_shutdown_script defends against is a compromised node trying to move its funds. With this exception, such a node can no longer send them to an address of its choosing, but it can burn its whole balance to fees, which an attacker with hashpower, or a deal with a miner, can collect. The receiver can't check "uneconomical" either: it's the closer's own judgment, so the exception applies to any amount.
I don't think that's a reason to block the exception, as the interaction is an edge case and simple close needs it to work at all. But the rationale should say what's actually being traded, e.g.:
When `option_simple_close` is negotiated, a node may instead use a valid
`OP_RETURN` script if it considers its output uneconomical. The corresponding
output amount is zero, as specified in [BOLT 3](03-transactions.md#closing-transaction),
so this exception cannot send funds to a new address. It does let the node
give its whole output to fees, so `option_upfront_shutdown_script` no longer
protects against a compromised node that can collect those fees, e.g. one
colluding with a miner.
If y'all would rather keep the original guarantee, the closee could only accept the exception when the closer's output would be below dust_limit_satoshis, which it can check from fee_satoshis and the balances. That is stricter than "uneconomical", though, so I'm not sure it's worth it.
The
option_upfront_shutdown_scriptfeature mandates that a node fails the connection if the counterparty provides a shutdown script that doesn't match what was previously negotiated during channel open.However,
option_simple_closeallows the counterparty to change its shutdown script to a 0-valueOP_RETURNif it doesn't deem its output economical to still meet the dust threshold. Here we simply clarify that both are compatible, i.e., theOP_RETURNspecial case foroption_simple_closeshouldn't be rejected even ifoption_upfront_shutdown_scriptwas negotiated.