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.

Sign up for GitHub

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

Jump to bottom

Tx metadata extracted from binary blocks #2077

Closed
rvl opened this issue Aug 26, 2020 · 3 comments
Closed

Tx metadata extracted from binary blocks #2077

rvl opened this issue Aug 26, 2020 · 3 comments
Assignees
@rvl
Labels
ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG
Milestone
(ADP-307) Transac...

Comments

@rvl
Copy link
Contributor

rvl commented Aug 26, 2020 •
edited
Loading

Context

Metadata must be serialized as CBOR in transactions. However, we should input and output JSON at the system boundary, and convert between JSON/CBOR internally.

User story ADP-307

As an API user,
I want to add metadata to a transaction and view them as JSON,
So that I can embed custom information within a transaction.

References

See Appendix E of Delegation Design Spec

Decision

There's an existing implementation in cardano-api for doing JSON/CBOR conversions. We can hopefully re-use this. See: https://github.com/input-output-hk/cardano-node/blob/495366939621d4a413d000494ec01a8b0f5edc26/cardano-api/src/Cardano/Api/MetaData.hs

Although it is possible to translate any JSON into CBOR, the reciprocal isn't true. There are therefore arbitrary choices made regarding the transformation JSON/CBOR to make sure it can roundtrip under several assumptions.

AC

  • cardano-wallet must show metadata in transaction that have some (as part of the response object).
  • metadata must be shown as JSON format in API responses
  • cardano-wallet-jormungandr should default to null values and fail if provided with a metadata object.

Development

  1. Extend TxMeta type to include metadata as an aeson Value.
  2. When getting transactions from a block, use the existing conversion functions to make Value from metadata.

QA

  • I have no actual chain data to use, so there are no golden tests.
  • integration test TRANS_CREATE_10 - Transaction with metadata in lib/core-integration/src/Test/Integration/Scenario/API/Shelley/Transactions.hs ⇒ Tx metadata API integration tests #2096.
@rvl rvl added the ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG label Aug 26, 2020
@rvl rvl added this to the (ADP-307) Transaction metadata milestone Aug 26, 2020
@rvl rvl added this to Backlog in Adrestia via automation Aug 26, 2020
@rvl rvl self-assigned this Aug 26, 2020
@KtorZ
Copy link
Member

KtorZ commented Aug 26, 2020

Extend TxMeta type to include metadata as an aeson Value.

@rvl from quickly looking at cardano-api, it seems that they provide a Metadatum data-type as an intermediate Haskell representation and that transformation are done from this Metadatum type (for JSON & CBOR). Perhaps re-using this data-type is preferable over an opaque aeson Value ? Thoughts?

@rvl rvl mentioned this issue Aug 26, 2020
Merged
@rvl
Copy link
Contributor Author

rvl commented Aug 26, 2020

Yes it could be better actually because it helps with validating new transactions.

Also I am thinking it's better to have TxMetadata inside the Tx type rather than TxMeta.

@rvl rvl mentioned this issue Aug 28, 2020
Merged
@rvl rvl moved this from Backlog to In Progress in Adrestia Aug 28, 2020
iohk-bors bot added a commit that referenced this issue Aug 31, 2020
@iohk-bors @rvl
Merge #2079 #2088 #2092
d695c6f 
2079: Get TxMetadata from blocks on chain r=rvl a=rvl

### Issue Number

ADP-307 / #2077

### Overview

- Initial types, etc.

### Comments

- I don't have any serialized block data to make golden tests. But the testing can be done with integration tests.


2088: Bump cardano-base so that shelley-spec-ledger-test compiles r=rvl a=rvl

### Issue Number

Found during #2077.

### Overview

If you are keen enough to build the test suites of all our dependencies, you would have found `shelley-spec-ledger-test` failing to compile, because it needs a later revision of `cardano-base`. This will fix it.


2092: Fix stack build on windows r=rvl a=rvl

### Issue Number

None

### Overview

On windows, the stack build would fail with some missing dependencies. There is some stack issue with GHC boot packages. The recommended course of action is to add them as `extra-deps` in `stack.yaml`. This fixes the build.

### Comments

Since the libsodium VRF was added, there is now an extra step to get stack builds working on windows. I have updated our [iohk-setup.ps1](https://github.com/input-output-hk/adrestia/wiki/scripts/iohk-setup.ps1) script to install libsodium and configure stack to use it.


Co-authored-by: Rodney Lorrimar <rodney.lorrimar@iohk.io>
Co-authored-by: IOHK <devops+stack-project@iohk.io>
iohk-bors bot added a commit that referenced this issue Aug 31, 2020
@iohk-bors @rvl
Merge #2088 #2092
0fc3538 
2088: Bump cardano-base so that shelley-spec-ledger-test compiles r=rvl a=rvl

### Issue Number

Found during #2077.

### Overview

If you are keen enough to build the test suites of all our dependencies, you would have found `shelley-spec-ledger-test` failing to compile, because it needs a later revision of `cardano-base`. This will fix it.


2092: Fix stack build on windows r=rvl a=rvl

### Issue Number

None

### Overview

On windows, the stack build would fail with some missing dependencies. There is some stack issue with GHC boot packages. The recommended course of action is to add them as `extra-deps` in `stack.yaml`. This fixes the build.

### Comments

Since the libsodium VRF was added, there is now an extra step to get stack builds working on windows. I have updated our [iohk-setup.ps1](https://github.com/input-output-hk/adrestia/wiki/scripts/iohk-setup.ps1) script to install libsodium and configure stack to use it.


Co-authored-by: Rodney Lorrimar <rodney.lorrimar@iohk.io>
Co-authored-by: IOHK <devops+stack-project@iohk.io>
iohk-bors bot added a commit that referenced this issue Sep 1, 2020
@iohk-bors @rvl
Merge #2079
3aa7d52 
2079: Get TxMetadata from blocks on chain r=rvl a=rvl

### Issue Number

ADP-307 / #2077

### Overview

- Initial types, etc.

### Comments

- I don't have any serialized block data to make golden tests. But the testing can be done with integration tests.


Co-authored-by: Rodney Lorrimar <rodney.lorrimar@iohk.io>
@rvl rvl moved this from In Progress to QA in Adrestia Sep 3, 2020
@piotr-iohk
Copy link
Contributor

lgtm.

Adrestia automation moved this from QA to Closed Sep 3, 2020
@CharlesMorgan-IOHK CharlesMorgan-IOHK removed this from Closed in Adrestia Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rvl rvl

Labels
ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG
Projects
None yet
Milestone
(ADP-307) Transaction metadata
Development

No branches or pull requests
3 participants
@rvl @KtorZ @piotr-iohk