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

Commit from external wallet #215

Closed
11 tasks done
ch1bo opened this issue Feb 11, 2022 · 31 comments
Closed
11 tasks done

Commit from external wallet #215

ch1bo opened this issue Feb 11, 2022 · 31 comments
Labels
amber ⚠️ Medium complexity or partly unclear feature 💬 feature A feature on our roadmap
Milestone

Comments

@ch1bo
Copy link
Collaborator

ch1bo commented Feb 11, 2022

Why

As a user if I want to commit inside a Head, I would need to send whatever I want to commit first to the internal wallet of my hydra node. This is non-ideal as the funds need to be sent there first and would potentially at a higher risk (of losing the internal signing key) in the transit.

What

This feature should allow users to use an any external wallet to commit funds to a Hydra Head. Which means, any outputs supported by the external wallets can be committed (multi-sig, scripts, withdrawals,..), which are not necessarily supported by the internal wallet!

There will still be an internal wallet for "Hydra fuel" though, but that is only used for funds to "drive" the Hydra Head protocol are in custody of a hydra-node and no fuel "marking" is required anymore (related #553). We keep the fuel and only remove fuel handling in a separate step.

How

  • Create a synchronous Request-Response API to get a commit draft transaction from the hydra-node
    • Updated api reference
      • Request, response and errors are documented
    • ADR to cover potential architecture changes
  • An external wallet can balance & sign the commit draft transaction and submit it
  • Deprecate but NOT remove the "internal commit" API, i.e. not a breaking change Non-mandatory fuel #924
    • API docs indicate deprecation
    • Documentations suggest to use the external commit now
    • Keep instructions about marking things as fuel, just in case somebody still wants to do commit from the internal wallet
    • Fallback to using whatever UTxO is available if nothing is marked as fuel internally
  • The TUI is updated to reflect the above Make the tui commit externally #953
  • Write a follow-up feature to do only external commits Remove commit from internal wallet #954

TBD (old)

  • Commit from scripts not possible this way as redeemers cannot be added after the fact

    • A) provide redeemers with original API interaction
    • B) give draft to user and ask it back to us for submission in another roundtrip to hydra-node
  • Does there exist a standard for signing externally?

    • Maybe CIP-30, could be inspiration
    • Nothing sufficient for all kinds of witnesses
  • Would greatly benefit from a request/response query API

@ch1bo ch1bo added 💬 feature A feature on our roadmap amber ⚠️ Medium complexity or partly unclear feature labels Feb 11, 2022
@ch1bo ch1bo moved this to In Planning in Hydra Head Roadmap Mar 10, 2022
@ch1bo ch1bo changed the title External Commits Commit from external wallet May 31, 2022
@ch1bo ch1bo moved this from Next to Later in Hydra Head Roadmap Jun 1, 2022
@ch1bo ch1bo moved this to Later in Hydra Head Roadmap Jun 21, 2022
@ch1bo
Copy link
Collaborator Author

ch1bo commented Sep 9, 2022

FWIW, we drew this a long time ago in this use case diagram:

Image

@parenthetical
Copy link
Contributor

This feature would be very useful to us for our work on a Hydra Head-as-a-Service platform, for the reasons outlined in the motivation. It would greatly reduce the needed trust in the platform provider and improve robustness :)

@ch1bo
Copy link
Collaborator Author

ch1bo commented Sep 21, 2022

@parenthetical Does sound the implementation idea reasonable to you?

As you might have seen we have the Commit API input and this feature would change it's semantics to not actually do commit a UTxO, but return a draft transaction which can be used to do so.

Furthermore, do you think it makes sense to keep both ways? i.e. be able to commit something from the hydra-node owned key when we have the ability to do so via any external wallet?

@parenthetical
Copy link
Contributor

@ch1bo the API with the draft transaction sounds good to me. Maybe this could even be done in one step together with Init to save on a round trip?

I can't think of a reason to keep both... Another process could always handle the commits for which the wallet key is needed, right?

Are there any other actions for which a Hydra node needs the external key?

@ch1bo
Copy link
Collaborator Author

ch1bo commented Sep 22, 2022

Maybe this could even be done in one step together with Init to save on a round trip?

Hm. For the initiator, yes. But for the other parties, this will be a different workflow. Probably not worth it to account for two different workflows, especially in the client application.

I can't think of a reason to keep both... Another process could always handle the commits for which the wallet key is needed, right?

Yes. Doing commits only external should be fine. Our end-to-end tests likely would use a cardano client (or the cli) to sign and submit the transaction then.

Are there any other actions for which a Hydra node needs the external key?

