Add collateral return outputs to the API and DB #3251

jonathanknowles · 2022-04-25T05:13:52Z

Tasks Implemented

ADP-1715 (Update API to support collateral return outputs)
ADP-1716 (Update DB schema to support collateral return outputs)
ADP-1717 (Update primitive types to support collateral return outputs)

Summary

This PR tackles the "edges" of support for collateral return outputs:

adjusting API types;
adjusting primitive types;
adjusting the DB schema;
adjusting the DB persistence layer.

The approach used for every commit (except where indicated) in this PR is to:

first make an adjustment;
then make the minimal possible number of changes to:
- get the code to compile;
- get the tests to pass.

This PR does not update our UTxO transition function to account for collateral outputs. Therefore, when reading blocks with transactions containing collateral outputs that affect a wallet, these outputs will currently be ignored. Updating the UTxO transition function will be handled by ADP-1718.

sevanspowell

Approved with the caveat that someone with more experience takes a look at the database changes. It looks sensible to me but just to be sure.

👍

piotr-iohk

LGTM.

paolino

LGTM

- no trailing whitespace at the end of lines. - use 4-space indents. - avoid variable-length indentation. - limit line lengths to 80 characters where practical. - use only a single blank line between top-level definitions. https://input-output-hk.github.io/adrestia/code/Coding-Standards

Instead of using `Proxy`, we can just use type applications. We also sort the list into alphabetical order to make it easier to spot if an existing item is already there. It would be nice if we could generate this list automatically.

To keep the size of the golden file small, we pick a type for `t` with a simple serialization that is unlikely to change.

In the API specification we use an array type for `collateral_outputs`, for consistency with the encoding of ordinary transaction outputs, and enabling forward compatibility if in future more than one collateral output is allowed in a transaction. But we clearly mark the fact that there can only be zero or one collateral output. Attempting to parse an `ApiTransaction` with more than one collateral output will fail with an error. For the moment, we populate this field with `Nothing`.

In this commit, we simply add the field, and fix up all places that the compiler requires us to fix. In many cases, we can simply assign `Nothing` to the field. In places where we need to assign something other than `Nothing`, we add a `TODO`.

Important: This commit breaks the DB tests, because it provides no way to write or read collateral outputs to or from the DB. A further commit will provide proper DB persistence for collateral outputs. Instead, in this commit, we simply add the field, and fix up all places that the compiler requires us to fix. In places where we are providing mock data for tests of transactions belonging to previous eras, we can simply assign `Nothing` to the field. In places where we will eventually need to assign something other than `Nothing`, we add a `TODO`, with a link to an appropriate ticket number.

This commit ensures that all DB tests pass.

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

jonathanknowles · 2022-04-27T08:10:46Z

bors r+

3251: Add collateral return outputs to the API and DB r=jonathanknowles a=jonathanknowles ## Tasks Implemented ADP-1715 (_Update API to support collateral return outputs_) ADP-1716 (_Update DB schema to support collateral return outputs_) ADP-1717 (_Update primitive types to support collateral return outputs_) ## Summary This PR tackles the "edges" of support for collateral return outputs: - adjusting API types; - adjusting primitive types; - adjusting the DB schema; - adjusting the DB persistence layer. The approach used for every commit (except where indicated) in this PR is to: - first make an adjustment; - then make the minimal possible number of changes to: - get the code to compile; - get the tests to pass. This PR does **_not_** update our UTxO transition function to account for collateral outputs. Therefore, when reading blocks with transactions containing collateral outputs that affect a wallet, these outputs will currently be ignored. Updating the UTxO transition function will be handled by ADP-1718. Co-authored-by: Jonathan Knowles <jonathan.knowles@iohk.io>

iohk-bors · 2022-04-27T08:58:23Z

Build failed:

ci/hydra-build:required

This was caused by this PR. One of the commits accidentally swapped two positional arguments that share the same type. I've now replaced the positional pattern match with a record-syntax-based pattern match, to avoid this problem.

