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

CIP-0030: Enhance API by adding a 'balanceTx' function #170

Closed

Conversation

koslambrou
Copy link

@koslambrou koslambrou commented Dec 9, 2021

Adds an additionnal API function called balanceTx which balances an unbalanced (or partial) transaction.

@rooooooooob

@koslambrou koslambrou marked this pull request as draft December 9, 2021 15:07
@koslambrou
Copy link
Author

Should the balanceTx also describe HOW to balance a tx?

@crptmppt crptmppt added Minor change Update Adds content or significantly reworks an existing proposal labels Dec 13, 2021
@MarcelKlammer
Copy link

What exactly would that function do? After reading it twice it's not clear to me what behaviour is expected.

@rooooooooob
Copy link
Contributor

I second that it's not 100% obvious what is to be done.

If this is just to add inputs (does it add witnesses too?) to match needed outputs? It also talks about outputs - is it only for change? Does it do either depending on if there's enough already? e.g. if there's not enough inputs it adds them (and possible adds a change output to return the excess), and if there's enough already it returns the rest as a change output.

I also don't understand why this is needed as part of the bridge communication spec. It seems like this could all be done on the dApp side using the available endpoints (getUtxos and getChangeAddress) quite easily and using some kind of library like the cardano-serialization-lib quite easily, since its TransactionBuilder can already do all of those things (at least at current master - the input selection part wasn't added until a couple of months ago) using info from those two endpoints.

If I'm understanding this correctly I think this belongs at the dApp ecosystem library level, and not as part of the communication spec.

@koslambrou
Copy link
Author

I agree that it is not obvious :)

Here's some context: I've been working on integrating the Nami browser wallet (which follows the communication protocol defined in CIP-0030) and the Plutus Application Backend (PAB). The PAB will try to create a transaction tx given some constraints, and it can output an unbalanced transaction (meaning, sum(inputs(tx)) != sum(outputs(tx))). Normally, I would ask a wallet backend (cardano-wallet) to balance this transaction given the funds inside the wallet which activated the PAB contract. However, in my case, the wallet is a browser extension (Nami). Therefore, I would expect it to provide a balanceTx endpoint which adds just enough inputs from the wallet to cover the outputs.

I agree that I can simply use TransactionBuilder.add_inputs_from. But I guess my question is: Shouldn't browser wallets provide an endpoint for this functionnality ?

The same reasoning can be applied with submitTx, which can surely be implemented on the dApp side using a service like BlockFrost for example, but nevertheless is still part of the communication spec. So, shouldn't balanceTx also be part of it?

@KtorZ
Copy link
Member

KtorZ commented Dec 20, 2021

1

Balancing a partially constructed a transaction, in the context of Plutus / PAB typically requires more than just the transaction. What would work nicely I reckon would be to stick to the same communication interface / approach that the PAB uses with the wallet (namely: https://input-output-hk.github.io/cardano-wallet/api/edge/#operation/balanceTransaction). That is, wallets typically need two extra arguments for balancing a partially constructed transactions:

  • A list of fully qualified UTxO entries (or resolved inputs) which have been picked by the DApp. This is because wallet usually don't know about script inputs but knowing how much is held in such inputs is mandatory for balancing a transaction.

  • A list of redeemers data and purpose. This one is a bit more subtle but inputs, minting policies, withdrawals and delegation certificates locked by script address require a redeemer. While there's usually no issue for minting policies, withdrawals and delegations which could be pre-included in the partial transaction, inputs redeemers (the most frequent kind) are annoying to deal with. In fact, input redeemers refer to inputs using a pointer on an input index within the input set and, because inputs are ordered lexicographically in the set, adding new inputs may change the ordering in the set, invalidating redeemers. Hence, to work-around that, (input) redeemers shall be added to transaction only after it has been balanced and, having clients provide a description of the redeemers on the side is necessary.

2

On a different note, transaction balancing is far from being trivial, especially when considering a generic solution which work nicely for all cases. Having this as part of the base API for CIP-0030 raises the bar for implementors quite significantly. While useful, I would strongly suggest to make this a separate CIP working on top of CIP-0030, as an extension which is optionally, albeit recommended, implemented by wallet providers.

@koslambrou
Copy link
Author

Thanks @KtorZ for the detailed comment!

Totally makes sense to create a separate CIP. I'll close this PR, and start working on the other CIP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Update Adds content or significantly reworks an existing proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants