remove store() from close()

[m2049r]

close should not store. otherwise there is no possibility to close without store.
also, the Wallet destructor calls close() which in turn stores the wallet - this takes a long time with no possibility of error detection/correction. destructors should not be persisting data.
the Wallet::close() closes and then deletes the WalletImpl on success which actually stores the wallet again (!!) wasting even more time.
In case someone is actively relying on this behaviour it would break!
I also think this change has no effect on simplewallet which stores and closes (stops) independently.

[moneromooo-monero]

It has indeed no impact on simplewallet, as it does not use that API.

[Jaqueeee]

This will break the GUI. Should be trivial to fix though. But since there might be other software relying on this behavior maybe better to add a bool store function parameter that defaults to true?

[m2049r]

good idea. implemented & squashed. BUT as we cannot parametrize the desctructor (can we?) it calls close(false) so we can delete a wallet object without force-saving.

[Jaqueeee]

Looks good. I'd prefer this being merged after todays code freeze/release tag since GUI relies on that close&save call in the destructor. @moneromooo-monero @fluffypony

[m2049r]

sorry - had forgotten that in the API the Wallet is closed through the WalletManager - added the store flag to the closeWallet function there too.

[hyc]

Was this rebased? How did readline commits get into this PR?

[m2049r]

i have no idea how that got in there! i synced to the upstream v0.11.0.0, fixed a bug (store=true was missing in the wallet2_api.h) and committed that. tried to squash/rebase and this is where I am now. any idea how to fix this? or I could make a new clean branch with the changes and file a new PR.

[m2049r]

so I rebased on release-v0.11.0.0

[fluffypony]

@m2049r ah - you're meant to rebase off master, not a release branch:) Can you re-rebase?

[moneromooo-monero]

If those updates are meant to have rebased, they did not.

That should do it:

git checkout master
git pull --rebase
git checkout monero
git rebase master

[m2049r]

sorry about all this!

git is killing me - it was getting all kind of conflicts in weird places. i think i got it now. if not i will close this and make a new PR.

[moneromooo-monero]

You probably didn't push your latest attempt.

[m2049r]

git checkout -b x origin/m2049r_nost
git diff master

only shows me my changes. so does git diff upstream/master - what am i missing?

[moneromooo-monero]

The "47" at the top, in the "Commits" page. Click on that, and see the huge amount of commits requested for merging. Maybe your master is shot ? What is its top hash ?

[m2049r]

applied the changes to a new branch from master and forced push! thank you for you patience.

[moneromooo-monero]

That did it, thanks.

[fluffypony]

Reviewed