Skip to content
This repository was archived by the owner on Jul 27, 2022. It is now read-only.

Problem:(CRO-542) no integration test for hd-wallet#585

Merged
bors[bot] merged 1 commit intocrypto-com:masterfrom
leejw51crypto:cro-542
Nov 19, 2019
Merged

Problem:(CRO-542) no integration test for hd-wallet#585
bors[bot] merged 1 commit intocrypto-com:masterfrom
leejw51crypto:cro-542

Conversation

@leejw51
Copy link
Copy Markdown
Contributor

@leejw51 leejw51 commented Nov 13, 2019

Solution: add hdwallet integration test

@leejw51crypto
Copy link
Copy Markdown
Collaborator

because, hd-wallet is the same with normal wallets except for wallet creation.
this test is enough for CI build.

also other test (mnemonic) is already done in unit test.

@codecov
Copy link
Copy Markdown

codecov bot commented Nov 13, 2019

Codecov Report

Merging #585 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #585      +/-   ##
==========================================
- Coverage   68.18%   68.17%   -0.01%     
==========================================
  Files         129      129              
  Lines       15299    15299              
==========================================
- Hits        10431    10430       -1     
- Misses       4868     4869       +1
Impacted Files Coverage Δ
chain-core/src/tx/fee/mod.rs 87.68% <0%> (-0.5%) ⬇️

Copy link
Copy Markdown
Contributor

@tomtau tomtau left a comment

Choose a reason for hiding this comment

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

i think it could be helpful to test the recovery RPC call (only internals are tested in unit tests, no the full flow via rpc) -- but it's fine to merge it without such tests if it blocks #586 (review)

Copy link
Copy Markdown
Collaborator

@calvinaco calvinaco left a comment

Choose a reason for hiding this comment

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

The test looks good.

It would be good to include a test for creating the HD wallet and restore the wallet with the returned mnemonic words; and a test to check if a transaction can be sent from a HD wallet and sync.

@leejw51crypto
Copy link
Copy Markdown
Collaborator

leejw51crypto commented Nov 14, 2019

ok, i will add recovery check
other checks (tx, sync)

@leejw51
Copy link
Copy Markdown
Contributor Author

leejw51 commented Nov 15, 2019

because it needs cro-540, i included it.
when i rebased with the latest, strange sync bug fixed

@leejw51 leejw51 force-pushed the cro-542 branch 2 times, most recently from d0461f2 to 6af2539 Compare November 15, 2019 15:57
@tomtau tomtau self-requested a review November 18, 2019 05:40
Copy link
Copy Markdown
Contributor

@tomtau tomtau left a comment

Choose a reason for hiding this comment

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

@leejw51 @leejw51crypto can you squash the commits?

@leejw51
Copy link
Copy Markdown
Contributor Author

leejw51 commented Nov 18, 2019

ok

@tomtau
Copy link
Copy Markdown
Contributor

tomtau commented Nov 18, 2019

bors r+

@bors
Copy link
Copy Markdown
Contributor

bors bot commented Nov 18, 2019

Merge conflict (retrying...)

@bors
Copy link
Copy Markdown
Contributor

bors bot commented Nov 18, 2019

Merge conflict

@leejw51crypto
Copy link
Copy Markdown
Collaborator

i'll rebase

Solution: add private, public keys to storage
Problem:(CRO-542) no integration test for hd-wallet

Solution: add hdwallet integration test
add hdwallet test


add hdwallet restore integration test
@leejw51crypto
Copy link
Copy Markdown
Collaborator

let's use viewkey from transfer index=0,
because viewkey is primarily used for hiding utxos

@tomtau
Copy link
Copy Markdown
Contributor

tomtau commented Nov 19, 2019

bors r+

bors bot added a commit that referenced this pull request Nov 19, 2019
585: Problem:(CRO-542) no integration test for hd-wallet r=tomtau a=leejw51

Solution: add hdwallet integration test

Co-authored-by: Jongwhan Lee <jonghwan@crypto.com>
@bors
Copy link
Copy Markdown
Contributor

bors bot commented Nov 19, 2019

@bors bors bot merged commit 8304e0f into crypto-com:master Nov 19, 2019
bors bot added a commit that referenced this pull request Nov 25, 2019
613: Problem: (CRO-603) Bech32 address always encoded with initialized network r=tomtau a=calvinlauco

Solution: Add network argument to to_cro()

---
`to_cro()` always encoded with the initialized network, making it mpossible to encode to arbitrary network

615: Bump serde_json from 1.0.41 to 1.0.42 r=tomtau a=dependabot-preview[bot]

Bumps [serde_json](https://github.com/serde-rs/json) from 1.0.41 to 1.0.42.
<details>
<summary>Release notes</summary>

*Sourced from [serde_json's releases](https://github.com/serde-rs/json/releases).*

> ## v1.0.42
> - Add `impl From<()> for Value` which produces Value::Null ([#585](https://github-redirect.dependabot.com/serde-rs/json/issues/585), thanks [@&#8203;Nilix007](https://github.com/Nilix007))
</details>
<details>
<summary>Commits</summary>

- [`f0471e6`](serde-rs/json@f0471e6) Release 1.0.42
- [`bf8cc66`](serde-rs/json@bf8cc66) Merge pull request [#585](https://github-redirect.dependabot.com/serde-rs/json/issues/585) from Nilix007/add_from_unit_for_value
- [`ff5a59c`](serde-rs/json@ff5a59c) Add `impl From\<()> for Value`
- [`7dda823`](serde-rs/json@7dda823) Merge pull request [#580](https://github-redirect.dependabot.com/serde-rs/json/issues/580) from andrisak/docs_read_without_blocking_eof
- [`2065227`](serde-rs/json@2065227) Added missing fake_main [#522](https://github-redirect.dependabot.com/serde-rs/json/issues/522)
- [`3a5aba3`](serde-rs/json@3a5aba3) Fix for Document how to deserialize from a prefix of an io::Read without bloc...
- [`0c72820`](serde-rs/json@0c72820) Ignore must_use_candidate pedantic lint
- [`265f1ca`](serde-rs/json@265f1ca) Upgrade to rustversion 1.0
- [`b63ad14`](serde-rs/json@b63ad14) Address needless_doctest_main lint
- See full diff in [compare view](serde-rs/json@v1.0.41...v1.0.42)
</details>
<br />

[![Dependabot compatibility score](https://api.dependabot.com/badges/compatibility_score?dependency-name=serde_json&package-manager=cargo&previous-version=1.0.41&new-version=1.0.42)](https://dependabot.com/compatibility-score.html?dependency-name=serde_json&package-manager=cargo&previous-version=1.0.41&new-version=1.0.42)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
- `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language
- `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language
- `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language
- `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language
- `@dependabot badge me` will comment on this PR with code to add a "Dependabot enabled" badge to your readme

Additionally, you can set the following in your Dependabot [dashboard](https://app.dependabot.com):
- Update frequency (including time of day and day of week)
- Pull request limits (per update run and/or open at any time)
- Out-of-range updates (receive only lockfile updates, if desired)
- Security updates (receive only security updates, if desired)



</details>

Co-authored-by: Calvin Lau <calvinlauco@gmail.com>
Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants