Add collateral inputs to wallet types, DB, and API. #2817

jonathanknowles · 2021-08-11T04:46:57Z

Issue Number

Overview

This PR adds all the boilerplate required for collateral inputs to:

the wallet database;
the wallet primitive types;
the wallet API.

This PR makes no attempt to manipulate collateral inputs in any way, other than to marshall data around from one layer to another. It also makes no attempt to parse collateral inputs from network blocks. Where necessary, future work has been marked with TODO comments. (With a link to ADP-957).

Currently, all API calls returning collateral inputs will return the empty list.

API

This PR adds API entries and types for collateral inputs, with the following differences from ordinary inputs:

while a transaction must have at least one ordinary input, it's possible for a transaction to have no collateral inputs.
while an ordinary input may be associated with one or more non-ada assets, a collateral input may not have non-ada assets.

This is reflected in the following differences between types:

- x-transactionInputs: &transactionInputs
+ x-transactionCollateral: &transactionCollateral
    description: |
-     A list of transaction inputs.
+     A list of transaction inputs used for collateral.
    type: array
    minItems: 0
    items:
      type: object
      required:
        - id
        - index
      properties:
        address: *addressId
        amount: *amount
-       assets: *walletAssets
        id: *transactionId
        index:
          type: integer
          minimum: 0

- x-transactionResolvedInputs: &transactionResolvedInputs
+ x-transactionResolvedCollateral: &transactionResolvedCollateral
-   description: A list of transaction inputs.
+   description: A list of transaction inputs used for collateral.
    type: array
-   minItems: 1
+   minItems: 0
    items:
      type: object
      required:
        - id
        - index
        - address
        - amount
        - derivation_path
      properties:
        address: *addressId
        amount: *amount
-       assets: *walletAssets
        id: *transactionId
        derivation_path: *derivationPath
        index:
          type: integer
          minimum: 0

Database

This PR adds a TxCollateral table, with analagous fields to the TxIn table.

Primitive Types

This PR adds fields for collateral inputs to the following primitive types:

UnsignedTx
TransactionInfo
Tx

This commit performs the minimum amount of work necessary to add fields for collateral inputs to the following types: - `Tx` - `UnsignedTx` - `TransactionInfo`

lib/core/src/Cardano/Wallet/Api/Server.hs

lib/core/src/Cardano/Wallet/Api/Types.hs

lib/core/src/Cardano/Wallet/DB/Sqlite.hs

sevanspowell · 2021-08-13T03:40:54Z

specifications/api/swagger.yaml

@@ -1212,6 +1230,27 @@ x-transactionResolvedInputs: &transactionResolvedInputs
        type: integer
        minimum: 0

+x-transactionResolvedCollateral: &transactionResolvedCollateral
+  description: A list of transaction inputs used for collateral


What does "resolved" here mean?

What does "resolved" here mean?

Good question. I'm following the existing terminology for inputs, as much as possible.

For "resolved inputs": in addition to including the txId and txIndex (which is enough to uniquely identify an input), we also include the following information, which shows what this input "resolves", or "maps" to:

the address

the derivationPath

the amount

This commit replaces the use of named field puns with generic field accessors in order to make the function a bit more concise. In response to: https://github.com/input-output-hk/cardano-wallet/pull/2817/files#r687606529

lib/core/src/Cardano/Wallet/DB/Sqlite.hs

This commit uses named record fields rather than a positional style.

sevanspowell · 2021-08-13T04:43:40Z

The subtleties are lost on me, but I've read through this carefully and it looks logical and consistent.

By the subtleties, I just mean I'm not sure I can answer the questions of:

Is the API in the schema we want?
Is the DB in the schema we want?

jonathanknowles · 2021-08-13T06:23:46Z

The subtleties are lost on me, but I've read through this carefully and it looks logical and consistent.

Thanks for looking through!

By the subtleties, I just mean I'm not sure I can answer the questions of:
* Is the API in the schema we want?
* Is the DB in the schema we want?

I think the main principle here is that "collateral inputs" should have a similar representation to ordinary inputs, in both the API and the DB.

The primary differences (at the type-level) are:

Collateral inputs cannot have non-ada asset quantities.
In general, a transaction must have at least one normal input, but it doesn't have to have collateral inputs.

So in the API, we have the following differences:

- x-transactionResolvedInputs: &transactionResolvedInputs
+ x-transactionResolvedCollateral: &transactionResolvedCollateral
-   description: A list of transaction inputs
+   description: A list of transaction inputs used for collateral
    type: array
