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

Multiwallet: simplest endpoint support #10849

Merged
merged 6 commits into from Jul 18, 2017

Conversation

@jonasschnelli
Copy link
Contributor

@jonasschnelli jonasschnelli commented Jul 17, 2017

Alternative for #10829 and #10650.
It adds the most simplest form of wallet based endpoint support (/wallet/<filename>).
No v1 and no node/wallet endpoint split.

@laanwj
Copy link
Member

@laanwj laanwj commented Jul 17, 2017

Looks good to me too, utACK c8115e3.

Loading

@laanwj laanwj added this to the 0.15.0 milestone Jul 17, 2017
@ryanofsky
Copy link
Contributor

@ryanofsky ryanofsky commented Jul 17, 2017

This has no documentation. If we think this approach is a good idea, I would like to see in what way it can be explained to users. Is there going to be a doc/JSPON-RPC-interface.md to complement https://github.com/bitcoin/bitcoin/blob/master/doc/REST-interface.md? Will endpoint be considered experimental (unchanging semantics but possibly temporary) or unstable (possibly changing semantics and possibly temporary)?

Also, from the other issue I think something like -rpcwallet=filename would be better than -usewallet=filename. It's hard to see how "use" could convey any meaning when you would expect any command line option that you specify to be used.

And, also from other issue I think '?wallet=filename makes more sense than /wallet/filename as more extensible and conventional way of passing options (as opposed to required arguments) in urls.

Loading

@laanwj
Copy link
Member

@laanwj laanwj commented Jul 17, 2017

And, also from other issue I think '?wallet=filename makes more sense than /wallet/filename as more extensible and conventional way of passing options (as opposed to required arguments) in urls.

Definitely no! Let's not repeat this: JSON-RPC uses POST only, and with POST it is weird to have ?a=b URL-encoded arguments in the URL as well. It also implies more arguments could be added later, which is not true. This is only to select the wallet, which is inherently hierarchical.
(also e.g. ?a=b syntax isn't deterministic, it's possible to add arguments that are ignored, change the order of the arguments, etc, whereas a /wallet/<name>/ URL is simply a fixed thing)

Loading

@ryanofsky
Copy link
Contributor

@ryanofsky ryanofsky commented Jul 17, 2017

POST it is weird to have ?a=b URL-encoded arguments in the URL as well

Don't know your source for this but this is done all the time.

It also implies more arguments could be added later, which is not true.

I don't know why it wouldn't be true. I can think of lots of RPC options you might want to add outside the RPC request: timeouts, completions handlers, etc.

?a=b syntax isn't deterministic, it's possible to add arguments that are ignored, change the order of the arguments, etc, whereas a /wallet// URL is simply a fixed thing

Not true, you don't have to ignore any option, but it is good practice to accept options in any order. This is one of the reasons people choose to used named options instead of positional arguments.

Loading

@laanwj
Copy link
Member

@laanwj laanwj commented Jul 17, 2017

I don't know why it it wouldn't be true, but ok. I can think of lots of RPC options you might want to add outside the RPC request: timeouts, completions handlers, etc.

Those really shouldn't be passed in the URL. The URL locates something, such as the wallet in this case.

Loading

@ryanofsky
Copy link
Contributor

@ryanofsky ryanofsky commented Jul 17, 2017

Those really shouldn't be passed in the URL. The URL locates something, such as the wallet in this case.

They shouldn't be part of the uri-path, but you would have to explain more why they shouldn't be options in a query string...

Loading

@gmaxwell
Copy link
Contributor

@gmaxwell gmaxwell commented Jul 17, 2017

ryanofsky: Among other reasons the path approach will already work with most but not all software, e.g. you change your RPC endpoint url to be one with the path, and tada-- done, no source code modifications in some things. I believe a query string would not achieve this, and in some cases require deep spelunking.

I could see an argument that something like a timeout might make sense as a query like parameter, but a wallet seems like a pretty perfect 'navigation' like element.

Loading

@laanwj
Copy link
Member

@laanwj laanwj commented Jul 17, 2017

Is there going to be a doc/JSPON-RPC-interface.md to complement https://github.com/bitcoin/bitcoin/blob/master/doc/REST-interface.md?

