Skip to content

Conversation

ryanofsky
Copy link
Contributor

@ryanofsky ryanofsky commented Nov 14, 2017

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 #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.

@luke-jr
Copy link
Member

luke-jr commented Nov 14, 2017

This is too dangerous. It's not clear at face value to users that the database/ directory will be created or must be maintained with the wallet file(s).

@gmaxwell
Copy link
Contributor

FWIW, if you separate a wallet from its database directory you reliably get a wallet that won't open. To see this for yourself, copy a wallet.dat from a running wallet to another directory and try starting another copy of bitcoin against it there.

@meshcollider
Copy link
Contributor

meshcollider commented Nov 14, 2017

Agree with @luke-jr, #11466 is safer until we decide on a better wallet directory structure (e.g. #11466 (comment)), which would involve supporting multiple BDB environments first.

@meshcollider
Copy link
Contributor

Note that you might want to cherrypick c36cb54 to help debug the travis failure

@laanwj
Copy link
Member

laanwj commented Nov 16, 2017

FWIW, if you separate a wallet from its database directory you reliably get a wallet that won't open.

Only if it has uncompacted log files. To prevent this, bitcoin spends so much time consolidating the wallet database at runtime (in the MaybeCompactWalletDB thread), and does it at least at a clean shutdown.

People are moving around wallet.dat already, from and to the data directory, so I'm not convinced that supporting multiple wallet directories in principle makes the risk much larger that wallets are separated from their database directory while "live".

But indeed, a database environment has to be created where the wallets are. Creating wallets in different places or even on different file systems but having the database environment in the main data directory is asking for trouble.

@@ -48,7 +48,7 @@ std::string HelpMessageCli()
strUsage += HelpMessageOpt("-rpcclienttimeout=<n>", strprintf(_("Timeout in seconds during HTTP requests, or 0 for no timeout. (default: %d)"), DEFAULT_HTTP_CLIENT_TIMEOUT));
strUsage += HelpMessageOpt("-stdinrpcpass", strprintf(_("Read RPC password from standard input as a single line. When combined with -stdin, the first line from standard input is used for the RPC password.")));
strUsage += HelpMessageOpt("-stdin", _("Read extra arguments from standard input, one per line until EOF/Ctrl-D (recommended for sensitive information such as passphrases). When combined with -stdinrpcpass, the first line from standard input is used for the RPC password."));
strUsage += HelpMessageOpt("-rpcwallet=<walletname>", _("Send RPC for non-default wallet on RPC server (argument is wallet filename in bitcoind directory, required if bitcoind/-Qt runs with multiple wallets)"));
strUsage += HelpMessageOpt("-rpcwallet=<walletname>", _("Send RPC for non-default wallet on RPC server (needs to exactly match corresponding -wallet option passed to bitcoind)"));
Copy link
Contributor

Choose a reason for hiding this comment

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

Full path? How will the url look like?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Full path? How will the url look like?

It doesn't have to be the full path, but it can be. Paths are urlencoded, and symlinks are also allowed. See the unit test which tests every imaginable type of path.

Copy link
Contributor

Choose a reason for hiding this comment

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

Feels weird to known server side details. Not to mention it can break the client if you move the wallet to a new place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Feels weird to known server side details. Not to mention it can break the client if you move the wallet to a new place.

The PR is not changing anything about this. Whatever string the server uses to identify the wallet, the client just has to use the same string. We've discussed adding and storing wallet identifiers in the past, but opted to just use filenames for simplicity. Server operator is free to choose a path, or alias or whatever he wants as the string used to identify the wallet.

Copy link
Contributor Author

@ryanofsky ryanofsky Nov 17, 2017

Choose a reason for hiding this comment

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

See discussion in #10650 "The walletID is currently defined by the filename (-wallet=), ideally, we later add a wallet RPC call to set the walletID (should be written to the wallet database)." It would still be possible to do this if you are interested, but it's tangential to this PR.

There is more discussion starting at "The walletID should in future not be a representation of the filename. User should be able to manually choose the walletID. Ideally, we write the walletID into the wallet database. There we could only allow alphanumeric chars." #10650 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

listwallets will expose the full path right?

No, or at least not by default. Listwallets just shows the wallet name. If the server is configured to use full paths for wallet name these will be full paths, but the server has to be specifically configured this way in order for this to happen.

I'm still little puzzled about what you think the underlying problem is with allowing server admins to use full paths as wallet names. But if you really think it is a problem it would literally be a one-line code change to use a different identifier. E.g. the name argument to CreateWalletFromFile() could be changed from walletFile to fs::path(walletFile).filename().

Copy link
Contributor

Choose a reason for hiding this comment

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

Suppose you have configured the server with an absolute wallet path. Then you have a couple of clients (in different hosts). You had to configure each client with the absolute path of the wallet. These clients can be others than bitcoin-cli. Now the server operator decides it has to move the wallet to a new location. The clients can handle a server restart, but now the wallet doesn't exists. And the operator simply doesn't know of these clients because they are managed by other operators etc..

Copy link
Contributor

Choose a reason for hiding this comment

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

My point is to not even allow such cases. An wallet id (filename or even relative path) is enough

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suppose you have configured the server with an absolute wallet path. Then you have a couple of clients (in different hosts). You had to configure each client with the absolute path of the wallet. These clients can be others than bitcoin-cli. Now the server operator decides it has to move the wallet to a new location.

I don't see how this is much different than the server renaming the wallet. If a server admin wants to create a stable name for a wallet, it's perfectly possible to do that by creating a symlink or just manipulating the -datadir or -walletdir (#11687) settings to be able store the wallets in different locations without changing the name. I believe this PR actually gives server admins more flexibility in this regard rather than less, because previously wallet symlinks were disallowed.

But to take the example at face value, you have a server admin who accidentally exposed a non-stable wallet name, instead of a stable wallet name over the rpc interface to clients on the internet outside of his control. What solutions would you like to see for this problem? Would it be good enough to use fs::path(walletFile).filename() instead of walletFile as the wallet name passed to CreateWalletFromFile like I suggested earlier? Do you want wallet names to be configurable on the command line? Do you want them to be configurable by RPC? Do you want wallet names to be stored inside wallet databases like Jonas suggested?

Copy link
Contributor

Choose a reason for hiding this comment

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

accidentally exposed a non-stable wallet

My point is to prevent this. IMO symlinks are a weak solution to remedy that case. Renaming is indeed a breaking change from the client point of view.

I'm not a server admin expert (not a bit either) but, correct me if I'm wrong, usually absolute paths aren't exposed outside.

@ryanofsky
Copy link
Contributor Author

This is too dangerous. It's not clear at face value to users that the database/ directory will be created or must be maintained with the wallet file(s).

Note that this was posted in response to an earlier version of this pr that did not include the third commit. With the third commit, the default for creating new wallets is as directories rather than a files (with each wallet directory containing its own wallet.dat file, db.log and database/log files). See the comments and release notes in the third commit for details.

But indeed, a database environment has to be created where the wallets are. Creating wallets in different places or even on different file systems but having the database environment in the main data directory is asking for trouble.

The PR doesn't do this (and never did). The first commit of the PR is a refactoring specifically to add support for opening multiple BDB environments.

Agree with @luke-jr, #11466 is safer until we decide on a better wallet directory structure (#11466 (comment)), which would involve supporting multiple BDB environments first.

Again, the very first commit of this PR adds support for multiple BDB environments, and if you combine this PR with #11466 the result is exactly what you described in your linked comment.

@meshcollider
Copy link
Contributor

Note that this was posted in response to an earlier version of this pr that did not include the third commit. With the third commit, the default for creating new wallets is as directories rather than a files (with each wallet directory containing its own wallet.dat file, db.log and database/log files). See the comments and release notes in the third commit for details.

Indeed, concept ACK on the new approach, will review soon

laanwj added a commit that referenced this pull request Nov 18, 2017
c1e5d40 Make debugging test crash easier (MeshCollider)
8263f6a Create walletdir if datadir doesn't exist and fix tests (MeshCollider)
9587a9c Default walletdir is wallets/ if it exists (MeshCollider)
d987889 Add release notes for -walletdir and wallets/ dir (MeshCollider)
80c5cbc Add test for -walletdir (MeshCollider)
0530ba0 Add -walletdir parameter to specify custom wallet dir (MeshCollider)

Pull request description:

  Closes #11348

  Adds a `-walletdir` parameter which specifies a directory to use for wallets, allowing them to be stored separately from the 'main' data directory. Creates a new `wallets/` directory in datadir if this is the first time running, and defaults to using it if it exists.

  Includes tests and release notes. Things which might need to be considered more:
  - there is no 'lock' on the wallets directory, which might be needed?
  - because this uses a new wallets/ directory by default, downgrading to an earlier version won't see the wallets in that directory (not a big deal though, users can just copy them up to the main dir)
  - jnewbery suggested putting each wallet in its own directory, which is a good idea, but out of scope for this PR IMO. EDIT: this is being done in #11687
  - doc/files.md needs updating (will do soon)

  I also considered including  a cleanup by removing caching of data directory paths and instead just initialise them once on startup (c.f. #3073), but decided it wasn't super relevant here will just complicate review.

Tree-SHA512: c8ac04bfe9a810c32055f2c8b8fa0d535e56125ceb8d96f12447dd3538bf3e5ee992b60b1cd2173bf5f3fa023a9feab12c9963593bf27ed419df929bb413398d
@luke-jr
Copy link
Member

luke-jr commented Nov 18, 2017

IMO we should still forbid paths for file-based wallets, and only allow it for directory-based ones.

Suggest renaming wallet.dat in the directory to wallet.bdb to convey the fact that you can't just copy the file to make a proper backup.

@sipa
Copy link
Member

sipa commented Nov 18, 2017

Concept ACK

@laanwj
Copy link
Member

laanwj commented Nov 20, 2017

The PR doesn't do this (and never did). The first commit of the PR is a refactoring specifically to add support for opening multiple BDB environments.

Sorry! I didn't read the code but was apparently confused by all the screaming about dangerousness. If you added support for multiple database environments you get my concept ACK.

Suggest renaming wallet.dat in the directory to wallet.bdb to convey the fact that you can't just copy the file to make a proper backup.

I don't think we should be changing the default naming of wallet databases. People have been backing up separate wallet.dats forever, and generally it works because bitcoin core leaves the wallet in consolidated state. So I'd prefer documenting that behavior instead.

Even a backup with the last update stripped is better than no backup. From that it's usually possible to recover due to the mempool or HD seed. But if they can't find 'wallet.dat', they might decide not to to a backup at all.

Turning the abstraction in the user's mind of wallets into directories is going to be an upstream fight as well. I'm not against it, but I think it will result in a lot of confusion.

@ryanofsky
Copy link
Contributor Author

IMO we should still forbid paths for file-based wallets, and only allow it for directory-based ones.

Good idea, added check for this.

src/wallet/db.h Outdated
@@ -85,7 +86,8 @@ class CDBEnv
}
};

extern CDBEnv bitdb;
/** Get CDBEnv and database filename given a wallet path. */
void GetWalletEnv(const fs::path& wallet_path, CDBEnv*& env, std::string& database_filename);
Copy link
Contributor

Choose a reason for hiding this comment

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

Ugh, can we return the CDBEnv* instead of return-by-setting-ref-to-ptr?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ugh. Good idea! Done in 75ed8c5

src/wallet/db.h Outdated
{
GetWalletEnv(wallet_path, env, strFile);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why create the env before checking for mock? Why not add a fMock parameter to GetWalletEnv or so instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't see how that would be an improvement. The env object needs to exist whether it is memory or file-backed. And GetWalletEnv is just intended to create and retrieve CDBEnv objects, not do that and then invoke unrelated CDBEnv methods that only one of its callers needs depending on a bool parameter...

Copy link
Contributor

Choose a reason for hiding this comment

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

It just seems very weird given mock seems like it shouldn't have a/care what the wallet_path is. In the future I kinda feel like it should become more RAII (ie Open in the GetWalletEnv call) but not in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It just seems very weird given mock seems like it shouldn't have a/care what the wallet_path is. In the future I kinda feel like it should become more RAII (ie Open in the GetWalletEnv call) but not in this PR.

RAII is not the model this uses because when there more than one wallet file open in the same directory they share the same environment. You could only resolve this by requiring all databases in the environment to be opened and closed at the same time (which would complicate dynamic loading/unloading) or by not allowing multiple databases in the same directory to be opened at the same time, or some combination these things, or just not using RAII.

On using fake "" path for mock databases, this is a little weird. Could be made less exposed by adding a separate constructor for mocks like proposed in the other comment: #11687 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but instead of env = GetWalletEnv(wallet_path, strFile); env->Open(); or env->Close(); env->MakeMock();, GetWalletEnv could return a reference which is already open or otherwise has some level of understanding of closing when all refs to that env are lost. Even if not that, would prefer to not use the fake "" path as you suggest.

src/wallet/db.h Outdated
@@ -35,17 +35,18 @@ class CDBEnv
void EnvShutdown();

public:
mutable CCriticalSection cs_db;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why make the lock global instead of per-env? It seems like there should be nothing preventing you from keeping that and adding a second lock that only exists to protect g_dbenvs, though likely not a big deal in terms of concurrency.

Copy link
Contributor Author

@ryanofsky ryanofsky Dec 6, 2017

Choose a reason for hiding this comment

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

One lock seems a simpler than multiple multiple locks unless there is a performance rationale for more fine grained locking. One thing which I would not look forward to is dealing with even more lock-order bugs in the wallet than the ones that already frequently arise.

Copy link
Contributor

Choose a reason for hiding this comment

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

It just seems weird given it'd be even less code change to not split the lock out, and as long as the global lock is only held in the creation of new envs, no lockorder should be worried about (cause lockorder is always trivially correct if locks are held only for the duration of a function which uses them, without making callbacks).

}

void CDBEnv::Close()
{
for (auto& db : mapDb) {
Copy link
Contributor

Choose a reason for hiding this comment

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

While you're at it, can you just merge EnvShutdown and Close? EnvShutdown is now only called from Close().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 75ed8c5

@@ -435,7 +467,9 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb
if (ret != 0) {
throw std::runtime_error(strprintf("CDB: Error %d, can't open database %s", ret, strFilename));
}
CheckUniqueFileid(*env, strFilename, *pdb_temp);
for (auto& env : g_dbenvs) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this required? Somehow I had assumed they only conficted if they were both in the same env, but I dunno where I got that from.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added comment in 75ed8c5. It's done to prevent opening two paths that refer to the same file at the same time.

Copy link
Contributor

Choose a reason for hiding this comment

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

Did you forget to push? Noting this is the only way we handle hardlinks is useful, indeed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did you forget to push?

Oops, yeah. It was on the pr/wfile.9 tag I linked to below. Should be updated now. Also I'll work on the "just leave this part of the diff" change you requested above.

// needed to restore wallet transaction meta data after -zapwallettxes
std::vector<CWalletTx> vWtx;

if (gArgs.GetBoolArg("-zapwallettxes", false)) {
uiInterface.InitMessage(_("Zapping all transactions from wallet..."));

std::unique_ptr<CWalletDBWrapper> dbw(new CWalletDBWrapper(&bitdb, walletFile));
std::unique_ptr<CWallet> tempWallet = MakeUnique<CWallet>(std::move(dbw));
std::unique_ptr<CWallet> tempWallet = MakeUnique<CWallet>(name, path);
Copy link
Contributor

Choose a reason for hiding this comment

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

Heh, I have to admit I really prefer the old version from the new one. Creating a DB and then giving it to the wallet makes more sense conceptually to me than telling the wallet where to find the DB and then having the CWallet constructor create the DB (especially as the slow-moving goal is to abstract the DB out of the wallet and make it an interface).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting, I guess you are imagining CWalletDBWrapper changing into some abstract interface that the wallet can call to load and store data. If that's the goal I can make the CWallet(name, dbw) constructor public instead of private and get rid of the other constructors. For callers, this means the current:

MakeUnique<CWallet>(name, path)
MakeUnique<CWallet>(CWallet::Mock())
MakeUnique<CWallet>(CWallet::Dummy()))

would become something like:

MakeUnique<CWallet>(name, MakeUnique<CWalletDBWrapper>(path))
MakeUnique<CWallet>("mock", MakeUnique<CWalletDBWrapper>(CWalletDBWrapper::Mock()))
MakeUnique<CWallet>("dummy", MakeUnique<CWalletDBWrapper>(CWalletDBWrapper::Dummy()))

I would probably want to hold off on a change like this until CWalletDBWrapper did become a real abstraction. But let me know if you want this change or something along these lines. Obviously this is more verbose than code in the current pr, but it's not much more verbose than code currently in master.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea, I really like that, tbh. If nothing else is much more clearly separates wallet.cpp from walletdb, etc, keeping wallet.cpp/CWallet only filled with wallet logic, and not database-backend stuff.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If nothing else is much more clearly separates wallet.cpp from walletdb, etc, keeping wallet.cpp/CWallet only filled with wallet logic

It really affects callers of wallet, not wallet itself. CWallet is barely affected and wallet.cpp doesn't change at all. Anyway, let me know if you would prefer to see this here or in another PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean given you're changing it here, I see no reason to change it to go in the opposite direction of where I think it should go - even if CWalletDBWrapper right now is just a BDB implementation, it still has a clean-ish interface, and having the CWallet cosntructor create one from a path instead of take one just seems like the wrong direction. Better to just leave this part of the diff out of the PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 39fa1e9

Copy link
Contributor Author

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

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

Added 2 commits 2cf4dc8 -> ec01271 (pr/wfile.7 -> pr/wfile.8, compare)
Squashed ec01271 -> c25ed1f (pr/wfile.8 -> pr/wfile.9)

}

void CDBEnv::Close()
{
for (auto& db : mapDb) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 75ed8c5

@@ -435,7 +467,9 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb
if (ret != 0) {
throw std::runtime_error(strprintf("CDB: Error %d, can't open database %s", ret, strFilename));
}
CheckUniqueFileid(*env, strFilename, *pdb_temp);
for (auto& env : g_dbenvs) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added comment in 75ed8c5. It's done to prevent opening two paths that refer to the same file at the same time.

src/wallet/db.h Outdated
@@ -35,17 +35,18 @@ class CDBEnv
void EnvShutdown();

public:
mutable CCriticalSection cs_db;
Copy link
Contributor Author

@ryanofsky ryanofsky Dec 6, 2017

Choose a reason for hiding this comment

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

One lock seems a simpler than multiple multiple locks unless there is a performance rationale for more fine grained locking. One thing which I would not look forward to is dealing with even more lock-order bugs in the wallet than the ones that already frequently arise.

src/wallet/db.h Outdated
@@ -85,7 +86,8 @@ class CDBEnv
}
};

