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 support in CLI for creating transactions #2076

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

Tx metadata support in CLI for creating transactions #2076

rvl opened this issue Aug 26, 2020 · 2 comments
Assignees
@rvl
Labels
ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG PRIORITY:LOW Would eventually require attention if times allow it.
Milestone
(ADP-307) Transac...

Comments

@rvl
Copy link
Contributor

rvl commented Aug 26, 2020 •
edited

Context

ADP-307

For completeness, the CLI should support metadata when creating transactions.

Decision

Add a --metadata option to the CLI:

Usage: cardano-wallet transaction create [--port INT] WALLET_ID
                                         --payment PAYMENT
                                         [--metadata JSON]
  Create and submit a new transaction.

Available options:
  -h,--help                Show this help text
  --port INT               port used for serving the wallet API. (default: 8090)
  --payment PAYMENT        address to send to and amount to send separated by @,
                           e.g. '<amount>@<address>'
  --metadata JSON    extra application-specific data to include in the transaction. The argument must be encoded in JSON format, and it must match the metadata JSON schema.

I considered supporting loading a JSON file, either a --metadata-file option, or through @ notation ala curl --data-raw. But it should be ok in all cases to simply put the entire JSON data as a command-line argument.

Acceptance Criteria

  1. It must be possible to supply metadata to cardano-wallet transaction create.
  2. Metadata must be validated as well-formed json before POSTing the request. It may also be validated according to the metadata schema before sending to the wallet server.
  3. Metadata format errors must be reported to the user.
  4. Wiki/docs updated with information on how to set transaction metadata.

Development

QA

  1. CLI golden tests updated in lib/cli/test/unit/Cardano/CLISpec.hs.
  2. Unit tests for metadata command line option added to lib/cli/test/unit/Cardano/CLISpec.hs - "Tx Metadata JSON option".
  3. TRANSMETA_CREATE_01 integration test added.
@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
@KtorZ
Copy link
Member

KtorZ commented Aug 26, 2020

To be tackle at the very end I think or could even be moved in another story.

@rvl rvl added the PRIORITY:LOW Would eventually require attention if times allow it. label Aug 26, 2020
@rvl rvl self-assigned this Sep 9, 2020
@rvl rvl added this to Backlog in Adrestia via automation Sep 9, 2020
@rvl rvl moved this from Backlog to In Progress in Adrestia Sep 9, 2020
@rvl rvl mentioned this issue Sep 9, 2020
Merged
iohk-bors bot added a commit that referenced this issue Sep 16, 2020
@iohk-bors @rvl @jonathanknowles
Merge #2125 #2136
f8b0eaf 
2125: CLI: Add option "cardano-wallet transaction create --metadata=JSON" r=rvl a=rvl

### Issue Number

ADP-307 / #2076

### Overview

- Adds `--metadata=JSON` option to transaction create and fee estimate commands.

### Comments

Will update the CLI wiki page after merging.


2136: Run existing pool DB property tests on model implementation. r=jonathanknowles a=jonathanknowles

# Issue Number

https://jira.iohk.io/browse/ADP-406

# Overview

Over time, we've built up a fairly large suite of [property tests for the pool database](https://github.com/input-output-hk/cardano-wallet/blob/ff181966317b50083a61aa3e4f3dfd2ac1bff1a3/lib/core/test/unit/Cardano/Pool/DB/Properties.hs).

However, up until now these tests were only ever run on the **SQLite** implementation. The **model** implementation was not tested.

This PR fixes this by adjusting the test suite to test both the **SQLite** and **model** implementations. For example:

```hs
ghcid \
    --command "stack ghci cardano-wallet-core:lib cardano-wallet-core:test:unit" \
    --test ':run Spec.main -m prop_listPoolLifeCycleData_multiplePools_multipleCerts'

Cardano.Pool.DB.MVar
  MVar
    Stake Pool Properties
      prop_listPoolLifeCycleData_multiplePools_multipleCerts
        +++ OK, passed 100 tests.
Cardano.Pool.DB.Sqlite
  Sqlite
    Stake Pool Properties
      prop_listPoolLifeCycleData_multiplePools_multipleCerts
        +++ OK, passed 100 tests.

Finished in 40.3082 seconds
2 examples, 0 failures
```

## Details

This PR:
1. Generalizes the `Pool.DB.Properties` module to work with any implementation of `Pool.DBLayer`.
2. Adds a `Pool.DB.MVarSpec` module to handle the required setup for testing the model implementation.
3. Fixes the small number of resulting failures.

Co-authored-by: Rodney Lorrimar <rodney.lorrimar@iohk.io>
Co-authored-by: Jonathan Knowles <jonathan.knowles@iohk.io>
iohk-bors bot added a commit that referenced this issue Sep 16, 2020
@iohk-bors @rvl
Merge #2125
4b2df86 
2125: CLI: Add option "cardano-wallet transaction create --metadata=JSON" r=rvl a=rvl

### Issue Number

ADP-307 / #2076

### Overview

- Adds `--metadata=JSON` option to transaction create and fee estimate commands.

### Comments

Will update the CLI wiki page after merging.


Co-authored-by: Rodney Lorrimar <rodney.lorrimar@iohk.io>
iohk-bors bot added a commit that referenced this issue Sep 17, 2020
@iohk-bors @rvl
Merge #2125 #2135 #2141
3642da8 
2125: CLI: Add option "cardano-wallet transaction create --metadata=JSON" r=rvl a=rvl

### Issue Number

ADP-307 / #2076

### Overview

- Adds `--metadata=JSON` option to transaction create and fee estimate commands.

### Comments

Will update the CLI wiki page after merging.


2135: Fix dylib references of bundled programs on macOS r=rvl a=rvl

### Issue Number

Resolves #2134

### Overview

- On macOS rewrite dylib references from `/nix/store` to `@executable_path` - for every executable in the bundle.
- Add linking tests to `check-bundle.rb` for macOS and Linux.


2141: Make Jörmungandr tests pass again r=rvl a=piotr-iohk

# Issue Number

<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->


# Overview

- 3665332
  Disable STAKE_POOLS_* tests due to #2140
  
- 6e276af
  Remove hard-coded mnemonics in case not necessary in integration tests
  
- 038c365
  Remove Shelley specific transaction suite. Add few more basic transaction tests to Jormungandr suite.
  
- 6e98d4b
  BYRON_MIGRATE_01 - make Jormungandr big wallet for migration smaller (the same as Shelley one) and adjust to pass on both backends
  
- e3d6a55
  adjust genMnemonics to be more standard



# Comments

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Rodney Lorrimar <rodney.lorrimar@iohk.io>
Co-authored-by: Piotr Stachyra <piotr.stachyra@iohk.io>
iohk-bors bot added a commit that referenced this issue Sep 17, 2020
@iohk-bors @rvl
Merge #2125
37ba484 
2125: CLI: Add option "cardano-wallet transaction create --metadata=JSON" r=rvl a=rvl

### Issue Number

ADP-307 / #2076

### Overview

- Adds `--metadata=JSON` option to transaction create and fee estimate commands.

### Comments

Will update the CLI wiki page after merging.


Co-authored-by: Rodney Lorrimar <rodney.lorrimar@iohk.io>
iohk-bors bot added a commit that referenced this issue Sep 20, 2020
@iohk-bors @rvl
Merge #2125
6d0691f 
2125: CLI: Add option "cardano-wallet transaction create --metadata=JSON" r=KtorZ a=rvl

### Issue Number

ADP-307 / #2076

### Overview

- Adds `--metadata=JSON` option to transaction create and fee estimate commands.

### Comments

Will update the CLI wiki page after merging.


Co-authored-by: Rodney Lorrimar <rodney.lorrimar@iohk.io>
iohk-bors bot added a commit that referenced this issue Sep 21, 2020
@iohk-bors @rvl
Merge #2125
d6a2474 
2125: CLI: Add option "cardano-wallet transaction create --metadata=JSON" r=rvl a=rvl

### Issue Number

ADP-307 / #2076

### Overview

- Adds `--metadata=JSON` option to transaction create and fee estimate commands.

### Comments

Will update the CLI wiki page after merging.


Co-authored-by: Rodney Lorrimar <rodney.lorrimar@iohk.io>
iohk-bors bot added a commit that referenced this issue Sep 21, 2020
@iohk-bors @rvl
Merge #2125
f1aeba1 
2125: CLI: Add option "cardano-wallet transaction create --metadata=JSON" r=rvl a=rvl

### Issue Number

ADP-307 / #2076

### Overview

- Adds `--metadata=JSON` option to transaction create and fee estimate commands.

### Comments

Will update the CLI wiki page after merging.


Co-authored-by: Rodney Lorrimar <rodney.lorrimar@iohk.io>
iohk-bors bot added a commit that referenced this issue Sep 21, 2020
@iohk-bors @rvl
Merge #2125
292b48a 
2125: CLI: Add option "cardano-wallet transaction create --metadata=JSON" r=rvl a=rvl

### Issue Number

ADP-307 / #2076

### Overview

- Adds `--metadata=JSON` option to transaction create and fee estimate commands.

### Comments

Will update the CLI wiki page after merging.


Co-authored-by: Rodney Lorrimar <rodney.lorrimar@iohk.io>
iohk-bors bot added a commit that referenced this issue Sep 21, 2020
@iohk-bors @rvl
Merge #2125
952e9ac 
2125: CLI: Add option "cardano-wallet transaction create --metadata=JSON" r=rvl a=rvl

### Issue Number

ADP-307 / #2076

### Overview

- Adds `--metadata=JSON` option to transaction create and fee estimate commands.

### Comments

Will update the CLI wiki page after merging.


Co-authored-by: Rodney Lorrimar <rodney.lorrimar@iohk.io>
iohk-bors bot added a commit that referenced this issue Sep 21, 2020
@iohk-bors @rvl @KtorZ
Merge #2125
76e4c09 
2125: CLI: Add option "cardano-wallet transaction create --metadata=JSON" r=KtorZ a=rvl

### Issue Number

ADP-307 / #2076

### Overview

- Adds `--metadata=JSON` option to transaction create and fee estimate commands.

### Comments

Will update the CLI wiki page after merging.


Co-authored-by: Rodney Lorrimar <rodney.lorrimar@iohk.io>
Co-authored-by: KtorZ <matthias.benkort@gmail.com>
Co-authored-by: IOHK <devops+stack-project@iohk.io>
iohk-bors bot added a commit that referenced this issue Sep 22, 2020
@iohk-bors @rvl @KtorZ
Merge #2125
a1a82e6 
2125: CLI: Add option "cardano-wallet transaction create --metadata=JSON" r=rvl a=rvl

### Issue Number

ADP-307 / #2076

### Overview

- Adds `--metadata=JSON` option to transaction create and fee estimate commands.

### Comments

Will update the CLI wiki page after merging.


Co-authored-by: Rodney Lorrimar <rodney.lorrimar@iohk.io>
Co-authored-by: KtorZ <matthias.benkort@gmail.com>
@rvl rvl moved this from In Progress to QA in Adrestia Oct 6, 2020
@piotr-iohk
Copy link
Contributor

lgtm.

There is actually one issue that is related -> #2169.

Also the way --metadata $(<tx-meta.json) of using metadata in cmdline didn't work for me in hash shell, whether the content in a file was single or double quoted. (That was suggested in https://github.com/input-output-hk/cardano-wallet/wiki/TxMetadata#cli)

I have updated https://github.com/input-output-hk/cardano-wallet/wiki/TxMetadata#cli on how to use metadata in CLI (which worked for me) for Linux/Mac (putting metadata argument in single quotes) and Windows (metadata argument in double quotes and double quotes inside metadata JSON escaped).

Updated Wallet Cmdline interface with metadata exemplary usage.

Adrestia automation moved this from QA to Closed Oct 6, 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 PRIORITY:LOW Would eventually require attention if times allow it.
Projects
None yet
Milestone
(ADP-307) Transaction metadata
Development

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