Age | Commit message (Collapse) | Author |
|
Change gui code to use gArgs, Params() functions directly instead of going
through interfaces::Node.
Remotely accessing bitcoin-node ArgsManager from bitcoin-gui works fine in
https://github.com/bitcoin/bitcoin/pull/10102, when bitcoin-gui spawns a new
bitcoin-node process and controls its startup, but for bitcoin-gui to support
-ipcconnect option in https://github.com/bitcoin/bitcoin/pull/19461 and connect
to an existing bitcoin-node process, it needs ability to parse arguments itself
before connecting out.
This change also simplifies https://github.com/bitcoin/bitcoin/pull/10102 a
bit, by making the bitcoin-gui -> bitcoin-node startup sequence more similar to
the bitcoin-node -> bitcoin-wallet startup sequence where the parent process
parses arguments and passes them to the child process instead of the parent
process using the child process to parse arguments.
|
|
|
|
This maintains a persistent list of wallets stored in settings that will
automatically be loaded on startup. Being able to load a wallet automatically
on startup will be more useful in the GUI when the option to create wallets is
added in #15006, but it's reasonable to expose this feature by RPC as well.
|
|
|
|
edc316020e8270dafc5e31465d532baebdafd3dd test: Remove duplicate NodeContext hacks (Russell Yanofsky)
Pull request description:
Qt tests currently are currently using two NodeContext structs at the same time, one in interfaces::NodeImpl::m_context, and the other in BasicTestingSetup::m_node, and the tests have hacks transferring state between them.
Fix this by getting rid of the NodeImpl::m_context struct and making it a pointer. This way a common BitcoinApplication object can be used for all qt tests, but they can still have their own testing setups.
Non-test code is changing but non-test behavior is still the same as before.
Motivation for this PR is to be able to remove the "std::move(test.m_node.connman)" and mempool hacks for swapping individual NodeContext members in Qt tests, because followup PR #19099 adds yet another member (wallet_client) that needs to be swapped. After this change, the whole NodeContext struct can be swapped instead of individual members, so the workarounds are less fragile and invasive.
ACKs for top commit:
MarcoFalke:
crACK edc316020e8270dafc5e31465d532baebdafd3dd 🌮
promag:
ACK edc316020e8270dafc5e31465d532baebdafd3dd.
Tree-SHA512: c1650e4127f43a4020304ca7c13b5d9122fb5723aacd8fa1cf855d03c6052fcfb7685810aa2a5ef708561015f0022fecaacbad479295104ca45d2c17579466a4
|
|
9c69cfe4c54e38edd2f54303be2f8a53dcf5bad8 Add <datadir>/settings.json persistent settings storage. (Russell Yanofsky)
eb682c5700e7a9176d0104d470b83ff9aa3589e8 util: Add ReadSettings and WriteSettings functions (Russell Yanofsky)
Pull request description:
Persistent settings are used in followup PRs #15936 to unify gui settings between bitcoin-qt and bitcoind, and #15937 to add a load_on_startup flag to the loadwallet RPC and maintain a dynamic list of wallets that should be loaded on startup that also can be shared between bitcoind and bitcoin-qt.
ACKs for top commit:
MarcoFalke:
Approach re-ACK 9c69cfe4c54e38edd2f54303be2f8a53dcf5bad8 🌾
jnewbery:
utACK 9c69cfe4c54e38edd2f54303be2f8a53dcf5bad8
Tree-SHA512: 39fcc6051717117c9141e934de1d0d3f739484be4685cdf97d54de967c8c816502b4fd0de12114433beaa5c5b7060c810fd8ae4e2b3ce7c371eb729ac01ba2e1
|
|
Qt tests currently are currently using two NodeContext structs at the same
time, one in interfaces::NodeImpl::m_context, and the other in
BasicTestingSetup::m_node, and the tests have hacks transferring state between
them.
Fix this by getting rid of the NodeImpl::m_context struct and making it a
pointer. This way a common BitcoinApplication object can be used for all qt
tests, but they can still have their own testing setups.
Non-test code is changing but non-test behavior is still the same as before.
Motivation for this PR is to be able to remove the
"std::move(test.m_node.connman)" and mempool hacks for swapping individual
NodeContext members in Qt tests, because followup PR #19099 adds yet another
member (wallet_client) that needs to be swapped. After this change, the whole
NodeContext struct can be swapped instead of individual members, so the
workarounds are less fragile and invasive.
|
|
Persistent settings are used in followup PRs #15936 to unify gui settings
between bitcoin-qt and bitcoind, and #15937 to add a load_on_startup flag to
the loadwallet RPC and maintain a dynamic list of wallets that should be loaded
on startup that also can be shared between bitcoind and bitcoin-qt.
|
|
This refactor does not change behavior
|
|
|
|
fa927ff884ae373c676deed63180a8d238872cdc Enable Wswitch for OutputType (MarcoFalke)
faddad71f648ed99734f4f8811bd4bc7232ca670 Remove confusing OutputType::CHANGE_AUTO (MarcoFalke)
fa2eb383522249a5f4d48726c520cec5de496614 interfaces: Remove unused getDefaultChangeType (MarcoFalke)
Pull request description:
`OutputType::CHANGE_AUTO` is problematic for several reasons:
* An output that is not change must never be described by `CHANGE_AUTO`. Simply allowing that option makes the code confusing and review harder than it needs to be.
* To make review even harder, `CHANGE_AUTO` requires `-Wswitch` to be disabled for `OutputType`
Fix both issues by removing `CHANGE_AUTO` and then enabling `-Wswitch` for `OutputType`
ACKs for top commit:
promag:
Code review ACK fa927ff884ae373c676deed63180a8d238872cdc.
laanwj:
Code review ACK fa927ff884ae373c676deed63180a8d238872cdc
Tree-SHA512: 24fd809757aa343866c94dafe9a7130b50cda4f77c97666d407f99b813f75b115a7d8e688a6bc2a737e87cba64ddd4e43f2b3c5538fd35fabb5845807bb39134
|
|
-BEGIN VERIFY SCRIPT-
# Move files
git mv src/ui_interface.h src/node/ui_interface.h
git mv src/ui_interface.cpp src/node/ui_interface.cpp
sed -i -e 's/BITCOIN_UI_INTERFACE_H/BITCOIN_NODE_UI_INTERFACE_H/g' src/node/ui_interface.h
# Adjust includes and makefile
sed -i -e 's|ui_interface|node/ui_interface|g' $(git grep -l ui_interface)
# Sort includes
git diff -U0 | clang-format-diff -p1 -i -v
-END VERIFY SCRIPT-
|
|
ui_interface is in libbitcoin_server and can not be included in the
wallet because the wallet does not link with server symbols.
|
|
|
|
In FillPSBT, optionally report the number of inputs we successfully
signed, as an out parameter. If "sign" is false, instead report the
number of inputs for which GetSigningProvider does not return nullptr.
(This is a potentially overbroad estimate of inputs we could sign.)
|
|
fs.cpp:35:17: error: no member named 'strerror' in namespace 'std'
return std::strerror(errno);
~~~~~^
fs.cpp:49:9: error: use of undeclared identifier 'close'
close(fd);
^
2 errors generated.
./interfaces/chain.h:265:55: error: ‘std::function’ has not been declared
virtual void rpcRunLater(const std::string& name, std::function<void()> fn, int64_t seconds) = 0;
^~~
|
|
|
|
Make it consistent with `Chain::initError`.
|
|
4a7253ab6c3bb323581cea54573529c2f823f035 Remove g_rpc_chain global (Russell Yanofsky)
e783197bf0f7429f80fea94b44c59857bc8cfef9 refactor: replace RegisterWalletRPCCommands with GetWalletRPCCommands (Russell Yanofsky)
Pull request description:
Replace with RPC request reference to new WalletContext struct similar to the existing NodeContext struct and reference.
This PR is a followup to #18740 removing the g_rpc_node global.
Some later PRs will follow this up and move more wallet globals to the WalletContext struct.
ACKs for top commit:
MarcoFalke:
ACK 4a7253ab6c3bb323581cea54573529c2f823f035 🎋
ariard:
Code Review ACK 4a7253a, feel free to ignore comment it's super nit.
Tree-SHA512: 5bb5561c89f81811ca5232a58bf450e230d4218e62471c03227d142395fd36131672e99cb88329b33b9680a235db01e8b9d1c1e2a18288349e57205528deabab
|
|
notifications
7eaf86d3bfc83f2beb3ef449707d5156853126fb trivial: Suggested cleanups to surrounding code (Russell Yanofsky)
b604c5c8b5892842f13dee89ae31812a28ab25d1 wallet: Minimal fix to restore conflicted transaction notifications (Russell Yanofsky)
Pull request description:
This fix is a based on the fix by Antoine Riard (ariard) in https://github.com/bitcoin/bitcoin/pull/18600.
Unlike that PR, which implements some new behavior, this just restores previous wallet notification and status behavior for transactions removed from the mempool because they conflict with transactions in a block. The behavior was accidentally changed in two `CWallet::BlockConnected` updates: a31be09bfd77eed497a8e251d31358e16e2f2eb1 and 7e89994133725125dddbfa8d45484e3b9ed51c6e from https://github.com/bitcoin/bitcoin/pull/16624, causing issue https://github.com/bitcoin/bitcoin/issues/18325.
The change here could be improved and replaced with a more comprehensive cleanup, so it includes a detailed comment explaining future considerations.
Fixes #18325
Co-authored-by: Antoine Riard (ariard)
ACKs for top commit:
jonatack:
Re-ACK 7eaf86d3bfc83f
ariard:
ACK 7eaf86d, reviewed, built and ran tests.
MarcoFalke:
ACK 7eaf86d3bfc83f2beb3ef449707d5156853126fb 🍡
Tree-SHA512: 9a1efe975969bb522a9dd73c41064a9348887cb67883cd92c6571fd2df4321b9f4568363891abdaae14a3b9b168ef8142e95c373fc04677e46289b251fb84689
|
|
da73f1513a637a9f347b64de66564d6cdb2541f8 qt: Fix shutdown when waitfor* cmds are called from RPC console (Hennadii Stepanov)
Pull request description:
On master (7eed413e72a236b6f1475a198f7063fd24929e23), if the GUI has been started with`-server=1`, `bitcoin-qt` hangs on shutdown during calling any of the `waitfor*` commands in the GUI RPC console.
This PR suggests minimal changes to fix this bug.
Fix #17495
ACKs for top commit:
jonasschnelli:
utACK da73f1513a637a9f347b64de66564d6cdb2541f8
Tree-SHA512: 469f5332945a5f2c57d19336cda5df79b123ccc494aea6d58a85eb1293be52708b2b9c5bb6bc2c402a90b7b4e9e8d7ab8fe84cf201cf7ce612c9290c57e43681
|
|
Replace with RPC request reference to new WalletContext struct similar to the
existing NodeContext struct and reference.
This PR is a followup to 25ad2c623af30056ffb36dcd203a52edda2b170f
https://github.com/bitcoin/bitcoin/pull/18740 removing the g_rpc_node global.
Some later PRs will follow this up and move more wallet globals to the
WalletContext struct.
Co-authored-by: João Barbosa <joao.paulo.barbosa@gmail.com>
|
|
|
|
signals.
|
|
[ClientModel] best header/block hash cached.
|
|
b3f7f375efb9a9ca9a7a4f2caf41fe3df2262520 refactor: Remove g_rpc_node global (Russell Yanofsky)
ccb5059ee89f6e8dc31ba5b82830b384890bb65e scripted-diff: Remove g_rpc_node references (Russell Yanofsky)
6fca33b2edc09ed62dab2323c780b31585de1750 refactor: Pass NodeContext to RPC and REST methods through util::Ref (Russell Yanofsky)
691c817b340d10e806dc3b1834d2a8fcc5e681fd Add util::Ref class as temporary alternative for c++17 std::any (Russell Yanofsky)
Pull request description:
This PR removes the `g_rpc_node` global, to get same benefits we see removing other globals and make RPC code more testable, modular, and reusable.
This uses a hybrid of the approaches suggested in #17548. Instead of using `std::any`, which isn't available in c++11, or `void*`, which isn't type safe, it uses a small new `util::Ref` helper class, which acts like a simplified `std::any` that only holds references, not values.
Motivation for writing this was to provide an simpler alternative to #18647 by Harris Brakmić (brakmic) which avoids some shortcomings of that PR (https://github.com/bitcoin/bitcoin/pull/18647#issuecomment-617878826)
ACKs for top commit:
MarcoFalke:
re-ACK b3f7f375ef, only change is adding back const and more tests 🚾
ajtowns:
ACK b3f7f375efb9a9ca9a7a4f2caf41fe3df2262520
Tree-SHA512: 56292268a001bdbe34d641db1180c215351503966ff451e55cc96c9137f1d262225d7d7733de9c9da7ce7d7a4b34213a98c2476266b58c89dbbb0f3cb5aa5d70
|
|
WalletModel::pollBalanceChanged
d3a56be77a9d112cde4baef4314882170b9f228f Revert "gui: Avoid Wallet::GetBalance in WalletModel::pollBalanceChanged" (Russell Yanofsky)
bf0a510981ddc28c754881ca21c50ab18e5f2b59 gui: Avoid wallet tryGetBalances calls before TransactionChanged or BlockTip notifications (Russell Yanofsky)
2bc9b92ed8b7736ad67876398a0bb8287f57e9b3 Cancel wallet balance timer when shutdown requested (Russell Yanofsky)
83f69fab3a1ae97c5cff8ba1e6fd191b0fa264bb Switch transaction table to use wallet height not node height (Russell Yanofsky)
Pull request description:
Main commit `gui: Avoid wallet tryGetBalances calls` is one-line change to `WalletModel::pollBalanceChanged` that returns early if there hasn't been a new `TransactionChanged` or `BlockTip` notification since the previous poll call. This is the same behavior that was implemented in #18160, now implemented in a simpler way.
The other commits are a straight revert of #18160, and two tweaks to avoid relying on `WalletModel::m_client_model` lifetime which were causing travis failures with earlier versions of this PR.
Motivation for this change is to be able to revert #18160 and cut down on unnecessary cross-process calls that happen when #18160 is combined with #10102
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).
ACKs for top commit:
jonasschnelli:
utACK d3a56be77a9d112cde4baef4314882170b9f228f
Tree-SHA512: 3cd31ca515e77c3bd7160d3f1ea0dce5050d4038b2aa441b6f66b8599bd413d81ca5542a197806e773d6092dd1d26830932b1cecbc95298b1f1ab41099e2f12f
|
|
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
|
|
This fix is a based on the fix by Antoine Riard <ariard@student.42.fr> in
https://github.com/bitcoin/bitcoin/pull/18600.
Unlike that PR, which implements some new behavior, this just restores previous
wallet notification and status behavior for transactions removed from the
mempool because they conflict with transactions in a block. The behavior was
accidentally changed in two `CWallet::BlockConnected` updates:
a31be09bfd77eed497a8e251d31358e16e2f2eb1 and
7e89994133725125dddbfa8d45484e3b9ed51c6e from
https://github.com/bitcoin/bitcoin/pull/16624, causing issue
https://github.com/bitcoin/bitcoin/issues/18325.
The change here could be improved and replaced with a more comprehensive
cleanup, so it includes a detailed comment explaining future considerations.
Fixes #18325
Co-authored-by: Antoine Riard <ariard@student.42.fr>
|
|
This commit does not change behavior
|
|
78be8d97d3d750fcfbbe509a72639b7b30b7bd18 util: Drop OpOriginal() and OpTranslated() (Hennadii Stepanov)
da16f95c3fecf4ee1e9a1dc4333b0b92cd981afd gui: Do not translate InitWarning messages in debug.log (Hennadii Stepanov)
4c9b9a4882e68835f16a52f1f0657efe47e589b5 util: Enhance Join() (Hennadii Stepanov)
fe05dd0611cfc2929a7fdec04ca5948bccf0233b util: Enhance bilingual_str (Hennadii Stepanov)
Pull request description:
This PR forces the `bitcoin-qt` to write `InitWarning()` messages to the `debug.log` file in untranslated form, i.e., in English.
On master (376294cde6b1588cb17055d8fde567eaf5848c3c):
```
$ ./src/qt/bitcoin-qt -lang=nl -debug=vladidation -printtoconsole | grep 'vladi'
Warning: Niet-ondersteunde logcategorie -debug=vladidation.
2020-05-09T12:39:59Z Warning: Niet-ondersteunde logcategorie -debug=vladidation.
2020-05-09T12:40:02Z Command-line arg: debug="vladidation"
```
With this PR:
```
$ ./src/qt/bitcoin-qt -lang=nl -debug=vladidation -printtoconsole | grep 'vladi'
Warning: Unsupported logging category -debug=vladidation.
2020-05-09T12:42:04Z Warning: Unsupported logging category -debug=vladidation.
2020-05-09T12:42:35Z Command-line arg: debug="vladidation"
```
![Screenshot from 2020-05-09 15-42-31](https://user-images.githubusercontent.com/32963518/81474073-c7a50e00-920b-11ea-8775-c41122dacafe.png)
Related to #16218.
ACKs for top commit:
laanwj:
ACK 78be8d97d3d750fcfbbe509a72639b7b30b7bd18
jonasschnelli:
utACK 78be8d97d3d750fcfbbe509a72639b7b30b7bd18
MarcoFalke:
ACK 78be8d97d3d750fcfbbe509a72639b7b30b7bd18 📢
Tree-SHA512: 48e9ecd23c4dd8ec262e3eb94f8e30944bcc9c6c163245fb837b2e0c484d4d0b4f47f7abc638c14edc27d635d340ba3ee4ba4506b062399e9cf59a1564c98755
|
|
|
|
d044e0ec7d37bbcdf10bbdb903b9119741c7297d refactor: Remove override for final overriders (Hennadii Stepanov)
1551cea2d52cac403ff506a7cc955d8de8fd6f3e refactor: Use override for non-final overriders (Hennadii Stepanov)
Pull request description:
Two commits are split out from #16710 to make reviewing [easier](https://github.com/bitcoin/bitcoin/pull/16710#issuecomment-625760894).
From [C++ FAQ](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines.html#c128-virtual-functions-should-specify-exactly-one-of-virtual-override-or-final):
> C.128: Virtual functions should specify exactly one of virtual, override, or final
> **Reason** Readability. Detection of mistakes. Writing explicit `virtual`, `override`, or `final` is self-documenting and enables the compiler to catch mismatch of types and/or names between base and derived classes. However, writing more than one of these three is both redundant and a potential source of errors.
ACKs for top commit:
practicalswift:
ACK d044e0ec7d37bbcdf10bbdb903b9119741c7297d: consistent use of `override` prevents bugs + patch looks correct + Travis happy
MarcoFalke:
ACK d044e0ec7d37bbcdf10bbdb903b9119741c7297d, based on my understanding that adding `override` or `final` to a function must always be correct, unless it doesn't compile!?
vasild:
ACK d044e0ec7
Tree-SHA512: 245fd9b99b8b5cbf8694061f892cb3435f3378c97ebed9f9401ce86d21890211f2234bcc39c9f0f79a4d2806cb31bf8ce41a0f9c2acef4f3a2ac5beca6b077cf
|
|
|
|
|
|
|
|
|
|
Also, mark feebumper bilingual_str as Untranslated
They are technical and have previously not been translated either.
It is questionable whether they can even appear in the GUI.
|
|
Common errors and warnings should be translated when displayed in the
GUI, but not translated when displayed elsewhere. The wallet method
CreateWalletFromFile does not know its caller, so this commit changes it
to return a bilingual_str to the caller.
|
|
This reverts commit 0933a37078e1ce3a3d70983c3e7f4b3ac6c3fa37 from
https://github.com/bitcoin/bitcoin/pull/18160 which no longer an optimization
since commit "gui: Avoid wallet tryGetBalances calls before TransactionChanged
or BlockTip notifications".
|
|
This change is intended to make the bitcoin node and its rpc, network
and gui interfaces more responsive while the wallet is in use. Currently
because the node's cs_main mutex is always locked before the wallet's
cs_wallet mutex (to prevent deadlocks), cs_main currently stays locked
while the wallet does relatively slow things like creating and listing
transactions.
This commit only remmove chain lock tacking in wallet code, and invert
lock order from cs_main, cs_wallet to cs_wallet, cs_main.
must happen at once to avoid any deadlock. Previous commit were only
removing Chain::Lock methods to Chain interface and enforcing they
take cs_main.
Remove LockChain method from CWallet and Chain::Lock interface.
|
|
Remove findPruned and findFork, no more used after 17954.
|
|
|
|
Add HaveChain to assert chain access for wallet-tool in LoadToWallet.
|
|
Instead of calling getHeight, we rely on CWallet::m_last_block
processed_height where it's possible.
|
|
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
|
|
9986608ba93de040490ee0d5584ea33e605f1df0 test: Verify findCommonAncestor always initializes outputs (Russell Yanofsky)
Pull request description:
Also add code comment to clarify surprising code noted by practicalswift
https://github.com/bitcoin/bitcoin/pull/18657#issuecomment-614278450
ACKs for top commit:
MarcoFalke:
ACK 9986608ba93de040490ee0d5584ea33e605f1df0
jonatack:
ACK 9986608ba93de04 modulo @practicalswift's https://github.com/bitcoin/bitcoin/pull/18660#issuecomment-614487724
Tree-SHA512: d79c910291d68b770ef4b09564d274c0e19a6acf43ef1a6691dc889e3944ab3462b86056eeb794fd0c6f2464cfad6cc00711a833f84b32079c69ef9b3c8da24c
|
|
Also add code comment to clarify surprising code noted by practicalswift
https://github.com/bitcoin/bitcoin/pull/18657#issuecomment-614278450
|
|
|