Use `(TxIn, Address)` as a unique input identifier within coin selection modules. #3149

jonathanknowles · 2022-02-22T08:56:02Z

Issue Number

Background

We eventually want to change the internal coin selection library so that it:

does not depend on the following wallet-specific types:
- TxIn
- TxOut
- Address
- UTxO

produces selections of the form:

data Selection input = Selection
    { inputs     = [(input, TokenBundle)]
    , collateral = [(input, Coin       )]
    , ...
    }

(where input is a type parameter that uniquely identifies a selected input)

However, the wallet currently expects to manipulate selections of the following form:

data Selection = Selection
    { inputs =     [(TxIn, TxOut)]
    , collateral = [(TxIn, TxOut)]
    , ...
    }

Since (for the time being) we wish to avoid making major changes to the wallet codebase, we need a way to translate a Selection produced by the internal algorithm to a wallet-friendly Selection.

One way to achieve this is to instantiate type input as (TxIn, Address), so that our internal selection type is instantiated to:

data Selection = Selection
    { inputs     = [((TxIn, Address), TokenBundle)]
    , collateral = [((TxIn, Address), Coin       )]
    , ...
    }

With this substitution, it's trivial to convert from internal Selection values to wallet-friendly Selection values, because the choice of concrete type for input has all the context necessary to reconstruct a (TxIn, TxOut) pair.

This PR

This PR doesn't introduce any new type parameters, but instead makes an evolutionary step towards the goal outlined above.

This PR:

Introduces a (temporary) type synonym InputId = (TxIn, Address).
Amends core coin selection modules to use InputId as the unique identifier for inputs.
Amends core coin selection modules so that they no longer depend on the TxOut and UTxO types.
Amends the readWalletUTxOIndex function to perform the translation from UTxO (equivalent to Map TxIn TxOut) to Map InputId TokenBundle.
Amends the wallet-facing Cardano.Wallet.CoinSelection module to perform the reverse translation from (InputId, TokenBundle) pairs to (TxIn, TxOut) pairs.

Future Work

A future PR will replace the InputId type synonym with a type parameter on the following types:

Selection
SelectionParams
UTxOIndex
UTxOSelection

Performance Considerations

It's important that the translations stated above do not add any performance overhead.

The translation from (TxIn, TxOut) to (InputId, TokenBundle) within readWalletUTxOIndex does need to traverse the UTxO, but it only does so a single time. It was already necessary to traverse the UTxO in order to construct the UTxOIndex. When estimating a fee, the function readWalletUTxOIndex is only evaluated a single time.
The translation from (InputId, TokenBundle) to (TxIn, TxOut) is only applied to the inputs that are eventually selected.

Anviking

Can't quite see the full picture and wondering if we might not want some output parameters anyway down the line, but also LGTM.

Anviking · 2022-02-23T11:25:52Z

lib/core/src/Cardano/Wallet/CoinSelection/Internal.hs

        -- ^ Selected inputs.
    , collateral
-        :: ![(TxIn, Coin)]
+        :: ![(InputId, Coin)]
        -- ^ Selected collateral inputs.
    , outputs
        :: ![(Address, TokenBundle)]


There are some remaining TxOut and Address types... Are you sure we won't end up needing a output type-parameter anyway? Maybe this interplays with the choices in this PR

There are some remaining TxOut and Address types... Are you sure we won't end up needing a output type-parameter anyway? Maybe this interplays with the choices in this PR

I do eventually plan to remove all of the TxOut and Address usages, I promise! I haven't got there yet, as I'm currently trying to do these changes one area at a time, partly in order to limit the size and complexity of each PR. But all of them will go away!

As for the outputs field: I suspect this is one area where we might need to use an output type parameter, simply because it's helpful to the caller to be able to link the outputs they supplied to the outputs they eventually get back in the final selection.