And yes, there should be. Documentation is good.
But that's not a requirement for the feature freeze. This is an experimental API, it could be changed completely for next release, so not having it documented 100% on first release is not a problem.
(we should make an attempt in the release notes, of course)

Loading

@ryanofsky
Copy link
Contributor

@ryanofsky ryanofsky commented Jul 17, 2017

you change your RPC endpoint url to be one with the patch, and tada-- done

This is true for this PR but not #10650 which is a big reason why I think this PR is better and less user-hostile than #10650.

@laanwj, my bigger point is about extensibility . We want to add a wallet option right now. Great, so that can be as simple as adding ?wallet=filename to the query string to provide the nice user benefit Greg cites. And maybe you have a problem with timeouts or completion handlers, but lets say we want to add a ?network=name parameter in the future (maybe to add extra way to specify that an operation is done on mainnet or testnet, or in some dystopia where there are multiple chains, but no need to go there...). The point is you want to be able to add some new option. With query strings you have a standard way to do this. ?wallet=filename&network=name&timeout=30s works just as well as ?timeout=30s&wallet=filename&network=name as established by long-running convention.

If we use paths instead, are we requiring /wallet/filename/network/name or /network/name/filename or /wallet/filename?network=name, or something else completely arbitrary based on historical accident? Or because a filename is a "location" but a network name isn't?

I guess another thing that is motivating me is that I would like to see less uri-path space annexed for this one multiwallet feature. Right now json-rpc calls go directly to / handler and REST calls go directly to /rest handler. Everything else right now (but not with these endpoint prs) is wide open right now. I'd be happier if more of the path space could be free for other applications like a web interface. If these prs added stuff under an /rpc path, that would also seem like an improvement to me.

Also, so my other points doesn't get lost above, it would be great to see something done on documentation and -usewallet if we are taking this endpoint approach. Even if there is not "100% documentation" I think it would be very helpful to see uri-path endpoints explained from user point of view.

Loading

@gmaxwell
Copy link
Contributor

@gmaxwell gmaxwell commented Jul 17, 2017

I'd be happier if more of the path space could be free for other applications like a web interface. If all this stuff was moved under an /rpc path

It's under /wallet/ at least, so I think it won't have any clashes with other use.

As far as usewallet-- I think we intend that argument to eventually specify all wallets that get loaded, not just specific to rpc. e.g. it would also tell it which ones to load in the UI when that support is complete.

Loading

@ryanofsky
Copy link
Contributor

@ryanofsky ryanofsky commented Jul 17, 2017

It's under /wallet/ at least, so I think it won't have any clashes with other use.

I could easily imagine a web interface wanting to handle /wallet path. The point is that if RPCs are restricted to exactly one path / (current case) or two paths / and /rpc then it's then it's clean and easy to do anything else with the rest of the space. If there's a bigger changing list of RPC paths, that would seem to make things messier.

As far as usewallet-- I think we intend that argument to eventually specify all wallets that get loaded, not just specific to rpc. e.g. it would also tell it which ones to load in the UI when that support is complete.

That's interesting. I thought the UI was would just have tabs or a dropdown to choose between any of the loaded wallets. But I guess the advantage of having the bitcoin-qt executable accept a -usewallet=filename instead of a more descriptive -showwallet=filename or -onlyshowwallet=filename would be that you can specify -usewallet in your bitcoind.conf and have it picked up by both bitcoin-cli and bitcoin-qt.

Loading

@@ -241,7 +242,20 @@ UniValue CallRPC(const std::string& strMethod, const UniValue& params)
assert(output_buffer);
evbuffer_add(output_buffer, strRequest.data(), strRequest.size());

int r = evhttp_make_request(evcon.get(), req.get(), EVHTTP_REQ_POST, "/");
// check if we should use a special wallet endpoint
std::string endpoint;
Copy link
Member

@jnewbery jnewbery Jul 17, 2017

Choose a reason for hiding this comment

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

bug: if no wallet is specified then the endpoint will be "" instead of "/"

result: bitcoin-cli doesn't work without a -usewallet parameter:

→ bitcoin-cli getinfo
error: no response from server

Fix is:

    std::string endpoint = "/";

Loading

@jnewbery
Copy link
Member

@jnewbery jnewbery commented Jul 17, 2017

It's a shame that this PR doesn't prevent individual wallet endpoints from accessing server-level RPCs. However that can be added at a later date.

