mlabs-haskell / bot-plutus-interface

Unofficial PAB implementation
21 stars 11 forks source link

Better collateral handling #89

Closed szg251 closed 2 years ago

szg251 commented 2 years ago

Problem

Currently, for each transaction we pick the first ada-only utxo we can find in the user's wallet, and use that as a collateral. There are two problems with this approach:

Solution ideas

1. Always return at least one ada change

This would be the easier solution, with a change to balancing logic so that it always returns at least one ada-only change output. https://github.com/mlabs-haskell/bot-plutus-interface/issues/56

Problems:

2. Separate utxo for collateral use only

This is what the nami wallet does, it separates 5 adas as collateral, and never uses that as regular input for transaction. We could do the same, by using a separate address for collaterals and regular inputs, or storing the collateral utxo in state and modifying queries so that it could never get used.

Problems:

GeorgeFlerovsky commented 2 years ago

I strongly prefer Solution 2.

mikekeke commented 2 years ago

There are two problems with this approach

There is one more issue found - case when Ada value of picked collateral is not enough for calculated fee:

Result: ExecutionResult {outcome = Left (ContractExecutionError "WalletContractError (OtherError \"ExitCode 1: Command failed: transaction submit Error: Error while submitting tx: ShelleyTxValidationError ShelleyBasedEraAlonzo (ApplyTxError [UtxowFailure (WrappedShelleyEraFailure (UtxoFailure (InsufficientCollateral (Coin 2000000) (Coin 5460195))))])\\n\")")

Link to source for InsufficientCollateral

GeorgeFlerovsky commented 2 years ago

So, balancing requires collateral to be selected, because balancing involves calculating the tx fee and thus the change output?

If that's the case, then I would suggest the following silly workaround:

  1. Select the largest utxo available and set it as the collateral input, for the purposes of balancing. (This minimizes the chance that balancing will fail due to insufficient collateral during fee calculation -- if that's possible)
  2. After balancing, replace the collateral input in the balanced transaction with the smallest utxo that is larger than the required collateral amount.

Assuming that replacing one collateral input for another in the transaction balanced transaction does not affect fees (I don't think it should), this should give you what you want, improving upon the "random collateral" selection rule.

--

Alternatively, if fee calculation during balancing does not check for collateral sufficiency/existence, then don't set collateral before balancing --- do balancing first and then set collateral to the smallest sufficient utxo.

TotallyNotChase commented 2 years ago

Some more thoughts:-

I was looking into some plutip transactions case by case in an interactive environment and it dawned on me that the "No collateral" issue pops up primarily after a minting transaction where the minted token is paid to your wallet. The new tokens must be put within some ada utxo, but it looks like the balancing algo BPI uses is very eager to jumble up a lot of your existing "small ada change" utxos into one and then put the new tokens inside it. So even if you start with a lot of small ada utxos in change, if your mint + pay to self transactions don't explicitly distribute ada, it might be jumbled up after a few transactions.

This can be solved by putting an explicit constraint that says "create this much ada only utxos as outputs" but the crux of the issue really seems like it lies on the balancing algo IMO. The contract endpoint needs to know how much ada a particular user has to do a "distribute ada" in a UX friendly manner, but that's not the contract's job! It should be the balancer's job.

mikekeke commented 2 years ago

After discussion session current plan is to enable bpi to use separate constant utxo as collateral in automatic manner, such that:

mikekeke commented 2 years ago

Done in #124 and #133