To clarify: External key == payment key owning UTxOs of the user.
No, after committing the funds will be owned by the script on L1.

However, there is one open question / thing to consider here:

  • The current on-chain protocol enforces that one of the parties' public keys was used to sign the commit transaction (determined by the participation token spent in the commit tx)
  • In this design there would still be an internal cardano key identifying the party to the protocol
  • The external wallet would not have that key!
  • So we might return an already signed (by the internal key) transaction and the wallet just adds the signature to spend the committed output.

@perturbing
Copy link
Contributor

To add to this, right now, the committing of UTxO's is conflated with the readiness of the head participants. When all parties added something (it might be an empty utxo) they are marked as ready and the node automatically collects the UTxO's and opens the head.

It might happen due to transaction size limits that a party needs to commit twice. Furthermore, after this first commit, it should not be possible for the other parties to collect and open the head, preventing a party to commit more to the head.

@ch1bo
Copy link
Collaborator Author

ch1bo commented Oct 14, 2022

It might happen due to transaction size limits that a party needs to commit twice.

@perturbing Yes, having two commit transactions is not possible right now. It's also not possible in the basic Hydra Head protocol (as specified and implemented right now).

We even limit the number of UTxOs to commit to 1 right now to not run into limits when collecting & opening the Head. We may be able to increase this limit, but the bottleneck is the collectCom transaction.

Let's open another issue for doing that. It's a bit orthogonal to this effort here about outsourcing the actual commit transaction creation / submission.

@Yasuke
Copy link

Yasuke commented Oct 25, 2022

The way I see it, we need two new Client Inputs; One for Fuel and One for Commit, both would return balanced transactions. The main reason for balanced transactions is that while cardano-wallet exposes an endpoint to balance a transaction, Light Wallets expect to sign balanced transactions. Also I don't know of a Light Wallet that exposes any balancing functionality, nor does a "balance" endpoint exist in the CIP-30 specification.

That being said wallets do usually have the ability to balance transactions internally (An example is when doing a simple send), so if balancing ends up being too much to expect the node to do, you could potentially just make it a requirement that you must balance the transaction yourself, this adds a small amount of friction, though we could likely bridge the gap in Hydra Pay.

The need for Commit is fairly self explanatory as it was already addressed in the initial proposal; The Fuel Client Input would be a balanced transaction that actually funds the internal wallet of the node. A simpler alternative to this is to have a InteralAddress or similar Client Input that would give you the internal wallet address to build a transaction out of yourself.

As for the details of fuel in the internal wallet, how do you envision fuel management from the perspective of the participants managing their node, or the user running the network of nodes for a Head?

@ch1bo ch1bo moved this from Later to Next in Hydra Head Roadmap Nov 8, 2022
@ch1bo
Copy link
Collaborator Author

ch1bo commented Nov 9, 2022

two new Client Inputs; One for Fuel and One for Commit

By having the external wallet do the commit, we can separate it completely from the fuel. That means, anything owned by the --cardano-signing-key given to the hydra-node will be fuel.

As for the details of fuel in the internal wallet, how do you envision fuel management from the perspective of the participants managing their node, or the user running the network of nodes for a Head?

Knowing how much fuel a node has is basically just knowing the balance of the internal wallet's address. Refueling is sending more ADA to the internal wallet's address (like now).

A simpler alternative to this is to have a InteralAddress or similar Client Input that would give you the internal wallet address

We can make this query-able, but wouldn't we have that information available "outside" the hydra-node as we also take the --cardano-signing-key as an argument?

return balanced transactions

