defer selection improvement in coin selection

[paweljakubas]

Issue Number

#461

Overview

• I have implemented random algo with input selection for each output and then improvement
• I have extended coin selection errors by adding ErrInputsDepleted

Comments

[paweljakubas]

this is one of very interesting cases. Occassionally, random is better than this if for 41 it chooses [20,20,5] and for 6 output 10. largestFirst gives ErrInputsDepleted here deterministically. random can give non-error. If the dices are against us here in case of random we have fallback to largestFirst. Saying that, at this moment, I see only one additional property - there are cases when largestFirst give deterministically error, but in such cases, random can in non-deterministic way choose valid inputs. The question for me is if we want to add such a property

[KtorZ]

It'd be hard to express such property actually 🤔 ... Have you "rough" idea in mind already of what this could look like ?

[paweljakubas]

agree here. it is very specific case, but I believe it is going to pop up someday. We need additional coin to select to cover one output, and there are two coins left, both can cover the output. But we have also last output to cover, and here only larger one coin can cover it. in largestFirst we are doing always this bad choice ending up in error. In case of random we have 50% chance to do it right (if we are already facing this last random pick). And here random shows its precedence. So in order to provide property like this we need to prepare very specific Coin Selection generator emulating such last choice....

[KtorZ]

Looks good overall, some remarks about readability of type signatures mainly ?

[KtorZ]

If we swap the arguments of improveTxOut, we can simplify this expression a bit and avoid the naming 0, 1, and 2:

Just (utxo', res) -> 
    lift $ foldM 
        (improveTxOut opt)
        (mempty, utxo')
        (reverse res)

[KtorZ]

Why did we get rid of the CoinSelection type here? Because we do this in two steps, the type shouldn't be much different since the second step is somewhat similar to (UTxO, CoinSelection) -> (UTxO, CoinSelection).

It becomes very noisy quickly with all the TxIn and TxOut sums and products. So if nothing justifies it strongly, I'd rather stay with a simpler type-signature 👍

[paweljakubas]

in the first phase, we are constructing selections for all outputs, also draggng along utxo that is diminished by doing so. basically, in the phase two we start for each output with the selected inputs and try to improve with the current utxo. only then we construct coin selection (in which also change is computed). So, for me, it was easier to operate in phase one and as an input to phase two with type having just already selected inputs for each output plus current UTxO. Probably we can use CoinSelection here - I need a while to think about it

[paweljakubas]

I see one obstacle to use CoinSelection at phase 1. If I use <> when processing outputs, I will coalesce all inputs from different outputs. We have :

72 data CoinSelection = CoinSelection                                                                                                                                                         
73     { inputs  :: [(TxIn, TxOut)]                                                                                                                                                           
74       -- ^ Picked inputs                                                                                                                                                                   
75     , outputs :: [TxOut]                                                                                                                                                                   
76       -- ^ Picked outputs                                                                                                                                                                  
77     , change  :: [Coin]                                                                                                                                                                    
78       -- ^ Resulting changes                                                                                                                                                               
79     } deriving (Generic, Show, Eq) 

And then I will not have this knowledge at stage 2 (when doing improvement). And here I need exact inputs per output. So using coin selection at phase 1 somewhat blur boundaries upon <> Maybe I am missing something...

[paweljakubas]

Nevertheless, [([(TxIn, TxOut)], TxOut)] may look cryptic, although this represents the list of (inps, txout) ie., list of all inps selected for a given txout. For me very natural, and very closely reflecting the nature of the algorithm we used

[KtorZ]

🤔 ... hmmm. Since we treat each output independently, would it make sense to actually consider [CoinSelection] at this stage, with "singleton" selections? This way, we preserve the relationship between inputs/outputs and we still get a rather readable type ?

[KtorZ]

Yeah. I am just grooming hoping to find something slightly better, but it's not like it's totally cryptic. In the end, as you said, it's quite natural and understandable from the context.

[KtorZ]

Suggested change

	theTarget = mkTargetRange txout
	target = mkTargetRange txout

[KtorZ]

It'd be hard to express such property actually 🤔 ... Have you "rough" idea in mind already of what this could look like ?