extern CDBEnv bitdb;
/** Get CDBEnv and database filename given a wallet path. */
void GetWalletEnv(const fs::path& wallet_path, CDBEnv*& env, std::string& database_filename);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ugh. Good idea! Done in 75ed8c5

src/wallet/db.h Outdated
{
GetWalletEnv(wallet_path, env, strFile);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't see how that would be an improvement. The env object needs to exist whether it is memory or file-backed. And GetWalletEnv is just intended to create and retrieve CDBEnv objects, not do that and then invoke unrelated CDBEnv methods that only one of its callers needs depending on a bool parameter...

// needed to restore wallet transaction meta data after -zapwallettxes
std::vector<CWalletTx> vWtx;

if (gArgs.GetBoolArg("-zapwallettxes", false)) {
uiInterface.InitMessage(_("Zapping all transactions from wallet..."));

std::unique_ptr<CWalletDBWrapper> dbw(new CWalletDBWrapper(&bitdb, walletFile));
std::unique_ptr<CWallet> tempWallet = MakeUnique<CWallet>(std::move(dbw));
std::unique_ptr<CWallet> tempWallet = MakeUnique<CWallet>(name, path);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting, I guess you are imagining CWalletDBWrapper changing into some abstract interface that the wallet can call to load and store data. If that's the goal I can make the CWallet(name, dbw) constructor public instead of private and get rid of the other constructors. For callers, this means the current:

MakeUnique<CWallet>(name, path)
MakeUnique<CWallet>(CWallet::Mock())
MakeUnique<CWallet>(CWallet::Dummy()))

would become something like:

MakeUnique<CWallet>(name, MakeUnique<CWalletDBWrapper>(path))
MakeUnique<CWallet>("mock", MakeUnique<CWalletDBWrapper>(CWalletDBWrapper::Mock()))
MakeUnique<CWallet>("dummy", MakeUnique<CWalletDBWrapper>(CWalletDBWrapper::Dummy()))

I would probably want to hold off on a change like this until CWalletDBWrapper did become a real abstraction. But let me know if you want this change or something along these lines. Obviously this is more verbose than code in the current pr, but it's not much more verbose than code currently in master.

Copy link
Contributor Author

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

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

Added 2 commits c25ed1f -> b9440ab (pr/wfile.9 -> pr/wfile.10, compare)
Squashed b9440ab -> 9993492 (pr/wfile.10 -> pr/wfile.11)

// needed to restore wallet transaction meta data after -zapwallettxes
std::vector<CWalletTx> vWtx;

if (gArgs.GetBoolArg("-zapwallettxes", false)) {
uiInterface.InitMessage(_("Zapping all transactions from wallet..."));

std::unique_ptr<CWalletDBWrapper> dbw(new CWalletDBWrapper(&bitdb, walletFile));
std::unique_ptr<CWallet> tempWallet = MakeUnique<CWallet>(std::move(dbw));
std::unique_ptr<CWallet> tempWallet = MakeUnique<CWallet>(name, path);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 39fa1e9

PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 15, 2020
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 15, 2020
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 15, 2020
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 15, 2020
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 15, 2020
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 16, 2020
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 16, 2020
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 17, 2020
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 18, 2020
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 18, 2020
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
UdjinM6 pushed a commit to UdjinM6/dash that referenced this pull request Apr 23, 2020
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 23, 2020
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 23, 2020
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
meshcollider added a commit that referenced this pull request Oct 15, 2020
…d use it for new descriptor wallets

c4a29d0 Update wallet_multiwallet.py for descriptor and sqlite wallets (Russell Yanofsky)
310b0fd Run dumpwallet for legacy wallets only in  wallet_backup.py (Andrew Chow)
6c6639a Include sqlite3 in documentation (Andrew Chow)
f023b7c wallet: Enforce sqlite serialized threading mode (Andrew Chow)
6173269 Set and check the sqlite user version (Andrew Chow)
9d3d2d2 Use network magic as sqlite wallet application ID (Andrew Chow)
9af5de3 Use SQLite for descriptor wallets (Andrew Chow)
9b78f3c walletutil: Wallets can also be sqlite (Andrew Chow)
ac38a87 Determine wallet file type based on file magic (Andrew Chow)
6045f77 Implement SQLiteDatabase::MakeBatch (Andrew Chow)
727e6b2 Implement SQLiteDatabase::Verify (Andrew Chow)
b4df8fd Implement SQLiteDatabase::Rewrite (Andrew Chow)
010e365 Implement SQLiteDatabase::TxnBegin, TxnCommit, and TxnAbort (Andrew Chow)
ac5c161 Implement SQLiteDatabase::Backup (Andrew Chow)
f6f9cd6 Implement SQLiteBatch::StartCursor, ReadAtCursor, and CloseCursor (Andrew Chow)
bf90e03 Implement SQLiteBatch::ReadKey, WriteKey, EraseKey, and HasKey (Andrew Chow)
7aa4562 Add SetupSQLStatements (Andrew Chow)
6636a26 Implement SQLiteBatch::Close (Andrew Chow)
9382535 Implement SQLiteDatabase::Close (Andrew Chow)
a0de833 Implement SQLiteDatabase::Open (Andrew Chow)
3bfa0fe Initialize and Shutdown sqlite3 globals (Andrew Chow)
5a488b3 Constructors, destructors, and relevant private fields for SQLiteDatabase/Batch (Andrew Chow)
ca8b7e0 Implement SQLiteDatabaseVersion (Andrew Chow)
7577b6e Add SQLiteDatabase and SQLiteBatch dummy classes (Andrew Chow)
e87df82 Add sqlite to travis and depends (Andrew Chow)
54729f3 Add libsqlite3 (Andrew Chow)

Pull request description:

  This PR adds a new class `SQLiteDatabase` which is a subclass of `WalletDatabase`. This provides access to a SQLite database that is used to store the wallet records. To keep compatibility with BDB and to complexity of the change down, we don't make use of many SQLite's features. We use it strictly as a key-value store. We create a table `main` which has two columns, `key` and `value` both with the type `blob`.

  For new descriptor wallets, we will create a `SQLiteDatabase` instead of a `BerkeleyDatabase`. There is no requirement that all SQLite wallets are descriptor wallets, nor is there a requirement that all descriptor wallets be SQLite wallets. This allows for existing descriptor wallets to work as well as keeping open the option to migrate existing wallets to SQLite.

  We keep the name `wallet.dat` for SQLite wallets. We are able to determine which database type to use by searching for specific magic bytes in the `wallet.dat` file. SQLite begins it's files with a null terminated string `SQLite format 3`. BDB has `0x00053162` at byte 12 (note that the byte order of this integer depends on the system endianness). So when we see that there is a `wallet.dat` file that we want to open, we check for the magic bytes to determine which database system to use.

  I decided to keep the `wallet.dat` naming to keep things like backup script to continue to function as they won't need to be modified to look for a different file name. It also simplifies a couple of things in the implementation and the tests as `wallet.dat` is something that is specifically being looked for. If we don't want this behavior, then I do have another branch which creates `wallet.sqlite` files instead, but I find that this direction is easier.

ACKs for top commit:
  Sjors:
    re-utACK c4a29d0
  promag:
    Tested ACK c4a29d0.
  fjahr:
    reACK c4a29d0
  S3RK:
    Re-review ACK c4a29d0
  meshcollider:
    re-utACK c4a29d0
  hebasto:
    re-ACK c4a29d0, only rebased since my [previous](#19077 (review)) review, verified with `git range-diff master d18892d c4a29d0`.
  ryanofsky:
    Code review ACK c4a29d0. I am honestly confused about reasons for locking into `wallet.dat` again when it's so easy now to use a clean format. I assume I'm just very dense, or there's some unstated reason, because the only thing that's been brought up are unrealistic compatibility scenarios (all require actively creating a wallet with non-default descriptor+sqlite option, then trying to using the descriptor+sqlite wallets with old software or scripts and ignoring the results) that we didn't pay attention to with previous PRs like #11687, which did not require any active interfaction.
  jonatack:
    ACK c4a29d0, debug builds and test runs after rebase to latest master @ c2c4dba, some manual testing creating, using, unloading and reloading a few different new sqlite descriptor wallets over several node restarts/shutdowns.

Tree-SHA512: 19145732e5001484947352d3175a660b5102bc6e833f227a55bd41b9b2f4d92737bbed7cead64b75b509decf9e1408cd81c185ab1fb4b90561aee427c4f9751c
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Oct 16, 2020
…base and use it for new descriptor wallets

c4a29d0 Update wallet_multiwallet.py for descriptor and sqlite wallets (Russell Yanofsky)
310b0fd Run dumpwallet for legacy wallets only in  wallet_backup.py (Andrew Chow)
6c6639a Include sqlite3 in documentation (Andrew Chow)
f023b7c wallet: Enforce sqlite serialized threading mode (Andrew Chow)
6173269 Set and check the sqlite user version (Andrew Chow)
9d3d2d2 Use network magic as sqlite wallet application ID (Andrew Chow)
9af5de3 Use SQLite for descriptor wallets (Andrew Chow)
9b78f3c walletutil: Wallets can also be sqlite (Andrew Chow)
ac38a87 Determine wallet file type based on file magic (Andrew Chow)
6045f77 Implement SQLiteDatabase::MakeBatch (Andrew Chow)
727e6b2 Implement SQLiteDatabase::Verify (Andrew Chow)
b4df8fd Implement SQLiteDatabase::Rewrite (Andrew Chow)
010e365 Implement SQLiteDatabase::TxnBegin, TxnCommit, and TxnAbort (Andrew Chow)
ac5c161 Implement SQLiteDatabase::Backup (Andrew Chow)
f6f9cd6 Implement SQLiteBatch::StartCursor, ReadAtCursor, and CloseCursor (Andrew Chow)
bf90e03 Implement SQLiteBatch::ReadKey, WriteKey, EraseKey, and HasKey (Andrew Chow)
7aa4562 Add SetupSQLStatements (Andrew Chow)
6636a26 Implement SQLiteBatch::Close (Andrew Chow)
9382535 Implement SQLiteDatabase::Close (Andrew Chow)
a0de833 Implement SQLiteDatabase::Open (Andrew Chow)
3bfa0fe Initialize and Shutdown sqlite3 globals (Andrew Chow)
5a488b3 Constructors, destructors, and relevant private fields for SQLiteDatabase/Batch (Andrew Chow)
ca8b7e0 Implement SQLiteDatabaseVersion (Andrew Chow)
7577b6e Add SQLiteDatabase and SQLiteBatch dummy classes (Andrew Chow)
e87df82 Add sqlite to travis and depends (Andrew Chow)
54729f3 Add libsqlite3 (Andrew Chow)

Pull request description:

  This PR adds a new class `SQLiteDatabase` which is a subclass of `WalletDatabase`. This provides access to a SQLite database that is used to store the wallet records. To keep compatibility with BDB and to complexity of the change down, we don't make use of many SQLite's features. We use it strictly as a key-value store. We create a table `main` which has two columns, `key` and `value` both with the type `blob`.

  For new descriptor wallets, we will create a `SQLiteDatabase` instead of a `BerkeleyDatabase`. There is no requirement that all SQLite wallets are descriptor wallets, nor is there a requirement that all descriptor wallets be SQLite wallets. This allows for existing descriptor wallets to work as well as keeping open the option to migrate existing wallets to SQLite.

  We keep the name `wallet.dat` for SQLite wallets. We are able to determine which database type to use by searching for specific magic bytes in the `wallet.dat` file. SQLite begins it's files with a null terminated string `SQLite format 3`. BDB has `0x00053162` at byte 12 (note that the byte order of this integer depends on the system endianness). So when we see that there is a `wallet.dat` file that we want to open, we check for the magic bytes to determine which database system to use.

  I decided to keep the `wallet.dat` naming to keep things like backup script to continue to function as they won't need to be modified to look for a different file name. It also simplifies a couple of things in the implementation and the tests as `wallet.dat` is something that is specifically being looked for. If we don't want this behavior, then I do have another branch which creates `wallet.sqlite` files instead, but I find that this direction is easier.

ACKs for top commit:
  Sjors:
    re-utACK c4a29d0
  promag:
    Tested ACK c4a29d0.
  fjahr:
    reACK c4a29d0
  S3RK:
    Re-review ACK c4a29d0
  meshcollider:
    re-utACK c4a29d0
  hebasto:
    re-ACK c4a29d0, only rebased since my [previous](bitcoin#19077 (review)) review, verified with `git range-diff master d18892d c4a29d0`.
  ryanofsky:
    Code review ACK c4a29d0. I am honestly confused about reasons for locking into `wallet.dat` again when it's so easy now to use a clean format. I assume I'm just very dense, or there's some unstated reason, because the only thing that's been brought up are unrealistic compatibility scenarios (all require actively creating a wallet with non-default descriptor+sqlite option, then trying to using the descriptor+sqlite wallets with old software or scripts and ignoring the results) that we didn't pay attention to with previous PRs like bitcoin#11687, which did not require any active interfaction.
  jonatack:
    ACK c4a29d0, debug builds and test runs after rebase to latest master @ c2c4dba, some manual testing creating, using, unloading and reloading a few different new sqlite descriptor wallets over several node restarts/shutdowns.

Tree-SHA512: 19145732e5001484947352d3175a660b5102bc6e833f227a55bd41b9b2f4d92737bbed7cead64b75b509decf9e1408cd81c185ab1fb4b90561aee427c4f9751c
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Dec 16, 2020
13c3a65 Qt/Bugfix: fix handling default wallet with no name (João Barbosa)

Pull request description:

  If one loads a wallet via RPC (`loadwallet w2`), then select w2, select back to the default wallet (which is an empty string), that default wallet cannot be access through the RPC console because the current code only points to the wallet endpoint if the wallet name is not empty.

  This is a quick fix that reenables accessing the default wallet in case an additional wallet has been loaded.

  Using "" for the default wallet may not be ideal in other cases and it may make more sense to change it at a deeper level (wallet.cpp). See discussion here which where the reasons for the current behaviour in master:
  bitcoin#11687 (comment)

  @jnewbery @promag @ryanofsky

Tree-SHA512: 74b935886b4e4a6033a2f5e1f44bb69a252e31f4021e19a2054445a8e3e4db1d8ee256290850a84d8569d2d0e21412fce0170e7f0e881259156057587181ee05
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Dec 18, 2020
13c3a65 Qt/Bugfix: fix handling default wallet with no name (João Barbosa)

Pull request description:

  If one loads a wallet via RPC (`loadwallet w2`), then select w2, select back to the default wallet (which is an empty string), that default wallet cannot be access through the RPC console because the current code only points to the wallet endpoint if the wallet name is not empty.

  This is a quick fix that reenables accessing the default wallet in case an additional wallet has been loaded.

  Using "" for the default wallet may not be ideal in other cases and it may make more sense to change it at a deeper level (wallet.cpp). See discussion here which where the reasons for the current behaviour in master:
  bitcoin#11687 (comment)

  @jnewbery @promag @ryanofsky

Tree-SHA512: 74b935886b4e4a6033a2f5e1f44bb69a252e31f4021e19a2054445a8e3e4db1d8ee256290850a84d8569d2d0e21412fce0170e7f0e881259156057587181ee05
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Jun 30, 2021
…param

c1e5d40 Make debugging test crash easier (MeshCollider)
8263f6a Create walletdir if datadir doesn't exist and fix tests (MeshCollider)
9587a9c Default walletdir is wallets/ if it exists (MeshCollider)
d987889 Add release notes for -walletdir and wallets/ dir (MeshCollider)
80c5cbc Add test for -walletdir (MeshCollider)
0530ba0 Add -walletdir parameter to specify custom wallet dir (MeshCollider)

Pull request description:

  Closes bitcoin#11348

  Adds a `-walletdir` parameter which specifies a directory to use for wallets, allowing them to be stored separately from the 'main' data directory. Creates a new `wallets/` directory in datadir if this is the first time running, and defaults to using it if it exists.

  Includes tests and release notes. Things which might need to be considered more:
  - there is no 'lock' on the wallets directory, which might be needed?
  - because this uses a new wallets/ directory by default, downgrading to an earlier version won't see the wallets in that directory (not a big deal though, users can just copy them up to the main dir)
  - jnewbery suggested putting each wallet in its own directory, which is a good idea, but out of scope for this PR IMO. EDIT: this is being done in bitcoin#11687
  - doc/files.md needs updating (will do soon)

  I also considered including  a cleanup by removing caching of data directory paths and instead just initialise them once on startup (c.f. dashpay#3073), but decided it wasn't super relevant here will just complicate review.

Tree-SHA512: c8ac04bfe9a810c32055f2c8b8fa0d535e56125ceb8d96f12447dd3538bf3e5ee992b60b1cd2173bf5f3fa023a9feab12c9963593bf27ed419df929bb413398d
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Jun 30, 2021
a14dbff Allow multiwallet.py to be used with --usecli (Russell Yanofsky)
f6ade9c [tests] allow tests to be run with --usecli (John Newbery)
ff9a363 TestNodeCLI batch emulation (Russell Yanofsky)
ca9085a Prevent TestNodeCLI.args mixups (Russell Yanofsky)
fcfb952 Improve TestNodeCLI output parsing (Russell Yanofsky)

Pull request description:

  Lack of test coverage was pointed out by @jnewbery in bitcoin#11687 (comment)

Tree-SHA512: 5f10e31abad11a5edab0da4e2515e39547adb6ab9e55e50427ab2eb7ec9a43d6b896b579b15863e5edc9beee7d8bf1c84d9dabd247be0760a1b9ae39e1e8ee02
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Jun 30, 2021
2f3bd47 Abstract directory locking into util.cpp (MeshCollider)
5260a4a Make .walletlock distinct from .lock (MeshCollider)
64226de Generalise walletdir lock error message for correctness (MeshCollider)
c9ed4bd Add a test for wallet directory locking (MeshCollider)
e60cb99 Add a lock to the wallet directory (MeshCollider)

Pull request description:

  Fixes bitcoin#11888, needs a 0.16 milestone

  Also adds a test that the lock works.

  bitcoin#11687 will probably rework this to a per-wallet lock instead of just the walletdir, but this fixes the current issue

Tree-SHA512: 98e52d67f820e3b8f919cf361ffbb7d928f1bd67603e0ed26c5076ea02d9b3a90c3535ddf7329f3b88171396fa28dd3c87adab3577a8a217bd1e4247bda99138
furszy added a commit to PIVX-Project/PIVX that referenced this pull request Jul 23, 2021
…xternal wallet files capabilities

056f4e8 [GUI] settings information widget setting the correct data directory. (furszy)
f765611 bugfix: Remove dangling wallet env instance (João Barbosa)
524103f Implement and connect CWallet::GetPathToFile to GUI. (furszy)
e7aa6bd [Refactor] First load all wallets, then back em (random-zebra)
91b112b [Refactor][Bug] Fix automatic backups, final code deduplication (random-zebra)
12a1e39 [BUG] Sanitize wallet name in GetUniqueWalletBackupName (random-zebra)
7aa251d wallet: Fix backupwallet for multiwallets (Daniel Kraft)
351d2c8 wallet_tests: mock wallet db. (furszy)
565abcd db: fix db path not removed from the open db environments map. (furszy)
4cae8dc test: Add unit test for LockDirectory  Add a unit test for LockDirectory, introduced in btc#11281. (W. J. van der Laan)
16b4651 util: Fix multiple use of LockDirectory  This commit fixes problems with calling LockDirectory multiple times on the same directory, or from multiple threads. It also fixes the build on OpenBSD. (W. J. van der Laan)
9ae619a Test: Use specific testing setups for wallet_zkeys_tests tests (furszy)
d86cd4f wallet: Add missing check for backup to source wallet file. (furszy)
d9e1c6b Abstract VerifyWalletPath and connect it to init and GUI. (furszy)
23458ca GUI: Implement and connect WalletModel::getWalletPath(). (furszy)
c2d3a07 Create new wallet databases as directories rather than files (Russell Yanofsky)
daa7fe5 Allow wallet files not in -walletdir directory (Russell Yanofsky)
9b2dae1 Allow wallet files in multiple directories (furszy)
d36185a wallet: unify backup creation process. (furszy)
8b8725d wallet_tests: Use dummy wallet instance instead of wallet pointer. (furszy)
434ed75 Abstract LockDirectory into system.cpp (furszy)
6a0380a Make .walletlock distinct from .lock (MeshCollider)
d8539bb Generalise walletdir lock error message for correctness (MeshCollider)
ddcfd4a Enable test for wallet directory locking (furszy)
a238a8d Add a lock to the wallet directory (MeshCollider)
1dc2219 Don't allow relative -walletdir paths (Russell Yanofsky)
dcb43ea Create walletdir if datadir doesn't exist and correct tests (furszy)
03db5c8 Default walletdir is wallets/ if it exists (MeshCollider)
359b01d Add release notes for -walletdir and wallets/ dir (MeshCollider)
71a4701 Add -walletdir parameter to specify custom wallet dir (furszy)
5b31813 Use unique_ptr for dbenv (DbEnv) (practicalswift)
a1bef4f Refactor: Modernize disallowed copy constructors/assignment (Dan Raviv)

Pull request description:

  > Adding more flexibility in where the wallets directory can be located. Before, the wallet database files were stored at the top level of the PIVX data directory. Now the location of the wallets directory can be overridden by specifying a `-walletdir=<path>` option where `<path>` can be an absolute path to a directory or directory symlink.
  >
  >Another advantage of this change is that if two wallets are located in the same directory, they will now use their own BerkeleyDB environments instead using a shared environment. Using a shared environment makes it difficult to manage and back up wallets separately because transaction log files will contain a mix of data from all wallets in the environment.

  Coming from:
  * bitcoin#11351 -> Refactor: Modernize disallowed copy constructors/assignment.
  * bitcoin#11466 -> Specify custom wallet directory with -walletdir param.
  * bitcoin#11726 -> Cleanups + nit fixes for -walletdir work.
  * bitcoin#11904 -> Add lock to the wallet directory.
  * bitcoin#11687 -> External wallet files support.
  * bitcoin#12166 -> Doc better -walletdir description.
  * bitcoin#12220 -> Error if relative -walletdir is specified.

  This finishes the work started in #2337, enabling all the commented tests inside `wallet_multiwallet.py` and more.

  PR built on top of #2369.

  Note: Test this properly.

ACKs for top commit:
  random-zebra:
    utACK 056f4e8
  Fuzzbawls:
    ACK 056f4e8

Tree-SHA512: 98ae515dd664f959d35b63a0998bd93ca3bcea30ca67caccd2a694a440d10e18f831a54720ede0415d8f5e13af252bc6048a820491863d243df70ccc9d5fa7c6
malbit pushed a commit to malbit/raptoreum that referenced this pull request Nov 5, 2021
c1e5d40 Make debugging test crash easier (MeshCollider)
8263f6a Create walletdir if datadir doesn't exist and fix tests (MeshCollider)
9587a9c Default walletdir is wallets/ if it exists (MeshCollider)
d987889 Add release notes for -walletdir and wallets/ dir (MeshCollider)
80c5cbc Add test for -walletdir (MeshCollider)
0530ba0 Add -walletdir parameter to specify custom wallet dir (MeshCollider)

Pull request description:

  Closes #11348

  Adds a `-walletdir` parameter which specifies a directory to use for wallets, allowing them to be stored separately from the 'main' data directory. Creates a new `wallets/` directory in datadir if this is the first time running, and defaults to using it if it exists.

  Includes tests and release notes. Things which might need to be considered more:
  - there is no 'lock' on the wallets directory, which might be needed?
  - because this uses a new wallets/ directory by default, downgrading to an earlier version won't see the wallets in that directory (not a big deal though, users can just copy them up to the main dir)
  - jnewbery suggested putting each wallet in its own directory, which is a good idea, but out of scope for this PR IMO. EDIT: this is being done in bitcoin/bitcoin#11687
  - doc/files.md needs updating (will do soon)

  I also considered including  a cleanup by removing caching of data directory paths and instead just initialise them once on startup (c.f. #3073), but decided it wasn't super relevant here will just complicate review.

Tree-SHA512: c8ac04bfe9a810c32055f2c8b8fa0d535e56125ceb8d96f12447dd3538bf3e5ee992b60b1cd2173bf5f3fa023a9feab12c9963593bf27ed419df929bb413398d
malbit pushed a commit to malbit/raptoreum that referenced this pull request Nov 5, 2021
a14dbff39e Allow multiwallet.py to be used with --usecli (Russell Yanofsky)
f6ade9ce1a [tests] allow tests to be run with --usecli (John Newbery)
ff9a363ff7 TestNodeCLI batch emulation (Russell Yanofsky)
ca9085afc5 Prevent TestNodeCLI.args mixups (Russell Yanofsky)
fcfb952bca Improve TestNodeCLI output parsing (Russell Yanofsky)

Pull request description:

  Lack of test coverage was pointed out by @jnewbery in bitcoin/bitcoin#11687 (comment)

Tree-SHA512: 5f10e31abad11a5edab0da4e2515e39547adb6ab9e55e50427ab2eb7ec9a43d6b896b579b15863e5edc9beee7d8bf1c84d9dabd247be0760a1b9ae39e1e8ee02
malbit pushed a commit to malbit/raptoreum that referenced this pull request Nov 5, 2021
2f3bd47 Abstract directory locking into util.cpp (MeshCollider)
5260a4a Make .walletlock distinct from .lock (MeshCollider)
64226de Generalise walletdir lock error message for correctness (MeshCollider)
c9ed4bd Add a test for wallet directory locking (MeshCollider)
e60cb99 Add a lock to the wallet directory (MeshCollider)

Pull request description:

  Fixes bitcoin/bitcoin#11888, needs a 0.16 milestone

  Also adds a test that the lock works.

  bitcoin/bitcoin#11687 will probably rework this to a per-wallet lock instead of just the walletdir, but this fixes the current issue

Tree-SHA512: 98e52d67f820e3b8f919cf361ffbb7d928f1bd67603e0ed26c5076ea02d9b3a90c3535ddf7329f3b88171396fa28dd3c87adab3577a8a217bd1e4247bda99138
malbit pushed a commit to malbit/raptoreum that referenced this pull request Nov 5, 2021
be8ab7d08 Create new wallet databases as directories rather than files (Russell Yanofsky)
26c06f24e Allow wallet files not in -walletdir directory (Russell Yanofsky)
d8a99f65e 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/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
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Feb 12, 2022
…param

c1e5d40 Make debugging test crash easier (MeshCollider)
8263f6a Create walletdir if datadir doesn't exist and fix tests (MeshCollider)
9587a9c Default walletdir is wallets/ if it exists (MeshCollider)
d987889 Add release notes for -walletdir and wallets/ dir (MeshCollider)
80c5cbc Add test for -walletdir (MeshCollider)
0530ba0 Add -walletdir parameter to specify custom wallet dir (MeshCollider)

Pull request description:

  Closes bitcoin#11348

  Adds a `-walletdir` parameter which specifies a directory to use for wallets, allowing them to be stored separately from the 'main' data directory. Creates a new `wallets/` directory in datadir if this is the first time running, and defaults to using it if it exists.

  Includes tests and release notes. Things which might need to be considered more:
  - there is no 'lock' on the wallets directory, which might be needed?
  - because this uses a new wallets/ directory by default, downgrading to an earlier version won't see the wallets in that directory (not a big deal though, users can just copy them up to the main dir)
  - jnewbery suggested putting each wallet in its own directory, which is a good idea, but out of scope for this PR IMO. EDIT: this is being done in bitcoin#11687
  - doc/files.md needs updating (will do soon)

  I also considered including  a cleanup by removing caching of data directory paths and instead just initialise them once on startup (c.f. dashpay#3073), but decided it wasn't super relevant here will just complicate review.

Tree-SHA512: c8ac04bfe9a810c32055f2c8b8fa0d535e56125ceb8d96f12447dd3538bf3e5ee992b60b1cd2173bf5f3fa023a9feab12c9963593bf27ed419df929bb413398d
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.