* test: Introduce uint256_to_string
* test: Use uint256_to_string in some places
* Avoid converting back and forth, reuse known string represntations of hashes for blocks and txes instead of converting sha256
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
bd9d67b752 Don't test against min relay fee information in mining_prioritisetransaction.py (Kristaps Kaupe)
Pull request description:
Follow-up on #13032.
As advised by @MarcoFalke.
Tree-SHA512: a690ff2e6499b0b4ee6a952c849e363738b9a5ae553266e1780fc7910a6b01e240d71b434fe87da53dc43f6e26cec23a900ec16aab64bb8f6765c26d7aa37c56
1accfbc Output values for "min relay fee not met" error (Kristaps Kaupe)
Pull request description:
It is already done this way for "mempool min fee not met" error.
Tree-SHA512: 829db78ecc066cf93b8e93ff1aeb4e7b98883cf45f341d5be6e6b4dff4135f3f54fa49b3a6f12eb43f676a9ba54f981143c9887f786881e584370434a9566cfd
80a5e59 [qa] Attach node index to test_node AssertionError and print messages (James O'Beirne)
Pull request description:
In the midst of fighting with https://github.com/bitcoin/bitcoin/pull/12873 it became apparent that there're a number of assertions and print statements which are emitted by test nodes but don't identify the node in question. This change makes debugging a bit easier by adding identifying information to non-logger test_node-related error messages.
Tree-SHA512: 7cc86f2c81f4b3fdba15ec9a2d21a84c4b083629e845e82288087c3affbbdc5c68e74067621856cc97fe84fbc8cb4f5ca4977a51ef381e5d74515df8eb001239
fix 13022
Signed-off-by: pasta <pasta@dashboost.org>
aee80b0ef9 qt: Don't log to console by default (Wladimir J. van der Laan)
Pull request description:
Default `-printtoconsole` to false for the GUI. GUI programs should not print to the console unnecessarily. For example, when launched by the window manager, the output might end up in the X session log file,
resulting in duplicate logging. On Windows, it is pointless as well because bitcoin-qt isn't a console application.
This same mechanism is used to set `-server` to true by default for bitcoind: https://github.com/bitcoin/bitcoin/blob/master/src/bitcoind.cpp#L116
(fixes#13004)
Tree-SHA512: 24ae460d9d97130a063f7bf7fa6da1e6cc46643a94ea0827aa64d0f4a80647e5e7394695b24ea0f49a147a1fa07329659d224f04511fc24b97a9869d1c29b890
54a5a21 [MOVEONLY] Turn CScript::GetOp2 into a function and move to cpp (Pieter Wuille)
6a7456a [MOVEONLY] Move CSCript::FindAndDelete to interpreter (Pieter Wuille)
33a8ecf Delete unused non-const-iterator CSCript::GetOp overloads (Pieter Wuille)
2fb168b Make iterators in CScript::FindAndDelete const (Pieter Wuille)
Pull request description:
This PR moves `FindAndDelete` and `GetOp2` out of CScript (the first is only used inside the interpreter and moved there, the second does not actually depend on any script specifics and works on any vector). Furthermore, all non-const-iterator versions of GetOp are replaced by const ones, removing a number of methods in the process.
The longer term goal here is making the script interpreter independent from the CScript representation.
Note for reviewers: both `FindAndDelete` and `GetScriptOp` are consensus critical.
Tree-SHA512: c4ccf91c0b33c37cff0d474aa8dd2dab25b5b7655e2ed69a9b15e29daf0a67b21d51c23e1defb3a72ec762bd6138de96f69c6db1fb9c1fe1e976e421261aedb7
Ensure change is move only
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
fix assign_to
Signed-off-by: pasta <pasta@dashboost.org>
1f83839 [wallet] [tests] Test disallowed multiwallet params (John Newbery)
3476e3c [wallet] Fix zapwallettxes/multiwallet interaction. (John Newbery)
Pull request description:
`-zapwallettxes` should be disallowed when starting bitcoin in multiwallet mode.
There's code in `WalletInit::ParameterInteraction()` to disallow `-zapwallettxes` when running in multiwallet mode. This code functioned as expected when passing the parameter `-zapwallettxes=1`, but not when passing the parameter `-zapwallettxes` (ie without the value specified). Fix that and add a test.
The new test in the
_[wallet] [tests] Test disallowed multiwallet params_ commit reproduces the bug and should fail against master.
Fixes#12505
Tree-SHA512: 6cd921717e9c7d2773ca84c946c310c2adec8430e37cbecdb33a620f510db3058a72bd328411812ba415111bc52a3367b332c9d15a37a92ccfd7ae785d2f32ab
0851a75 rpc: Interrupt block generation on shutdown request (João Barbosa)
Pull request description:
With this simple change, after running `bitcoin-cli -regtest generate 100000`, it is possible to interrupt `bitcoind` cleanly without waiting for the generation to complete.
Tree-SHA512: f0f7cdde242e595cfdaea31ae8bddbc25933621b63f639e813d272c2b00ce2ef52f0c14ae44954ba8c49f0fc846bcc3bfd5419e52b3347a68bb0341ce6b02d26
e4d0b44 Consistently log CValidationState on failure (Ben Woosley)
Pull request description:
This replaces potential silent failures and partial logging with full logging. Seems providing at least minimal visibility to the failure is a good practice. E.g. `FlushStateToDisk` can return a rare but meaningful out of disk space error that would be better to note than leave out.
Note many of these are related to `ActivateBestChain` or `FlushStateToDisk`. Only a few cases of ignored state remain, e.g. LoadExternalBlockFile and RelayWalletTransaction, where I expect logging would likely be spammy.
Tree-SHA512: fb0e521039e5a5250cd9c82e7a8676423b5e3899d495649c0e71752059d1984e5175f556386ade048f51a7d59f5c8e467df7fe91d746076f97d24c000ccf7891
13020 continued
Signed-off-by: pasta <pasta@dashboost.org>
cf02779 Add logging and error handling for file syncing (Wladimir J. van der Laan)
Pull request description:
Add logging and error handling inside, and outside of FileCommit.
Functions such as fsync, fdatasync will return error in case of hardware I/O errors, and ignoring this means it can silently continue through data corruption.
(c.f. https://lwn.net/SubscriberLink/752063/12b232ab5039efbe/)
EINVAL is handled specially to avoid crashing out on (network, fuse) filesystems that don't handle `f[data]sync`.
I checked that the syncing inside leveldb is already generating an I/O error as appropriate.
Tree-SHA512: 64cc9bbedca3ecc97ff4bac0a7b7ac6526a7ed763c66f6786d03ca4f2e9e366e42b152cb908299c060448d98ca39ff03395280bffaca51d592e728aa2516f5dd
To reflect its actual status as a member rather than a global value.
g_failed_blocks was previously global: 2862aca
-BEGIN VERIFY SCRIPT-
sed -i 's/g_failed_blocks/m_failed_blocks/g' src/validation.cpp
-END VERIFY SCRIPT-
Signed-off-by: pasta <pasta@dashboost.org>
1bf3f33b46 node: Removed unused wallet-related methods from the Node interface. (Thomas Snider)
b38200459f benchmark: Removed bench/perf.cpp (Thomas Snider)
Pull request description:
Not sure if these should be separate PRs.
First is removal of a platform abstraction for getting cycle counters where possible. Since the benchmarking switch to counting number of iterations over a fixed window instead of counting cycles per iteration, these are unused.
Second is removal of a few methods from the Node interface that seem vestigial from when the concepts of wallet/node were not as clearly separated.
Tree-SHA512: de1460a7d4473ca19db4e2ca845185c63c765d12462c2685044a1f27dedab266cd908bc52235a881a7ad98bc251a4abf4eae523e5f599c169e3511e489f19a0d
ae1d2b030 Give an error when rescan is aborted by the user (Andrew Chow)
69b01e6f8 Add cancel button to rescan progress dialog (Andrew Chow)
Pull request description:
A cancel button is added to the `showProgress` dialog that is used only for rescans. When clicked, `AbortRescan` is called directly to cancel the rescan.
Rescans triggered from the debug console will now be cancelable by clicking the cancel button.
Rescans triggered by a command (e.g. `importmulti`) will now give an error indicating that the rescan was aborted by the user (either by the `abortrescan` command or by clicking cancel).
Tree-SHA512: 4bb14998766de686e2318fbc9805758eccf5dbe628a7257d072c9ae2fb4f61303a0876f49988d6e5eddb261969b8a307c81c0c2df0a42ae909a43d738af3dc1b
* Implement new hardfork for v17 to be used in future changes
Signed-off-by: pasta <pasta@dashboost.org>
* fix comment
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
fa72f34 bitcoin-tx: Remove unused for loop (MarcoFalke)
Pull request description:
This flattens out a for loop and gets rid of the then unused vector `txVariants`.
Tree-SHA512: 68081b313d846ce235a97a642c9d0097c3641350e819d6254001f332b053e41fa63ce49faca68120f5aaf5d5f4bfda104662eae781e2956d76a8915770344045
6cba60ace2 speed up wallet_hd.py and clarify/augment checks (Gregory Sanders)
Pull request description:
With `keypool=0` I see no reason to do 300 addresses and sends.
(with --enable-debug)
Before patch:
real 1m10.412s
user 0m49.772s
sys 0m3.988s
After:
real 0m11.566s
user 0m3.344s
sys 0m4.648s
Also added check, since I failed to understand that on startup the wallet already knows about funds by rescanning blocks newer than oldest key birthdate.
Tree-SHA512: cf90f7fe6a437b8b7b1f0707464b9c06085233167826f1a12c3871684664d4d572e13f03e13a718e4537cac39713271c4ac3d9b983e10080b50647caf3cbe82d
459ea58 rpc: Drop redundant testing of signrawtransaction prevtxs args (Ben Woosley)
Pull request description:
These other types are already tested on line 736.
Tree-SHA512: 2efe777c8a63c69ffe0fafcb2f37f134d324a8bc9525510f1079d2215535b511d6308e5e6eec702a3444f87701236c5e7a22f10bb24e5a454010ef421e5ae900
* Merge #18894: gui: Fix manual coin control with multiple wallets loaded
a8b5f1b133d4f23975a3fbfb7a415b17261466ee gui: Fix manual coin control with multiple wallets loaded (João Barbosa)
Pull request description:
This PR ensures each loaded wallet has a dedicated coin control in the send view which is manipulated by the coin control dialog.
This is an alternative to #17457. Two main differences are:
- scope reduced - no unnecessary changes unrelated to the fix;
- approach taken - coin control instance now belongs to the send view.
All problems raised in #17457 reviews no longer apply due to the approach taken - https://github.com/bitcoin/bitcoin/pull/17457#pullrequestreview-319297589 and https://github.com/bitcoin/bitcoin/pull/17457#issuecomment-555920829)
No change in behavior if only one wallet is loaded.
Closes#15725.
ACKs for top commit:
jonasschnelli:
utACK a8b5f1b133d4f23975a3fbfb7a415b17261466ee
ryanofsky:
Code review ACK a8b5f1b133d4f23975a3fbfb7a415b17261466ee. Code changes are very straightforward, just replacing global CCoinControl object with SendCoinsDialog member. Not sure if this means coin control settings are reset between payments. It would be good to note in the PR description or release notes if single wallet behavior is affected
hebasto:
ACK a8b5f1b133d4f23975a3fbfb7a415b17261466ee
Sjors:
tACK a8b5f1b133d4f23975a3fbfb7a415b17261466ee
Tree-SHA512: 3ad9c51bab6f28ec0e90efbd6f43fa510c81dafb2eff0b8c3724efcee3e030054a10be013e27cefe35763374c5f6d7af8c02658736964f733d7e38b646b5df65
* Update src/qt/coincontroldialog.cpp
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
Co-authored-by: Jonas Schnelli <dev@jonasschnelli.ch>
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
* contrib: Update makeseeds.py
The `dnspython` package complained about `dns.resolver.Resolver.query()` being deprecated and suggested to use `dns.resolver.Resolver.resolve()` instead.
* contrib: Update dependency install instructions in seeds/README.md
* Rename QToolButton-s
* Move buttons from createActions() to createToolBars()
Also, drop `showNormalIfMinimized()` connect for buttons cause it makes no sense
* Tweak `BitcoinGUI` to let `updateToolBarShortcuts()` work with `QButtonGroup` only
b6c748f84909212dce73e4b77aa125ed1e108a10 doc: Add release notes for 15730 (João Barbosa)
d3e8458365ab29017241bc43204fe81cb7fd8530 rpc: Show scanning details in getwalletinfo (João Barbosa)
90e27abe37cc84c7b206f20d28aafe32e71e7209 wallet: Track current scanning progress (João Barbosa)
2ee811e6930cf76ea51e6826fe437ed888688adc wallet: Track scanning duration (João Barbosa)
Pull request description:
Closes#15724.
ACKs for commit b6c748:
MarcoFalke:
re-utACK b6c748f849 (Only change since my last review is rebase, adding release notes, and returning false instead of null)
laanwj:
utACK b6c748f84909212dce73e4b77aa125ed1e108a10
jonatack:
ACK b6c748f84909212dce73e4b77aa125ed1e108a10, only changes appear to be rebase for https://github.com/bitcoin/bitcoin/pull/15730#discussion_r280030617 and release notes.
Tree-SHA512: 8ee98f971c15f66ce8138fc92c55e51abc9faf01866a31ac7ce2ad766aa2bb88559eabee3b5815d645c84cdf1c19dc35ec03f31461e39bc5f6040edec0b87116
Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
0000d8f Document how FlushStateMode::NONE is handled (practicalswift)
2311c7c Call FlushStateToDisk(...) regardless of fCheckForPruning (practicalswift)
Pull request description:
FlushStateToDisk(...) won't do anything besides check if we need to prune if
FLUSH_STATE_NONE is given. We avoid reading the variable fCheckForPruning
which is guarded by the mutex cs_LastBlockFile.
Tree-SHA512: 89df06256f73503a74b9e26d580ce9ed09efaef347fae1ff6a5759a2993b0db52edd2fadb65694d27e579a5aed92127753bdf41b5bb1bd516e577fcf17f17999
12e7c55 Ignore macOS daemon() depracation warning (Jonas Schnelli)
Pull request description:
`daemon()` is deprecated on OSX since 10.5 (should migrate to `posix_spawn()`). There are no signs `daemon()` will get removed by Apple.
Tree-SHA512: d5bcdc5d6b507576e0358906a73f9c766f2072f4a9aef6bdc559e10dbec95337ffa50a1ccb60f7197591e2e74f87c74c13387de880aaedc6dbf3796253f69561
9b9d717263 wallet: Remove redundant lambda function arg in handleTransactionChanged (Wladimir J. van der Laan)
Pull request description:
Makes the build warning-clean again here:
bitcoin/src/interfaces/wallet.cpp:425:18: warning: lambda capture 'this' is not used [-Wunused-lambda-capture]
[fn, this](CWallet*, const uint256& txid, ChangeType status) { fn(txid, status); }));
^
Tree-SHA512: 82362c8fb3638246a6c3b28998165eed8a6490f9e13cf2741fb2776ef7f82a1c0bb5ea1f58b7ff842bd59c9830f59f5a478fb63860693055c4d8dd72a94d8a5a
9142dfea81 Use explicit casting in cuckoocache's compute_hashes(...) to clarify integer conversion (practicalswift)
Pull request description:
Use explicit casting in cuckoocache's `compute_hashes(...)` to clarify integer conversion.
I discussed this code with the code's author @JeremyRubin who suggested patching it to avoid any confusion.
At least one static analyzer incorrectly warns about a shift past bitwidth (UB) here, so this patch will help avoid confusion for human reviewers and static analyzers alike :-)
Tree-SHA512: 0419ee31b422d2ffedbd1a100688ec0ff5b0c1690d6d92592f638ca8db07a21a9650cb467923108c6f14a38d2bf07d6e6c85d2d1d4b7da53ffe6919f94f32655
b120f7b [test] Add tests for self usage in arith_uint256 (Karl-Johan Alm)
08b17de [arith_uint256] Do not destroy *this content if passed-in operator may reference it (Karl-Johan Alm)
Pull request description:
Before this fix (see test commit), `v *= v` would result in `0` because `operator*=` set `*this` (`==b`) to `0` at the start. This patch changes the code to use `a` as temporary for `*this`~~, with drawback that `*this` is set to `a` at the end, an extra `=` operation in other words~~.
Tree-SHA512: 8028a99880c3198a39c4bcc5056169735ba960625d553e15c0317510a52940c875f7a1fefe14e1af7fcf10c07a246411994a328cb1507bf3eaf1b6e7425390dc
01189ab9ae build: Show enabled sanitizers in configure output (practicalswift)
Pull request description:
Show enabled sanitizers in `configure` output.
Context: @eklitzke excellent addition of `--with-sanitizers` in #12692
Tree-SHA512: b2d52308e3476488fe47cbc059d7f3235aaeefaa2b987003923f6eaacbadf67f0cf22a32a04873d0f54c1867757841e01c8053f86a4d2f59a407b960ac15105f
0e7c52d Shut down if trying to connect a corrupted block (Suhas Daftuar)
Pull request description:
(Updated OP after reworking the approach)
Shut down if a corrupted block is found in ConnectBlock(). This prevents an infinite loop trying to connect such a block, and alerts the node operator that there may be potential hardware failure.
Tree-SHA512: f20d56aa9d36d6eeff4c3d13c0fbd14f06a57701bd13c2416d36f0cc4235f81f752139e336a073617e8e803782c5096c960108af122b19a51227de512e9095ee
4d74c78 Add username and ip logging for RPC method requests (Gabriel Davidian)
Pull request description:
Adds username and IP logging (if enabled via -logips command) to RPC method request logging.
This closes#12223
Tree-SHA512: a441228e80ea6884ec379c66e949d86df3689770f1b3c3608015cf5a36d2dfb38051298a7f6ea6dfdfbf0b3b6c896e414c8dc54e9833bb73dd65bdb1832f4395
fc7c32fc6 do not truncate .dat extension for wallets in gui (Gregory Sanders)
Pull request description:
Truncating the extension results in wallet name ambiguity and the inability to use the wallet in GUI debug rpc console.
Resolves#12794
Tree-SHA512: 89507918f597e9274148b45233b893c9f653da4f9e929415822165d47c67b55ad0b2d5ff53b508e942831d5213d5c15bce3fbdfbcb592a5c7f3dd5c1ca02cfb8
