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

Remove cardano-wallet-byron package #1919

Closed
7 of 9 tasks
rvl opened this issue Jul 17, 2020 · 7 comments
Closed
7 of 9 tasks

Remove cardano-wallet-byron package #1919

rvl opened this issue Jul 17, 2020 · 7 comments
Assignees

Comments

@rvl
Copy link
Contributor

rvl commented Jul 17, 2020

Context

ADP-356

With the hard fork combinator we have a wallet which works on both byron and shelley networks. This will be in the cardano-wallet-shelley package.

We no longer require wallets that can only work with single-era nodes.

Decision

Remove the cardano-wallet-byron package.

Salvage any tests or benchmarks and copy them across to the cardano-wallet-shelley package.

Note: grep is your friend, especially when tracking down byron references in the nix build.

Acceptance Criteria

  1. There must be only one cardano-node backed cardano-wallet.
  2. Test cases, nightly tests, benchmarks, etc should not be lost because we need more automated QA not less.

Development

QA

  • cardano-wallet-byron was removed.
  • cardano-wallet-shelley was renamed to cardano-wallet
  • Existing shelley tests pass.
@piotr-iohk
Copy link
Contributor

I is this completed? I suppose there are integration tests to be "merged" to shelley, e.g. -> https://github.com/input-output-hk/cardano-wallet/blob/master/lib/byron/test/integration/Main.hs#L128, not sure about other stuff?

QA section filled would be helpful 😅

@Anviking
Copy link
Member

@piotr-iohk This is not in the QA column. Are you talking about another ticket?

@piotr-iohk
Copy link
Contributor

piotr-iohk commented Jul 29, 2020

Sorry, my bad. I thought I saw it in QA... Jesus 😫

@Anviking Anviking removed their assignment Jul 31, 2020
iohk-bors bot added a commit that referenced this issue Aug 5, 2020
1695: Add shelley mainnet and testnet to nightly restore tests r=rvl a=rvl

### Issue Number

ADP-356 / #1919 

### Overview

- Ports restore benchmark to shelley backend, using cardano-mode mainnet and testnet networks.
- Add option handling to restore benchmark instead of using environment variables.
- Change restore benchmark to use cardano-node configuration in such a way that it can easily be run without the assistance of nix.

### Comments

- CLI usage below.


Co-authored-by: Rodney Lorrimar <[email protected]>
Co-authored-by: IOHK <[email protected]>
Co-authored-by: Johannes Lund <[email protected]>
@Anviking Anviking self-assigned this Aug 10, 2020
iohk-bors bot added a commit that referenced this issue Aug 10, 2020
2012: Remove cardano-wallet-byron package r=Anviking a=Anviking



# Issue Number

#1919 


# Overview

- [x] `rm -rf lib/byron`
- [x] Remove nix and CI stuff relating to it


# Comments

```
Since cardano-wallet-shelley is now targeting the mainnet, we can remove
the byron package.

We should perhaps concider renaming cardano-wallet-shelley to simply
cardano-wallet.
```

<!-- 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: Johannes Lund <[email protected]>
iohk-bors bot added a commit that referenced this issue Aug 11, 2020
2013: Rename cardano-wallet-shelley to cardano-wallet r=Anviking a=Anviking

# Issue Number

#1919 


# Overview


- [x] Rename `cardano-wallet-shelley` to `cardano-wallet`
- [x] Check that everything still works (Think it might do)
- [ ] Update wiki 

# Comments

