Age | Commit message (Collapse) | Author |
|
|
|
|
|
This test can now be run even with the Bitcoin Core wallet disabled.
|
|
e844115deae8a11c591704a84e56ed45fa81409c test: use MiniWallet for rpc_scantxoutset.py (Sebastian Falbesoner)
983ca0456c0fd32dc6ce166cb1e9aeb925e81161 test: introduce `address_to_scriptpubkey` helper (Sebastian Falbesoner)
e704d4d26f62f83068dc7d4082ab9b57bc33d1fb test: introduce `getnewdestination` helper for generating various address types (Sebastian Falbesoner)
Pull request description:
This PR enables one more of the non-wallet functional tests (rpc_scantxoutset.py) to be run even with the Bitcoin Core wallet disabled by using the MiniWallet instead, as proposed in #20078 and https://github.com/bitcoin/bitcoin/pull/23858#issue-1088320649 more recently.
Reviewer's guide:
* [commit 1/3] For replacing the getnewaddress/getaddressinfo RPC calls a helper `getnewdestination` is introduced which allows to create addresses with the common address format types ('legacy', 'p2sh-segwit' and 'bech32'), but also additionally returns the corresponding pubkey and output script.
* [commit 2/3] In order to send to addresses with MiniWallet, a helper `address_to_scriptpubkey` is introduced. It only supports legacy addresses (Base58Check) so far, which is sufficient for the scantxoutset test.
* [commit 3/3] With those helpers, the use of MiniWallet in the test is quite straight-forward. To avoid repeatedly specifying parameters like `from_node` to MiniWallet's `send_to` method, another test-internal helper `sendtodestination` is introduced which supports specifying the destination both as outputscript or as address.
ACKs for top commit:
w0xlt:
reACK [e844115](https://github.com/bitcoin/bitcoin/pull/23866/commits/e844115deae8a11c591704a84e56ed45fa81409c)
Tree-SHA512: e4823dc507019b2b8e479602963c5dddc4c78211e1d934218ee0f0e32c068ab7e44e9793307f549127946364f57d684c4ea1d70f25865ea70d30d4f3855836d0
|
|
d3b0f82a438ae60060d0d4d83245bf75f5de433d build: Fix regression introduced in PR23603 (Hennadii Stepanov)
Pull request description:
It appears 7629efcc2c3a8a8a7c17b1300cd466ec6c8c1f3f from bitcoin/bitcoin#23603 introduced a regression in build tool flag evaluation.
On macOS system:
- pre-PR23603 master (ae017b81604761b57e22c28913c4ce81bf2e31ce):
```
% make -C depends print-darwin_CXXFLAGS
darwin_CXXFLAGS=-pipe
% make -C depends print-host_CXXFLAGS
host_CXXFLAGS=-pipe
```
- the current master (369978686e156ad34df703f1e60bd90aeaa8f2d6):
```
% make -C depends print-darwin_CXXFLAGS
darwin_CXXFLAGS=-pipe
% make -C depends print-host_CXXFLAGS
host_CXXFLAGS=-arch x86_64
```
It's obvious a flag being set in `depends/hosts/darwin.mk`, i.e., `-pipe`, is lost.
With this PR:
```
% make -C depends print-darwin_CXXFLAGS
darwin_CXXFLAGS=-pipe
% make -C depends print-host_CXXFLAGS
host_CXXFLAGS=-pipe -arch x86_64
```
ACKs for top commit:
fanquake:
ACK d3b0f82a438ae60060d0d4d83245bf75f5de433d
Tree-SHA512: 643099ce6858475ac9f3a4dfa72a4e493fec6fdd7042ae0f0d5fe44c5cd175e4eda63cb39fc46ac1501cadcd3466507ec88d9089235e005fe43ea7ab47ce37c1
|
|
Qt RCC
11736dbe3dbe34bcb430d05810b9e9aa66ec1cd6 build, qt: Hardcode last modified timestamp in Qt RCC (Hennadii Stepanov)
Pull request description:
Our Guix build system sets the [`SOURCE_DATE_EPOCH`](https://reproducible-builds.org/specs/source-date-epoch/) and propagates it to the depends build subsystem. Its [default value](https://github.com/bitcoin/bitcoin/blob/master/contrib/guix/README.md#recognized-environment-variables) is the top commit timestamp.
After bumping Qt version up to 5.15.2, due to [this](https://github.com/qt/qtbase/commit/1ffcca4cc208c48ddb06b6a23abf1756f9724351) change, every time they are going to make new Guix builds for another branch/commit they must ensure that the `qt` package will be rebuilt from scratch. Otherwise, Bitcoin Core GUI binaries will be non-deterministic.
Such behavior makes working with Guix builds suboptimal.
This PR fixes the described issue by patching Qt RCC and hardcoding last modified timestamps with `1`.
It's worth to mention that this change is compatible with a possible future [improvement](https://github.com/bitcoin/bitcoin/pull/21995) which makes each dependency package reproducible.
A drawback of such an approach is not currently applied to our project, as it effectively makes [QML cache files](https://bugreports.qt.io/browse/QTBUG-57182) useless. I can't say it's a problem for the https://github.com/bitcoin-core/gui-qml project.
---
**A note for thinkers:** For now this change is enough as only Qt source contains `SOURCE_DATE_EPOCH`. But in general we should re-think about treating the `SOURCE_DATE_EPOCH` variable in the depends build subsystem. For instance, its default value could be the output of `git log --format=%at -1 -- depends`.
ACKs for top commit:
fanquake:
ACK 11736dbe3dbe34bcb430d05810b9e9aa66ec1cd6
Tree-SHA512: 31f104010a0a78d217aafcc5bc4606351f9060fc2a827277935b85fc8ced9f3d90a31d812c7db8c2711fb6daccd279cf0945dc1d7a7199e0eb0ade451cdbcd5d
|
|
After parsing the checksum, make sure that it is the size that we expect
it to be.
|
|
This test can now be run even with the Bitcoin Core wallet disabled.
|
|
94a7f09de6ed1899372bf14f6202629e27dc1512 doc: Drop outdated note (Hennadii Stepanov)
Pull request description:
It is outdated since bitcoin/bitcoin#23060.
ACKs for top commit:
MarcoFalke:
cr ACK 94a7f09de6ed1899372bf14f6202629e27dc1512
theStack:
Code-review ACK 94a7f09de6ed1899372bf14f6202629e27dc1512
Tree-SHA512: 5a98cf39c3939845d1abb64cb62dbb9bdb1135aa07a5cd0d85de7b6a6d9a3397b0e5da06cdb89991aac31151cb3c0d38eacf4898c53014cb7f7600e992230f87
|
|
Works only with legacy addresses (Base58Check) right now.
|
|
This serves as a replacement for the getnewaddress RPC if no wallet is
available. In addition to the address, it also returns the corresponding
public key and output script (scriptPubKey).
|
|
fafe4dea16c93ac8d5cd5eee872dfb8427d7809a test: Fix pep8 of touched file (MarcoFalke)
fa0ac9d7e36ec616cd673b9041f0e54b9f32d5b0 test: Fix rpc_scantxoutset intermittent issue (MarcoFalke)
Pull request description:
I fail to see how this could have ever worked, since there is nothing that prevents the wallet from spending the coins in the utxo set.
Fixes https://github.com/bitcoin/bitcoin/issues/23847
Longer term it would be nice to remove the wallet and use MiniWallet here.
ACKs for top commit:
brunoerg:
tACK fafe4dea16c93ac8d5cd5eee872dfb8427d7809a
theStack:
Code-review ACK fafe4dea16c93ac8d5cd5eee872dfb8427d7809a
Tree-SHA512: d92b7be9a81eb62f496488dd15b8e564e9b7a64b55634af2714b53f985e6a35fdae788323833ff59c40ae7c6a0cf54fe069db8eb3be03686f7c100379f54a292
|
|
This change allows the already built qt package to be reused even with
the SOURCE_DATE_EPOCH variable set, e.g., for Guix builds.
|
|
It is outdated since bitcoin/bitcoin#23060.
|
|
crypto_diff_fuzz_chacha20.cpp
faaf9da20a2920fe52ec4efa1e9a07090c089cb3 test: Add missing suppressions for crypto_diff_fuzz_chacha20.cpp (MarcoFalke)
Pull request description:
Without them, CI and fuzzing is broken
ACKs for top commit:
stratospher:
ACK faaf9da. The changes look good to me.
Tree-SHA512: 46604ba1b6eb8ea321e55e043835ca1e7a4f562855007b08cf9cf96d9a7f2a0155009ca1397f34338e478b8cc5b5a6a0dc94e1878a2f00bfab11595ae58bb014
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
Assignments in builders/darwin.mk actually override previous assignments
in hosts/default.mk. Therefore, the append operator must be used
instead.
|
|
Can be reviewed with --word-diff-regex=. --ignore-all-space
|
|
|
|
parameters
c27bba96723384488fb6995e21bb43969d94b0f6 test: check for invalid listtransactions RPC parameters (Sebastian Falbesoner)
Pull request description:
This PR adds missing test coverage for RPC errors that are thrown if invalid parameters are passed to `listtransactions`:
https://github.com/bitcoin/bitcoin/blob/887796a5ffcbafcd281b920f8d55fcb6e8347584/src/wallet/rpc/transactions.cpp#L508
https://github.com/bitcoin/bitcoin/blob/887796a5ffcbafcd281b920f8d55fcb6e8347584/src/wallet/rpc/transactions.cpp#L524
https://github.com/bitcoin/bitcoin/blob/887796a5ffcbafcd281b920f8d55fcb6e8347584/src/wallet/rpc/transactions.cpp#L526
ACKs for top commit:
shaavan:
ACK c27bba96723384488fb6995e21bb43969d94b0f6
Tree-SHA512: e5a23590186b4d9663261ff6cea52ac45e9bf2f2ef693c22b3452bb07af9b800fdabc2a94fd2852c686c28214a496d7afe296e41831759f2182feac2482635d0
|
|
7b481f015a0386f5ee3e13415474952653ddc198 Fix Racy ParseOpCode function initialization (Jeremy Rubin)
Pull request description:
If multiple callers call ParseOpCode concurrently it will cause a race condition. We can either move the static to it's own area and require init be called explicitly, or just allow concurrent first callers to race to fill in an atomic variable that never changes thereafter. The second approach is taken here.
Static initialization *is* threadsafe, but only insofar as definining the variable being guaranteed to be called once. This is used incorrectly here.
practicalswift --> are there tools we can deploy to catch usage like this?
ACKs for top commit:
MarcoFalke:
re-ACK 7b481f015a0386f5ee3e13415474952653ddc198 🗣
Tree-SHA512: cbf9dc3af26d7335305026f32ce8472a018309b89b3d81a67357e59fbeed72c37b5b8a6e30325ea68145c3b2403867be82de01f22decefb6e6717cf0c0045633
|
|
|
|
|
|
|
|
|
|
|
|
native_qt5 CI
2da97b271b23225c394217ebc7b37999e34e345e ci: use GCC 8 when building packages in native_qt5 CI (fanquake)
Pull request description:
Our minimum required GCC is GCC 8, and this change in required for
PRs like #23839 which take advantage of flags introduced in that
version of GCC.
This should have been included as part of 182de7ba10811ec39e24ec5bec7cd2119f776f2f.
ACKs for top commit:
MarcoFalke:
cr ACK 2da97b271b23225c394217ebc7b37999e34e345e
hebasto:
ACK 2da97b271b23225c394217ebc7b37999e34e345e
Tree-SHA512: 2b64c21119fb95b959ac0f7d8d2d38f6ed98309695bb35425fad45b3b628247c2c78d0647c4d1f511669e8d333c6febe1cc44fac8027ed0bd7593eb630add548
|
|
to interfaces::WalletLoader
ff5f6dea5336075d1a6cace1112be3252d97540b scripted-diff: Rename interfaces::WalletClient to interfaces::WalletLoader (Russell Yanofsky)
Pull request description:
Name has been confusing since it was introduced, and it was pointed in recent review club https://bitcoincore.reviews/10102 that it was particularly unclear how `interfaces::WalletClient` was different from `interfaces::Wallet`.
ACKs for top commit:
w0xlt:
ACK ff5f6de
Tree-SHA512: 26fa10baa457e76da1933adab187e9be61b8d76cff1cf2c73ad4320461c7e31fb9db07b7c2486998294826beb4a1aca255c14903920b443db6213e653c5f7e0a
|
|
|
|
ChainTestingSetup
826e12b010eda4238f9e8cd875e8915a405bed0d test: call VerifyLoadedChainstate during ChainTestingSetup (James O'Beirne)
Pull request description:
for additional coverage and similarity to actual init process.
Followup to #23280.
ACKs for top commit:
dongcarl:
Code Review ACK 826e12b010eda4238f9e8cd875e8915a405bed0d
ryanofsky:
Code review ACK 826e12b010eda4238f9e8cd875e8915a405bed0d
Tree-SHA512: a4e7fd25e5d7a08b1e154ae6daf67c3048260a2684b0e569b544dd826693b7b969db9923b191e499cb8d8d0a2a73eb9330ff45909313145a9abb6052eb8c3ad9
|
|
Our minimum required GCC is GCC 8, and this change in required for
changes like #23839 which take advantage of flags introduced in that
version of GCC.
This should have been included as part of
182de7ba10811ec39e24ec5bec7cd2119f776f2f.
|
|
|
|
|
|
|
|
|
|
|
|
Name has been confusing since it was introduced, and it was pointed in
recent review club as https://bitcoincore.reviews/10102 that it was
particularly unclear how interfaces::WalletClient was different from
interfaces::Wallet.
-BEGIN VERIFY SCRIPT-
ren() { git grep -l "$1" src | xargs sed -i "s/$1/$2/g"; }
ren WalletClient WalletLoader
ren walletClient walletLoader
ren wallet_client wallet_loader
ren "wallet clients release the wallet" "wallet pointer owners release the wallet"
ren "wallet client" "wallet loader"
ren "Wallet client" "Wallet loader"
-END VERIFY SCRIPT-
|
|
c9a77e227eecf357c7dd550efb8c1827bb99a5de gui: address type dropdown, add bech32m (Sjors Provoost)
56113daef4830cad1af3c00f6b3c447c9e2a8e05 wallet: add taprootEnabled() to interface (Sjors Provoost)
Pull request description:
This PR replaces the Bech32 checkbox with an address type dropdown It "Use Taproot" checkbox to the receive screen for any wallet with a Taproot descriptor. The Bech32m option is hidden for wallets that don't have taproot descriptors.
Bech32 is kept as the default even for Taproot enabled wallets until it's more widely supported.
(an earlier attempt of this PR added a second checkbox)
<img width="469" alt="Schermafbeelding 2021-12-21 om 11 44 05" src="https://user-images.githubusercontent.com/10217/146872660-339fae1f-c0b8-4673-b8be-33c25f3933fd.png">
**Suggested testing**
* notice that the Bech32m entry only appears for wallet with a taproot descriptor
* verify that it generates a bc1p instead of bc1q address
1. Legacy wallet
2. Default descriptor wallet (current does not have taproot descriptor)
3. Wallet with taproot descriptor:
* just create a new descriptor wallet
Replaces https://github.com/bitcoin/bitcoin/pull/22260
ACKs for top commit:
Rspigler:
tACK c9a77e227eecf357c7dd550efb8c1827bb99a5de. I like the changes made now, thanks!
kristapsk:
re-ACK c9a77e227eecf357c7dd550efb8c1827bb99a5de
Tree-SHA512: bae66ac90ed55372e6c94878db0e37d32b7b5c24ed00c0f2ebb10fd127dddce3a061162df915d67e92d7b35b3da093137db17b73931a0ae1a470fff20be1f30b
|
|
This was forgotten in commit 3ac38058ce0e80a9f4276bfa82951decdb237e9a
|
|
Co-authored-by: João Barbosa <joao.paulo.barbosa@gmail.com>
|
|
|
|
wallet backups
a2a92317ad4ab88cfca234f68337a7cd37897f10 rpc: Add warning to user about newkeypool command (Samuel Dobson)
Pull request description:
This PR prevents `newkeypool` from being run on non-HD wallets, because this would require a new backup every time, so it isn't very safe.
David Harding also suggested [here](https://github.com/bitcoin/bitcoin/pull/23093#issuecomment-945350003) that the RPC help text should include a warning to the users about the interaction between newkeypool.
ACKs for top commit:
achow101:
ACK a2a92317ad4ab88cfca234f68337a7cd37897f10
Tree-SHA512: 0aa497900f1d179764bce13ffce0bb081ba2ca354492bf2e04b21d0212e960b3ed13a386fbf65602b6b50461f4056a0285752ef707d312da28e82449cd8ea048
|
|
improve privacy
fada6c65d23f7bd4682fac281026612b835c4f8b wallet: Strictly match tx change type to improve privacy (MarcoFalke)
Pull request description:
Currently the change type will only match a destination by accident, making it easier to determine the change.
Fix that by strictly matching one of the destinations.
ACKs for top commit:
S3RK:
Concept & Approach ACK fada6c6. Also did light code review .
achow101:
ACK fada6c65d23f7bd4682fac281026612b835c4f8b
prayank23:
tACK https://github.com/bitcoin/bitcoin/pull/23789/commits/fada6c65d23f7bd4682fac281026612b835c4f8b
w0xlt:
tACK fada6c6
Tree-SHA512: 2b072c3c32debac7b0bef07a6df9a8f1a631e0f7d556b859973f18894ca490225582dc13e4588b29fa205ffbcd30fb632d5313b304d10ad17a26adc3f7684471
|