Skip to content
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

Add CustomClaimOutput in PackageSolvingData (custom scripts support) #2300

Closed
wants to merge 1 commit into from

Conversation

ariard
Copy link

@ariard ariard commented May 16, 2023

This PR adds an experimental CustomClaimOutput enum variant in PackageSolvingData and this variant containt a new Box<dyn CustomClaimOutput>. This trait can be implemented by a structure representing the claim data required to solve a generic output. This generic output can be any non-BOLT-standard outputs such as a Discreet Log Contract, a custom Taptree or an atomic swap (non-exhaustive list: only limitations are Bitcoin Script and ECDSA/Schnorr/Taproot).

How CustomClaimOutput can integrate within LDK monitoring backend ?

The ChannelManager and Channel structures would have support for generic packets a la #619. New ChannelMonitorUpdateStep::CustomUpdate is introduced and feeded to a new trait object of ChannelMonitor, CustomMonitorHandler. This trait object is responsible to parse watchted transactions spending generic outputs (CustomMonitorHandler::check_custom_transaction()) and return a set of outputs to watch and outpoints to claim to OnchainTxHandler. As usual, OnchainTxHandler build and broadcast packages generated with CustomClaimOutput.

How an implementation of CustomClaimOutput can be rolled out ?

Let’s say you would like to have a simple 2-of-3 CHECKMULTISIG as a custom script. The struct CheckMultisigOutput can implement CustomClaimOutput in its own out-of-tree checkmultisigscript.rs file. The methods of CustomClaimOutput should ensure this output claim is processed to any standard one like RevokedOutput. This CustomClaimOutput construction should stay internal to the corresponding CheckMultisigMonitorHandler::check_custom_transaction(). Applications should embed those custom implementations in their logic thanks to LDK flexible API.

Do you have pitfalls to use the custom scripts API ?

Yes custom scripts can be vulnerable to transaction-relay issues, witness malleability and changes any default fee-estimation in terms of fee-bumping reserves at least. BOLT 2 channels parameters should be adapted in consequence. The scripts must be written to respect the LN-penalty model, even if in the future Eltoo can simplify this exercise.

This PR is opened to collect feedback on the Box dyn trait approach. Other approaches have been explored such as using types. I’ll move the informative question in its own custom script Tracking Issue

@ariard ariard marked this pull request as draft May 16, 2023 00:10
@TheBlueMatt
Copy link
Collaborator

I think for custom scripts we should consider going the opposite direction here - like we're doing for the anchors stuff generate an event letting the user know there's an output they need to deal with. That would reduce having to touch internal LDK enums like this and also avoid reentrancy into user code.

@ariard
Copy link
Author

ariard commented May 16, 2023

I think for custom scripts we should consider going the opposite direction here - like we're doing for the anchors stuff generate an event letting the user know there's an output they need to deal with. That would reduce having to touch internal LDK enums like this and also avoid reentrancy into user code.

Yes I can see this approach working for the backend though I think you have the 2 following observations:

  • I think you still need a CustomMonitorHandler for the on-chain output detection ?
  • I think you still need one more enum variant in PackageSolvingData like GenericOutput to let our package-management (e.g rebroadcasting frequency) in OnchainTxHandler unmodified ?
  • Can we adopt the same approach for frontend support at the ChannelManager/Channel-level ?

The approach you’re proposing can be definitely worthy it and I’ll try on another branch. One key design requirement I have in mind about “true” custom script support is the fact you can have both BOLT outputs and “custom script” on the same commitment transaction. Fast-forward, if you would like a single liquidity market (and not one splitted in sub-Lightning per-type of script support) I think that’s the way to go? Do we have more design requirements to care about “custom scripts” ?

@TheBlueMatt
Copy link
Collaborator

I think you still need a CustomMonitorHandler for the on-chain output detection ?

Indeed, however a user can implement that completely in parallel with ours, there's no need to have it be connected to our ChannelMonitor logic at all.

@ariard
Copy link
Author

ariard commented May 23, 2023

Indeed, however a user can implement that completely in parallel with ours, there's no need to have it be connected to our ChannelMonitor logic at all.

I disagree. Anyone implementing a custom script use-case would have to connect with OnchainTxHandler and ChainMonitor and replicate the flow of yielding back events to the off-chain coordinator (i.e ChannelManager) to route back on the backward link the witness payload extracted on-chain from the forward link (e.g for routed DLCs, see https://lists.linuxfoundation.org/pipermail/lightning-dev/2020-April/002647.html).

Beyond, it means anyone following the direction you’re indicating won’t benefit from future security hardening we might support like monitor replicas (e.g https://github.com/lightningdevkit/rust-lightning/blob/main/GLOSSARY.md#monitor-replicas) and we already land PRs for monitor replicas in the past (e.g #679). And this is the type of design approach we have followed so far for custom messages or custom onion message handling (i.e CustomOnionMessageHandler).

Would you like more technical explanations on the type of Lightning packets aimed to be supported by our custom scripts API ? Note, things like payments pools and channel factory are clearly out-of-scope. Or do you have other considerations in mind like e.g we would refactor our APIs to support some type of Lightning packets, of which we don’t wish to associate the LDK project image with them in reasons of the “trust model” or very experimental safety of the funds ? (Note the Taro thing has been added on the “official” roadmap without this having being made the topic of any public discussion to the best of my knowledge).

@moneyball
Copy link
Contributor

Note the Taro thing has been added on the “official” roadmap without this having being made the topic of any public discussion to the best of my knowledge

I don't appreciate the continued insinuations that the LDK project is conducted behind closed doors. You keep painting an inaccurate picture.

The LDK roadmap was proposed to the LDK project in the public LDK Discord, soliciting feedback prior to publication. No one raised a concern over "the Taro thing."

The LDK roadmap was also a topic of discussion in the biweekly public LDK meeting. No one raised a concern about it there either.

Taro as an example of what custom scripts could enable was a topic of light discussion on August 24, 2022 in the public LDK Discord. You even participated.

It also should be noted the mention of Taro is in the "Prospective Future Projects" section of the roadmap. No one had signed up for these projects yet so they're clearly ill-defined and very early and subject to change.

It also should be noted the roadmap begins by stating "As a reminder, the LDK project is community-driven with no centralized authority. Anyone is welcome to contribute and propose ideas at any point."

@TheBlueMatt
Copy link
Collaborator

I disagree. Anyone implementing a custom script use-case would have to connect with OnchainTxHandler and ChainMonitor and replicate the flow of yielding back events to the off-chain coordinator (i.e ChannelManager) to route back on the backward link the witness payload extracted on-chain from the forward link (e.g for routed DLCs, see https://lists.linuxfoundation.org/pipermail/lightning-dev/2020-April/002647.html).

I don't see why the calls into user code for signing needs to be in the LDK code, however - we can monitor for outpoints which need to be claimed and give it to the user via an event, even doing so every time we get a rebroadcast call or every time we think they need to bump the fee. This is how we handle broadcasts now with anchors anyway, and avoids reentrancy while still pushing the user towards doing things "the right way".

The only material drawback here is that the user wouldn't be able to trivially batch claims with their custom stuff with the traditional lightning parts of the channel, but I'm not sure I buy that a ton of extra complexity both for the user and for us is worth adding for that.

@ariard
Copy link
Author

ariard commented May 25, 2023

I don't appreciate the continued insinuations that the LDK project is conducted behind closed doors. You keep painting an inaccurate picture.

Please if we can stay polite towards each other. There is no need to assign malicious or deceptive intents by the usage of the word "insinuations".

I think we're doing good as a community in terms of transparency and openness in the discussion of technical priorities. If you're referring to other things source of disagreement between us, thanks if you can clarify your mind.

About Taro, I remember the discussion on Discord in August when Taro was mentioned for the first time iirc. I think I've missed the roadmap discussion and it sounds to me the roadmap PR to update the blog was merged quite quickly.

More fundamentally, I would like to say the issue is not about Taro itself. Note, the following sentence of my previous post "of which we don't wish to associate the LDK project image with them in reasons of the "trust model" or very experimental safety of the funds”. This is more a general remark than a lot of custom scripts are coming with new assumptions (e.g for Taro the universe or assets creators) and they could be source of loss of funds in their early days. This holds for DLC too, where there is no consistent security model for the oracles incentives to the best of my knowledge. As a project, we might be better to underscore the sheer novelty of the security model of those use-cases leveraging custom scripts, by conservatism for the user funds.

That’s all fine, there is no rush with custom scripts and I think we can take time to advance step by step.

@ariard
Copy link
Author

ariard commented May 25, 2023

we can monitor for outpoints which need to be claimed and give it to the user via an event, even doing so every time we get a rebroadcast call or every time we think they need to bump the fee.

Yeah I think this is what I meant by CustomMonitorHandler doing the monitoring and detection of the outpoints, so I dunno if we have the same mental model in mind. The anchor-like approach where we delegate an event to the user to be signed on its own sounds good to me.

The only material drawback here is that the user wouldn't be able to trivially batch claims with their custom stuff with the traditional lightning parts of the channel,

Well batching of custom scripts with the traditional lightning parts of the channels sounds the easy way to screw up things where you might issue non-standard package because each parts is valid on its own, though not when you aggregate them. I believe I think the same here, coming up with code will be more speaking.

@moneyball
Copy link
Contributor

Please if we can stay polite towards each other. There is no need to assign malicious or deceptive intents by the usage of the word "insinuations".

For several months now you've been consistently painting a picture of some kind of Block or Spiral conspiracy. This is pure FUD without any substance or evidence. For example:

From rust-bitcoin on IRC you jump to this conclusion before gathering facts: "if block inc is doing corporate capture of this open-source project please be transparent about it at last"

In LDK GitHub posts you suggest the project might be compromised due to lost trust between Matt, you, and me.

This bothers me because a long held explicit goal of the LDK project (and BDK and BDC) is to be a community project and one that is not controlled by Spiral (let alone Block). I'm not claiming these projects are perfect, and we seek feedback on how to improve. But in order to improve, we need specific, actionable feedback. Making claims that there is corporate capture with zero evidence isn't constructive. I did appreciate your feedback today on LDK Discord about how the spec process can delay things beyond the quarterly time estimates in the roadmap post.

sounds to me the roadmap PR to update the blog was merged quite quickly.

There was 1 week between sharing the proposed roadmap doc with the LDK community and publication. There was no feedback. And there has been no feedback or concern expressed since then except for your stated concern here over the reference to Taro. If others in the LDK community feel there should have been a longer window of time or different process or approach, please let me know!

More fundamentally, I would like to say the issue is not about Taro itself. Note, the following sentence of my previous post "of which we don't wish to associate the LDK project image with them in reasons of the "trust model" or very experimental safety of the funds”. This is more a general remark than a lot of custom scripts are coming with new assumptions (e.g for Taro the universe or assets creators) and they could be source of loss of funds in their early days. This holds for DLC too, where there is no consistent security model for the oracles incentives to the best of my knowledge. As a project, we might be better to underscore the sheer novelty of the security model of those use-cases leveraging custom scripts, by conservatism for the user funds.

All of this sounds like great discussion to have, but I fail to see how the roadmap post gets in the way of any of it or somehow commits the project in a wrong direction.

@ariard
Copy link
Author

ariard commented May 26, 2023

[ed: a number of parts of this comment have been removed as unrelated to the discussion around the roadmap/taro and in violation of the CoC's references to publication of private communications]

And I'm not composing this statement without a feeling of sadness, as I think most of us are aligned in term of technical and philosophical ideas concerning Lightning, and we have all showed a high-level of disinteressment and dedication during the past years to make LDK what it is today.

You're free to reach out offline to pursue the conversation at a time and setting both at our convenience. I’m very open to listen more to your viewpoint. Beyond I have code to get done and too many things to review and get fixed, no shit.


I don’t know about talking further about the LDK roadmap, I don’t think we’re substantially in disagreement about its
process. We’re just in a situation where we don’t have sufficient trust in each other statements to be efficient.

@ariard
Copy link
Author

ariard commented May 27, 2023

@moneyball As someone with GH admin rights deleted my replied comment to yours on #2300, due to a different opinion than mine on the disclosure of private communications in the public interest, I proposed to Matt to have an offline conversation with the Code of Conduct team on our disagreement. With the goal to solve my current deficit of trust towards the Spiral Inc team and dried up the bad ambiance from the last months. I'm all good to shrug about the past miscommunication issues between us and I just aim to reestablish "good faith" communication between us to work all together on future legal issues that all contributors of the LDK project might face as a group, based on Core experience.

The day we're attacked by CSW-like adversaries, it will be more than one man or one organisation job to defend ourselves, as such very humbly I think we all have to win to heal drama from the past in a healthy fashion.

@moneyball
Copy link
Contributor

I think we all have to win to heal drama from the past in a healthy fashion.

This is what I've been trying to do for months, yet just 2 days ago you claimed "Yes - I accuse Spiral and yourself to be in a conspirary at the instigation of [redacted] to attack my personal interests, the LDK users and the long-term survival of the Lightning Network." Yet you offer zero evidence of how I have attacked LDK users or the survival of LN.

@lightningdevkit lightningdevkit locked and limited conversation to collaborators May 27, 2023
@TheBlueMatt
Copy link
Collaborator

Locking this since it's so far off topic.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants