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

Fix #588 Wallet list index out of range error#589

Open
AlexCato wants to merge 1 commit into
JoinMarket-Org:developfrom
AlexCato:fix_wallet_index_error
Open

Fix #588 Wallet list index out of range error#589
AlexCato wants to merge 1 commit into
JoinMarket-Org:developfrom
AlexCato:fix_wallet_index_error

Conversation

@AlexCato

@AlexCato AlexCato commented Jul 4, 2016

Copy link
Copy Markdown
Contributor

Uses the number of mixdepths from the wallet file

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.09%) to 79.945% when pulling 141be19 on AlexCato:fix_wallet_index_error into 984626b on JoinMarket-Org:develop.

@AlexCato

AlexCato commented Jul 4, 2016

Copy link
Copy Markdown
Contributor Author

I think I found a problem with this solution:

Given there's a standard wallet with 5 mixdepths, but you start a yieldgenerator with a max_mixdepth-configuration of fewer than 5: then there will be wallet.json-file will be changed to contain the fewer index caches.
This PR then will make it impossible to ever be able to use the original number of mixdepths again.

Proposed solution:
Changing the default value of max_mixdepth in the configuration to -1 (from 5) and only read the number of mixdepths from the wallet file if the user does not specify the max_mixdepth.
Will code that later if noone sees a problem with that.

        auto-detect number of mixdepths if not overridden by user
@AlexCato

AlexCato commented Jul 4, 2016

Copy link
Copy Markdown
Contributor Author

Changed default in wallet-tool.py to -1 now. This will auto-detect the number of mixdepths only if the user does not explicitly configure it.

Therefor the default behavior is improved without any change in behavior if the user intentionally sets the parameter.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.08%) to 79.958% when pulling b7db6d1 on AlexCato:fix_wallet_index_error into 984626b on JoinMarket-Org:develop.

@chris-belcher

Copy link
Copy Markdown
Collaborator

utACK on this. Anyone else have any thoughts?

@AdamISZ

AdamISZ commented Jan 4, 2017

Copy link
Copy Markdown
Member

Agreed. No conceivable negative I can see. Merging. Edit: on second thoughts, need to think a bit :)

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.

4 participants