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

Backport Wallet refactoring and external wallet files 11687 and 13017 #3426

Merged
merged 8 commits into from
Apr 26, 2020

Conversation

PastaPastaPasta
Copy link
Member

@PastaPastaPasta PastaPastaPasta commented Apr 17, 2020

Currently based on #3412

Will be rebased once that is merged.

This is a dependency for backporting bitcoin#10740

In the mean time, I need some advise from @UdjinM6 as to how I should fix the autobackup system now that the wallet files are "named" "" if they aren't specified by -wallet
I tried moving the InitAutoBackup call in init.cpp down below wallets are opened, and then running backups via CWallet* and not by name. However, when I did this it resulted in no backups being created. (I also tried a bunch of other methods, but each had their own problems associated)

Thanks

@PastaPastaPasta PastaPastaPasta marked this pull request as ready for review April 18, 2020 16:08
@PastaPastaPasta
Copy link
Member Author

This breaks auto-backup functionality, need some advise regarding that

@PastaPastaPasta PastaPastaPasta changed the title Backport 11687 Backport Wallet refactoring and external wallet files 11687 and 13017 Apr 20, 2020
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

Looks good, just one small issue in docs + pls see 450954d for the autobackup fix (I tested it a bit starting with different/multiple wallets and mixing, seems to be working as expected on my mac but pls test it on more systems if you can).

doc/release-notes.md Outdated Show resolved Hide resolved
doc/release-notes.md Outdated Show resolved Hide resolved
@PastaPastaPasta
Copy link
Member Author

@UdjinM6 starting dashcore ./src/qt/dash-qt --testnet which opens wallet.dat results in a backup being created, however, ./src/qt/dash-qt --testnet --wallet=wallet.dat does not result in a backup

laanwj and others added 7 commits April 23, 2020 14:33
3c058fd wallet: Add HasWallets (João Barbosa)
373aee2 wallet: Add AddWallet, RemoveWallet, GetWallet and GetWallets (João Barbosa)
6efd964 refactor: Drop CWalletRef typedef (João Barbosa)

Pull request description:

  This is a small step towards dynamic wallet load/unload. The wallets *registry* `vpwallets` is used in several places. With these new functions all `vpwallets` usage are removed and `vpwallets` is now a static variable (no external linkage).

  The typedef `CWalletRef` is also removed as it is narrowly used.

Tree-SHA512: 2ea19da2e17b521ad678bfe10f3257e497ccaf7ab9fd0b6647f9d829f1d6131cfa68db8e8492421711c6da399859432b963a568bdd4ca40a77dd95b597839423
Signed-off-by: pasta <pasta@dashboost.org>
be8ab7d Create new wallet databases as directories rather than files (Russell Yanofsky)
26c06f2 Allow wallet files not in -walletdir directory (Russell Yanofsky)
d8a99f6 Allow wallet files in multiple directories (Russell Yanofsky)

Pull request description:

  This change consists of three commits:

  * The first commit is a pure refactoring that removes the restriction that two wallets can only be opened at the same time if they are contained in the same directory.
  * The second commit removes the restriction that `-wallet` filenames can only refer to files in the `-walletdir` directory.
  * The third commit makes second commit a little safer by changing bitcoin to create wallet databases as directories rather than files, so they can be safely backed up.

  All three commits should be straightforward:

  *  The first commit adds around 20 lines of new code and then updates a bunch of function signatures (generally updating them to take plain fs::path parameters, instead of combinations of strings, fs::paths, and objects like CDBEnv and CWalletDBWrapper).
  * The second commit removes two `-wallet` filename checks and adds some test cases to the multiwallet unit test.
  * The third commit just changes the mapping from specified wallet paths to bdb environment & data paths.

    ---

  **Note:** For anybody looking at this PR for the first time, I think you can skip the comments before _20 Nov_ and start reading at bitcoin#11687 (comment). Comments before  _20 Nov_ were about an earlier version of the PR that didn't include the third commit, and then confusion from not seeing the first commit.

Tree-SHA512: 00bbb120fe0df847cf57014f75f1f7f1f58b0b62fa0b3adab4560163ebdfe06ccdfff33b4231693f03c5dc23601cb41954a07bcea9a4919c8d42f7d62bcf6024
Signed-off-by: Pasta <pasta@dashboost.org>
Signed-off-by: Pasta <pasta@dashboost.org>
Co-Authored-By: UdjinM6 <UdjinM6@users.noreply.github.com>
@UdjinM6 UdjinM6 added this to the 16 milestone Apr 24, 2020
@UdjinM6 UdjinM6 added the RPC Some notable changes to RPC params/behaviour/descriptions label Apr 24, 2020
@UdjinM6
Copy link

UdjinM6 commented Apr 24, 2020

Good catch! 3a20581 should fix that.

@PastaPastaPasta
Copy link
Member Author

Cherry-picked

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

ACK

@UdjinM6 UdjinM6 merged commit 7bca963 into dashpay:develop Apr 26, 2020
@PastaPastaPasta PastaPastaPasta deleted the backport-11687 branch April 26, 2020 03:14
UdjinM6 added a commit to UdjinM6/dash that referenced this pull request Jun 29, 2020
UdjinM6 added a commit that referenced this pull request Sep 29, 2020
* Archive v0.15 release notes

* Drop changes in release notes accidentally introduced by #3426

* Draft v0.16 release notes

* Update doc/release-notes.md

Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com>

* Some additions to 0.16 release notes

* Fix typos

* Format/TODO

* Drop `is`

* Block Reward Reallocation, Dynamic Activation Thresholds, PrivateSend coins and fees, PrivateSend Random Round Mixing, GUI, sporks, cmd, commits, contributors

* Apply suggestions from code review

Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>

* Apply few more suggestions

* docs: some release note suggestions

* Update PRs/commits list

Replace "Merged branch .."+commits with the regular "<PR name> (<PR number)" thing

* Apply suggestions from code review

Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>

* Update doc/release-notes.md

Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>

* Update doc/release-notes.md

Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>

* Add recent PRs

* Apply suggestions from code review

Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>

* Format/fix trailing whitespaces

* Fix duplicate whitespaces

* doc: Add details about block reward reallocation

* Update doc/release-notes.md

Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com>

* Apply suggestions from code review - wrap file names in ``

Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com>

* doc: Some adjustments in the GUI part

Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com>
Co-authored-by: Alexander Block <ablock84@gmail.com>
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
Co-authored-by: thephez <thephez@users.noreply.github.com>
Co-authored-by: xdustinface <xdustinfacex@gmail.com>
knst added a commit to knst/dash that referenced this pull request Oct 28, 2024
knst added a commit to knst/dash that referenced this pull request Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RPC Some notable changes to RPC params/behaviour/descriptions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants