[ADP-3344] Implement createPayment using balanceTx

[HeinrichApfelmus]

This pull request implements the createPayment function in terms of balanceTx.

Comments

• We make the scenario test for createPayment executable, but still mark it as pending, as we need to wait for the getCustomerDeposits function to support rollForward.

Issue Number

ADP-3344

[Anviking]

👍 first look

[Anviking]

It not being IO is neat! But I wonder if there are any downsides with either getting it from WalletState in general or from walletTip specifically 🤔

E.g. possibly re 1) security/privacy or 2) concurrency or 3) retries for different selections

[HeinrichApfelmus]

🤔 My thinking was that balanceTx uses a randomized algorithm to improve the result, but the presence or absence of a result should not materially depend on randomization.

1. Hm. In principle, it could be possible to reverse-engineer information about the StdGen and therefore information about the ChainPoint from the observed selection of coins.
2. You're probably referring to the fact that concurrent calls to balanceTx with the same UTxO may have a higher likelihood of selecting different inputs.
3. Similar to 2). The necessity to retry would indicate a material defect in the balanceTx, though. ("Sometimes it gives a result, sometimes it doesn't".)

Alternative designs:

• Keep track of a StdGen in WalletState. Avoids 1 by revealing only uninteresting information.
• Use a cryptographic hash of the transaction as source of randomness. Avoids 1 and 2.

The point 3 cannot be changed in a stateless manner — you'd need to know about previous attempts at calling balanceTx. A fast-evolving source of randomness can mitigate this, but I think that it's better to have balanceTx not fail randomly.

[Anviking]

1. Hm. In principle, it could be possible to reverse-engineer information about the StdGen and therefore information about the ChainPoint from the observed selection of coins.

I was more thinking that: if utxo, walletTip, guessPartialTx onchainTx can determinsitically produce the resulting onchainTx, it could make correlating enterprise addresses from the same wallet easer, and possibly in some new sense leak information about the utxo set as a whole. However... one would have to guess the walletTip at the time of tx construction, which might make the approach worthless even with just a few likely tips.

You're probably referring to the fact that concurrent calls to balanceTx with the same UTxO may have a higher likelihood of selecting different inputs.

*the same inputs, yes. And I think there's a decent chance it's guaranteed. But... this may possibly not be terrible as you shouldn't rely on randomness to prevent selecting the same inputs anyway 🤷‍♂️

Similar to 2). The necessity to retry would indicate a material defect in the balanceTx, though. ("Sometimes it gives a result, sometimes it doesn't".)

Not necessarily. There could be cases where the user as strong opinions on the selection and would like to discard it to try again.

As for retries not being an intended solution for failing balanceTx results, I do agree. (Whether balanceTx should be guaranteed to find the only succeeding selection in tricky situations if such exists is another question though. I suspect we only need/want some balance >= paymentAmt + delta => can make payment property to hold, and are ok with the result sometimes succeeding and sometimes failing when paymentAmt <= balance <= paymentAmt + delta)

Keep track of a StdGen in WalletState. Avoids 1 by revealing only uninteresting information.

If you provide a non-public initial seed, probably 🤔

Use a cryptographic hash of the transaction as source of randomness. Avoids 1 and 2.

What tx? The one that isn't constructed yet? Identical partial txs may be balanced and submitted repeatedly. We could use all arguments to balanceTx as source of randomness though (in particular the utxo set). This wouldn't avoid 1 though (or it might depend on how far-fetched or realistic 1 is).

[Anviking]

Anyway, happy to go with the approach you have, as it's interesting and I see no clear problem with it, but I did want to raise the points.

[Anviking]

Frankly — I don't think eventually duplicating createPaymentConway and createPaymentNextEra seem that appealing. Why not have the implementation for any IsRecentEra era?

[Anviking]

IsRecentEra era

On this note, I actually still suspect it would be better to consistently keep two eras as "recent" so that we don't forget details which matter only when supporting an era which isn't the latest one, for when we have to before the HF. For instance, that the utxo in theory could be newer than the era where toConwayUTxO might fail (although in practice, this might require both switching nodes and a race condition... 🤔)

[HeinrichApfelmus]

I generally agree on the IsRecentEra era polymorphism.

However, for the Deposit Wallet MVP, I figured that I would limit the scope to Conway — that's why Cardano.Wallet.Deposit.Write.Tx is pinned to a single era for now.

As for the actual implementation of era-polymorphic functions: It's a matter of code reuse — the function createPaymentConway can be reused in an era-polymorphic implementation, or it can be cheaper to write createPaymentPolymorphic directly.

Compare

fooPolymorphic :: IsRecentEra era => Thing era
fooPolymorphic = … barConway … barDonway …

barConway :: Otherthing Conway
barDonway :: Otherthing Donway

vs

fooPolymorphic :: IsRecentEra era => Thing era
fooPolymorphic = … barPolymorphic …

barPolymorphic :: IsRecentEra era => Otherthing Conway era

The second style tends to use lines of code more efficiently, but not always due to case distinctions and type inference. The first style can always be wrapped into the second style by adding code rather than changing it.

[Anviking]

lgtm!