I have a branch here: https://github.com/jnewbery/bitcoin/tree/pr10849 that does a couple of things:

  • fixes #10849 (comment) (in commit bugfix - allow bitcoin-cli to work without -wallet parameter)
  • prevents bitcoin-cli from reading -wallet arguments from the conf file and removes the -usewallet argument. That means that to run a command on a wallet in multiwallet mode, the bitcoin-cli user must specify -wallet on the command line.

Feel free to squash the first commit and cherry-pick the next two if you think they're useful.

Loading

@jonasschnelli jonasschnelli force-pushed the 2017/07/mw_endpoint_simple branch from c8115e3 to 6b9faf7 Jul 17, 2017
@jonasschnelli
Copy link
Contributor Author

@jonasschnelli jonasschnelli commented Jul 17, 2017

Force push fixed the endpoint issue (thanks @jnewbery).
I think we should keep it minimal with a hope this can slip in before 0.15 freeze.

Fixes can be done later.

Loading

@jnewbery
Copy link
Member

@jnewbery jnewbery commented Jul 17, 2017

Fixes can be done later.

Agree. Multiwallet interface should be considered unstable for v0.15.

Loading

@achow101
Copy link
Member

@achow101 achow101 commented Jul 17, 2017

So this will remove the concept of "default wallet" from the RPCs? Any attempt to use a wallet RPC without specifying a wallet will fail, right?

Loading

@jonasschnelli
Copy link
Contributor Author

@jonasschnelli jonasschnelli commented Jul 17, 2017

@achow101: only if you call a wallet RPC and if you run with multiple wallets.

Loading

@achow101
Copy link
Member

@achow101 achow101 commented Jul 17, 2017

@jonasschnelli Ok, cool!

utACK 6b9faf7

Loading

super().__init__()
self.setup_clean_chain = True
self.num_nodes = 1
self.extra_args = [['-wallet=w1', '-wallet=w2', '-wallet=w3']]
Copy link
Member

@theuni theuni Jul 17, 2017

Choose a reason for hiding this comment

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

Could you change one of these to just 'w' to catch possible length extension bugs? If we're asking for w2, we want to make sure that we don't get w. on accident.

Loading

@theuni
Copy link
Member

@theuni theuni commented Jul 18, 2017

Code review utACK 6b9faf7. No opinion on the endpoint discussion.

Loading

@NicolasDorier
Copy link
Contributor

@NicolasDorier NicolasDorier commented Sep 2, 2017

Ok disregard my comment. I thought that adding /wallet/w1 to a non wallet RPC method would fail. This is not the case, so no issue!

Loading

Copy link
Member

@MarcoFalke MarcoFalke left a comment

Post-merge utACK 6b9faf7

Loading

walletinfo = w1.getwalletinfo()
assert_equal(walletinfo['immature_balance'], 50)

#check w1 wallet balance
Copy link
Member

@MarcoFalke MarcoFalke Nov 10, 2017

Choose a reason for hiding this comment

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

nit: w2

Loading

MarcoFalke added a commit that referenced this issue Nov 22, 2017
fa61c6f qa: Add multiwallet prefix test (MarcoFalke)

Pull request description:

  Fixes #10849 (comment)

Tree-SHA512: 7967be04e76d935398b3bea60c864ffc9e38dbb4cfb55890bb146a6f16c28d81ca5d89736275e2d0b03642806f6f7093beeea979f5257c464f437c4e5a9684f1
ftrader pushed a commit to ftrader-bitcoinabc/bitcoin-abc that referenced this issue Feb 5, 2018
Summary:
Merge #11743: qa: Add multiwallet prefix test

fa61c6f qa: Add multiwallet prefix test (MarcoFalke)

Pull request description:

  Fixes bitcoin/bitcoin#10849 (comment)

Tree-SHA512: 7967be04e76d935398b3bea60c864ffc9e38dbb4cfb55890bb146a6f16c28d81ca5d89736275e2d0b03642806f6f7093beeea979f5257c464f437c4e5a9684f1

Core PR11743

Fixes T177

Test Plan: ./test/functional/test_runner.py multiwallet

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Subscribers: teamcity

Maniphest Tasks: T177

