Age | Commit message (Collapse) | Author |
|
Can be easily reviewed with `--color-moved=dimmed-zebra`.
|
|
Can be easily reviewed with `--color-moved=dimmed-zebra`.
|
|
-BEGIN VERIFY SCRIPT-
sed -i s/FromBinary/from_binary/g ./contrib/signet/miner
-END VERIFY SCRIPT-
|
|
Pass the whole CoinsResult struct to SelectCoins instead of only a
vector. This means we now have to remove preselected coins from each
OutputType vector and shuffle each vector individually.
Pass the whole CoinsResult struct to AttemptSelection. This involves
moving the logic in AttemptSelection to a newly named function,
ChooseSelectionResult. This will allow us to run ChooseSelectionResult
over each OutputType in a later commit. This ensures the backoffs work
properly.
Update unit and bench tests to use CoinResult.
|
|
Store COutputs by OutputType in CoinsResult.
The struct stores vectors of `COutput`s by `OutputType`
for more convenient access
|
|
IWYU confirms that they are unused
|
|
|
|
They are needed, otherwise the next commit will not compile
|
|
This is needed for the next commit
|
|
d68ca4ef64405d0d7c628a71d0f2bbae68e63cda Fix `-Wparentheses` gcc warning (Hennadii Stepanov)
Pull request description:
This PR fixes `-Wparentheses` gcc warning which has been introduced in bitcoin/bitcoin#25624.
On the master branch (6d8707b21d3a5b4abb76b773a69f2bfac22bcd93):
```
$ gcc --version
gcc (Ubuntu 11.2.0-19ubuntu1) 11.2.0
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
$ make > /dev/null
In file included from ./net.h:29,
from ./net_processing.h:9,
from test/fuzz/txorphan.cpp:7:
test/fuzz/txorphan.cpp: In lambda function:
test/fuzz/txorphan.cpp:116:70: warning: suggest parentheses around comparison in operand of ‘==’ [-Wparentheses]
116 | Assert(!have_tx == GetTransactionWeight(*tx) > MAX_STANDARD_TX_WEIGHT);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
./util/check.h:74:50: note: in definition of macro ‘Assert’
74 | #define Assert(val) inline_assertion_check<true>(val, __FILE__, __LINE__, __func__, #val)
| ^~~
```
ACKs for top commit:
MarcoFalke:
ACK d68ca4ef64405d0d7c628a71d0f2bbae68e63cda
Tree-SHA512: 5c98df4d6a6124d048b16eb3caf29bb396223d3394c1f48efc0fe0c8fd334d67dbf64d0b2e40faf9eda6f6a537885abcff05c61e410cfb317737e3dc361791ee
|
|
This fixes a non-determinism issue in the asm produced for
this function when cross-compiling on x86_64 and aarch64 for
the arm-linux-gnueabihf HOST.
Related to #21194.
|
|
|
|
The descriptor wallets allow an application to track coins of multiple
descriptors in a single wallet. However, such an application would not
previously be able to (easily) tell what received coin "belongs" to what
descriptor.
This commit tackles this issues by adding a "wallet_desc" entry to the
entries for received coins in 'listsinceblock'.
|
|
messages
fae5ce8795080018875227aee8613677f92e99ce univalue: Return more detailed type check error messages (MacroFake)
fafab147e7ff41ab1b961349f20a364f6bf847d2 move-only: Move UniValue::getInt definition to keep class with definitions only (MacroFake)
Pull request description:
Print the current type and the expected type
ACKs for top commit:
aureleoules:
ACK fae5ce8795080018875227aee8613677f92e99ce.
Tree-SHA512: 4ae720a012ff8245baf5cd7f844f93b946c58feebe62de6dfd84ebc5c8afb988295a94de7c01aef98aaf4c6228f7184ed622f37079c738924617e0f336ac5b6e
|
|
|
|
|
|
4c495413e138ec1dd6874e41b44e689f0c15e0e3 Disallow encryption of watchonly wallets (Andrew Chow)
Pull request description:
Watchonly wallets do not have any private keys to encrypt. It does not make sense to encrypt such wallets, so disable the option to encrypt them.
This avoids an assertion that can be hit when encrypting watchonly descriptor wallets.
As our current behavior allows for encrypting watchonly wallets (no crash with legacy, crash, but still encrypted with descriptors), the new `NoKeys` status is only returned for unencrypted watchonly wallets. This allows any watchonly wallets that were previously encrypted to show the correct encryption status (they have encryption keys, and so should be indicated as being encrypted).
ACKs for top commit:
w0xlt:
tACK https://github.com/bitcoin-core/gui/pull/631/commits/4c495413e138ec1dd6874e41b44e689f0c15e0e3
hebasto:
ACK 4c495413e138ec1dd6874e41b44e689f0c15e0e3, tested on Ubuntu 22.04.
Tree-SHA512: 054dba0a8c1343a0df17689508cd628a974555828955a3c8820bf020868b95a3df98c47253b0ffe2252765b020160bb76ea21647d76d59ba748b3b41c481f2ae
|
|
inputs' UTXOs
d2ed97656bba050051cfc677f1fa7eb3fc633f7d wallet: Precompute Txdata after setting PSBT inputs' UTXOs (Andrew Chow)
Pull request description:
If we are given a PSBT that is missing one or more input UTXOs, our
PrecomputedTransactionData will be incorrect and missing information
that it should otherwise have, and therefore we may not produce a
signature when we should. To avoid this problem, we can do the
precomputation after we have set the UTXOs the wallet is able to set for
the PSBT.
Also adds a test for this behavior.
ACKs for top commit:
instagibbs:
reACK https://github.com/bitcoin/bitcoin/pull/25590/commits/d2ed97656bba050051cfc677f1fa7eb3fc633f7d
Sjors:
ACK d2ed97656bba050051cfc677f1fa7eb3fc633f7d
aureleoules:
ACK d2ed97656bba050051cfc677f1fa7eb3fc633f7d.
Tree-SHA512: 71beb6c7946096e82cfca83f36277302aa9e69d27b4f6d73d7d8f2f9f0ea1c0d653e846fa6aebee5e4763f56f950b4481240e953f6a2412caa84908d519171e1
|
|
|
|
|
|
LTO (with GCC)
658685af935e89ddd2ca326b3e13e271d166014d depends: default to using GCC tool wrappers (with GCC) (fanquake)
6fdc13c61f0e5ef6be0c03fa475741ab2cd57672 build: Fix autoconf variable names for tools found by `AC_PATH_TOOL` (Hennadii Stepanov)
Pull request description:
This improves support for LTO by using gcc wrappers for `ar`, `nm`, `ranlib`,
that correctly setup plugin arguments for LTO, when using GCC.
Other HOSTS are using clang.
Portion of #25391.
Guix Build (x86_64):
```bash
```
Guix Build (arm64):
```bash
```
ACKs for top commit:
dongcarl:
Code Review ACK 658685af935e89ddd2ca326b3e13e271d166014d
hebasto:
ACK 658685af935e89ddd2ca326b3e13e271d166014d
jarolrod:
ACK 658685af935e89ddd2ca326b3e13e271d166014d
Tree-SHA512: 28d6127c118f74336c97e2523117f8a0d11b32cd565124cd4052baeb7cc53e71909d3037cb080d996ae4e3ce600326fced37ee36adcc53d839ba7dd7974ebcd2
|
|
a02f3f19f52e628248f81acc2410e67f3d49baf5 tidy: use misc-unused-using-decls (fanquake)
d6787bc19b1032d3f46a60625105f30199c41b00 refactor: remove unused using directives (fanquake)
3617634324d647956c621db407db6d82a91b91ec validation: remove unused using directives (eugene)
Pull request description:
Adds https://clang.llvm.org/extra/clang-tidy/checks/misc/unused-using-decls.html to our clang-tidy.
PR'd after the discussion in #25433 (which it includes).
ACKs for top commit:
jamesob:
Github ACK https://github.com/bitcoin/bitcoin/pull/25466/commits/a02f3f19f52e628248f81acc2410e67f3d49baf5
Tree-SHA512: 2bb937c1cc90006e69054458d845fb54f287567f4309c773a3fc859f260558c32ff51fc1c2ce9b43207426f3547e7ce226c87186103d741d5efcca19cd355253
|
|
CNode::nLocalServices to Peer
8d8eeb422e64ccd08bce92eed2ee9cbbf55ffd67 [net processing] Remove CNode::nLocalServices (John Newbery)
5961f8eea1ad5be1a4bf8da63651e197a20359b2 [net] Return CService from GetLocalAddrForPeer and GetLocalAddress (dergoegge)
d9079fe18dc5d81ce290876353555b51125127d1 [net processing] Remove CNode::nServices (John Newbery)
7d1c0369340cb752f0d78e24f4251595534bf5e9 [net processing] Replace fHaveWitness with CanServeWitnesses() (John Newbery)
f65e83d51bfb6a34f1d5efccfb3d8786a51a4534 [net processing] Remove fClient and m_limited_node (John Newbery)
fc5eb528f7d7b33e2f2e443c5610a1551c7f099b [tests] Connect peer in outbound_slow_chain_eviction by sending p2p messages (John Newbery)
1f52c47d5c09b59fd3153700751c74e63edc7d7e [net processing] Add m_our_services and m_their_services to Peer (John Newbery)
Pull request description:
Another step in #19398. Which services we offer to a peer and which services they offer to us is application layer data and should not be stored on `CNode`.
This is also a prerequisite for adding `PeerManager` unit tests (See #25515).
ACKs for top commit:
MarcoFalke:
ACK 8d8eeb422e64ccd08bce92eed2ee9cbbf55ffd67 🔑
jnewbery:
utACK 8d8eeb422e64ccd08bce92eed2ee9cbbf55ffd67
mzumsande:
Code Review ACK 8d8eeb422e64ccd08bce92eed2ee9cbbf55ffd67
Tree-SHA512: e772eb2a0a85db346dd7b453a41011a12756fc7cbfda6a9ef6daa9633b9a47b9770ab3dc02377690f9d02127301c3905ff22905977f758bf90b17a9a35b37523
|
|
817326a828d6148dc63d9ef08f641b9c0c522411 wallet: avoid rescans if under the snapshot (James O'Beirne)
Pull request description:
This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11) (parent PR: #15606)
---
Refuse to load a wallet if it requires a rescan lower than the height of assumed-valid blocks.
Of course in live code right now, `BLOCK_ASSUMED_VALID` block index entries don't exist since they're a unique flag introduced by the use of UTXO snapshots, so this is prophylactic code exercised only by unittests.
ACKs for top commit:
achow101:
ACK 817326a828d6148dc63d9ef08f641b9c0c522411
ryanofsky:
Code review ACK 817326a828d6148dc63d9ef08f641b9c0c522411. This seems like the simplest change we can make to avoid wallet problems when an assumeutxo snapshot is loaded.
Tree-SHA512: cfa44b2eb33d1818d30df45210d0dde1e9b78cc9b7c88cb985054dc28427bba9e0905debe4196065d1d3a5ce7bca7e605e629d5ce5f0225b25395746e6d3d596
|
|
Replace CommitInternal method with CustomCommit and use interfaces::Chain
instead of CChainState to generate block locator.
This commit does not change behavior in any way, except in the
(m_best_block_index == nullptr) case, which was added recently in
https://github.com/bitcoin/bitcoin/pull/24117 as part of an ongoing attempt to
prevent index corruption if bitcoind is interrupted during startup. New
behavior in that case should be slightly better than the old behavior (skipping
the entire custom+base commit now vs only skipping the base commit previously)
and this might avoid more cases of corruption.
|
|
Replace Rewind method with CustomRewind and pass block hashes and
heights instead of CBlockIndex* pointers
This commit does not change behavior in any way.
|
|
Replace WriteBlock method with CustomAppend and pass BlockInfo struct
instead of CBlockIndex* pointer
This commit does not change behavior in any way.
|
|
Replace overriden index Init() methods that use the best block
CBlockIndex* pointer with pure CustomInit() callbacks that are passed
the block hash and height.
This gets rid of more CBlockIndex* pointer uses so indexes can work
outside the bitcoin-node process. It also simplifies the initialization
call sequence so index implementations are not responsible for
initializing the base class.
There is a slight change in behavior here since now the best block
pointer is loaded and checked before the custom index init functions are
called instead of while they are called.
|
|
This commit does not change behavior in any way.
|
|
Passing abstract Chain interface will let indexes run in separate
processes.
This commit does not change behavior in any way.
|
|
notifications
Add new interfaces::BlockInfo struct to be able to pass extra block
information (file and undo information) to indexes which they are
updated to use high level interfaces::Chain notifications.
This commit does not change behavior in any way.
|
|
Second attempt
1be796418934ae7370cb0ed501877db59e738106 test, wallet: Add mempool rescan test for import RPCs (Fabian Jahr)
833ce76df712932c19e99737e87b5569e2bca34b rpc, wallet: Document mempool rescan after importdescriptor, importwallet (Fabian Jahr)
0e396d1ba701c9ac6280a98bf37f53352167e724 rpc, wallet: Document mempool scan after importmulti (Fabian Jahr)
e6d3ef85867545a5a66a211e35e818e8a1b166fa rpc, wallet: Document mempool scan after importpubkey (Fabian Jahr)
6d3db52e667474b6c0c2e4eeb9fb5b3ba4063205 rpc, wallet: Document and test mempool scan after importprivkey (João Barbosa)
3abdbbb90a4a8f2041fec37506268e66a0b3eb31 rpc, wallet: Document and test mempool scan after importaddress (João Barbosa)
236239bd40ae1175537fc932df5af27902326329 wallet: Rescan mempool for transactions as well (Fabian Jahr)
Pull request description:
This PR picks up the work from #18964 and closes #18954.
It should incorporate all the unaddressed feedback from the PR:
- Mempool rescan now expanded to all relevant import* RPCs
- Added documentation in the help of each RPC
- More tests
ACKs for top commit:
Sjors:
re-utACK 1be796418934ae7370cb0ed501877db59e738106 (only a test change)
achow101:
ACK 1be796418934ae7370cb0ed501877db59e738106
w0xlt:
reACK https://github.com/bitcoin/bitcoin/pull/25351/commits/1be796418934ae7370cb0ed501877db59e738106
Tree-SHA512: b62fed5f97c6c242b2af417b41c9696a1f18878483d9e1c9429791f9c05257f57a00540a9a84df23c49faf6a61c3109c22972de81540083f38b506217804fcc5
|
|
Co-authored-by: Larry Ruane <larryruane@gmail.com>
|
|
https://clang.llvm.org/extra/clang-tidy/checks/misc/unused-using-decls.html
|
|
|
|
The following were unused from the node namespace:
- BLOCKFILE_CHUNK_SIZE
- nPruneTarget
- OpenBlockFile
- UNDOFILE_CHUNK_SIZE
|
|
This improves support for LTO by using gcc wrappers for ar, nm, ranlib,
that correctly setup plugin arguments for LTO.
Other HOSTS are using clang.
|
|
See the `AC_PATH_TOOL` macro implementation.
|
|
than std::atomic_exchange
4de4221ab4b645ff77503c777c9b195a962e9fa1 build: Check for std::atomic::exchange rather than std::atomic_exchange (Andrew Chow)
Pull request description:
Our usage of std::atomic is with it's own exchange function, not std::atomic_exchange. So we should be looking specifically for that function.
This removes the need for -latomic for riscv builds, which resolves a guix cross architecture reproducibility issue.
ACKs for top commit:
hebasto:
ACK 4de4221ab4b645ff77503c777c9b195a962e9fa1
fanquake:
ACK 4de4221ab4b645ff77503c777c9b195a962e9fa1
Tree-SHA512: dd8225fc9c6a335601f611700003d0249b9ef941efa502db39306129677929d013048e9221be1d6d7f0ea2d90313d4b87de239f441be21b25bea40a6c19a031e
|
|
from `ArgsManager`
cb3e9a1e3f8d72daaa361fc45dd853775e754b9d Move {Load,Dump}Mempool to kernel namespace (Carl Dong)
aa306765419f7dbea12b12e15553039835ba0e4d Move DEFAULT_PERSIST_MEMPOOL out of libbitcoinkernel (Carl Dong)
06b88ffb8ae7f2b2a93a32908cd80e77fafd270c LoadMempool: Pass in load_path, stop using gArgs (Carl Dong)
b857ac60d9a0433036519c26675378bbf56a1de1 test/fuzz: Invoke LoadMempool via CChainState (Carl Dong)
b3267258b052557fc136b9a4dcb754afb9219470 Move FopenFn to fsbridge namespace (Carl Dong)
ae1e8e37567fa603a5977d7d05105c682dd3f7db mempool: Use NodeClock+friends for LoadMempool (Carl Dong)
f9e8e5719f28d84f68f7d75e26c8e7fccac8e7d3 mempool: Improve comments for [GS]etLoadTried (Carl Dong)
813962da0b17b918941c6849996845e35d84a451 scripted-diff: Rename m_is_loaded -> m_load_tried (Carl Dong)
413f4bb52b72e082ad8716664ede48352b8e7e5a DumpMempool: Pass in dump_path, stop using gArgs (Carl Dong)
bd4407817e523e3c5b347bc6be25ed007cb27034 DumpMempool: Use std::chrono instead of weird int64_t arthmetics (Carl Dong)
c84390b741ab7b61c9f702d8b447c8cadc1257c8 test/mempool_persist: Test manual savemempool when -persistmempool=0 (Carl Dong)
Pull request description:
This is part of the `libbitcoinkernel` project: #24303, https://github.com/bitcoin/bitcoin/projects/18
-----
This PR moves `{Dump,Load}Mempool` into its own `kernel/mempool_persist` module and introduces `ArgsManager` `node::` helpers in `node/mempool_persist_args`to remove the scattered calls to `GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)`.
More context can be gleaned from the commit messages.
-----
One thing I was reflecting on as I wrote this was that in the long run, I think we should probably invert the validation <-> mempool relationship. Instead of mempool not depending on validation, it might make more sense to have validation not depend on mempool. Not super urgent since `libbitcoinkernel` will include both validation and mempool, but perhaps something for the future.
ACKs for top commit:
glozow:
re ACK cb3e9a1e3f8d72daaa361fc45dd853775e754b9d via `git range-diff 7ae032e...cb3e9a1`
MarcoFalke:
ACK cb3e9a1e3f 🔒
ryanofsky:
Code review ACK cb3e9a1e3f8d72daaa361fc45dd853775e754b9d
Tree-SHA512: 979d7237c3abb5a1dd9b5ad3dbf3b954f906a6d8320ed7b923557f41a4472deccae3e8a6bca0018c8e7a3c4a93afecc502acd1e26756f2054f157f1c0edd939d
|
|
When a transaction arrives, the wallet mark its inputs (prev-txs) as dirty.
Clearing the wallet transaction cache, triggering a balance recalculation.
If this does not happen due a db write error during `AddToWallet`, the wallet
will be in an invalid state: The transaction that spends certain wallet UTXO will
exist inside the in-memory wallet tx map, having the credit/debit calculated,
while its inputs will still have the old cached data (like if them were never
spent).
|
|
Our usage of std::atomic is with it's own exchange function, not
std::atomic_exchange. So we should be looking specifically for that
function.
Additionally, -pthread and -lpthread have an effect on whether -latomic
will be needed, so the atomics check needs to use these flags as well.
This will make the flags in use better match what is actually used when
linking.
This removes the need for -latomic for riscv builds, which resolves a
guix cross architecture reproducibility issue.
|
|
-Context:
If `AddToWallet` db write fails, the method returns a wtx nullptr without
removing the recently added transaction from the wallet's map.
-Problem:
When a db write error occurs, `AddToWalletIfInvolvingMe` return false even
when the tx is on the wallet's map already --> which makes `SyncTransaction`
skip the `MarkInputsDirty` call --> which leads to a wallet invalid state
where the inputs of this new transaction are not marked dirty, while the
transaction that spends them still exist on the in-memory wallet tx map.
Plus, as we only store arriving transaction inside `AddToWalletIfInvolvingMe`
when we synchronize/scan blocks from the chain and nowhere else, it makes sense
to treat the tx db write error as a runtime error to notify the user about the
problem. Otherwise, the user will lose all the not stored transactions after a
wallet shutdown (without be able to recover them automatically on the next
startup because the chain sync would be above the block where the txs arrived).
|
|
2315830491b2cfa6b6e3e277700238e5ac92a8e0 fuzz: Fix assert bug in txorphan target (chinggg)
Pull request description:
Fix https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=48914.
It is possible to construct big tx that got rejected in `AddTx`, so we cannot assume tx will be added successfully. We can only guarantee tx will not be added if orphanage already has it.
ACKs for top commit:
MarcoFalke:
lgtm ACK 2315830491b2cfa6b6e3e277700238e5ac92a8e0
Tree-SHA512: e173bc1a932639746de1192ed238e2e2318899f55371febb598facd0e811d8c54997f074f5e761757e1ffd3ae76d8edf9d673f020b2d97d5762ac656f632be81
|
|
|
|
on Linux
3442865360b18b757aee718de585c31e28c457ce build: Use Link Time Optimization for Qt code on Linux (Hennadii Stepanov)
ebce66e532914b433df38a0ee5a2aebcecbd7987 build: pass -fno-lto when building expat (fanquake)
Pull request description:
See: https://www.qt.io/blog/2019/01/02/qt-applications-lto
`bitcon-qt` unstripped size:
| host | master (31c6309cc60ae3fee2d3ecc2aff9576596fb98ac) | this PR, depends built with `LTO=1` |
|---|:-:|:-:|
| x86_64-pc-linux-gnu | 42 MB | 35 MB |
| arm-linux-gnueabihf | 31 MB | 26 MB |
| aarch64-linux-gnu | 41 MB | 32 MB |
| powerpc64-linux-gnu | 51 MB | 41 MB |
| powerpc64le-linux-gnu | 48 MB | 39 MB |
| riscv64-linux-gnu | 35 MB | 29 MB |
Based on the first commit from bitcoin/bitcoin#25391.
Using LTO for macOS and Windows hosts has some issues which could be addressed in follow ups.
x86_64 build:

ACKs for top commit:
fanquake:
ACK 3442865360b18b757aee718de585c31e28c457ce
Tree-SHA512: 03eef2568358df9336e24d6c4e12f28b89d649076fb74e7e5303d61e52865c2360c5345a4fb2b1e4bdfdae194f273fc27a5f67e6cf797ed01a154f3da9117247
|
|
cached debit/credit amount
757216e31cac7dcd45e11b2a2c6148420b3b99da wallet: don't iter twice when getting the cached debit/credit amount (Antoine Poinsot)
Pull request description:
A small optimization i stumbled upon while looking at something else. Figured it could be worth a PR.
Instead of calling GetCachableAmount twice, which will result in
iterating through all the transaction txins/txouts and calling
GetDebit/GetCredit (which lock cs_wallet), just merge the filters and do
it once.
ACKs for top commit:
achow101:
ACK 757216e31cac7dcd45e11b2a2c6148420b3b99da
aureleoules:
ACK 757216e31cac7dcd45e11b2a2c6148420b3b99da.
Tree-SHA512: 0dbbdd24231380196e929dce572752e6be1d69457252a7215e279e71d6199483b516f64019ae999a91dbce7fdd86f8bf0336b6e151cca93cbcf51bc854e838a2
|
|
|
|
Can be reviewed with the git options
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
|
|
1f0c83f43092f6bc959bcb1036a7076cb1235309 refactor: remove BOOST_*_TEST_ macros (fanquake)
70d807c35514f999faba49701fd26f6fb8947564 refactor: integrate no_nul into univalue unitester (fanquake)
98a0ae6b245dca088610094ff92c14ce395d4958 doc: remove references to downstream (fanquake)
Pull request description:
Remove references to "downstream" from makefiles, as they are now redundant.
Remove `BOOST_TEST` macros in favour of just using functions.
Add missing call to `univalue_push_throw` tests.
ACKs for top commit:
MarcoFalke:
ACK 1f0c83f43092f6bc959bcb1036a7076cb1235309 🍎
Tree-SHA512: e0e1ec159a82ece9b364c656b3b49d98f72a04f2614eeb2a386825c3e37bb5a10416446a8ea22d9048227d96aca3e5c1a3dbf3264a290443add382ded073575c
|