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

tapchannel: add new AuxChanCloser to enable co-op close for asset channels #919

Merged
merged 16 commits into from
Jun 4, 2024

Conversation

Roasbeef
Copy link
Member

In this Pr, we add a new AuxChanCloser that allows co-op close for asset channels. As we haven't fully unified the wallet yet, a "fully loaded" coop close transaction will have 4 output: 2 for the BTC balance, and 2 for the asset amounts. We use a new interface added in lnd to allow the aux chan closer to add additional outputs, and also ensure that sorting works out the same on both sides.

Along the way, we also modify the chan funder to push the funding proof to a Universe if the proof courier addr is specified. This sets us up to ensure that the receiver can properly handle a proof discovery.

For the initiator, we rely on the porter to push out the proofs for our coop close with the specified proof courier addr (currently global, should allow a side to modify). As the non-initiator, since all our asset outputs will be the result of a split (we force a split for full value sends), we simply import the asset output as an addr into the custodian. This way, the custodian is able to watch for on-chain confirmation, then handle obtaining all the proofs we need to be able to send again.

@guggero guggero force-pushed the custom-channels-integration-invoice branch 8 times, most recently from 13f4f29 to d6d9dc9 Compare May 28, 2024 12:58
Base automatically changed from custom-channels-integration-invoice to main May 28, 2024 14:07
@guggero guggero force-pushed the custom-channels-poc-coop-close branch 2 times, most recently from 90243a9 to f2f4d6a Compare May 29, 2024 18:26
@guggero guggero marked this pull request as ready for review May 29, 2024 18:27
@guggero guggero requested review from ffranr and GeorgeTsagk May 29, 2024 18:27
@guggero guggero force-pushed the custom-channels-poc-coop-close branch 2 times, most recently from 9e1986d to f02e269 Compare May 31, 2024 13:21
@guggero guggero force-pushed the custom-channels-poc-coop-close branch from f02e269 to 96e2951 Compare June 3, 2024 11:19
Copy link
Member

@guggero guggero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed almost all commits and added a couple of my own, which I guess counts as a review as well. LGTM 🎉

Copy link
Member

@GeorgeTsagk GeorgeTsagk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only Q while leaving the diff was how are we able to spend a declared_known script?

Looks good!

Roasbeef and others added 6 commits June 4, 2024 17:02
Adds a new RPC method that allows RPC users to easily encode payment
related records into the required TLV custom record format required by
the payment RPCs of lnd.
This commit fixes an issue where an error is bubbled up too far when it
should instead have been handled in the HasProof method of the proof
archive itself.
This lead to proofs not being imported when they should've been.
guggero added 7 commits June 4, 2024 17:02
We'll want to be able to detect that a script key is explicitly declared
as being known to a software that operates on the local node.
This is useful for example when the script key uses an internal key that
is not local (e.g. it cannot be derived by the local wallet, for example
a NUMS key), but the Tapscript spend paths are known to the software
operating on the tapd RPCs. So funds on such script keys should still
show up in the balances.
To be able to distinguish between locally known and potentially
script-encumbered script keys, we add two new methods on the script key
that help us to do that.
To recognize the funding output as belonging to the wallet, we now also
insert it as a declared key into the wallet.
And to make sure the asset is created in the asset DB, we also don't
skip any transfer outputs that are marked as declared keys.
@guggero guggero force-pushed the custom-channels-poc-coop-close branch from 96e2951 to c5e4352 Compare June 4, 2024 15:03
@guggero
Copy link
Member

guggero commented Jun 4, 2024

My only Q while leaving the diff was how are we able to spend a declared_known script?

There isn't an easy way yet, at least not without knowing some internals.
I think we'll probably add a new flag to the FundVirtualPsbt RPC in the future that allows you to select such an asset.
But for now you'd basically export a proof, then use the tappsbt.FromProofs() function to create a vPSBT that you can then spend (which is basically what we're doing in the custom channels use case).

@dstadulis dstadulis added this to the v0.4 milestone Jun 4, 2024
@guggero guggero added this pull request to the merge queue Jun 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 4, 2024
@guggero guggero added this pull request to the merge queue Jun 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 4, 2024
@guggero guggero added this pull request to the merge queue Jun 4, 2024
Merged via the queue into main with commit 3b816e8 Jun 4, 2024
14 checks passed
@guggero guggero deleted the custom-channels-poc-coop-close branch June 4, 2024 17:22
// payment or other channel related RPCs. This RPC is completely stateless and
// does not perform any checks on the data provided, other than pure format
// validation.
func (r *rpcServer) EncodeCustomRecords(_ context.Context,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still this as something that's temp? As in we'd just wrap the payment specific stuff in an entirely new RPC?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, yes. Or maybe not, given that with this it should be pretty easy to do the RPC calls in any language. I guess the question is: how much effort is it for the app devs to call 3 RPCs (buy/sell order, encode records, send payment) with the correct values vs. us threading all the payment and channel flags through some new RPC methods in tapd...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

4 participants