-   minItems: 1
+   minItems: 0
    items:
      type: object
      required:
        - id
        - index
        - address
        - amount
        - derivation_path
      properties:
        address: *addressId
        amount: *amount
-       assets: *walletAssets
        id: *transactionId
        derivation_path: *derivationPath
        index:
          type: integer
          minimum: 0

- x-transactionInputs: &transactionInputs
+ x-transactionCollateral: &transactionCollateral
    description: |
-     A list of transaction inputs.
+     A list of transaction inputs used for collateral.
    type: array
    minItems: 0
    items:
      type: object
      required:
        - id
        - index
      properties:
        address: *addressId
        amount: *amount
-       assets: *walletAssets
        id: *transactionId
        index:
          type: integer
          minimum: 0

jonathanknowles · 2021-08-13T06:45:38Z

Is the DB in the schema we want?

The schema for TxCollateral should be the same as the schema for TxIn, just with different field name prefixes.

You might have wondered whether TxIn would include an extra assets field, but assets are actually stored separately (in another table), because they're in a many-to-one relationship with each UTxO entry. So as a result, the TxCollateral table is the same shape (has the same fields) as the TxIn table.

For brevity, use "collateral" rather than "collateralInputs" in record field names. This is consistent with the API naming.

sevanspowell · 2021-08-16T00:29:58Z

lib/core/src/Cardano/Wallet/Api/Server.hs

@@ -2856,7 +2856,7 @@ mkApiTransaction
    -> Lens' (ApiTransaction n) (Maybe ApiBlockReference)
    -> IO (ApiTransaction n)
 mkApiTransaction
-    ti txid mfee ins cins outs ws (meta, timestamp) txMeta setTimeReference = do
+    ti txid mfee cins ins outs ws (meta, timestamp) txMeta setTimeReference = do


Ooh, this looks like a very likely place to make a mistake, is there anyway we can distinguish between the two types of [(TxIn, Maybe TxOut)] that makes sense in this context?

@sevanspowell I agree that it would be easy for a caller to get these arguments muddled up, especially given that the types are identical.

I've created a record type MkApiTransactionParams, and altered mkApiTransaction to take this record instead of all the positional parameters. Hopefully this should make it easier at call sites to determine which parameter is which.

Commit: c9bc8e6

sevanspowell · 2021-08-16T00:32:08Z

Is the DB in the schema we want?

The schema for TxCollateral should be the same as the schema for TxIn, just with different field name prefixes.

You might have wondered whether TxIn would include an extra assets field, but assets are actually stored separately (in another table), because they're in a many-to-one relationship with each UTxO entry. So as a result, the TxCollateral table is the same shape (has the same fields) as the TxIn table.

Thanks for this! I've re-reviewed and added my ✔️

This change replaces the use of positional arguments with a record type in function `mkApiTransaction`, making it clearer at call sites which argument is which, and hopefully less likely for arguments with the same type to be supplied in the wrong order.

jonathanknowles · 2021-08-16T05:07:21Z

bors r+

iohk-bors · 2021-08-16T06:13:01Z

Build succeeded:

jonathanknowles self-assigned this Aug 11, 2021

jonathanknowles requested review from Anviking and paweljakubas August 11, 2021 04:58

jonathanknowles force-pushed the jonathanknowles/tx-collateral-inputs branch 4 times, most recently from 05b8429 to 55a7d0f Compare August 12, 2021 03:13

jonathanknowles changed the title ~~Add fields for collateral inputs to types Tx and UnsignedTx.~~ Add collateral input fields to primitive transaction types. Aug 12, 2021

Add collateral input fields to primitive transaction types.

cd90151

This commit performs the minimum amount of work necessary to add fields for collateral inputs to the following types: - `Tx` - `UnsignedTx` - `TransactionInfo`

jonathanknowles force-pushed the jonathanknowles/tx-collateral-inputs branch from 55a7d0f to cd90151 Compare August 12, 2021 03:21

jonathanknowles changed the title ~~Add collateral input fields to primitive transaction types.~~ Add collateral input fields to wallet types. Aug 12, 2021

jonathanknowles removed request for Anviking and paweljakubas August 12, 2021 03:35

Add collateral inputs to the database.

172279a

jonathanknowles changed the title ~~Add collateral input fields to wallet types.~~ Add collateral inputs to primitive types and database. Aug 12, 2021

Add collateral inputs to ApiCoinSelection.

837aecd

jonathanknowles changed the title ~~Add collateral inputs to primitive types and database.~~ Add collateral inputs to wallet types and database. Aug 12, 2021

Add collateral inputs to ApiTransaction.

a42a642

jonathanknowles marked this pull request as ready for review August 12, 2021 10:36

jonathanknowles requested review from Anviking, paweljakubas and sevanspowell August 12, 2021 10:36

jonathanknowles changed the title ~~Add collateral inputs to wallet types and database.~~ Add collateral inputs to database, wallet types, and API. Aug 12, 2021

jonathanknowles changed the title ~~Add collateral inputs to database, wallet types, and API.~~ Add collateral inputs to wallet types, DB, and API. Aug 12, 2021

jonathanknowles commented Aug 12, 2021

View reviewed changes

lib/core/src/Cardano/Wallet/Api/Server.hs Outdated Show resolved Hide resolved

jonathanknowles commented Aug 12, 2021

View reviewed changes

lib/core/src/Cardano/Wallet/Api/Types.hs Show resolved Hide resolved

jonathanknowles commented Aug 12, 2021

View reviewed changes

lib/core/src/Cardano/Wallet/DB/Sqlite.hs Show resolved Hide resolved

sevanspowell reviewed Aug 13, 2021

View reviewed changes

jonathanknowles added 2 commits August 13, 2021 03:47

sevanspowell reviewed Aug 13, 2021

View reviewed changes

lib/core/src/Cardano/Wallet/DB/Sqlite.hs Outdated Show resolved Hide resolved

Use named record fields in selectionToUnsignedTx.

cbf8790

This commit uses named record fields rather than a positional style.

jonathanknowles force-pushed the jonathanknowles/tx-collateral-inputs branch 2 times, most recently from cfb159c to fd0c082 Compare August 13, 2021 04:24

Unify terminology for collateral inputs.

73203d5

For brevity, use "collateral" rather than "collateralInputs" in record field names. This is consistent with the API naming.

jonathanknowles force-pushed the jonathanknowles/tx-collateral-inputs branch from fd0c082 to 73203d5 Compare August 13, 2021 06:58

sevanspowell reviewed Aug 16, 2021

View reviewed changes

sevanspowell self-requested a review August 16, 2021 00:30

sevanspowell approved these changes Aug 16, 2021

View reviewed changes

iohk-bors bot merged commit 385768a into master Aug 16, 2021

iohk-bors bot deleted the jonathanknowles/tx-collateral-inputs branch August 16, 2021 06:13

WilliamKingNoel-Bot pushed a commit that referenced this pull request Aug 16, 2021

docs: Merge #2817

f2fc2ca

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add collateral inputs to wallet types, DB, and API. #2817

Add collateral inputs to wallet types, DB, and API. #2817

jonathanknowles commented Aug 11, 2021 •

edited

Loading

sevanspowell Aug 13, 2021

jonathanknowles Aug 13, 2021 •

edited

Loading

sevanspowell commented Aug 13, 2021

jonathanknowles commented Aug 13, 2021 •

edited

Loading

jonathanknowles commented Aug 13, 2021

sevanspowell Aug 16, 2021

jonathanknowles Aug 16, 2021 •

edited

Loading

sevanspowell commented Aug 16, 2021

jonathanknowles commented Aug 16, 2021

iohk-bors bot commented Aug 16, 2021

Add collateral inputs to wallet types, DB, and API. #2817

Add collateral inputs to wallet types, DB, and API. #2817

Conversation

jonathanknowles commented Aug 11, 2021 • edited Loading

Issue Number

Overview

API

Database

Primitive Types

sevanspowell Aug 13, 2021

Choose a reason for hiding this comment

jonathanknowles Aug 13, 2021 • edited Loading

Choose a reason for hiding this comment

sevanspowell commented Aug 13, 2021

jonathanknowles commented Aug 13, 2021 • edited Loading

jonathanknowles commented Aug 13, 2021

sevanspowell Aug 16, 2021

Choose a reason for hiding this comment

jonathanknowles Aug 16, 2021 • edited Loading

Choose a reason for hiding this comment

sevanspowell commented Aug 16, 2021

jonathanknowles commented Aug 16, 2021

iohk-bors bot commented Aug 16, 2021

jonathanknowles commented Aug 11, 2021 •

edited

Loading

jonathanknowles Aug 13, 2021 •

edited

Loading

jonathanknowles commented Aug 13, 2021 •

edited

Loading

jonathanknowles Aug 16, 2021 •

edited

Loading