Get TxMetadata from blocks on chain #2079

rvl · 2020-08-26T07:00:22Z

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.

KtorZ · 2020-08-26T08:08:27Z

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

+    , metadata
+        :: !(Maybe TxMetadata)
+        -- ^ Semi-structured application-specific extension data stored in the
+        -- transaction on chain.


KtorZ · 2020-08-26T08:09:57Z

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

+
+-- Compare metadatas by json encoding
+instance Ord TxMetadata where
+    compare = comparing (Aeson.encode . unTxMetadata)


What's the rationale here ? Why not rely on the existing Ord instance for Value ?

Value has no Ord ... nor does Cardano.Api.TxMetadata.
I define it by string encoding so that other types can keep their Ord constraint.

KtorZ · 2020-08-27T12:50:03Z

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

@@ -555,6 +558,9 @@ data ApiTransaction (n :: NetworkDiscriminant) = ApiTransaction
    , outputs :: ![AddressAmount (ApiT Address, Proxy n)]
    , withdrawals :: ![ApiWithdrawal n]
    , status :: !(ApiT TxStatus)
+    -- fixme: #2073 not sure whether empty metadata should be null or missing
+    -- key.
+    , metadata :: !(Maybe (ApiT TxMetadata))


My own preference is to have a null value so that it is more explicit client side but I admit not being 100% sure about how the default aeson generic options treat this in our API at the moment.

That is also my preference actually. But when submitting a transaction, it is more convenient for users if they can omit the key. Our default generic options are to omit Nothing fields.

Will sort it out in #2089.

KtorZ · 2020-08-27T14:35:23Z

cardano-1.19.x.yaml

@@ -60,7 +60,7 @@ packages:
 - Win32-2.6.2.0

 - git: https://github.com/input-output-hk/cardano-base
-  commit: df8687488449f71dce3d881800c21e41fe1b7fc1
+  commit: 3e2d92b25c675fd923a07237eb1d0859b29a79d3


Damn. This completely breaks the linker when building with stack for me. I am trying on a completely clean stack root now because It reminds me of a past issue with stack not correctly figuring out links between same packages of different snapshots.

Yeah I think the YAML filename needs to be different so that stack knows that something's changed. But I couldn't think of a good name for a version somewhere between 1.19.0 and 1.20.0.

I needed to bump this library so that the shelley-spec-ledger-test package would build. This was just to get access to the TxMetadata test generators. But that has caused more problems with multiple Arbitrary instances of other types. It's just not worth it. I will copy&paste the Arbitrary instance.

KtorZ · 2020-08-31T07:52:23Z

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

+        -- transaction on chain.
+        --
+        -- This is not to be confused with 'TxMeta', which is information about
+        -- a transaction derived from the ledger.


yeah, we might want to rename TxMeta at some point indeed.

KtorZ · 2020-08-31T07:53:55Z

lib/core/test/unit/Cardano/Wallet/Primitive/TypesSpec.hs

+                , "        - \"v1\""
+                , "  - key: 0"
+                , "  - val: \"value\""
+                ]


KtorZ

OK

rvl · 2020-08-31T12:34:36Z

bors r+

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 <[email protected]> Co-authored-by: IOHK <[email protected]>

iohk-bors · 2020-08-31T13:02:55Z

Build failed (retrying...)

ci/hydra-build:required

iohk-bors · 2020-08-31T13:10:07Z

Canceled

rvl · 2020-09-01T02:30:17Z

Since we have added shelley libs as dependencies to the core library, all executables now link with libsodium. So the nix needed to be updated so that libsodium-23.dll was copied for all windows exes.

bors r+

iohk-bors · 2020-09-01T04:12:46Z

Build succeeded

2091: Add tx metadata to sqlite database r=rvl a=rvl ### Issue Number ADP-307 / #2072 ### Overview Includes transaction metadata in the sqlite database transaction history. ### Comments - Based on PR #2079 branch - DB state machine tests fail. Possibly due to mutations during the TxMetadata json round-trip. Co-authored-by: Rodney Lorrimar <[email protected]> Co-authored-by: KtorZ <[email protected]>

rvl added the ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG label Aug 26, 2020

rvl added this to the (ADP-307) Transaction metadata milestone Aug 26, 2020

rvl self-assigned this Aug 26, 2020

KtorZ reviewed Aug 26, 2020

View reviewed changes

rvl force-pushed the rvl/2077/tx-metadata branch from 59d4386 to 95aac1f Compare August 27, 2020 06:21

KtorZ reviewed Aug 27, 2020

View reviewed changes

rvl force-pushed the rvl/2077/tx-metadata branch 2 times, most recently from dcd6bfe to 9660fd1 Compare August 28, 2020 10:17

rvl marked this pull request as ready for review August 28, 2020 10:18

This was referenced Aug 28, 2020

Add metadata to transaction layer #2090

Merged

Add tx metadata to sqlite database #2091

Merged

KtorZ reviewed Aug 31, 2020

View reviewed changes

lib/core/test/unit/Cardano/Wallet/Primitive/TypesSpec.hs

, " - \"v1\""

, " - key: 0"

, " - val: \"value\""

]

Copy link

Member

KtorZ Aug 31, 2020

Choose a reason for hiding this comment

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

👍

KtorZ approved these changes Aug 31, 2020

View reviewed changes

KtorZ force-pushed the rvl/2077/tx-metadata branch from 9660fd1 to c003df8 Compare August 31, 2020 07:58

rvl force-pushed the rvl/2077/tx-metadata branch from b0c33cf to b9d1b1a Compare August 31, 2020 14:52

rvl added 2 commits September 1, 2020 12:19

Get TxMetadata from blocks on chain

109c5e8

nix: all windows executables now require libsodium-23.dll

d7ca998

rvl force-pushed the rvl/2077/tx-metadata branch from b9d1b1a to d7ca998 Compare September 1, 2020 02:28

iohk-bors bot merged commit 3aa7d52 into master Sep 1, 2020

iohk-bors bot deleted the rvl/2077/tx-metadata branch September 1, 2020 04:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get TxMetadata from blocks on chain #2079

Get TxMetadata from blocks on chain #2079

rvl commented Aug 26, 2020 •

edited

Loading

KtorZ Aug 26, 2020

KtorZ Aug 26, 2020

rvl Aug 28, 2020

KtorZ Aug 27, 2020

rvl Aug 28, 2020

rvl Aug 28, 2020

KtorZ Aug 27, 2020

rvl Aug 28, 2020

rvl Aug 28, 2020

KtorZ Aug 31, 2020

KtorZ Aug 31, 2020

KtorZ left a comment

rvl commented Aug 31, 2020

iohk-bors bot commented Aug 31, 2020

iohk-bors bot commented Aug 31, 2020

rvl commented Sep 1, 2020

iohk-bors bot commented Sep 1, 2020

Get TxMetadata from blocks on chain #2079

Get TxMetadata from blocks on chain #2079

Conversation

rvl commented Aug 26, 2020 • edited Loading

Issue Number

Overview

Comments

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KtorZ left a comment

Choose a reason for hiding this comment

rvl commented Aug 31, 2020

iohk-bors bot commented Aug 31, 2020

Build failed (retrying...)

iohk-bors bot commented Aug 31, 2020

Canceled

rvl commented Sep 1, 2020

iohk-bors bot commented Sep 1, 2020

Build succeeded

rvl commented Aug 26, 2020 •

edited

Loading