Age | Commit message (Collapse) | Author |
|
fa4e09924b11b0dc94e377005f86a83c09761265 refactor: Replace validation.h include with forward-decl in miner.h (MarcoFalke)
fa0739a7d398aea952a07b73ef565e7c2da75898 style: Sort file list after rename (MarcoFalke)
fa53e3a58c94731a90514fe92fad365a49adb10c scripted-diff: Move miner to src/node (MarcoFalke)
Pull request description:
It is impossible to run the miner without a node (validation, chainstate, mempool, rpc, ...). Also, the module is in the node library. Thus, it should be moved to `src/node`.
Also, replace the `validation.h` include in the header with a forward-declaration.
ACKs for top commit:
theStack:
Code-review ACK fa4e09924b11b0dc94e377005f86a83c09761265
Tree-SHA512: 791e6caa5839d8dc83b0f58f3f49bc0a7e3c1710822e8a44dede254c87b6f7531a0586fb95e8a067c181457a3895ad6041718aa2a2fac64cfc136bf04bb851d5
|
|
fa3e0da06b491b8c0fa2dbae37682a9112c9deb8 policy: Treat taproot as always active (MarcoFalke)
Pull request description:
Now that taproot is active, it can be treated as if it was always active for policy for the next major release. This simplifies the code and changes two things:
* Importing `tr` descriptors can be done before the chain is fully synced. This is fine, because the wallet will already generate `tr` descriptors by default (regardless of the taproot status) after commit 47fe7445e7f54aee10ec6dbc53f1db1adbeb43de.
* Valid taproot spends won't be rejected from the mempool before taproot is active. This is strictly speaking a bugfix after commit 47fe7445e7f54aee10ec6dbc53f1db1adbeb43de, since the wallet may generate taproot spends before the chain is fully synced. For example, a slow node or a purposefully offline node. Currently, the wallet needs the mempool to account for change. See https://github.com/bitcoin/bitcoin/issues/11887.
A similar change was done for segwit v0 in https://github.com/bitcoin/bitcoin/pull/13120 .
This effectively reverts commit c5ec0367d718544caa3a1578d6c730fc92ee4e94.
ACKs for top commit:
mjdietzx:
Code Review ACK fa3e0da06b491b8c0fa2dbae37682a9112c9deb8
achow101:
ACK fa3e0da06b491b8c0fa2dbae37682a9112c9deb8
sipa:
utACK fa3e0da06b491b8c0fa2dbae37682a9112c9deb8
gruve-p:
ACK https://github.com/bitcoin/bitcoin/pull/23512/commits/fa3e0da06b491b8c0fa2dbae37682a9112c9deb8
gunar:
Code Review + tACK fa3e0da06
rajarshimaitra:
code review + tACK https://github.com/bitcoin/bitcoin/pull/23512/commits/fa3e0da06b491b8c0fa2dbae37682a9112c9deb8
Tree-SHA512: c6dc7a4e6c345bdec33f256847dc63906ab1696aa683ab9b32a79e715613950884ac3a1a7a44e95f31bb28e58dd64679a616175f7e152b21f5550f3337c8e622
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's/\<proxyType\>/Proxy/g' $(git grep -l proxyType)
-END VERIFY SCRIPT-
|
|
This fixes -Wbitwise-instead-of-logical compiler warnings:
node/interfaces.cpp:544:16: warning: use of bitwise '&' with boolean operands [-Wbitwise-instead-of-logical]
return FillBlock(ancestor, ancestor_out, lock, active) & FillBlock(block1, block1_out, lock, active) & FillBlock(block2, block2_out, lock, active);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
&&
node/interfaces.cpp:544:16: note: cast one or both operands to int to silence this warning
node/interfaces.cpp:544:16: warning: use of bitwise '&' with boolean operands [-Wbitwise-instead-of-logical]
return FillBlock(ancestor, ancestor_out, lock, active) & FillBlock(block1, block1_out, lock, active) & FillBlock(block2, block2_out, lock, active);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
&&
node/interfaces.cpp:544:16: note: cast one or both operands to int to silence this warning
2 warnings generated.
A similar change was recently made to libsecp in commit 16d13221
for the same reason.
|
|
|
|
-BEGIN VERIFY SCRIPT-
# Move module
git mv src/miner.cpp src/node/
git mv src/miner.h src/node/
# Replacements
sed -i 's:miner\.h:node/miner.h:g' $(git grep -l miner)
sed -i 's:miner\.cpp:node/miner.cpp:g' $(git grep -l miner)
sed -i 's:MINER_H:NODE_MINER_H:g' $(git grep -l MINER_H)
-END VERIFY SCRIPT-
|
|
faba1abe469833b2dad01bac4e4d8a4ebb4bc97a Sort file list after rename (MarcoFalke)
fa8f60e31102e1153ad1452fbced51e54487a3d4 scripted-diff: Move minisketchwrapper to src/node (MarcoFalke)
Pull request description:
The newly added wrapper is currently in the node library, but not placed in the node directory. While it is possible to use the wrapper outside of a node context (for example in a utility), it seems unlikely. Either way, I think the wrapper should either be moved to the util lib+dir or the node lib+dir, not something in-between.
Also, fix incorrect comment `BITCOIN_DBWRAPPER_H`.
ACKs for top commit:
fanquake:
ACK faba1abe469833b2dad01bac4e4d8a4ebb4bc97a. I saw the comment in #21515, however given there hasn't been any new activity there, I'm going to merge this now.
Tree-SHA512: fccc0cfd1fee661152a1378587b96795ffb7a7eceb6d2c27ea5401993fd8b9c0a92579fdba61203917ae6565269cb28d0973464fb6201dabf72a5143495d3e77
|
|
interfaces::Node::broadCastTransaction method
0e0f4fdd892223766db4a83194b6576847b757f7 multiprocess: Add interfaces::Node::broadCastTransaction method (Russell Yanofsky)
Pull request description:
This fixes a null pointer crash in the bitcoin-gui PSBT dialog. The bitcoin-gui interfaces::Node object has a null NodeContext pointer, and can't broadcast transactions directly. It needs to broadcast transactions through the bitcoin-node process instead.
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).
ACKs for top commit:
lsilva01:
Code Review ACK 0e0f4fd
Tree-SHA512: cd2c1fe8dc15e7cecf01a21d64319d6add1124995305a9ef9cb72f8492dc692c62d4f846182567d47a5048a533178a925419250941a47cb39932467c36bea3e1
|
|
|
|
ad085f9ba15c131fc5cc77086a620f2e366aac7c multiprocess: Delay wallet client construction (Russell Yanofsky)
Pull request description:
Delay wallet client construction until after logging, thread and other init for two reasons:
- More responsive multiprocess GUI startup. When bitcoin-gui is started this moves the call from bitcoin-gui to bitcoin-node that spawns bitcoin-wallet off of the GUI event thread and onto the background GUI init executor thread.
- Avoids feature_logging.py test failures with bitcoin-node by making bitcoin-wallet logging start after bitcoin-node logging starts,
because the tests are not written to handle the bitcoin-wallet logging init code running first.
This partially reverts commit b266b3e0bf29d0f3d5deaeec62d57c5025b35525, moving wallet client creation back to the place it was located before.
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).
ACKs for top commit:
laanwj:
code review ACK ad085f9ba15c131fc5cc77086a620f2e366aac7c
hebasto:
ACK ad085f9ba15c131fc5cc77086a620f2e366aac7c, I have reviewed the code and it looks OK.
Tree-SHA512: 74d957ce2ee096db745c517124f60800185814b06c20db676090e10dce1b90311adbab02865a69731f8c39b9365f9ee14be0830ca1368cac9b474801ea92bad5
|
|
a032fa30d282fa69c304e0afd1f95f67c55d22e3 multiprocess: add interfaces::ExternalSigner class (Russell Yanofsky)
Pull request description:
Add `interfaces::ExternalSigner` class to let signer objects be passed between processes and let signer code run in the original process where the object was created.
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).
ACKs for top commit:
laanwj:
Concept and code review ACK a032fa30d282fa69c304e0afd1f95f67c55d22e3
hebasto:
re-ACK a032fa30d282fa69c304e0afd1f95f67c55d22e3
Tree-SHA512: 99a729fb3a64d010e142cc778a9f1f358e58345b77faaf2664de7d2277715d59df3352326e8f0f2a6628038670eaa4556310a549079fb28af6d2eeb05aea1460
|
|
This fixes a null pointer crash in the bitcoin-gui PSBT dialog. The
bitcoin-gui interfaces::Node object has a null NodeContext pointer, and
can't broadcast transactions directly. It needs to broadcast
transactions through the bitcoin-node process instead.
|
|
|
|
-BEGIN VERIFY SCRIPT-
# Move module
git mv src/minisketchwrapper.cpp src/node/
git mv src/minisketchwrapper.h src/node/
# Replacements
sed -i 's:minisketchwrapper:node/minisketchwrapper:g' $(git grep -l minisketchwrapper)
sed -i 's:MINISKETCHWRAPPER_H:NODE_MINISKETCHWRAPPER_H:g' $(git grep -l MINISKETCHWRAPPER_H)
sed -i 's:DBWRAPPER_H:NODE_MINISKETCHWRAPPER_H:g' ./src/node/minisketchwrapper.h
-END VERIFY SCRIPT-
|
|
|
|
Allowing callers to pass in a uint256 (which could be txid or wtxid)
but then always assuming that it's a txid is a footgunny interface.
|
|
fs::path(std::string) constructor and fs::path::string() method
6544ea5035268025207d2402db2f7d90fde947a6 refactor: Block unsafe fs::path std::string conversion calls (Russell Yanofsky)
b39a477ec69a51b2016d3a8c70c0c77670f87f2b refactor: Add fs::PathToString, fs::PathFromString, u8string, u8path functions (Russell Yanofsky)
Pull request description:
The `fs::path` class has a `std::string` constructor which will implicitly convert from strings. Implicit conversions like this are not great in general because they can hide complexity and inefficiencies in the code, but this case is especially bad, because after the transition from `boost::filesystem` to `std::filesystem` in #20744 the behavior of this constructor on windows will be more complicated and can mangle path strings. The `fs::path` class also has a `.string()` method which is inverse of the constructor and has the same problems.
Fix this by replacing the unsafe method calls with `PathToString` and `PathFromString` function calls, and by forbidding unsafe method calls in the future.
ACKs for top commit:
kiminuo:
ACK 6544ea5035268025207d2402db2f7d90fde947a6
laanwj:
Code review ACK 6544ea5035268025207d2402db2f7d90fde947a6
hebasto:
re-ACK 6544ea5035268025207d2402db2f7d90fde947a6, only added `fsbridge_stem` test case, updated comment, and rebased since my [previous](https://github.com/bitcoin/bitcoin/pull/22937#pullrequestreview-765503126) review. Verified with the following command:
Tree-SHA512: c36324740eb4ee55151146626166c00d5ccc4b6f3df777e75c112bcb4d1db436c1d9cc8c29a1e7fb96051457d317961ab42e6c380c3be2771d135771b2b49fa0
|
|
b5950dd59ca3e144721a5f15568a65be43bd2f20 validation: put coins cache write log into bench debug log (Anthony Towns)
31b2b802b5b8f214e91ec47fbc953c6ca95c7cf9 blockstorage: use debug log category (Anthony Towns)
da94ebc2facd75c6105a7bd31765c6d2b37fc73b validation: move header validation error logging to VALIDATION debug category (Anthony Towns)
1d7d835ec3a7cc17924f80222784fe3afb567b67 validation: include block hash when reporting prev block not found errors (Anthony Towns)
Pull request description:
Moves the following log messages into debug log categories:
* "AcceptBlockHeader: ..." to validation
* "Prune: deleted blk/rev" to new blockstorage log category
* "Leaving block file" moves from validation to blockstorage
* "write coins cache to disk" to bench
Also adds the hash of the block to the log message when AcceptBlockHeader is rejecting because of problems with the prev block.
ACKs for top commit:
practicalswift:
cr ACK b5950dd59ca3e144721a5f15568a65be43bd2f20
Empact:
Code review ACK https://github.com/bitcoin/bitcoin/pull/23235/commits/b5950dd59ca3e144721a5f15568a65be43bd2f20
laanwj:
Code review ACK b5950dd59ca3e144721a5f15568a65be43bd2f20
promag:
Code review ACK b5950dd59ca3e144721a5f15568a65be43bd2f20.
meshcollider:
Code review ACK b5950dd59ca3e144721a5f15568a65be43bd2f20
Tree-SHA512: a73fdbfe8d36da48a3e89c2d5e0b6a3c5045d280c1a57f61c38d0d21f4f198aece4bd85155be3439e179d5dabdb523bf15fa0395e0e3ceff19c878ba3112c840
|
|
interfaces::Chain::isTaprootActive non-const
7e88f61b285e6a356ea4f6ba384858f109559985 multiprocess: Make interfaces::Chain::isTaprootActive non-const (Russell Yanofsky)
Pull request description:
`interfaces::Chain` is an abstract class, so declaring the method const would be exposing internal implementation details of subclasses to interface callers. And specifically this doesn't work because the multiprocess implementation of the `interfaces::Chain::isTaprootActive` method can't be const because IPC connection state and request state is not constant during the call.
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).
ACKs for top commit:
jamesob:
ACK https://github.com/bitcoin/bitcoin/pull/23003/commits/7e88f61b285e6a356ea4f6ba384858f109559985
Tree-SHA512: 1c5ed89870aeb7170b9048c41299ab650dfa3d0978088e08c4c866fa0babb292722710b16f25540f26667220cb4747b1c256c4bd42893c552291eccc155346a3
|
|
|
|
Add interfaces::ExternalSigner to let signer objects be passed between
processes and signer code to run in the original process, without
multiple processes linking and running signer code.
|
|
There is no change in behavior. This just helps prepare for the
transition from boost::filesystem to std::filesystem by avoiding calls
to methods which will be unsafe after the transaction to std::filesystem
to due lack of a boost::filesystem::path::imbue equivalent and inability
to set a predictable locale.
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Co-authored-by: Kiminuo <kiminuo@protonmail.com>
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
|
|
details
021f86953e8a1dff8ecc768186368d345c865cc2 [style] Run changed files through clang formatter. (Amiti Uttarwar)
375750387e35ed751d1f5ab48860bdec93977f64 scripted-diff: Rename CAddrInfo to AddrInfo (Amiti Uttarwar)
dd8f7f250095e58bbf4cd4effb481b52143bd1ed scripted-diff: Rename CAddrMan to AddrMan (Amiti Uttarwar)
3c263d3f63c3598954ee2b65a0e721e3c22e52f8 [includes] Fix up included files (Amiti Uttarwar)
29727c2aa1233f7c5b91a17884c405e0aef10c6e [doc] Update comments (Amiti Uttarwar)
14f9e000d05f82b364d5a142cafc70b10406b660 [refactor] Update GetAddr_() function signature (Amiti Uttarwar)
40acd6fc9a8098fed85abf4fb727a5f0dff8a2ff [move-only] Move constants to test-only header (Amiti Uttarwar)
7cf41bbb38db5008f9b69037b88138076d6a6cc5 [addrman] Change CAddrInfo access (Amiti Uttarwar)
e3f1ea659c9eb1e8be4579923d6acaaab148c2ef [move-only] Move CAddrInfo to test-only header file (Amiti Uttarwar)
7cba9d56185b9325ce41d79364e448462fff0f6a [net, addrman] Remove external dependencies on CAddrInfo objects (Amiti Uttarwar)
8af5b54f973e11c847345418d8631bc301b96130 [addrman] Introduce CAddrMan::Impl to encapsulate addrman implementation. (Amiti Uttarwar)
f2e5f38f09ee40933f752680fe7d75ee8e529fae [move-only] Match ordering of CAddrMan declarations and definitions (Amiti Uttarwar)
5faa7dd6d871eac1a0ec5c4a93f2ad7577781a56 [move-only] Move CAddrMan function definitions to cpp (Amiti Uttarwar)
Pull request description:
Introduce the pimpl pattern for AddrMan to separate the implementation details from the externally used object representation. This reduces compile-time dependencies and conceptually clarifies AddrMan's interface from the implementation specifics.
Since the unit & fuzz tests currently rely on accessing AddrMan internals, this PR introduces addrman_impl.h, which is exclusively imported by addrman.cpp and test files.
ACKs for top commit:
jnewbery:
ACK 021f86953e8a1dff8ecc768186368d345c865cc2
GeneFerneau:
utACK [021f869](https://github.com/bitcoin/bitcoin/pull/22950/commits/021f86953e8a1dff8ecc768186368d345c865cc2)
mzumsande:
ACK 021f86953e8a1dff8ecc768186368d345c865cc2
rajarshimaitra:
Concept + Code Review ACK https://github.com/bitcoin/bitcoin/pull/22950/commits/021f86953e8a1dff8ecc768186368d345c865cc2
theuni:
ACK 021f86953e8a1dff8ecc768186368d345c865cc2
Tree-SHA512: aa70cb77927a35c85230163c0cf6d3872382d79048b0fb79341493caa46f8e91498cb787d8b06aba4da17b2f921f2230e73f3d66385519794fff86a831b3a71d
|
|
9d0379cea6c164610d05287ae6dd4e66f35b92b3 consensus: use <cstdint> over <stdint.h> in amount.h (fanquake)
863e52fe63a67fa020fb1ef527b9095a35ab77a5 consensus: make COIN & MAX_MONEY constexpr (fanquake)
d09071da5bc997f2de1f55ca7a9babc3d7619329 [MOVEONLY] consensus: move amount.h into consensus (fanquake)
Pull request description:
A first step (of a few) towards some source code reorganization, as well as making libbitcoinconsensus slightly more self contained.
Related to #15732.
ACKs for top commit:
MarcoFalke:
concept ACK 9d0379cea6c164610d05287ae6dd4e66f35b92b 🏝
Tree-SHA512: 97fc79262dcb8c00996852a288fee69ddf8398ae2c95700bba5b326f1f38ffcfaf8fa66e29d0cb446d9b3f4e608a96525fae0c2ad9cd531ad98ad2a4a687cd6a
|
|
secure computing mode)
4747da3a5b639b5a336b737e7e3cbf060cf2efcf Add syscall sandboxing (seccomp-bpf) (practicalswift)
Pull request description:
Add experimental syscall sandboxing using seccomp-bpf (Linux secure computing mode).
Enable filtering of system calls using seccomp-bpf: allow only explicitly allowlisted (expected) syscalls to be called.
The syscall sandboxing implemented in this PR is an experimental feature currently available only under Linux x86-64.
To enable the experimental syscall sandbox the `-sandbox=<mode>` option must be passed to `bitcoind`:
```
-sandbox=<mode>
Use the experimental syscall sandbox in the specified mode
(-sandbox=log-and-abort or -sandbox=abort). Allow only expected
syscalls to be used by bitcoind. Note that this is an
experimental new feature that may cause bitcoind to exit or crash
unexpectedly: use with caution. In the "log-and-abort" mode the
invocation of an unexpected syscall results in a debug handler
being invoked which will log the incident and terminate the
program (without executing the unexpected syscall). In the
"abort" mode the invocation of an unexpected syscall results in
the entire process being killed immediately by the kernel without
executing the unexpected syscall.
```
The allowed syscalls are defined on a per thread basis.
I've used this feature since summer 2020 and I find it to be a helpful testing/debugging addition which makes it much easier to reason about the actual capabilities required of each type of thread in Bitcoin Core.
---
Quick start guide:
```
$ ./configure
$ src/bitcoind -regtest -debug=util -sandbox=log-and-abort
…
2021-06-09T12:34:56Z Experimental syscall sandbox enabled (-sandbox=log-and-abort): bitcoind will terminate if an unexpected (not allowlisted) syscall is invoked.
…
2021-06-09T12:34:56Z Syscall filter installed for thread "addcon"
2021-06-09T12:34:56Z Syscall filter installed for thread "dnsseed"
2021-06-09T12:34:56Z Syscall filter installed for thread "net"
2021-06-09T12:34:56Z Syscall filter installed for thread "msghand"
2021-06-09T12:34:56Z Syscall filter installed for thread "opencon"
2021-06-09T12:34:56Z Syscall filter installed for thread "init"
…
# A simulated execve call to show the sandbox in action:
2021-06-09T12:34:56Z ERROR: The syscall "execve" (syscall number 59) is not allowed by the syscall sandbox in thread "msghand". Please report.
…
Aborted (core dumped)
$
```
---
[About seccomp and seccomp-bpf](https://en.wikipedia.org/wiki/Seccomp):
> In computer security, seccomp (short for secure computing mode) is a facility in the Linux kernel. seccomp allows a process to make a one-way transition into a "secure" state where it cannot make any system calls except exit(), sigreturn(), and read() and write() to already-open file descriptors. Should it attempt any other system calls, the kernel will terminate the process with SIGKILL or SIGSYS. In this sense, it does not virtualize the system's resources but isolates the process from them entirely.
>
> […]
>
> seccomp-bpf is an extension to seccomp that allows filtering of system calls using a configurable policy implemented using Berkeley Packet Filter rules. It is used by OpenSSH and vsftpd as well as the Google Chrome/Chromium web browsers on Chrome OS and Linux. (In this regard seccomp-bpf achieves similar functionality, but with more flexibility and higher performance, to the older systrace—which seems to be no longer supported for Linux.)
ACKs for top commit:
laanwj:
Code review and lightly tested ACK 4747da3a5b639b5a336b737e7e3cbf060cf2efcf
Tree-SHA512: e1c28e323eb4409a46157b7cc0fc29a057ba58d1ee2de268962e2ade28ebd4421b5c2536c64a3af6e9bd3f54016600fec88d016adb49864b63edea51ad838e17
|
|
|
|
std::from_chars(…) (C++17)
test: Add test cases for LocaleIndependentAtoi
fuzz: Assert legacy atoi(s) == LocaleIndependentAtoi<int>(s)
fuzz: Assert legacy atoi64(s) == LocaleIndependentAtoi<int64_t>(s)
|
|
Move amount.h to consensus/amount.h.
Renames, adds missing and removes uneeded includes.
|
|
-BEGIN VERIFY SCRIPT-
git grep -l CAddrMan src/ test/ | xargs sed -i 's/CAddrMan/AddrMan/g'
-END VERIFY SCRIPT-
|
|
Delay wallet client construction until after logging, thread and other
init for two reasons:
- More responsive multiprocess GUI startup. When bitcoin-gui is started
this moves the call from bitcoin-gui to bitcoin-node that spawns
bitcoin-wallet off of the GUI event thread and onto the background GUI
init executor thread.
- Avoids feature_logging.py test failures with bitcoin-node by making
bitcoin-wallet logging start after bitcoin-node logging starts,
because the tests are not written to handle the bitcoin-wallet logging
init code running first.
This partially reverts commit b266b3e0bf29d0f3d5deaeec62d57c5025b35525,
moving wallet client creation back to the place it was located before.
|
|
interfaces::Chain is an abstract class, so declaring the method const
would be exposing internal implementation details of subclasses to
interface callers. And specifically this doesn't work because the
multiprocess implementation of the interfaces::Chain::isTaprootActive
method can't be const because IPC connection state and request state is
not constant during the call.
|
|
ReadBlockFromDisk without cs_main lock
350e034e64d175f3db4c85ddca42e76e279912f6 consensus: don't call GetBlockPos in ReadBlockFromDisk without lock (Jon Atack)
Pull request description:
Commit ccd8ef65 "Reduce cs_main lock in ReadBlockFromDisk, only read GetBlockPos under the lock" in #11281 moved the cs_main lock from caller to `ReadBlockFromDisk()` for calling `CBlockIndex::GetBlockPos()`, but the second invocation doesn't have the lock, and IIUC there is no guarantee the compiler can know if state has changed.
Use the `blockPos` local variable instead, rename it to `block_pos`, and make it const.
ACKs for top commit:
laanwj:
Code review ACK 350e034e64d175f3db4c85ddca42e76e279912f6
theStack:
Code-review ACK 350e034e64d175f3db4c85ddca42e76e279912f6
promag:
Code review ACK 350e034e64d175f3db4c85ddca42e76e279912f6.
Tree-SHA512: 0df0614ab1876885c85f7b53c604a759a29008da8027e95503b4726d2b820ec6d27546020c613337ff954406e01cb5d191978ba4a12124052fed6e1b0e9a226f
|
|
makeChain, etc methods
e4709c7b56612553fb7cbf16ef2d5099c5b732d0 Start using init makeNode, makeChain, etc methods (Russell Yanofsky)
Pull request description:
Use `interfaces::Init::make*` methods instead of `interfaces::Make*` functions, so interfaces can be constructed differently in different executable without having to change any code. (So for example `bitcoin-gui` can make an `interfaces::Node` pointer that communicates with a `bitcoin-node` subprocess, while `bitcoin-qt` can make an `interfaces::Node` pointer that controls node code in the same process.)
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10). The commit was first part of larger PR #10102.
ACKs for top commit:
jamesob:
reACK https://github.com/bitcoin/bitcoin/commit/e4709c7b56612553fb7cbf16ef2d5099c5b732d0
achow101:
ACK e4709c7b56612553fb7cbf16ef2d5099c5b732d0
benthecarman:
utACK e4709c7b56612553fb7cbf16ef2d5099c5b732d0
Tree-SHA512: 580c1979dbb2ef444157c8e53041e70d15ddeee77e5cbdb34f70b6d228cc2d2fe3843825f172da84e506200c58f7e0932f7cd4c006bb5058c1f4e43259394834
|
|
|
|
Improve readability of code, simplify future scripted diff cleanup PRs, and be
more consistent with naming for GetBoolArg.
This will also be useful for replacing runtime settings type checking
with compile time checking.
-BEGIN VERIFY SCRIPT-
git grep -l GetArg | xargs sed -i 's/GetArg(\([^)]*\( [0-9]\+\|-1\|port\|BaseParams().RPCPort()\|Params().GetDefaultPort()\|_TIMEOUT\|Height\|_WORKQUEUE\|_THREADS\|_CONNECTIONS\|LIMIT\|SigOp\|Bytes\|_VERSION\|_AGE\|_CHECKS\|Checks() ? 1 : 0\|_BANTIME\|Cache\|BLOCKS\|LEVEL\|Weight\|Version\|BUFFER\|TARGET\|WEIGHT\|TXN\|TRANSACTIONS\|ADJUSTMENT\|i64\|Size\|nDefault\|_EXPIRY\|HEIGHT\|SIZE\|SNDHWM\|_TIME_MS\)\))/GetIntArg(\1)/g'
-END VERIFY SCRIPT-
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
|
|
listunspent output
6cb60f3e6d652ffa4cf570426a7cf1f690d15c45 doc/release-notes: Add new listunspent fields (Luke Dashjr)
0be2f17ef5649c2d77efbbbdd9222332b2ebf0d2 QA: Add tests for listunspent ancestor{count,size,fees} to mempool_packages (Luke Dashjr)
6966e80f453c46d5d0a923118205f19ac2f4e336 RPC: Add ancestor{count,size,fees} to listunspent output (Luke Dashjr)
3f77dfdaf0f0bfe0c4662a616d6943f31bdd5bf4 Expose ancestorsize and ancestorfees via getTransactionAncestry (Luke Dashjr)
Pull request description:
Requested by a user
ACKs for top commit:
prayank23:
reACK https://github.com/bitcoin/bitcoin/commit/6cb60f3e6d652ffa4cf570426a7cf1f690d15c45
fjahr:
Code review re-ACK 6cb60f3e6d652ffa4cf570426a7cf1f690d15c45
kiminuo:
ACK [6cb60f3](https://github.com/bitcoin/bitcoin/commit/6cb60f3e6d652ffa4cf570426a7cf1f690d15c45)
achow101:
Code Review ACK 6cb60f3e6d652ffa4cf570426a7cf1f690d15c45
naumenkogs:
ACK 6cb60f3e6d652ffa4cf570426a7cf1f690d15c45
darosior:
utACK 6cb60f3e6d652ffa4cf570426a7cf1f690d15c45
Tree-SHA512: 5d16e5799558691e5853ab7ea2cc85514cb45da3ce69134d855c71845beef32ec6af5ab28d4462683e9800c8ea126f162773a9d3d5660edac08fd8edbfeda173
|
|
settings file
49ee2a0ad88e0e656234b769d806987784ff1e28 Avoid wallet code writing node settings file (Russell Yanofsky)
Pull request description:
Change wallet loading code to access settings through the Chain interface instead of writing settings.json directly. This is for running wallet and node in separate processes, since multiprocess code wouldn't easily work with different processes updating the same file.
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10). The commit was first part of larger PR #10102.
ACKs for top commit:
jamesob:
ACK 49ee2a0ad88e0e656234b769d806987784ff1e28 ([`jamesob/ackr/22217.1.ryanofsky.refactor_avoid_wallet_co`](https://github.com/jamesob/bitcoin/tree/ackr/22217.1.ryanofsky.refactor_avoid_wallet_co))
ryanofsky:
> ACK [49ee2a0](https://github.com/bitcoin/bitcoin/commit/49ee2a0ad88e0e656234b769d806987784ff1e28) ([`jamesob/ackr/22217.1.ryanofsky.refactor_avoid_wallet_co`](https://github.com/jamesob/bitcoin/tree/ackr/22217.1.ryanofsky.refactor_avoid_wallet_co))
Zero-1729:
crACK 49ee2a0ad88e0e656234b769d806987784ff1e28
meshcollider:
Code review ACK 49ee2a0ad88e0e656234b769d806987784ff1e28
Tree-SHA512: a81c63b87816f739e02e3992808f314294d6c7213babaafdaaf3c4650ebc97ee4f98f9a4684ce4ff87372df59989b8ad5929159c5686293a7cce04e97e2fabba
|
|
Use interfaces::Init::make* methods instead of interfaces::Make*
functions, so interfaces can be constructed differently in different
executables without having to change any code. (So for example
bitcoin-gui can make an interfaces::Node pointer that communicates with
a bitcoin-node subprocess, while bitcoin-qt can make an interfaces::Node
pointer that starts node code in the same process.)
|
|
5c5d0b62648e1b144b7b93c199f45265dac100e5 Add FoundBlock.found member (Russell Yanofsky)
Pull request description:
This change lets IPC serialization code handle FoundBlock arguments more simply and efficiently. Without this change there was no way to determine from a FoundBlock object whether a block was found or not. So in order to correctly implement behavior of leaving FoundBlock output variables unmodified when a block was not found, IPC code would have to read preexisting output variable values from the local process, send them to the remote process, receive output values back from the remote process, and save them to output variables unconditionally. With FoundBlock.found method, the process is simpler. There's no need to read or send preexisting local output variable values, just to read final output values from the remote process and set them conditionally if the block was found.
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10). The commit was first part of larger PR #10102.
ACKs for top commit:
fjahr:
Code review ACK 5c5d0b62648e1b144b7b93c199f45265dac100e5
theStack:
Concept and code review ACK 5c5d0b62648e1b144b7b93c199f45265dac100e5
jamesob:
ACK 5c5d0b62648e1b144b7b93c199f45265dac100e5 ([`jamesob/ackr/22215.1.ryanofsky.refactor_add_foundblock`](https://github.com/jamesob/bitcoin/tree/ackr/22215.1.ryanofsky.refactor_add_foundblock))
Zero-1729:
crACK 5c5d0b6
Tree-SHA512: d906e1b7100ff72c3aa06d80bd77673887b2db670ebd52dce7c4f6f557a23a1744c6109308228a37fda6c6ea74f05ba0efecff0ef235ab06ea8acd861fbb8675
|
|
|
|
|
|
node/transaction.cpp
f685a13bef0418663015ea6d8f448f075510c0ec doc: GetTransaction()/getrawtransaction follow-ups to #22383 (John Newbery)
abc57e1f0882a1a2bb20474648419979af6e383d refactor: move `GetTransaction(...)` to node/transaction.cpp (Sebastian Falbesoner)
Pull request description:
~This PR is based on #22383, which should be reviewed first~ (merged by now).
In [yesterday's PR review club session to PR 22383](https://bitcoincore.reviews/22383), the idea of moving the function `GetTransaction(...)` from src/validation.cpp to src/node/transaction.cpp came up. With this, the circular dependency "index/txindex -> validation -> index/txindex" is removed (see change in `lint-circular-dependencies.sh`). Thanks to jnewbery for suggesting and to sipa for providing historical background.
Relevant IRC log:
```
17:52 <jnewbery> Was anyone surprised that GetTransaction() is in validation.cpp? It seems to me that node/transaction.cpp would be a more appropriate place for it.
17:53 <raj_> jnewbery, +1
17:53 <stickies-v> agreed!
17:54 <glozow> jnewbery ya
17:54 <jnewbery> seems weird that validation would call into txindex. I wonder if we remove this function, then validation would no longer need to #include txindex
17:54 <sipa> GetTransaction predates node/transaction.cpp, and even the generic index framework itself :)
17:55 <sipa> (before 0.8, validation itself used the txindex)
17:55 <jnewbery> (and GetTransaction() seems like a natural sibling to BroadcastTransaction(), which is already in node/transaction.cpp)
17:55 <jnewbery> sipa: right, this is not meant as a criticism of course. Just wondering if we can organize things a bit more rationally now that we have better separation between things.
17:55 <sipa> jnewbery: sure, just providing background
17:56 <sipa> seems very reasonable to move it elsewhere now
```
The commit should be trivial to review with `--color-moved`.
ACKs for top commit:
jnewbery:
Code review ACK f685a13bef0418663015ea6d8f448f075510c0ec
rajarshimaitra:
tACK https://github.com/bitcoin/bitcoin/pull/22528/commits/f685a13bef0418663015ea6d8f448f075510c0ec
mjdietzx:
crACK f685a13bef0418663015ea6d8f448f075510c0ec
LarryRuane:
Code review, test ACK f685a13bef0418663015ea6d8f448f075510c0ec
Tree-SHA512: 0e844a6ecb1be04c638b55bc4478c2949549a4fcae01c984eee078de74d176fb19d508fc09360a62ad130677bfa7daf703b67870800e55942838d7313246248c
|
|
779e638ca9b2b37c247577d225b93ac762b0602f coinstats: Add comments for new coinstatsindex values (Fabian Jahr)
5b3d4e724f377834e24b1f014787cc7aa7fc30fe Index: Improve logging in coinstatsindex (Fabian Jahr)
d4356d4e48f59c63894b68691cc21ed4892ee716 rpc: Block until synced if coinstatsindex is used in gettxoutsetinfo (Fabian Jahr)
a5f6791139554936d13f367660283899a37ff5c7 rpc: Add missing gettxoutsetinfo help docs (Fabian Jahr)
01386bfd88019397237256cb16f91de346eb66f2 Index: Return early from failed coinstatsindex init (Fabian Jahr)
1e3842385b8c0d15086c7cd8736f8c67e6c0c285 index: Use batch writing in coinstatsindex WriteBlock (Fabian Jahr)
fb65dde147f63422c4148b089c2f5be0bf5ba80f scripted-diff: Fix coinstats data member names (Fabian Jahr)
8ea8c927ac05980d6a81252e40b7444e9abb74f9 index: Avoid unnecessary type casts in coinstatsindex (Fabian Jahr)
Pull request description:
This is a collection of smaller follow-ups to #19521, addressing several post-merge review comments.
ACKs for top commit:
Sjors:
re-utACK 779e638ca9b2b37c247577d225b93ac762b0602f
jonatack:
re-ACK 779e638ca9b2b37c247577d225b93ac762b0602f diff since last review involves doc changes only; rebased to current master and verified clean debug build/no silent conflicts, unit tests, and feature_coinstatsindex functional test
laanwj:
Code review ACK 779e638ca9b2b37c247577d225b93ac762b0602f
Talkless:
re-utACK 779e638ca9b2b37c247577d225b93ac762b0602f after cosmetic changes.
Tree-SHA512: cb0d038d230c582d7fe3041c89b1e04d39971fab3739d540c609cf826754c6c513b12ded08ac92180aec7a9d7a70114ece50357bd1a902de4adaae9f30b8d699
|
|
|
|
|
|
can be reviewed with --color-moved
|
|
faa54e375782b21cbc2761c763128131c569e903 Move pblocktree global to BlockManager (MarcoFalke)
fa27f03b4943540aa2eab283d4cf50ad4a1a01f8 Move LoadBlockIndexDB to BlockManager (MarcoFalke)
Pull request description:
The block tree db is used within BlockManager to write and read the block index, so make the db global a member variable of BlockManager.
ACKs for top commit:
jamesob:
ACK faa54e375782b21cbc2761c763128131c569e903 ([`jamesob/ackr/22371.1.MarcoFalke.move_pblocktree_global_t`](https://github.com/jamesob/bitcoin/tree/ackr/22371.1.MarcoFalke.move_pblocktree_global_t))
theStack:
re-ACK faa54e375782b21cbc2761c763128131c569e903 🥧
ryanofsky:
Code review ACK faa54e375782b21cbc2761c763128131c569e903. I was thinking this looked like a change Carl would like, so no surprised he [Mega-acked](https://github.com/bitcoin/bitcoin/pull/22371#pullrequestreview-696450475)
Tree-SHA512: 1b7badbf503d53f5d4dbd9ed8f2e5c1ebfe48102665197048cc9e37bc87b5cec5f2277f3aae9f73a1095bfe879b19d288286ca3daa28031f5f1b64b1184439a9
|
|
|
|
|
|
This commit fixes some slightly unexpected behaviour when:
- there is already transaction in the mempool (the "mempool tx")
- BroadcastTransaction() is called for a transaction with the same txid
as the mempool transaction but a different witness (the "new tx")
Prior to this commit, if BroadcastTransaction() is called with
relay=true, then it'll call RelayTransaction() using the txid/wtxid of
the new tx, not the txid/wtxid of the mempool tx. For wtxid relay peers,
in SendMessages(), the wtxid of the new tx will be taken from
setInventoryTxToSend, but will then be filtered out from the vector of
wtxids to announce, since m_mempool.info() won't find the transaction
(the mempool contains the mempool tx, which has a different wtxid from
the new tx).
Fix this by calling RelayTransaction() with the wtxid of the mempool
transaction in this case.
|