We now have two fields relating to collateral within a `Tx`. To disambiguate them, we now use the following naming scheme everywhere (where practical): - inputs - outputs - collateral inputs - collateral outputs We also change some construction/pattern match sites that use positional arguments to instead use record syntax. This lowers the risk that fields with identical types are accidentally juxtaposed.

In response to review feedback: https://github.com/input-output-hk/cardano-wallet/pull/3251/files#r859371410

jonathanknowles · 2022-04-27T12:09:08Z

bors r+

3251: Add collateral return outputs to the API and DB r=jonathanknowles a=jonathanknowles ## Tasks Implemented ADP-1715 (_Update API to support collateral return outputs_) ADP-1716 (_Update DB schema to support collateral return outputs_) ADP-1717 (_Update primitive types to support collateral return outputs_) ## Summary This PR tackles the "edges" of support for collateral return outputs: - adjusting API types; - adjusting primitive types; - adjusting the DB schema; - adjusting the DB persistence layer. The approach used for every commit (except where indicated) in this PR is to: - first make an adjustment; - then make the minimal possible number of changes to: - get the code to compile; - get the tests to pass. This PR does **_not_** update our UTxO transition function to account for collateral outputs. Therefore, when reading blocks with transactions containing collateral outputs that affect a wallet, these outputs will currently be ignored. Updating the UTxO transition function will be handled by ADP-1718. Co-authored-by: Jonathan Knowles <jonathan.knowles@iohk.io>

iohk-bors · 2022-04-27T12:31:16Z

Build failed:

buildkite/cardano-wallet

Yet another instance of:

hspec-discover: error while loading shared libraries: libffi.so.7: cannot open shared object file: No such file or directory
--


<span class="term-fg34 term-fg1">  |</span><span class="term-fg31 term-fg1"> ^</span>

[7 of 7] Compiling Demo.Database    ( src/Demo/Database.hs, /var/lib/buildkite-agent-iohk/builds/packet-ipxe-1-ci1-2/input-output-hk/cardano-wallet/dist-newstyle/build/x86_64-linux/ghc-8.10.7/dbvar-2021.8.23/build/Demo/Database.o, /var/lib/buildkite-agent-iohk/builds/packet-ipxe-1-ci1-2/input-output-hk/cardano-wallet/dist-newstyle/build/x86_64-linux/ghc-8.10.7/dbvar-2021.8.23/build/Demo/Database.dyn_o )
--
cabal: Failed to build test:unit from dbvar-2021.8.23.

jonathanknowles · 2022-04-27T12:41:07Z

bors r+

3251: Add collateral return outputs to the API and DB r=jonathanknowles a=jonathanknowles ## Tasks Implemented ADP-1715 (_Update API to support collateral return outputs_) ADP-1716 (_Update DB schema to support collateral return outputs_) ADP-1717 (_Update primitive types to support collateral return outputs_) ## Summary This PR tackles the "edges" of support for collateral return outputs: - adjusting API types; - adjusting primitive types; - adjusting the DB schema; - adjusting the DB persistence layer. The approach used for every commit (except where indicated) in this PR is to: - first make an adjustment; - then make the minimal possible number of changes to: - get the code to compile; - get the tests to pass. This PR does **_not_** update our UTxO transition function to account for collateral outputs. Therefore, when reading blocks with transactions containing collateral outputs that affect a wallet, these outputs will currently be ignored. Updating the UTxO transition function will be handled by ADP-1718. Co-authored-by: Jonathan Knowles <jonathan.knowles@iohk.io>

iohk-bors · 2022-04-27T17:00:21Z

Timed out.

This build seems to have completely succeeded: https://hydra.iohk.io/build/14079546#tabs-summary

But it still timed out. 😕

jonathanknowles · 2022-04-27T21:17:39Z

bors r+

3251: Add collateral return outputs to the API and DB r=jonathanknowles a=jonathanknowles ## Tasks Implemented ADP-1715 (_Update API to support collateral return outputs_) ADP-1716 (_Update DB schema to support collateral return outputs_) ADP-1717 (_Update primitive types to support collateral return outputs_) ## Summary This PR tackles the "edges" of support for collateral return outputs: - adjusting API types; - adjusting primitive types; - adjusting the DB schema; - adjusting the DB persistence layer. The approach used for every commit (except where indicated) in this PR is to: - first make an adjustment; - then make the minimal possible number of changes to: - get the code to compile; - get the tests to pass. This PR does **_not_** update our UTxO transition function to account for collateral outputs. Therefore, when reading blocks with transactions containing collateral outputs that affect a wallet, these outputs will currently be ignored. Updating the UTxO transition function will be handled by ADP-1718. Co-authored-by: Jonathan Knowles <jonathan.knowles@iohk.io>

iohk-bors · 2022-04-27T21:47:44Z

Build failed:

ci/hydra-build:required

Failure:
https://hydra.iohk.io/build/14091728/nixlog/2/tail

Failures:

  src/Test/Integration/Scenario/API/Shelley/Wallets.hs:555:9: 
  1) API Specifications, SHELLEY_WALLETS, WALLETS_GET_02, WALLETS_DELETE_01 - Deleted wallet is not available
       From the following response: Left
           ( DecodeFailure "Something went wrong" "Error in $: Failed reading: not a valid json value at 'Somethingwentwrong'" )
       expected: Status {statusCode = 404, statusMessage = "Not Found"}
        but got: Status {statusCode = 500, statusMessage = "Internal Server Error"}

  To rerun use: --match "/API Specifications/SHELLEY_WALLETS/WALLETS_GET_02, WALLETS_DELETE_01 - Deleted wallet is not available/"

Randomized with seed 384653871

Finished in 994.7649 seconds, used 607.6841 seconds of CPU time
916 examples, 1 failure, 44 pending

jonathanknowles · 2022-04-27T22:24:09Z

bors r+

iohk-bors · 2022-04-27T22:58:08Z

Build succeeded:

jonathanknowles self-assigned this Apr 25, 2022

jonathanknowles force-pushed the jonathanknowles/collateral-return-outputs branch 4 times, most recently from 7ec8948 to 6e870c3 Compare April 25, 2022 07:25

jonathanknowles changed the title ~~WIP: Add collateral return outputs to the API and DB schema~~ Add collateral return outputs to the API and DB Apr 25, 2022

jonathanknowles changed the title ~~Add collateral return outputs to the API and DB~~ Add collateral return outputs to the API and DB schema Apr 25, 2022

jonathanknowles force-pushed the jonathanknowles/collateral-return-outputs branch from 6e870c3 to 2bf1a58 Compare April 25, 2022 07:36

jonathanknowles marked this pull request as ready for review April 26, 2022 06:07

jonathanknowles requested review from sevanspowell, paolino and piotr-iohk April 26, 2022 06:41

sevanspowell approved these changes Apr 26, 2022

View reviewed changes

piotr-iohk approved these changes Apr 26, 2022

View reviewed changes

jonathanknowles force-pushed the jonathanknowles/collateral-return-outputs branch 2 times, most recently from fa9d678 to 4d5664f Compare April 26, 2022 12:06

paolino approved these changes Apr 26, 2022

View reviewed changes

jonathanknowles added the DO NOT MERGE label Apr 27, 2022

jonathanknowles force-pushed the jonathanknowles/collateral-return-outputs branch 2 times, most recently from bdb04fe to b2a5ec6 Compare April 27, 2022 02:28

jonathanknowles changed the title ~~Add collateral return outputs to the API and DB schema~~ Add collateral return outputs to the API and DB Apr 27, 2022

jonathanknowles force-pushed the jonathanknowles/collateral-return-outputs branch from b2a5ec6 to 3a95baa Compare April 27, 2022 02:32

jonathanknowles added 8 commits April 27, 2022 04:08

Correct description of transactionOutputs in API specification.

3fc4046

Add TxCollateralOut and TxCollateralOutToken to database schema.

587c562

Add utility type ApiAsArray.

3c0304e

Add JSON roundtrip test for ApiAsArray (Maybe t).

0af160e

To keep the size of the golden file small, we pick a type for `t` with a simple serialization that is unlikely to change.

Add JSON parsing failure tests for ApiAsArray (Maybe t).

6128a32

jonathanknowles added 5 commits April 27, 2022 04:08

Add field txCollateralOutput to type MkApiTransactionParams.

ac7a0cc

Add field collateralOutputs to Buildable instance for type Tx.

b2fe29c

Provide DB persistence for collateral outputs.

2d9da99

This commit ensures that all DB tests pass.

jonathanknowles force-pushed the jonathanknowles/collateral-return-outputs branch from 3a95baa to d79ab96 Compare April 27, 2022 04:27

jonathanknowles commented Apr 27, 2022

View reviewed changes

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

jonathanknowles removed the DO NOT MERGE label Apr 27, 2022

sevanspowell approved these changes Apr 27, 2022

View reviewed changes

jonathanknowles added 3 commits April 27, 2022 12:03

Make error message more useful on failure to parse ApiAsArray.

2fc0a28

In response to review feedback: https://github.com/input-output-hk/cardano-wallet/pull/3251/files#r859371410

Factor out common type expression into ApiTxOutput type synonym.

f7b1137

jonathanknowles force-pushed the jonathanknowles/collateral-return-outputs branch from fbb95b3 to f7b1137 Compare April 27, 2022 12:05

iohk-bors bot merged commit d2d430a into master Apr 27, 2022

iohk-bors bot deleted the jonathanknowles/collateral-return-outputs branch April 27, 2022 22:58

WilliamKingNoel-Bot pushed a commit that referenced this pull request Apr 27, 2022

docs: Merge #3251

2fd1905

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add collateral return outputs to the API and DB #3251

Add collateral return outputs to the API and DB #3251

jonathanknowles commented Apr 25, 2022 •

edited

Loading

sevanspowell left a comment

piotr-iohk left a comment

paolino left a comment

jonathanknowles commented Apr 27, 2022

iohk-bors bot commented Apr 27, 2022 •

edited by jonathanknowles

Loading

jonathanknowles commented Apr 27, 2022

iohk-bors bot commented Apr 27, 2022 •

edited by jonathanknowles

Loading

jonathanknowles commented Apr 27, 2022

iohk-bors bot commented Apr 27, 2022 •

edited by jonathanknowles

Loading

jonathanknowles commented Apr 27, 2022

iohk-bors bot commented Apr 27, 2022 •

edited by jonathanknowles

Loading

jonathanknowles commented Apr 27, 2022

iohk-bors bot commented Apr 27, 2022

Add collateral return outputs to the API and DB #3251

Add collateral return outputs to the API and DB #3251

Conversation

jonathanknowles commented Apr 25, 2022 • edited Loading

Tasks Implemented

Summary

sevanspowell left a comment

Choose a reason for hiding this comment

piotr-iohk left a comment

Choose a reason for hiding this comment

paolino left a comment

Choose a reason for hiding this comment

jonathanknowles commented Apr 27, 2022

iohk-bors bot commented Apr 27, 2022 • edited by jonathanknowles Loading

jonathanknowles commented Apr 27, 2022

iohk-bors bot commented Apr 27, 2022 • edited by jonathanknowles Loading

jonathanknowles commented Apr 27, 2022

iohk-bors bot commented Apr 27, 2022 • edited by jonathanknowles Loading

jonathanknowles commented Apr 27, 2022

iohk-bors bot commented Apr 27, 2022 • edited by jonathanknowles Loading

jonathanknowles commented Apr 27, 2022

iohk-bors bot commented Apr 27, 2022

jonathanknowles commented Apr 25, 2022 •

edited

Loading

iohk-bors bot commented Apr 27, 2022 •

edited by jonathanknowles

Loading

iohk-bors bot commented Apr 27, 2022 •

edited by jonathanknowles

Loading

iohk-bors bot commented Apr 27, 2022 •

edited by jonathanknowles

Loading

iohk-bors bot commented Apr 27, 2022 •

edited by jonathanknowles

Loading