I do agree on this and I think the journey could be:

  • User determines what UTxO they want to commit (get's asked by a Hydra client application).
    • This might include preparing UTxOs to only include a chosen Value to be committed as a single UTxO (current limitation).
  • User (or client application) sends the Commit client input, which does take the UTxO to be committed as an argument.
  • The hydra-node prepares a commit transaction, signs it with it's --cardano-signing-key (the actual Hydra Head participant), and returns the balanced transaction. (This is new )
  • The external wallet needs to also sign and submit the transaction.
    • This is assuming the UTxO given was a pubkey address. Making commits from script outputs would require more parmeters to the Commit client input and I would make this out of scope for now.

@Yasuke
Copy link

Yasuke commented Feb 21, 2023

@ch1bo just to be explicit I think the path you have laid out for commit from external wallet makes sense, and hits the right blend of user experience/functionality.

Upon originally reading your response, I exclaimed "That checks out" but didn't actually make that clear in this back and forth, so I figured I would do so now.

@uhbif19
Copy link
Contributor

uhbif19 commented Apr 26, 2023

@ch1bo

Commit from scripts not possible this way as redeemers cannot be added after the fact

So our (hydra-auction) usecase will not be covered?

Commit client input returns a commit draft transaction as a server output

What is draft exactly? I guess TxBodyContent.

@v0d1ch v0d1ch mentioned this issue May 9, 2023
4 tasks
@ch1bo
Copy link
Collaborator Author

ch1bo commented May 9, 2023

I have drawn a sequence diagram for a potential implementation. The architectural changes are not 100% clear yet, but likely we need new interfaces to the HeadState/NodeState and at least a new method to draft transactions on the Chain handle, e.g. draftTx :: ChainStateType tx -> PostChainTx tx -> tx. Interestingly, this is exposing the tx type to the caller and tie up the L1 transaction to be tx as well (which was hidden before "in side" of postTx).

Anyhow, here the draft diagram:

sequenceDiagram
    Client->>+API: GET /commit + UTxO + redeemers

    API->>+QueryHead: getDraft CommitTx
    Note left of QueryHead: New interface for <br/>query-only access to Head
    QueryHead->>QueryHead: check "can commit" in headState
    QueryHead->>QueryHead: getChainState headState

    QueryHead->>+Chain: draftTx (chainState, CommitTx)
    Note left of Chain: New method for <br/>drafting transactions
    Chain->>Chain.State: commit (chainState, args) :: Tx
    Chain->>Wallet: balance & sign :: BalancedTx

    Chain-->>-QueryHead: BalancedTx
    QueryHead-->>-API: BalancedTx
    API-->>-Client: BalancedTx

    Client->>Client: sign & submit Tx
Loading

@ch1bo ch1bo added this to the 0.11.0 milestone May 11, 2023
@ch1bo
Copy link
Collaborator Author

ch1bo commented May 16, 2023

We still need to figure out whether we want to do option A or B:

  • Option A: Submit everything to build the commit transactions witnesses on the original request
sequenceDiagram
    Client->>+HydraNode: GET /commit + UTxO + datums, scripts, redeemers
    HydraNode->>HydraNode: sign Tx
    HydraNode-->>-Client: Tx
    Client->>Client: sign Tx
    Client->>CardanoNode: submit Tx
    CardanoNode-->>Client: Success/Fail
Loading
  • Option B: Provide a draft transaction and ask it back in another request + submit it
sequenceDiagram
    Client->>+HydraNode: GET /commit + UTxO
    HydraNode-->>-Client: TxBody
    Client->>Client: add scripts, data, redeemers, balance & sign Tx
    Client->>+HydraNode: POST /commit + Tx
    HydraNode->>HydraNode: check Tx & sign Tx
    HydraNode->>CardanoNode: submit Tx
    CardanoNode-->>HydraNode: Success/Fail
    HydraNode-->>-Client: Success/Fail
Loading

@ffakenz
Copy link
Contributor

ffakenz commented May 16, 2023

what about a combination of the two?
I mean under "/commit" you accept:

  • UTxO
  • optional scripts
  • optional redeemers
  • optional data
  • sign: true | false

and then we provide the other endpoint for POST /commit using the drafted TX + optional customization from the client, and we check wether or not this TX require further signing and we submit it :)

wdyt? does it makes sense?

@ch1bo ch1bo moved this from Next to Now in Hydra Head Roadmap May 16, 2023
@ch1bo
Copy link
Collaborator Author

ch1bo commented May 16, 2023

Discussion of today's grooming

  • Maybe both approaches? Some clients might not be able to submit the tx
  • Security concerns: We would need to be careful what to sign when given a tx!
  • The other direction is also true, if a client provides all the information to spend from scripts for external commit (in Option A), they would also trust the hydra-node does not "run with the money"
    • Is this really a problem? Committing into a Head is actually giving away funds into the custody of the head
  • Could we ask clients to just build the commit transactions themselves
    • This would require building the commit datum / output correctly, which is maybe tricky for some contexts (having the right serializer)
  • Maybe it just boils down to: Who does the last round of signing (and submits the transaction)

Currently we lean towards option A, but we will make sure to check back with you @Yasuke, @uhbif19 and other users. Please chime in :)

@uhbif19
Copy link
Contributor

uhbif19 commented May 17, 2023

As I said previously, I think option B may be better, because in some cases Hydra client could drop L1 connection completely, and made its configuration simpler.

Also I have another concern. I think that client should have some meant to validate transaction sent by Hydra Node. Otherwise it can always send some "transfer me all ADA" transaction for signing. That means that there should be some public API in hydra-api package which does checking.

@ch1bo
Copy link
Collaborator Author

ch1bo commented May 22, 2023

As I said previously, I think option B may be better, because in some cases Hydra client could drop L1 connection completely, and made its configuration simpler.

This is also possible with Option A by providing a "just submit to L1" endpoint. It's orthogonal to the two options A and B, which are mostly about how the transaction is built and signed.

@GeorgeFlerovsky
Copy link

GeorgeFlerovsky commented May 23, 2023

My vote is for Option B. I think it provides more flexibility for different use cases than Option A.

However, I would refactor Option B into three distinct endpoints:

  1. GET /prepare-commit + UTXO -- a pure query to the hydra node to help the client fill in the hydra-related inputs/outputs in the tx body of the commit, as a convenience that allows the client to avoid monitoring the hydra state utxo or tracking the participant token utxos. The client sends the utxo ref that she intends to commit to the hydra head, and the hydra node responds with the partial+unbalanced transaction body that includes the head state utxo, the participant token utxo, the committed utxo input, and the committed utxo output (with correctly serialized datum).

  2. GET /sign-commit + Tx -- another pure query to the hydra head (impure if you consider signing an effect 😄). Client sends a complete and balanced commit transaction to the hydra node for signature, and the hydra node responds with its signature if the transaction is valid.

  3. POST /submit-tx + SignedTx -- client sends a complete, balanced, and signed transaction to the hydra node for submission. The hydra node keeps the client synchronously informed about the tx status on L1.

Analogous to the self-service (1 + 2) and full-service (1 + 2 + 3) options at gas stations. 😄

@ch1bo
Copy link
Collaborator Author

ch1bo commented May 23, 2023

@GeorgeFlerovsky Unfortunately we cannot do your number 2. due to security reasons, or at least we would not feel very comfortable doing it. It could be used to have the hydra-node sign arbitrary transactions remotely.

Your option 3. is orthogonal to this feature and might be implemented in any case for convenience.

@GeorgeFlerovsky
Copy link

GeorgeFlerovsky commented May 23, 2023

@ch1bo In a two-party transaction, there are two distinct roles:

  1. The first signer ("offeror" in contract law) gets to draft the transaction before signing.
  2. The second signer ("offeree" in contract law) has to review the transaction before signing.

The first signer is inherently in a more secure position because he drafts the transaction, whereas the second signer needs to review a wider surface of unverified information.

Regarding external commits in Hydra:

  • If the client is the second signer, then the hydra node feels more secure against fuel leaks, but the client is more concerned about leaking more funds than intended into the hydra head (or elsewhere).
  • If the hydra node is the second signer, then the client is more secure but the hydra node has to do more review to safeguard its fuel.

Personally, I think that it is better for the hydra node to be the second signer, because it has a narrower scope of utxos to safeguard against leaks.

@GeorgeFlerovsky
Copy link

GeorgeFlerovsky commented May 23, 2023

RE POST /submit-tx + SignedTx, agreed that it is orthogonal. That was exactly my point -- since it is orthogonal, we only need to worry about the tx preparation and signing in the scope of this issue.

@GeorgeFlerovsky
Copy link

GeorgeFlerovsky commented May 23, 2023

@ch1bo
On the other hand, the narrower scope of utxos controlled by the hydra head means that it should be easier for the client to verify the full transaction — filter out the utxos containing hydra tokens and make sure that the rest of the inputs/outputs aren't evil.

I guess it's fine for the client to be the second signer, as your current PR implements it. 👍

@ch1bo
Copy link
Collaborator Author

ch1bo commented May 23, 2023

@GeorgeFlerovsky I see and thanks for bringing up this point.

It makes Option B, where the hydra-node would be checking the submitted tx to be indeed a commit tx and only signing / submitting it if everything is fine, more favorable. In fact, it's the exact same operation as observing a commit tx, which is naturally available in the hydra-node.

Sticking to Option A and the need for the client to check whether the returned transaction does what was intended is yielding more work for the client (= bad UX/DevX) and I already see it coming that 99% of applications running on Hydra would not do this check.

@madeline-os
Copy link

@Yasuke is on vacation, so I will answer on behalf of him and the hydra-pay team. After discussing the two options internally, we are confident that either solution is workable for us, but we think that Option B is more favorable for the ecosystem. It makes it easier to implement hydra clients, and the UX is somewhat nicer for the end user.

@pgrange
Copy link
Contributor

pgrange commented May 24, 2023

With option B, one difficulty I can see is that the burden to construct a well-formed commit transaction is now on the client.

@GeorgeFlerovsky I now you've done it already in your solution. What's your opinion on the difficulty level to do so? And can you think of things we can provide to the client to ease this process?