Package structure after renaming as a reminder:
![wreq](https://user-images.githubusercontent.com/304423/89775246-b7047c80-db07-11ea-8f8f-b54eec0570a7.png)

[Hydra jobset](https://hydra.iohk.io/jobset/Cardano/cardano-wallet-pr-2013#tabs-jobs)


2014: Run coin selection properties with non-zero withdrawal r=Anviking a=Anviking

# Issue Number

From looking at  #2006, but surely not related.


# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] I added a withdrawal field to `CoinSelProp` and used it in coin selection properties

# Comments

- I'm not that familiar with the coin selection, but this seems like a good idea, right?

<!-- 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: Johannes Lund <[email protected]>
iohk-bors bot added a commit that referenced this issue Aug 12, 2020
2028: Filter Tracers before adding LogObject and converting to Traces r=rvl a=rvl

### Issue Number

~Relates to #2005, I think.~

**edit by @KtorZ**: relates to #1997 

### Overview

Changes the tracer minimum severity so that it filters logs before adding `LogObject` and converting to `Trace`.

Users noticed that setting `--trace-db=off` improved performance, especially when restoring many wallets in parallel.

### Comments

Benchmarks not fixed yet.


2029: Fix lingering references to old package names r=rvl a=rvl

### Issue Number

Relates to #1919.

### Overview

- Fixes nightly windows tests and benchmarks which were using the cardano-wallet-shelley name.
- Removes the last unneeded references to shelley and byron in names.


Co-authored-by: Rodney Lorrimar <[email protected]>
iohk-bors bot added a commit that referenced this issue Aug 18, 2020
2045: Fix restore bench and docker r=rvl a=Anviking

# Issue Number

#1919, #2044 


# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] Fix broken restore bench  https://buildkite.com/input-output-hk/cardano-wallet-nightly/builds/608#f02f336b-c7be-4b3f-b86b-bc825c9b5ac2
- [ ] _Hope_ `docker` should be fixed, but can't easily test locally ⚠️ 


# 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: Johannes Lund <[email protected]>
iohk-bors bot added a commit that referenced this issue Aug 18, 2020
2045: Fix restore bench and docker r=Anviking a=Anviking

# Issue Number

#1919, #2044 


# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] Fix broken restore bench  https://buildkite.com/input-output-hk/cardano-wallet-nightly/builds/608#f02f336b-c7be-4b3f-b86b-bc825c9b5ac2
- [ ] _Hope_ `docker` should be fixed, but can't easily test locally ⚠️ 


# 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: Johannes Lund <[email protected]>
iohk-bors bot added a commit that referenced this issue Aug 18, 2020
2045: Fix restore bench and docker r=Anviking a=Anviking

# Issue Number

#1919, #2044 


# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] Fix broken restore bench  https://buildkite.com/input-output-hk/cardano-wallet-nightly/builds/608#f02f336b-c7be-4b3f-b86b-bc825c9b5ac2
- [ ] _Hope_ `docker` should be fixed, but can't easily test locally ⚠️ 


# 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: Johannes Lund <[email protected]>
iohk-bors bot added a commit that referenced this issue Aug 18, 2020
2045: Fix restore bench and docker r=Anviking a=Anviking

# Issue Number

#1919, #2044 


# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] Fix broken restore bench  https://buildkite.com/input-output-hk/cardano-wallet-nightly/builds/608#f02f336b-c7be-4b3f-b86b-bc825c9b5ac2
- [ ] _Hope_ `docker` should be fixed, but can't easily test locally ⚠️ 


# 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: Johannes Lund <[email protected]>
iohk-bors bot added a commit that referenced this issue Aug 18, 2020
2045: Fix restore bench and docker r=Anviking a=Anviking

# Issue Number

#1919, #2044 


# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] Fix broken restore bench  https://buildkite.com/input-output-hk/cardano-wallet-nightly/builds/608#f02f336b-c7be-4b3f-b86b-bc825c9b5ac2
- [ ] _Hope_ `docker` should be fixed, but can't easily test locally ⚠️ 


# 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: Johannes Lund <[email protected]>
iohk-bors bot added a commit that referenced this issue Aug 20, 2020
2048: Adjust travis deployment for renamed cardano-wallet binary r=rvl a=Anviking

# Issue Number

#1919 


# Overview

- [x] Fix naming issue caused by the renaming from `cardano-wallet-shelley` to `cardano-wallet`
- [x] ~Simply use the archives from hydra as-is. No need to unzip an re-zip. We don't rename the executable anymore.~ @rvl has made it use same filenames as previous releases.
- [x] Use a script for picking builds of the exact revision from hydra.

# Comments

- The adjustment of the cardano-wallet-jormungandr archives is left as-is.

Comparing latest release to manually calling `curl -L https://hydra.iohk.io/job/Cardano/cardano-wallet/cardano-wallet-linux64/latest/download/1 --output cardano-wallet-$TRAVIS_TAG-linux64.tar.gz`

The extracted folder appears different. Does that matter?

<img width="543" alt="Skärmavbild 2020-08-18 kl  11 58 07" src="https://user-images.githubusercontent.com/304423/90499685-2141a080-e14a-11ea-8843-ec72c47988bd.png">



Co-authored-by: Johannes Lund <[email protected]>
Co-authored-by: Rodney Lorrimar <[email protected]>
@piotr-iohk
Copy link
Contributor

Test cases, nightly tests, benchmarks, etc should not be lost because we need more automated QA not less.

Perhaps that is not a result of this change but Benchmarks and Windows tests are failing for quite a while now. (Although benchmarks are hopefully fixed 🤞 )

@piotr-iohk
Copy link
Contributor

As per #1970 (comment) this item was supposed to "fix" Windows tests...

@rvl
Copy link
Contributor Author

rvl commented Aug 25, 2020

The windows tests were fixed but then there was a new failure. I have opened #2067 for it.

@piotr-iohk
Copy link
Contributor

lgtm

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

No branches or pull requests

3 participants