User-specified outputs can be changed by the algorithm (if they didn't specify any ada, then we allow ourselves to add the minimum ada quantity).

But perhaps there's a way to avoid even this, in future, if we take the responsibility for "preparing outputs" away from coin selection.

Anviking · 2022-02-23T11:36:40Z

lib/core/src/Cardano/Wallet/Primitive/Types/UTxOSelection.hs

 --
 -- Otherwise, returns 'False'.
 --
-isMember :: IsUTxOSelection s => TxIn -> s -> Bool
+isMember :: IsUTxOSelection s => InputId -> s -> Bool


What I found strange initially was: how would UTxOIndex.isMember and UTxOIndex.lookup be useful if the caller would have to provide the TxIn and the Address as a key? You can't lookup a output given its input anymore...

However, it seems UTxOIndex is never used like that anyway, so I guess it's fine 🤷‍♂️

What I found strange initially was: how would UTxOIndex.isMember and UTxOIndex.lookup be useful if the caller would have to provide the TxIn and the Address as a key?

UTxOIndex.lookup and UTxO.isMember can be used in one of two different contexts:

When the caller is coin selection: coin selection doesn't care about the shape of keys used within the UTxOIndex. Coin selection views keys as opaque unique identifiers.

When the caller is the wallet: the wallet knows the mapping between TxIn and Address values, so it can easily create the (TxIn, Address) key required for lookup.

However, it seems UTxOIndex is never used like that anyway, so I guess it's fine :man_shrugging

Yes, exactly.

Currently UTxOIndex is currently only used by coin selection. (It was written specifically to make coin selection easier to implement.) So we have the luxury of being able to limit its operations to just those required by coin selection.

…ion. ADP-1448 We eventually want to change the internal coin selection library so that it: - does not depend on the following wallet-specific types: - `TxIn` - `TxOut` - `Address` - `UTxO` - produces selections of the form: ```haskell data Selection input = Selection { inputs = [(input, TokenBundle)] , collateral = [(input, Coin )] , ... } ``` (here `input` is a **_type parameter_** that **_uniquely_** identifies a selected input) However, the **_wallet_** currently expects to manipulate selections of the following form: ```haskell data Selection = Selection { inputs = [(TxIn, TxOut)] , collateral = [(TxIn, TxOut)] , ... } ``` Since (for the time being) we wish to avoid making major changes to the wallet codebase, we need a way to translate a `Selection` produced by the internal algorithm to a wallet-friendly `Selection`. One way to achieve this is to instantiate type `input` as `(TxIn, Address)`, so that our internal selection type is instantiated to: ```haskell data Selection = Selection { inputs = [((TxIn, Address), TokenBundle)] , collateral = [((TxIn, Address), Coin )] , ... } ``` With this substitution, it's trivial to convert from internal `Selection` values to wallet-friendly `Selection` values, because the choice of concrete type for `input` has all the context necessary to reconstruct a `(TxIn, TxOut)` pair. This commit **_doesn't_** introduce any new type parameters, but instead makes an evolutionary step **_towards_** the goal outlined above. This commit: - Introduces a (temporary) type synonym `InputId = (TxIn, Address)`. - Amends core coin selection modules to use `InputId` as the unique identifier for inputs. - Amends core coin selection modules so that they no longer depend on the `TxOut` and `UTxO` types. - Amends the `readWalletUTxOIndex` function to perform the translation from `UTxO` (equivalent to `Map TxIn TxOut`) to `Map InputId TokenBundle`. - Amends the wallet-facing `Cardano.Wallet.CoinSelection` module to perform the reverse translation from `(InputId, TokenBundle)` pairs to `(TxIn, TxOut)` pairs. A **future commit** will replace the `InputId` type synonym with an `input` type parameter on the following types: - `Selection` - `SelectionParams` - `UTxOIndex` - `UTxOSelection` It's important that the translations stated above do not add any performance overhead. - The translation from `(TxIn, TxOut)` to `(InputId, TokenBundle)` within function `readWalletUTxOIndex` does need to traverse the UTxO, but it only does so a single time. When estimating a fee, this function is only evaluated a single time. - The translation from `(InputId, TokenBundle)` to `(TxIn, TxOut)` is only applied to the inputs that are eventually selected.

We use the following convention for identifier names: - `u` represents a unique identifier for an individual UTxO (unspent transaction output). - `b` represents a `TokenBundle` value. - `i` represents an index of type `UTxOIndex`.

The former field name was confusing because it overloaded the term "UTxO" (used elsewhere in the module to refer to a single UTxO) with the complete set of UTxOs in the index.

These names better reflect the types involved.

…e-parameter-for-utxo-identifier-in-utxo-index Generalize UTxO identifier type within `UTxOIndex`.

jonathanknowles · 2022-02-23T23:22:50Z

bors r+

3149: Use `(TxIn, Address)` as a unique input identifier within coin selection modules. r=jonathanknowles a=jonathanknowles ## Issue Number ADP-1417 ## Background We eventually want to change the internal coin selection library so that it: - does not depend on the following wallet-specific types: - `TxIn` - `TxOut` - `Address` - `UTxO` - produces selections of the form: ```haskell data Selection input = Selection { inputs = [(input, TokenBundle)] , collateral = [(input, Coin )] , ... } ``` (where `input` is a **_type parameter_** that **_uniquely_** identifies a selected input) However, the **_wallet_** currently expects to manipulate selections of the following form: ```haskell data Selection = Selection { inputs = [(TxIn, TxOut)] , collateral = [(TxIn, TxOut)] , ... } ``` Since (for the time being) we wish to avoid making major changes to the wallet codebase, we need a way to translate a `Selection` produced by the internal algorithm to a wallet-friendly `Selection`. One way to achieve this is to instantiate type `input` as `(TxIn, Address)`, so that our internal selection type is instantiated to: ```haskell data Selection = Selection { inputs = [((TxIn, Address), TokenBundle)] , collateral = [((TxIn, Address), Coin )] , ... } ``` With this substitution, it's trivial to convert from internal `Selection` values to wallet-friendly `Selection` values, because the choice of concrete type for `input` has all the context necessary to reconstruct a `(TxIn, TxOut)` pair. ## This PR This PR **_doesn't_** introduce any new type parameters, but instead makes an evolutionary step **_towards_** the goal outlined above. This PR: - [x] Introduces a (temporary) type synonym `InputId = (TxIn, Address)`. - [x] Amends core coin selection modules to use `InputId` as the unique identifier for inputs. - [x] Amends core coin selection modules so that they no longer depend on the `TxOut` and `UTxO` types. - [x] Amends the `readWalletUTxOIndex` function to perform the translation from `UTxO` (equivalent to `Map TxIn TxOut`) to `Map InputId TokenBundle`. - [x] Amends the wallet-facing `Cardano.Wallet.CoinSelection` module to perform the reverse translation from `(InputId, TokenBundle)` pairs to `(TxIn, TxOut)` pairs. ## Future Work A **future PR** will replace the `InputId` type synonym with a type parameter on the following types: - `Selection` - `SelectionParams` - `UTxOIndex` - `UTxOSelection` ## Performance Considerations It's important that the translations stated above do not add any performance overhead. - The translation from `(TxIn, TxOut)` to `(InputId, TokenBundle)` within `readWalletUTxOIndex` does need to traverse the UTxO, but it only does so a single time. It was already necessary to traverse the UTxO in order to construct the `UTxOIndex`. When estimating a fee, the function `readWalletUTxOIndex` is only evaluated a single time. - The translation from `(InputId, TokenBundle)` to `(TxIn, TxOut)` is only applied to the inputs that are eventually selected. Co-authored-by: Jonathan Knowles <jonathan.knowles@iohk.io>

iohk-bors · 2022-02-24T02:01:31Z

Build failed:

ci/hydra-build:required

Test timed out here:

  Sign transaction
    signTransaction adds reward account witness when necessary (AnyCardanoEra ByronEra)
      +++ OK, passed 100 tests (100% feature not supported in ByronEra.).
    signTransaction adds reward account witness when necessary (AnyCardanoEra ShelleyEra) (29562ms)
      +++ OK, passed 100 tests.
    signTransaction adds reward account witness when necessary (AnyCardanoEra AllegraEra) (29663ms)
      +++ OK, passed 100 tests.
    signTransaction adds reward account witness when necessary (AnyCardanoEra MaryEra) (29746ms)
      +++ OK, passed 100 tests.
    signTransaction adds reward account witness when necessary (AnyCardanoEra AlonzoEra) (29702ms)
      +++ OK, passed 100 tests.
    signTransaction adds extra key witness when necessary (AnyCardanoEra ByronEra)
      +++ OK, passed 100 tests (100% feature not supported in ByronEra.).
    signTransaction adds extra key witness when necessary (AnyCardanoEra ShelleyEra)
      +++ OK, passed 100 tests (100% feature not supported in ShelleyEra.).
    signTransaction adds extra key witness when necessary (AnyCardanoEra AllegraEra)
      +++ OK, passed 100 tests (100% feature not supported in AllegraEra.).
    signTransaction adds extra key witness when necessary (AnyCardanoEra MaryEra)
      +++ OK, passed 100 tests (100% feature not supported in MaryEra.).
    signTransaction adds extra key witness when necessary (AnyCardanoEra AlonzoEra) (69849ms)
      +++ OK, passed 100 tests.
    signTransaction adds tx in witnesses when necessary (AnyCardanoEra ByronEra) (1ms)
      +++ OK, passed 100 tests.
    signTransaction adds tx in witnesses when necessary (AnyCardanoEra ShelleyEra) (95213ms)
      +++ OK, passed 100 tests.
    signTransaction adds tx in witnesses when necessary (AnyCardanoEra AllegraEra) (95491ms)
      +++ OK, passed 100 tests.

jonathanknowles · 2022-02-24T02:50:47Z

bors r+

3149: Use `(TxIn, Address)` as a unique input identifier within coin selection modules. r=jonathanknowles a=jonathanknowles ## Issue Number ADP-1417 ## Background We eventually want to change the internal coin selection library so that it: - does not depend on the following wallet-specific types: - `TxIn` - `TxOut` - `Address` - `UTxO` - produces selections of the form: ```haskell data Selection input = Selection { inputs = [(input, TokenBundle)] , collateral = [(input, Coin )] , ... } ``` (where `input` is a **_type parameter_** that **_uniquely_** identifies a selected input) However, the **_wallet_** currently expects to manipulate selections of the following form: ```haskell data Selection = Selection { inputs = [(TxIn, TxOut)] , collateral = [(TxIn, TxOut)] , ... } ``` Since (for the time being) we wish to avoid making major changes to the wallet codebase, we need a way to translate a `Selection` produced by the internal algorithm to a wallet-friendly `Selection`. One way to achieve this is to instantiate type `input` as `(TxIn, Address)`, so that our internal selection type is instantiated to: ```haskell data Selection = Selection { inputs = [((TxIn, Address), TokenBundle)] , collateral = [((TxIn, Address), Coin )] , ... } ``` With this substitution, it's trivial to convert from internal `Selection` values to wallet-friendly `Selection` values, because the choice of concrete type for `input` has all the context necessary to reconstruct a `(TxIn, TxOut)` pair. ## This PR This PR **_doesn't_** introduce any new type parameters, but instead makes an evolutionary step **_towards_** the goal outlined above. This PR: - [x] Introduces a (temporary) type synonym `InputId = (TxIn, Address)`. - [x] Amends core coin selection modules to use `InputId` as the unique identifier for inputs. - [x] Amends core coin selection modules so that they no longer depend on the `TxOut` and `UTxO` types. - [x] Amends the `readWalletUTxOIndex` function to perform the translation from `UTxO` (equivalent to `Map TxIn TxOut`) to `Map InputId TokenBundle`. - [x] Amends the wallet-facing `Cardano.Wallet.CoinSelection` module to perform the reverse translation from `(InputId, TokenBundle)` pairs to `(TxIn, TxOut)` pairs. ## Future Work A **future PR** will replace the `InputId` type synonym with a type parameter on the following types: - `Selection` - `SelectionParams` - `UTxOIndex` - `UTxOSelection` ## Performance Considerations It's important that the translations stated above do not add any performance overhead. - The translation from `(TxIn, TxOut)` to `(InputId, TokenBundle)` within `readWalletUTxOIndex` does need to traverse the UTxO, but it only does so a single time. It was already necessary to traverse the UTxO in order to construct the `UTxOIndex`. When estimating a fee, the function `readWalletUTxOIndex` is only evaluated a single time. - The translation from `(InputId, TokenBundle)` to `(TxIn, TxOut)` is only applied to the inputs that are eventually selected. Co-authored-by: Jonathan Knowles <jonathan.knowles@iohk.io>

iohk-bors · 2022-02-24T05:57:30Z

Build failed:

ci/hydra-build:required

Another timeout:

  Sign transaction
    signTransaction adds reward account witness when necessary (AnyCardanoEra ByronEra)
      +++ OK, passed 100 tests (100% feature not supported in ByronEra.).
    signTransaction adds reward account witness when necessary (AnyCardanoEra ShelleyEra) (29438ms)
      +++ OK, passed 100 tests.
    signTransaction adds reward account witness when necessary (AnyCardanoEra AllegraEra) (29539ms)
      +++ OK, passed 100 tests.
    signTransaction adds reward account witness when necessary (AnyCardanoEra MaryEra) (29601ms)
      +++ OK, passed 100 tests.
    signTransaction adds reward account witness when necessary (AnyCardanoEra AlonzoEra) (29572ms)
      +++ OK, passed 100 tests.
    signTransaction adds extra key witness when necessary (AnyCardanoEra ByronEra)
      +++ OK, passed 100 tests (100% feature not supported in ByronEra.).
    signTransaction adds extra key witness when necessary (AnyCardanoEra ShelleyEra)
      +++ OK, passed 100 tests (100% feature not supported in ShelleyEra.).
    signTransaction adds extra key witness when necessary (AnyCardanoEra AllegraEra)
      +++ OK, passed 100 tests (100% feature not supported in AllegraEra.).
    signTransaction adds extra key witness when necessary (AnyCardanoEra MaryEra)
      +++ OK, passed 100 tests (100% feature not supported in MaryEra.).

I actually watched this test while it was running in CI. The output stops at the above point, as if something has got stuck, and then test infrastructure times out much later on.

jonathanknowles · 2022-02-24T07:17:27Z

Time comparison of test that times out (cardano-wallet:test:unit --match "Sign transaction"), when run locally on an ordinary Linux machine, between this branch and master.

This branch (486.4921 seconds)

Finished in 486.4921 seconds, used 487.6973 seconds of CPU time            
35 examples, 0 failures                                                                    
                                                                                           
Slow spec items:                                                                           
  test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs:388:29: /Cardano.Wallet.Shelley.Transaction/Sign transaction/signTransaction adds collateral witnesses when necessary (AnyCardan
oEra AlonzoEra)/ (58960ms)                                                                                                                                                            
  test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs:386:29: /Cardano.Wallet.Shelley.Transaction/Sign transaction/signTransaction adds tx in witnesses when necessary (AnyCardanoEra 
ShelleyEra)/ (58924ms)                                                                     
  test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs:386:29: /Cardano.Wallet.Shelley.Transaction/Sign transaction/signTransaction adds tx in witnesses when necessary (AnyCardanoEra 
AlonzoEra)/ (58743ms)                                                                                                                                                                 
  test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs:386:29: /Cardano.Wallet.Shelley.Transaction/Sign transaction/signTransaction adds tx in witnesses when necessary (AnyCardanoEra 
AllegraEra)/ (58700ms)                                                                                                                                                                
  test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs:386:29: /Cardano.Wallet.Shelley.Transaction/Sign transaction/signTransaction adds tx in witnesses when necessary (AnyCardanoEra 
MaryEra)/ (58681ms)                                                                        
  test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs:384:29: /Cardano.Wallet.Shelley.Transaction/Sign transaction/signTransaction adds extra key witness when necessary (AnyCardanoEr
a AlonzoEra)/ (52162ms)
  test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs:382:29: /Cardano.Wallet.Shelley.Transaction/Sign transaction/signTransaction adds reward account witness when necessary (AnyCard
anoEra MaryEra)/ (21584ms)
  test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs:382:29: /Cardano.Wallet.Shelley.Transaction/Sign transaction/signTransaction adds reward account witness when necessary (AnyCard
anoEra AllegraEra)/ (21583ms)
  test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs:382:29: /Cardano.Wallet.Shelley.Transaction/Sign transaction/signTransaction adds reward account witness when necessary (AnyCard
anoEra AlonzoEra)/ (21551ms)
  test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs:382:29: /Cardano.Wallet.Shelley.Transaction/Sign transaction/signTransaction adds reward account witness when necessary (AnyCard
anoEra ShelleyEra)/ (21418ms)

cardano-wallet> Test suite unit passed

Master branch (547.6825 seconds)

Finished in 547.6825 seconds, used 549.0480 seconds of CPU time
35 examples, 0 failures

Slow spec items:
  test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs:388:29: /Cardano.Wallet.Shelley.Transaction/Sign transaction/signTransaction adds collateral witnesses when necessary (AnyCardanoEra AlonzoEra)/ (71508ms)
  test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs:386:29: /Cardano.Wallet.Shelley.Transaction/Sign transaction/signTransaction adds tx in witnesses when necessary (AnyCardanoEra AlonzoEra)/ (71326ms)
  test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs:386:29: /Cardano.Wallet.Shelley.Transaction/Sign transaction/signTransaction adds tx in witnesses when necessary (AnyCardanoEra MaryEra)/ (71251ms)
  test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs:386:29: /Cardano.Wallet.Shelley.Transaction/Sign transaction/signTransaction adds tx in witnesses when necessary (AnyCardanoEra ShelleyEra)/ (71217ms)
  test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs:386:29: /Cardano.Wallet.Shelley.Transaction/Sign transaction/signTransaction adds tx in witnesses when necessary (AnyCardanoEra AllegraEra)/ (71201ms)
  test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs:384:29: /Cardano.Wallet.Shelley.Transaction/Sign transaction/signTransaction adds extra key witness when necessary (AnyCardanoEra AlonzoEra)/ (49614ms)
  test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs:382:29: /Cardano.Wallet.Shelley.Transaction/Sign transaction/signTransaction adds reward account witness when necessary (AnyCardanoEra AlonzoEra)/ (21273ms)
  test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs:382:29: /Cardano.Wallet.Shelley.Transaction/Sign transaction/signTransaction adds reward account witness when necessary (AnyCardanoEra MaryEra)/ (21253ms)
  test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs:382:29: /Cardano.Wallet.Shelley.Transaction/Sign transaction/signTransaction adds reward account witness when necessary (AnyCardanoEra AllegraEra)/ (21241ms)
  test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs:382:29: /Cardano.Wallet.Shelley.Transaction/Sign transaction/signTransaction adds reward account witness when necessary (AnyCardanoEra ShelleyEra)/ (21151ms)

cardano-wallet> Test suite unit passed

I've run this several times both on this branch, and on master, on a local machine. The total run time is consistently between 480 seconds and 550 seconds, with no obvious slowdown introduced by this branch.

Successful runs of this branch on CI:

jonathanknowles · 2022-02-24T07:42:26Z

bors r+

iohk-bors · 2022-02-24T08:31:43Z

Build succeeded:

3155: Generalize UTxO identifier type within `UTxOSelection`. r=jonathanknowles a=jonathanknowles ## Issue Number ADP-1450 ## Background Within coin selection modules, we are steadily replacing all uses of `TxIn`, `Address`, `TxOut` with type parameters, so that the coin selection library does not depend on these concrete wallet types. ## Summary This PR replaces all uses of `InputId` (a temporary type synonym introduced in #3149) with a type parameter within module `UTxOSelection`. We use the following convention for type parameters and identifier names: - `u` represents a unique identifier for an individual UTxO (unspent transaction output). - `s` represents a selection of type `UTxOSelection`. - `b` represents a `TokenBundle` value. - `i` represents an index of type `UTxOIndex`. Co-authored-by: Jonathan Knowles <jonathan.knowles@iohk.io>

3155: Generalize UTxO identifier type within `UTxOSelection`. r=Anviking a=jonathanknowles ## Issue Number ADP-1450 ## Background Within coin selection modules, we are steadily replacing all uses of `TxIn`, `Address`, `TxOut` with type parameters, so that the coin selection library does not depend on these concrete wallet types. ## Summary This PR replaces all uses of `InputId` (a temporary type synonym introduced in #3149) with a type parameter within module `UTxOSelection`. We use the following convention for type parameters and identifier names: - `u` represents a unique identifier for an individual UTxO (unspent transaction output). - `s` represents a selection of type `UTxOSelection`. - `b` represents a `TokenBundle` value. - `i` represents an index of type `UTxOIndex`. Co-authored-by: Jonathan Knowles <jonathan.knowles@iohk.io>

3155: Generalize UTxO identifier type within `UTxOSelection`. r=jonathanknowles a=jonathanknowles ## Issue Number ADP-1450 ## Background Within coin selection modules, we are steadily replacing all uses of `TxIn`, `Address`, `TxOut` with type parameters, so that the coin selection library does not depend on these concrete wallet types. ## Summary This PR replaces all uses of `InputId` (a temporary type synonym introduced in #3149) with a type parameter within module `UTxOSelection`. We use the following convention for type parameters and identifier names: - `u` represents a unique identifier for an individual UTxO (unspent transaction output). - `s` represents a selection of type `UTxOSelection`. - `b` represents a `TokenBundle` value. - `i` represents an index of type `UTxOIndex`. Co-authored-by: Jonathan Knowles <jonathan.knowles@iohk.io>

jonathanknowles force-pushed the jonathanknowles/coin-selection-compound-input-id branch 3 times, most recently from 51bdf6d to bac063e Compare February 22, 2022 10:40

jonathanknowles changed the title ~~WIP~~ Use (TxIn, Address) as a unique input identifier within coin selection. Feb 22, 2022

jonathanknowles self-assigned this Feb 22, 2022

jonathanknowles requested a review from Anviking February 22, 2022 10:40

jonathanknowles marked this pull request as ready for review February 22, 2022 10:43

jonathanknowles force-pushed the jonathanknowles/coin-selection-compound-input-id branch from bac063e to 59c9fbe Compare February 22, 2022 11:06

jonathanknowles changed the title ~~Use (TxIn, Address) as a unique input identifier within coin selection.~~ Use (TxIn, Address) as a unique input identifier within coin selection modules. Feb 23, 2022

Anviking approved these changes Feb 23, 2022

View reviewed changes

jonathanknowles force-pushed the jonathanknowles/coin-selection-compound-input-id branch from 59c9fbe to a269694 Compare February 23, 2022 23:06

jonathanknowles added 6 commits February 23, 2022 23:07

Format according to coding standards.

ff30aff

Add type parameter to UTxOIndex to uniquely identify UTxOs.

0b8bb02

Rename field utxo of type UTxOIndex to universe.

dced45c

The former field name was confusing because it overloaded the term "UTxO" (used elsewhere in the module to refer to a single UTxO) with the complete set of UTxOs in the index.

Rename {to,from}UTxO to {to,from}Map in module UTxOIndex.

6d8a198

These names better reflect the types involved.

Merge pull request #3154 from input-output-hk/jonathanknowles/use-typ…

82c8e38

…e-parameter-for-utxo-identifier-in-utxo-index Generalize UTxO identifier type within `UTxOIndex`.

jonathanknowles mentioned this pull request Feb 24, 2022

Generalize UTxO identifier type within UTxOSelection. #3155

Merged

iohk-bors bot merged commit 9bd6cfe into master Feb 24, 2022

iohk-bors bot deleted the jonathanknowles/coin-selection-compound-input-id branch February 24, 2022 08:31

WilliamKingNoel-Bot pushed a commit that referenced this pull request Feb 24, 2022

docs: Merge #3149

9c3c528

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `(TxIn, Address)` as a unique input identifier within coin selection modules. #3149

Use `(TxIn, Address)` as a unique input identifier within coin selection modules. #3149

jonathanknowles commented Feb 22, 2022 •

edited

Loading

Anviking left a comment

Anviking Feb 23, 2022

jonathanknowles Feb 23, 2022 •

edited

Loading

Anviking Feb 23, 2022

jonathanknowles Feb 23, 2022 •

edited

Loading

jonathanknowles commented Feb 23, 2022

iohk-bors bot commented Feb 24, 2022 •

edited by jonathanknowles

Loading

jonathanknowles commented Feb 24, 2022

iohk-bors bot commented Feb 24, 2022 •

edited by jonathanknowles

Loading

jonathanknowles commented Feb 24, 2022 •

edited

Loading

jonathanknowles commented Feb 24, 2022

iohk-bors bot commented Feb 24, 2022

Use (TxIn, Address) as a unique input identifier within coin selection modules. #3149

Use (TxIn, Address) as a unique input identifier within coin selection modules. #3149

Conversation

jonathanknowles commented Feb 22, 2022 • edited Loading

Issue Number

Background

This PR

Future Work

Performance Considerations

Anviking left a comment

Choose a reason for hiding this comment

Anviking Feb 23, 2022

Choose a reason for hiding this comment

jonathanknowles Feb 23, 2022 • edited Loading

Choose a reason for hiding this comment

Anviking Feb 23, 2022

Choose a reason for hiding this comment

jonathanknowles Feb 23, 2022 • edited Loading

Choose a reason for hiding this comment

jonathanknowles commented Feb 23, 2022

iohk-bors bot commented Feb 24, 2022 • edited by jonathanknowles Loading

jonathanknowles commented Feb 24, 2022

iohk-bors bot commented Feb 24, 2022 • edited by jonathanknowles Loading

jonathanknowles commented Feb 24, 2022 • edited Loading

jonathanknowles commented Feb 24, 2022

iohk-bors bot commented Feb 24, 2022

Use `(TxIn, Address)` as a unique input identifier within coin selection modules. #3149

Use `(TxIn, Address)` as a unique input identifier within coin selection modules. #3149

jonathanknowles commented Feb 22, 2022 •

edited

Loading

jonathanknowles Feb 23, 2022 •

edited

Loading

jonathanknowles Feb 23, 2022 •

edited

Loading

iohk-bors bot commented Feb 24, 2022 •

edited by jonathanknowles

Loading

iohk-bors bot commented Feb 24, 2022 •

edited by jonathanknowles

Loading

jonathanknowles commented Feb 24, 2022 •

edited

Loading