@GeorgeFlerovsky
Copy link

With option B, one difficulty I can see is that the burden to construct a well-formed commit transaction is now on the client.

@GeorgeFlerovsky I now you've done it already in your solution. What's your opinion on the difficulty level to do so? And can you think of things we can provide to the client to ease this process?

Redirect to @uhbif19, who implemented this in our solution

@ch1bo
Copy link
Collaborator Author

ch1bo commented May 24, 2023

With option B, one difficulty I can see is that the burden to construct a well-formed commit transaction is now on the client.

Option B as outlined above would return already a blueprint for a commit transaction to the client. However, a client would not know whether this is proper or not. As a first step it would only make sure its balanced (pay the fee), add its witnesses and sign it. To really check whether the returned transaction draft is a sound commit transaction, these steps, as also done by the hydra-node when observing a commit transaction on the L1, would need to be done.

@uhbif19
Copy link
Contributor

uhbif19 commented May 24, 2023

@pgrange @GeorgeFlerovsky

I think that I understood this issue a little bit wrong before.

Yes, Option A definitely does not make any sense, cuz client should validate tx then.

filter out the utxos containing hydra tokens and make sure that the rest of the inputs/outputs aren't evil

But this needs to be supported by Hydra API lib, or it might break on version change. And if it is covered by Hydra lib, why ask server for that?

So, AFAIK, there are this parts of this task:

  1. Get id of Initial Utxo from Hydra.
    Also seems like ScriptRegistry, L1/L2 Hydra public key
    and hydra ledger protocol params are required.
    Only L2 key is available now.
  2. Construct Tx
  3. Add fee and balance
  4. Sign Tx by client (optional, needed only if they spend their Utxos)
  5. Verify Tx by Head
  6. Sign Tx by Head
  7. Submit to network

I think that 1 should be returned by hydra-node. Maybe they might just be included in greeting.
2 should be done by Hydra API lib. Like app gives a list of Utxos with witnesses and maybe minting params and gets TxBody.
Then client handles 3 itself. There might be different strategies for selection coins for fee, so it should probably be able to add new TxInputs. I think this does not affect Hydra init script?
For balancing the list of all input UTxOs is needed, not sure if it can be retrieved from TxBody.

As for 7 (submiting), I am not sure if it need to be separated
from Hydra node signing. Maybe for some kind of protocol,
where not all tx are sumbited, but client must be ensured do be able to do that?

@GeorgeFlerovsky
Copy link

GeorgeFlerovsky commented May 24, 2023

As for 7 (submiting), I am not sure if it need to be separated from Hydra node signing. Maybe for some kind of protocol,
where not all tx are sumbited, but client must be ensured do be able to do that?

I think that for some dapp protocols, it would be advantageous for the client to submit and monitor the transaction themselves through their own node.

It's useful to have the hydra endpoint for convenience to submit transactions on the clients' behalf, but it should not be mandatory to use it.

@GeorgeFlerovsky
Copy link

GeorgeFlerovsky commented May 24, 2023

With option B, one difficulty I can see is that the burden to construct a well-formed commit transaction is now on the client.

George, I now you've done it already in your solution. What's your opinion on the difficulty level to do so? And can you think of things we can provide to the client to ease this process?

I think that external commits will be used by two types of users:

  1. Users that are just trying to commit pubkey-based utxos to a hydra head that they do not control.
  2. Users that are interacting with hybrid L1/L2 dapps (e.g. hydra auction) that host part of the dapps' state on L2.

For the simple case (1), the TxBody returned by GET /prepare-commit + UTXO is already a complete transaction (tx fees can come from the user's utxo) -- it's only missing the user and hydra signatures. The user's frontend can just double-check that the transaction is only consuming the user's chosen utxo and that the rest of the utxos are hydra-related (perhaps there should be a frontend helper function to check this).

For the more complicated case (2), the dapps already need to verify and construct transactions in this way.

@uhbif19
Copy link
Contributor

uhbif19 commented May 24, 2023

I think that for some dapp protocols, it would be advantageous for the client to submit and monitor the transaction themselves through their own node.

But if they monitor Tx, they would probably like to monitor them all. And other Txs could not be submitted by users.
So Hydra could just return all submitted Tx Ids on relevant events.

The user's frontend can just double-check that the transaction is only consuming the user's chosen utxo and that the rest of the utxos are hydra-related (perhaps there should be a frontend helper function to check this).

Why do this instead of creating Tx on client side?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
amber ⚠️ Medium complexity or partly unclear feature 💬 feature A feature on our roadmap
Projects
None yet
Development

No branches or pull requests

9 participants