Differential Revision: https://reviews.bitcoinabc.org/D1055
zkbot added a commit to zcash/zcash that referenced this issue Aug 4, 2018
ZIP 32 preparations

Includes Makefile changes cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#7689
- bitcoin/bitcoin#10849

Part of #3380.
zkbot added a commit to zcash/zcash that referenced this issue Aug 4, 2018
ZIP 32 preparations

Includes Makefile changes cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#7689
- bitcoin/bitcoin#10849

Part of #3380.
zkbot added a commit to zcash/zcash that referenced this issue Aug 5, 2018
ZIP 32 preparations

Includes Makefile changes cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#7689
- bitcoin/bitcoin#10849

Part of #3380.
zkbot added a commit to zcash/zcash that referenced this issue Aug 5, 2018
ZIP 32 preparations

Includes Makefile changes cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#7689
- bitcoin/bitcoin#10849

Part of #3380.
zkbot added a commit to zcash/zcash that referenced this issue Aug 5, 2018
ZIP 32 preparations

Includes Makefile changes cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#7689
- bitcoin/bitcoin#10849

Part of #3380.
attilaaf pushed a commit to attilaaf/bitcoin that referenced this issue May 25, 2019
Summary:
Backport of Core PR bitcoin#10849: Multiwallet: simplest endpoint support

    6b9faf7 [QA] add basic multiwallet test (Jonas Schnelli)
    979d0b8 [tests] [wallet] Add wallet endpoint support to authproxy (John Newbery)
    76603b1 Select wallet based on the given endpoint (Jonas Schnelli)
    32c9710 Fix test_bitcoin circular dependency issue (Jonas Schnelli)
    31e0720 Add wallet endpoint support to bitcoin-cli (-usewallet) (Jonas Schnelli)
    dd2185c Register wallet endpoint (Jonas Schnelli)

Depends on D985, and D1003
Closes T173

Test Plan: make check && ./test/functional/test_runner.py

Reviewers: deadalnix, jasonbcox, matiu, #bitcoin_abc

Reviewed By: deadalnix, #bitcoin_abc

Subscribers: teamcity

Maniphest Tasks: T186, T173

Differential Revision: https://reviews.bitcoinabc.org/D987
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Aug 2, 2019
6b9faf7 [QA] add basic multiwallet test (Jonas Schnelli)
979d0b8 [tests] [wallet] Add wallet endpoint support to authproxy (John Newbery)
76603b1 Select wallet based on the given endpoint (Jonas Schnelli)
32c9710 Fix test_bitcoin circular dependency issue (Jonas Schnelli)
31e0720 Add wallet endpoint support to bitcoin-cli (-usewallet) (Jonas Schnelli)
dd2185c Register wallet endpoint (Jonas Schnelli)

Pull request description:

  Alternative for bitcoin#10829 and bitcoin#10650.
  It adds the most simplest form of wallet based endpoint support (`/wallet/<filename>`).
  No v1 and no node/wallet endpoint split.

Tree-SHA512: 23de1fd2f9b48d94682928b582fb6909e16ca507c2ee19e1f989d5a4f3aa706194c4b1fe8854d1d79ba531b7092434239776cae1ae715ff536e829424f59f9be
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Aug 6, 2019
6b9faf7 [QA] add basic multiwallet test (Jonas Schnelli)
979d0b8 [tests] [wallet] Add wallet endpoint support to authproxy (John Newbery)
76603b1 Select wallet based on the given endpoint (Jonas Schnelli)
32c9710 Fix test_bitcoin circular dependency issue (Jonas Schnelli)
31e0720 Add wallet endpoint support to bitcoin-cli (-usewallet) (Jonas Schnelli)
dd2185c Register wallet endpoint (Jonas Schnelli)

Pull request description:

  Alternative for bitcoin#10829 and bitcoin#10650.
  It adds the most simplest form of wallet based endpoint support (`/wallet/<filename>`).
  No v1 and no node/wallet endpoint split.

Tree-SHA512: 23de1fd2f9b48d94682928b582fb6909e16ca507c2ee19e1f989d5a4f3aa706194c4b1fe8854d1d79ba531b7092434239776cae1ae715ff536e829424f59f9be
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Jan 17, 2020
fa61c6f qa: Add multiwallet prefix test (MarcoFalke)

