Run multiple coin selections to estimate fees

[rvl]

Issue Number

Relates to ADP-283 / #1648.

Overview

• Extends POST /wallets/{id}/payment-fees API endpoint with a "better" estimation. Adds fields for expected minimum and maximum fees.

Comments

• decide on whether the api change should be breaking or backwards compatible
• also do similar fee estimation for delegation transactions
• better handling of coin selection errors while sampling
• determine how many samples to take, or a criteria for stopping sampling.
• decide on quantile calculation method (currently using median).
• unit test estimateFeeForCoinSelection

[jonathanknowles]

Hi Rodney

I've not looked into the statistics yet, but this looks very reasonable.

I do have a question regarding backwards compatibility.

The approach taken (so far) is to add a pair of fields to the existing ApiFee type:

- newtype ApiFee = ApiFee
-     { amount :: !(Quantity "lovelace" Natural) }
+ data ApiFee = ApiFee
+     { amount :: !(Quantity "lovelace" Natural)
+     , estimatedMin :: !(Quantity "lovelace" Natural)
+     , estimatedMax :: !(Quantity "lovelace" Natural)
+    }

Values of this type are returned by the following endpoint:

POST /wallets/{id}/payment-fees

As an alternative to modifying this endpoint, have you considered introducing a new endpoint and new type? Something like:

POST /wallets/{id}/payment-fee-estimates

With the following type:

data ApiFeeEstimate = ApiFeeEstimate
    { estimatedMin :: !(Quantity "lovelace" Natural)
    , estimatedMax :: !(Quantity "lovelace" Natural)
   }

[jonathanknowles]

Hi @rvl

Given the requirements as stated, this PR looks very sensible to me!

Ideally, we'd have some experimental data available to indicate the time and space costs of making multiple coin selections for different kinds of UTxO distributions. Unfortunately we don't have such data yet. (Though I'd very much like to create such a benchmark in future.)

Having said that, our current coin selection property tests do give us some (limited) information. With our current configuration, we're able to generate around 1,000 coin selections per second on our test machine. But this time includes the time taken to generate arbitrary UTxO sets. We currently generate UTxOs with between 1 and 256 entries, chosen according to a uniform random distribution. If we were to start with the same UTxO every time (to match what this PR is doing), this would almost certainly reduce the calculation time.

Assuming 100 samples from a similarly-sized UTxO set, the fee estimation endpoint could take around 1/10 second on a similar machine. So this seems reasonable as a starting point. 👍

If we wanted to err on the side of caution, then we could watch the clock while taking samples, and stop prematurely if we've taken too long. (Though perhaps that could be a future PR.)

As for the rest of the PR: I only have a minor request, which is to fix a couple of formatting issues (see comments with suggested changes included).

Also, I have a question regarding the property test (relating to the fee maximum).

Apart from that, all looks good!

[rvl]

We are going to change the estimate-fees api without making it backwards compatible.

[rvl]

Thanks - rebased and squashed.

bors merge

[iohk-bors]

Build failed

• buildkite/cardano-wallet

[KtorZ]

bors r+

[iohk-bors]

Build failed

• ci/hydra-eval

[KtorZ]

bors rety

[KtorZ]

bors retry

[iohk-bors]

Build succeeded

• ci/hydra-eval
• buildkite/cardano-wallet
• ci/hydra-build:required