Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ADP-3215] Add TxOut type to Cardano.Wallet.Read.Tx #4698

Merged
merged 3 commits into from
Aug 23, 2024

Conversation

HeinrichApfelmus
Copy link
Contributor

@HeinrichApfelmus HeinrichApfelmus commented Jul 24, 2024

This pull request adds a type TxOut to Cardano.Wallet.Read.Tx.TxOut.

The type TxOut occupies the following point in the design space:

  • TxOut is era-independent — a value of this type can be any transaction output from a past era.
  • TxOut can — in principle — be deconstructed using functions from the latest or next era. This is possible because transaction outputs are upwards-compatible.
  • TxOut can be serialized and deserialized to a format that is close to the ledger CBOR. However, we allow serialize . deserialize ≠ id in order to allow internal era upgrades.

The above design choices can be realized with different internal representations. We choose the following:

  • TxOut is represented as a disjoint sum of Output era.
  • TxOut supports explicit an upgrade to Output era where era is the latest or next era — but this conversion is not zero-cost.

Comments

  • In addition to deconstruction, we also provide a constructor mkBasicTxOut for convenience and testing.

Issue Number

ADP-3215

@HeinrichApfelmus HeinrichApfelmus self-assigned this Jul 24, 2024
@HeinrichApfelmus HeinrichApfelmus changed the base branch from master to HeinrichApfelmus/ADP-3215/read-txin July 24, 2024 15:01
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-3215/read-txin branch 2 times, most recently from 110c102 to 6c4f4ef Compare July 26, 2024 15:57
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-3215/read-txout branch from d1f3523 to 1deb893 Compare July 26, 2024 17:02
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-3215/read-txin branch from 6c4f4ef to 2e76e46 Compare July 29, 2024 12:09
Base automatically changed from HeinrichApfelmus/ADP-3215/read-txin to master July 29, 2024 14:00
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-3215/read-txout branch 3 times, most recently from 98cb734 to 5c53c1d Compare August 1, 2024 07:46
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-3215/read-txout branch 4 times, most recently from 97a580a to bd7f665 Compare August 19, 2024 12:26
@HeinrichApfelmus HeinrichApfelmus marked this pull request as ready for review August 19, 2024 12:27
Copy link
Member

@Anviking Anviking left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

first look

lib/read/lib/Cardano/Read/Ledger/Tx/Output.hs Show resolved Hide resolved
lib/read/lib/Cardano/Read/Ledger/Tx/Output.hs Outdated Show resolved Hide resolved
lib/read/lib/Cardano/Wallet/Read/Tx/TxOut.hs Outdated Show resolved Hide resolved
lib/read/lib/Cardano/Wallet/Read/Tx/TxOut.hs Show resolved Hide resolved
@Anviking Anviking self-requested a review August 20, 2024 13:08
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-3215/read-txout branch from bd7f665 to c5c9701 Compare August 22, 2024 18:36
@HeinrichApfelmus
Copy link
Contributor Author

As discussed, I have

  • removed the instance Eq TxOut
  • reimagined the upgrade function as upgradeTxOutToBabbageOrLater
  • added a test case that describes the Conway treatment of invalid stake pointers

In lieu of the tests relating to toConwayTxOut and toBabbageTxOut, I have also

  • added a test case that upgradeTxOutToBabbageOrLater preserves getValue

In the interest of development time, I have refrained from adding further tests on upgradeTxOutToBabbageOrLater, especially relating to the Byron era — I think the code is self-explanatory enough so that additional testing provides diminishing returns.

lib/read/lib/Cardano/Wallet/Read/Tx/TxOut.hs Outdated Show resolved Hide resolved
lib/read/lib/Cardano/Wallet/Read/Tx/TxOut.hs Outdated Show resolved Hide resolved
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-3215/read-txout branch from c5c9701 to 933b15f Compare August 23, 2024 09:34
@HeinrichApfelmus HeinrichApfelmus added this pull request to the merge queue Aug 23, 2024
Merged via the queue into master with commit 1c99602 Aug 23, 2024
25 checks passed
@HeinrichApfelmus HeinrichApfelmus deleted the HeinrichApfelmus/ADP-3215/read-txout branch August 23, 2024 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants