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

RPC Wallet tests fix #567

Merged
merged 9 commits into from Aug 14, 2019
Merged

RPC Wallet tests fix #567

merged 9 commits into from Aug 14, 2019

Conversation

FreakyMaryk
Copy link
Contributor

PR intention

RPC Wallet tests fix for:

  • wallet-dump
  • walletbackup
  • wallet-hd

Code changes brief

Time of tests executing was increased due to tests long-run (restarting/sync/dampwallet)
There are some TODO in this tests which are can be removed during review.

@lgtm-com
Copy link

lgtm-com bot commented Aug 1, 2019

This pull request introduces 3 alerts when merging a32e423 into 53fb75c - view on LGTM.com

new alerts:

  • 3 for 'import *' may pollute namespace

@reubenyap reubenyap added this to Needs review in Firo Core via automation Aug 2, 2019
@reubenyap reubenyap added this to the v0.13.8.4 milestone Aug 2, 2019
@ultimaweapon
Copy link
Contributor

@FreakyMaryk could you provide the example of wallet dump file?

@lgtm-com
Copy link

lgtm-com bot commented Aug 2, 2019

This pull request introduces 3 alerts when merging a873714 into 53fb75c - view on LGTM.com

new alerts:

  • 3 for 'import *' may pollute namespace

riordant
riordant previously approved these changes Aug 2, 2019
Copy link
Contributor

@riordant riordant left a comment

Choose a reason for hiding this comment

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

LGTM

@lgtm-com
Copy link

lgtm-com bot commented Aug 2, 2019

This pull request introduces 3 alerts when merging e23965b into 53fb75c - view on LGTM.com

new alerts:

  • 3 for 'import *' may pollute namespace

@FreakyMaryk
Copy link
Contributor Author

@ultimaweapon
wallet.unencrypted.dump.txt

@thebevrishot
Copy link
Contributor

@riordant # - As wallet encryption creates a new master seed, it adds another set of hdmint keys. Could new keys can be used? Is this a bug?

@riordant
Copy link
Contributor

riordant commented Aug 2, 2019

@thebevrishot Not a bug. When you generate a new wallet, it is in unencrypted state, and has a particular master seed. When you encrypt, it generates a new master seed. Both seeds generate 21 keys to be used for mints. The newer keys will always be used, that is, the keys for latest master seed will always be used for new mints.

qa/rpc-tests/test_framework/test_helper.py Outdated Show resolved Hide resolved
qa/rpc-tests/wallet-dump.py Outdated Show resolved Hide resolved
qa/rpc-tests/wallet-dump.py Show resolved Hide resolved
qa/rpc-tests/wallet-dump.py Show resolved Hide resolved
qa/rpc-tests/wallet-hd.py Show resolved Hide resolved
qa/rpc-tests/walletbackup.py Show resolved Hide resolved
qa/rpc-tests/walletbackup.py Show resolved Hide resolved
qa/rpc-tests/walletbackup.py Show resolved Hide resolved
qa/rpc-tests/wallet-dump.py Outdated Show resolved Hide resolved
qa/rpc-tests/wallet-dump.py Outdated Show resolved Hide resolved
qa/rpc-tests/wallet-hd.py Outdated Show resolved Hide resolved
qa/rpc-tests/wallet-hd.py Outdated Show resolved Hide resolved
qa/rpc-tests/walletbackup.py Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Aug 6, 2019

This pull request introduces 7 alerts when merging 7ccaccd into d159d4c - view on LGTM.com

new alerts:

  • 6 for Asserting a tuple
  • 1 for Unused local variable

@lgtm-com
Copy link

lgtm-com bot commented Aug 6, 2019

This pull request introduces 1 alert when merging 142e9dd into d159d4c - view on LGTM.com

new alerts:

  • 1 for Unused import

qa/rpc-tests/wallet-dump.py Outdated Show resolved Hide resolved
qa/rpc-tests/wallet-dump.py Show resolved Hide resolved
qa/rpc-tests/wallet-dump.py Show resolved Hide resolved
Firo Core automation moved this from Needs review to Reviewer approved Aug 14, 2019
Copy link
Contributor

@riordant riordant left a comment

Choose a reason for hiding this comment

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

LGTM

@riordant riordant merged commit fcfe80a into master Aug 14, 2019
Firo Core automation moved this from Reviewer approved to Done Aug 14, 2019
@riordant riordant deleted the fix-rpc-tests branch August 14, 2019 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Firo Core
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

6 participants