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

Jump to bottom

CLI: Add option "cardano-wallet transaction create --metadata=JSON" #2125

Merged
merged 9 commits into from Sep 22, 2020
Merged

CLI: Add option "cardano-wallet transaction create --metadata=JSON" #2125

merged 9 commits into from Sep 22, 2020

Conversation

rvl
Copy link
Contributor

@rvl rvl commented Sep 9, 2020 •
edited

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.

@rvl rvl added the ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG label Sep 9, 2020
@rvl rvl added this to the (ADP-307) Transaction metadata milestone Sep 9, 2020
@rvl rvl self-assigned this Sep 9, 2020
@rvl rvl added this to In Progress in Adrestia Sep 9, 2020
@rvl rvl marked this pull request as ready for review September 15, 2020 05:15
@KtorZ KtorZ self-requested a review September 15, 2020 14:55
KtorZ
KtorZ approved these changes Sep 15, 2020
View reviewed changes
Copy link
Member

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

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

👌

lib/core-integration/src/Test/Integration/Scenario/CLI/Shelley/Transactions.hs Outdated
it "TRANSMETA_CREATE_01 - Transaction with metadata via CLI" $ \ctx -> do
(wSrc, wDest) <- (,) <$> fixtureWallet ctx <*> emptyWallet ctx
let amt = 100000
let md = Just "{ \"1\": \"hello\" }"
Copy link
Member

Choose a reason for hiding this comment

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

👋

@rvl rvl force-pushed the rvl/2076/tx-metadata-cli branch 3 times, most recently from bf40c68 to 7498249 Compare September 16, 2020 07:15
@rvl
Copy link
Contributor Author

rvl commented Sep 16, 2020

bors r+

iohk-bors bot added a commit that referenced this pull request 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
Copy link
Contributor

iohk-bors bot commented Sep 16, 2020

Build failed (retrying...):

iohk-bors bot added a commit that referenced this pull request 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
Copy link
Contributor

iohk-bors bot commented Sep 16, 2020

Build failed:

@rvl
Copy link
Contributor Author

rvl commented Sep 17, 2020

bors r+

iohk-bors bot added a commit that referenced this pull request 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
Copy link
Contributor

iohk-bors bot commented Sep 17, 2020

Build failed (retrying...):

iohk-bors bot added a commit that referenced this pull request 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
Copy link
Contributor

iohk-bors bot commented Sep 17, 2020

Build failed:

@KtorZ
Copy link
Member

KtorZ commented Sep 20, 2020

bors r+

iohk-bors bot added a commit that referenced this pull request 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
Copy link
Contributor

iohk-bors bot commented Sep 20, 2020 •
edited by rvl

Build failed:

stack --color always test --fast --skip jormungandr-integration --ta '--skip SERIAL' --flag cardano-wallet-cli:release --flag cardano-wallet-jormungandr:release --flag cardano-wallet:release --flag cardano-wallet-test-utils:release --flag cardano-wallet-launcher:release --flag cardano-wallet-core:release --flag text-class:release --flag cardano-wallet-core-integration:release
cardano-wallet-2020.9.11: test (suite: integration, args: --skip SERIAL)
cardano-wallet-cli-2020.9.11: test (suite: unit, args: --skip SERIAL)
cardano-wallet-core-2020.9.11: test (suite: unit, args: --skip SERIAL)
cardano-wallet-jormungandr-2020.9.11: test (suite: unit, args: --skip SERIAL)
cardano-wallet-launcher-2020.9.11: test (suite: unit, args: --skip SERIAL)
text-class-2020.9.11: test (suite: unit, args: --skip SERIAL)
text-class-2020.9.11: Test suite unit passed
cardano-wallet-cli-2020.9.11: Test suite unit passed
cardano-wallet-launcher-2020.9.11: Test suite unit passed
cardano-wallet-jormungandr-2020.9.11: Test suite unit passed
cardano-wallet-core-2020.9.11: Test suite unit passed
Progress 5/6: cardano-wallet-2020.9.11
Timed out after 60 minutes.

@rvl
Copy link
Contributor Author

rvl commented Sep 21, 2020

Oops - the integration test transaction did not exceed minimum UTxO.

bors r+

iohk-bors bot added a commit that referenced this pull request 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
Copy link
Contributor

iohk-bors bot commented Sep 21, 2020

Build failed:

@rvl
Copy link
Contributor Author

rvl commented Sep 21, 2020

bors r+

iohk-bors bot added a commit that referenced this pull request 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
Copy link
Contributor

iohk-bors bot commented Sep 21, 2020

Build failed:

@rvl
Copy link
Contributor Author

rvl commented Sep 21, 2020

Should be updated now.

bors r+

iohk-bors bot added a commit that referenced this pull request 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
Copy link
Contributor

iohk-bors bot commented Sep 21, 2020

Build failed:

@KtorZ
Copy link
Member

KtorZ commented Sep 21, 2020

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Sep 21, 2020

Canceled.

@KtorZ
Copy link
Member

KtorZ commented Sep 21, 2020

bors merge

iohk-bors bot added a commit that referenced this pull request 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
Copy link
Contributor

iohk-bors bot commented Sep 21, 2020

Build failed:

@rvl
Copy link
Contributor Author

rvl commented Sep 22, 2020

@KtorZ You force pushed over my other build fixes. 🙂 I have pushed them back, rebased, and updated for #2147.

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Sep 22, 2020

Build succeeded:

@iohk-bors iohk-bors bot merged commit a1a82e6 into master Sep 22, 2020
@iohk-bors iohk-bors bot deleted the rvl/2076/tx-metadata-cli branch September 22, 2020 03:18
@rvl rvl mentioned this pull request Oct 6, 2020
Closed
@piotr-iohk piotr-iohk moved this from In Progress to Closed in Adrestia 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
Reviewers

@KtorZ KtorZ KtorZ approved these changes

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

Successfully merging this pull request may close these issues.

None yet
2 participants
@rvl @KtorZ