Pull request description:

  Fixes bitcoin#10849 (comment)

Tree-SHA512: 7967be04e76d935398b3bea60c864ffc9e38dbb4cfb55890bb146a6f16c28d81ca5d89736275e2d0b03642806f6f7093beeea979f5257c464f437c4e5a9684f1
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Jan 22, 2020
fa61c6f qa: Add multiwallet prefix test (MarcoFalke)

Pull request description:

  Fixes bitcoin#10849 (comment)

Tree-SHA512: 7967be04e76d935398b3bea60c864ffc9e38dbb4cfb55890bb146a6f16c28d81ca5d89736275e2d0b03642806f6f7093beeea979f5257c464f437c4e5a9684f1
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Jan 22, 2020
fa61c6f qa: Add multiwallet prefix test (MarcoFalke)

Pull request description:

  Fixes bitcoin#10849 (comment)

Tree-SHA512: 7967be04e76d935398b3bea60c864ffc9e38dbb4cfb55890bb146a6f16c28d81ca5d89736275e2d0b03642806f6f7093beeea979f5257c464f437c4e5a9684f1
barrystyle added a commit to PACGlobalOfficial/PAC that referenced this issue Jan 22, 2020
6b9faf7 [QA] add basic multiwallet test (Jonas Schnelli)
979d0b8 [tests] [wallet] Add wallet endpoint support to authproxy (John Newbery)
76603b1 Select wallet based on the given endpoint (Jonas Schnelli)
32c9710 Fix test_bitcoin circular dependency issue (Jonas Schnelli)
31e0720 Add wallet endpoint support to bitcoin-cli (-usewallet) (Jonas Schnelli)
dd2185c Register wallet endpoint (Jonas Schnelli)

Pull request description:

  Alternative for bitcoin#10829 and bitcoin#10650.
  It adds the most simplest form of wallet based endpoint support (`/wallet/<filename>`).
  No v1 and no node/wallet endpoint split.

Tree-SHA512: 23de1fd2f9b48d94682928b582fb6909e16ca507c2ee19e1f989d5a4f3aa706194c4b1fe8854d1d79ba531b7092434239776cae1ae715ff536e829424f59f9be
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Jan 29, 2020
fa61c6f qa: Add multiwallet prefix test (MarcoFalke)

Pull request description:

  Fixes bitcoin#10849 (comment)

Tree-SHA512: 7967be04e76d935398b3bea60c864ffc9e38dbb4cfb55890bb146a6f16c28d81ca5d89736275e2d0b03642806f6f7093beeea979f5257c464f437c4e5a9684f1
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Jan 29, 2020
fa61c6f qa: Add multiwallet prefix test (MarcoFalke)

Pull request description:

  Fixes bitcoin#10849 (comment)

Tree-SHA512: 7967be04e76d935398b3bea60c864ffc9e38dbb4cfb55890bb146a6f16c28d81ca5d89736275e2d0b03642806f6f7093beeea979f5257c464f437c4e5a9684f1
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Jan 29, 2020
fa61c6f qa: Add multiwallet prefix test (MarcoFalke)

Pull request description:

  Fixes bitcoin#10849 (comment)

Tree-SHA512: 7967be04e76d935398b3bea60c864ffc9e38dbb4cfb55890bb146a6f16c28d81ca5d89736275e2d0b03642806f6f7093beeea979f5257c464f437c4e5a9684f1
charlesrocket added a commit to charlesrocket/axe that referenced this issue Mar 10, 2020
fa61c6f6a qa: Add multiwallet prefix test (MarcoFalke)

Pull request description:

  Fixes bitcoin/bitcoin#10849 (comment)

Tree-SHA512: 7967be04e76d935398b3bea60c864ffc9e38dbb4cfb55890bb146a6f16c28d81ca5d89736275e2d0b03642806f6f7093beeea979f5257c464f437c4e5a9684f1
ckti added a commit to ckti-ioncore-current/ion that referenced this issue Mar 28, 2021
fa61c6f qa: Add multiwallet prefix test (MarcoFalke)

Pull request description:

  Fixes bitcoin#10849 (comment)

Tree-SHA512: 7967be04e76d935398b3bea60c864ffc9e38dbb4cfb55890bb146a6f16c28d81ca5d89736275e2d0b03642806f6f7093beeea979f5257c464f437c4e5a9684f1
random-zebra added a commit to PIVX-Project/PIVX that referenced this issue Jun 9, 2021
d5526bd Wrap dumpwallet warning and note scripts aren't dumped (MeshCollider)
3711c6a Add wallet backup text to import*, add* and dumpwallet RPCs (MeshCollider)
dbda874 [Wallet] always show help-line of wallet encryption calls (Jonas Schnelli)
20c269b Avoid opening copied wallet databases simultaneously (Russell Yanofsky)
e411b70 [wallet] Fix leak in CDB constructor (João Barbosa)
f15aeea Change getmininginfo errors field to warnings (Andrew Chow)
c04390b Unify help text for GetWarnings output in get*info RPCs (random-zebra)
1d966ce Add warnings field to getblockchaininfo (Andrew Chow)
ffcd781 [Trivial] Cleanup after MOVE-ONLY commits (random-zebra)
e067235 MOVEONLY: Init functions wallet/wallet.cpp -> wallet/init.cpp (random-zebra)
e947eec MOVEONLY: Fee functions wallet/wallet.cpp -> wallet/fees.cpp (random-zebra)
2188c3e Move some static functions out of wallet.h/cpp (random-zebra)
f49acf7 [wallet] [moveonly] Move CAffectedKeysVisitor (random-zebra)
8bd979f [wallet] Specify wallet name in wallet loading errors (random-zebra)
900bbfa Reject invalid wallet files (João Barbosa)
a1f4e2a Reject duplicate wallet filenames (random-zebra)
ee52c2e Fix misleading "Method not found" multiwallet errors (Russell Yanofsky)
ce35e1e [Qt] Use wallet 0 in rpc console if running with multiple wallets (random-zebra)
37089d1 [tests] Update wallet_multiwallet.py functional test (random-zebra)
3955ee9 [Doc] Update release notes (random-zebra)
4fd5913 [wallet] [rpc] Add listwallets RPC (John Newbery)
1525281 [wallet] [rpc] print wallet name in getwalletinfo (John Newbery)
fdf5da0 [wallet] fix comment for CWallet::Verify() (John Newbery)
cf4a90b Remove factor of 3 from definition of dust. (random-zebra)
a1c56fd [Policy] Introduce -dustrelayfee (random-zebra)
9fb29cc [Doc] Update multiwallet release notes (random-zebra)
379255e [Tests][Trivial] Add wallet_multiwallet.py to test_runner (random-zebra)
808fbc3 [Bugfix] consider boolean value of -zapwallettxes ParameterInteraction (random-zebra)
f9141f8 [QA] Add wallet_multiwallet.py functional test (John Newbery)
2e02006 Rename -usewallet to -rpcwallet (Alex Morcos)
a64440b Select wallet based on the given endpoint (Jonas Schnelli)
5683a9c Complete previous commit by moving mn stuff out of libbitcoin_wallet (random-zebra)
b0fe92f Fix test_pivx circular dependency issue (random-zebra)
6cb2b92 Add wallet endpoint support to bitcoin-cli (-usewallet) (Jonas Schnelli)
7dd3916 Register wallet endpoint (Jonas Schnelli)
5bd1bd7 Properly forbid -salvagewallet and -zapwallettxes for multi wallet. (Alex Morcos)
41a7335 Remove unused variables (practicalswift)
5c3d73f Avoid CWalletTx copies in GetAddressBalances and GetAddressGroupings (Russell Yanofsky)
e7cafab [Refactoring] Mimic ListCoins for sapling notes (random-zebra)
54fa122 [qt] Move some WalletModel functions into CWallet (random-zebra)
494ba64 [test] Add test for getmemoryinfo (random-zebra)
2394083 [Wallet] unset change position when there is no change on exact match (Gregory Sanders)
7d977ac Remove unused C++ code not covered by unit tests (random-zebra)
60bb4da ApproximateBestSubset should take inputs by reference, not value (Ryan Havar)
3633d75 Initialize nRelockTime (Patrick Strateman)
3a599d0 [Refactor] Return safeTx boolean in CheckTXAvailability (random-zebra)
f219be9 Add safe flag to listunspent result (NicolasDorier)
0201065 Add COutput::fSafe member for safe handling of unconfirmed outputs (random-zebra)
75c8c6d Disallow copy of CReserveKeys (Gregory Sanders)
8378322 [Refactor] Replace optional reserveKey in PBF with unique pointer (random-zebra)

Pull request description:

  I think these are all the remaining Bitcoin Core v0.15 PRs in the wallet area that we don't have yet, and are useful to us.
  I've grouped them here since they are all pretty small, simple, and narrow-focused (on the wallet/rpcwallet files).

  This changeset is based on top of
  - [x] #2337

  as it keeps going with the multiwallet implementation, adding:
  - RPC endpoint support
  - `listwallets` RPC
  - wallet name in `getwalletinfo` RPC
  - `wallet_multiwallet.py` functional test

  As in some areas we are much closer to upstream, some of the commits needed adaptations (especially the functional tests). A couple of commits have been extended to include shield-related code.

  List of upstream PRs backported/adapted:

  - bitcoin#9906  : Disallow copy constructor CReserveKeys
  - bitcoin#9830  : Add safe flag to listunspent result
  - bitcoin#9993  : Initialize nRelockTime
  - bitcoin#10108 : ApproximateBestSubset should take inputs by reference, not value
  - bitcoin#10075 : Remove unused C++ code not covered by unit tests
  - bitcoin#10257 : Add test for getmemoryinfo
  - bitcoin#10295 : Move some WalletModel functions into CWallet
  - bitcoin#10500 : Avoid CWalletTx copies in GetAddressBalances and GetAddressGroupings
  - bitcoin#10522 : Remove unused variables
  - bitcoin#10816 : Properly forbid -salvagewallet and -zapwallettxes for multi wallet
  - bitcoin#10849 : Multiwallet: simplest endpoint support
  - bitcoin#9380  : Separate different uses of minimum fees (only dustRelayFee)
  - bitcoin#10817 : Redefine Dust and add a discard_rate (only first commit)
  - bitcoin#10883 : Rename -usewallet to -rpcwallet
  - bitcoin#10604 : Add listwallets RPC, include wallet name in getwalletinfo + tests
  - bitcoin#10893 : Avoid running multiwallet.py twice
  - bitcoin#10870 : Use wallet 0 in rpc console if running with multiple wallets
  - bitcoin#10931 : Fix misleading "method not found" multiwallet errors
  - bitcoin#10885 : Reject invalid wallets
  - bitcoin#11022 : Basic keypool topup
  - bitcoin#10976 : [MOVEONLY] Move some static functions out of wallet.h/cpp
  - bitcoin#11310 : Test listwallets RPC
  - bitcoin#10858 : "errors" to getblockchaininfo + unify "errors" field in get*info RPCs
  - bitcoin#11492 : Fix leak in CDB constructor
  - bitcoin#11476 : Avoid opening copied wallet databases simultaneously
  - bitcoin#11590 : always show help-line of wallet encryption calls
  - bitcoin#11289 : Add wallet backup text to import* and add* RPCs

ACKs for top commit:
  furszy:
    re-re-utACK d5526bd.
  Fuzzbawls:
    ACK d5526bd

Tree-SHA512: 115c4916ee212539b0a1b2cb25783ddf6753f5376de739a084191e874ed8717fff2c7cd9d744e397891f14eaa459ef2f48d675168621ef7316e81758fa6559f2
gades added a commit to cosanta/cosanta-core that referenced this issue Jun 30, 2021
fa61c6f qa: Add multiwallet prefix test (MarcoFalke)

Pull request description:

  Fixes bitcoin#10849 (comment)

Tree-SHA512: 7967be04e76d935398b3bea60c864ffc9e38dbb4cfb55890bb146a6f16c28d81ca5d89736275e2d0b03642806f6f7093beeea979f5257c464f437c4e5a9684f1
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
malbit added a commit to malbit/raptoreum that referenced this issue Nov 5, 2021
fa61c6f6a qa: Add multiwallet prefix test (MarcoFalke)

Pull request description:

  Fixes bitcoin/bitcoin#10849 (comment)

Tree-SHA512: 7967be04e76d935398b3bea60c864ffc9e38dbb4cfb55890bb146a6f16c28d81ca5d89736275e2d0b03642806f6f7093beeea979f5257c464f437c4e5a9684f1
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet