The tracepoint `validation:block_connected` was introduced in #22006.
The first argument was the hash of the connected block as a pointer
to a C-like String. The last argument passed the hash of the
connected block as a pointer to 32 bytes. The hash was only passed as
string to allow `bpftrace` scripts to print the hash. It was
(incorrectly) assumed that `bpftrace` cannot hex-format and print the
block hash given only the hash as bytes.
The block hash can be printed in `bpftrace` by calling
`printf("%02x")` for each byte of the hash in an `unroll () {...}`.
By starting from the last byte of the hash, it can be printed in
big-endian (the block-explorer format).
```C
$p = $hash + 31;
unroll(32) {
$b = *(uint8*)$p;
printf("%02x", $b);
$p -= 1;
}
```
See also: https://github.com/bitcoin/bitcoin/pull/22902#discussion_r705176691
This is a breaking change to the block_connected tracepoint API, however
this tracepoint has not yet been included in a release.
The COIN constant is critical in understanding Bitcoin's supply, but what it represents isn't clear from the name of the constant. Adding a comment clarifies the meaning of the constant for future readers.
For the generic wallet tests, make DescriptorScriptPubKeyMans. There are
still some wallet tests that test legacy wallet things. Those remain
unchanged.
The subtract fee from outputs assumes that the leftover input amount
will be dropped to fees. However this only happens if that amount is
less than the cost of change. In the event that it is higher than the
cost of change, the leftover amount will actually become a change
output. To avoid this scenario, force a change type which has a high
cost of change.
To avoid issues with test data leaking across tests cases, the global
vCoins and testWallet are removed from coinselector_tests and all of the
relevant functions reworked to not need them.
6544ea5035 refactor: Block unsafe fs::path std::string conversion calls (Russell Yanofsky)
b39a477ec6 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 6544ea5035
laanwj:
Code review ACK 6544ea5035
hebasto:
re-ACK 6544ea5035, 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
Presumably these stubs indicate to packagers that external leveldb is meant to
be supported in some way. It is not. Remove the stubs to avoid sending any
mixed messages.
6531599f42 test: Add check that newkeypool flushes change addresses too (Samuel Dobson)
84fa19c77a Add release notes for keypool flush changes (Samuel Dobson)
f9603ee4e0 Add test for flushing keypool with newkeypool (Samuel Dobson)
6f6f7bb36c Make legacy wallet upgrades from non-HD to HD always flush the keypool (Samuel Dobson)
2434b10781 Fix outdated keypool size default (Samuel Dobson)
22cc797ca5 Add newkeypool RPC to flush the keypool (Samuel Dobson)
Pull request description:
This PR makes two main changes:
1) Adds a new RPC `newkeypool` which will entirely flush and refill the keypool.
2) When upgradewallet is called on old, non-HD wallets upgrading them to HD, we now always flush the keypool and generate a new one, to immediately start using the HD generated keys.
This PR is motivated by a number of users with old, pre-compressed-key wallets upgrading them and being confused about why they still can't generate p2sh-segwit or bech32 addresses -- this is due to uncompressed keys remaining in the keypool post-upgrade and being illegal in these newer address formats. There is currently no easy way to flush the keypool other than to call `getnewaddress` a hundred/thousand times or an ugly hack of using a `sethdseed` call.
ACKs for top commit:
laanwj:
re-ACK 6531599f42
meshcollider:
Added new commit 6531599f42 to avoid invalidating previous ACKs.
instagibbs:
ACK 6531599f42
Tree-SHA512: 50c79c5d42dd27ab0ecdbfdc4071fdaa1b2dbb2f9195ed325b007106ff19226419ce57fe5b1539c0c24101b12f5e034bbcfb7bbb0451b766cb1071295383d774
b5950dd59c validation: put coins cache write log into bench debug log (Anthony Towns)
31b2b802b5 blockstorage: use debug log category (Anthony Towns)
da94ebc2fa validation: move header validation error logging to VALIDATION debug category (Anthony Towns)
1d7d835ec3 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 b5950dd59c
Empact:
Code review ACK b5950dd59c
laanwj:
Code review ACK b5950dd59c
promag:
Code review ACK b5950dd59c.
meshcollider:
Code review ACK b5950dd59c
Tree-SHA512: a73fdbfe8d36da48a3e89c2d5e0b6a3c5045d280c1a57f61c38d0d21f4f198aece4bd85155be3439e179d5dabdb523bf15fa0395e0e3ceff19c878ba3112c840
ce69e18947 scripts: remove pixie.py (fanquake)
00b85d0b13 scripts: only parse the binary once in security-check.py (fanquake)
cad40a5b16 scripts: use LIEF for ELF checks in security-check.py (fanquake)
8242ae230e scripts: only parse the binary once in symbol-check.py (fanquake)
309eac9019 scripts: use LIEF for ELF checks in symbol-check.py (fanquake)
610a8a8e39 test-*-check: Pass in *FLAGS and compile with them (Carl Dong)
Pull request description:
This finishes the transition to using LIEF for the ELF symbol and security checks.
Note that there's currently a work around used for identifying RISCV binaries (just checking the interpreter). I've sent a PR upstream, https://github.com/lief-project/LIEF/pull/562, and we should be able to drop that when using LIEF 0.12.0 and onwards.
ACKs for top commit:
dongcarl:
Code Review ACK ce69e18947
laanwj:
Code review ACK ce69e18947
Tree-SHA512: 911ba693cd9777ad1fc1f66dff6c4d3630a907351215380cbde5b14a4bbf5cf7eebf52eafa7e86b27deabd2d93d1b403f34aabd356b5ceaab3cc6e9941a01dd4
fa6f29de51 bitcoin-tx: Reject non-integral and out of range multisig numbers (MarcoFalke)
fafab8ea5e bitcoin-tx: Reject non-integral and out of range sequence ids (MarcoFalke)
fa53d3d826 test: Check that bitcoin-tx accepts whitespace around sequence id and multisig numbers (MarcoFalke)
Pull request description:
Seems odd to silently accept arbitrary strings that don't even represent integral values.
Fix that.
ACKs for top commit:
practicalswift:
cr ACK fa6f29de51
laanwj:
Code review ACK fa6f29de51
Empact:
Code review ACK fa6f29de51
promag:
Code review ACK fa6f29de51.
Tree-SHA512: e31f7f21fe55ac069e755557bdbcae8d5d29e20ff82e441ebdfc65153e3a31a4edd46ad3e6dea5190ecbd1b8ea5a8f94daa5d59a3b7558e46e794e30db0e6c79
7e88f61b28 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 7e88f61b28
Tree-SHA512: 1c5ed89870aeb7170b9048c41299ab650dfa3d0978088e08c4c866fa0babb292722710b16f25540f26667220cb4747b1c256c4bd42893c552291eccc155346a3
fa4d0aacf2 test: * -> & (MarcoFalke)
Pull request description:
This changes background_cs from being a pointer to a reference to work
around a gcc false warning. Also, this makes the test easier to read.
Fixes bitcoin#23101
Can be reviewed with --ignore-all-space.
ACKs for top commit:
practicalswift:
cr ACK fa4d0aacf2
jamesob:
ACK fa4d0aacf2
hebasto:
ACK fa4d0aacf2, tested on Linux Mint 20.2 (x86_64) by merging this PR on top of the current master.
Tree-SHA512: 93a0d8859201f7074bea52fab8f6701409148bc50cfbb142cacfa6c991fc12c07584df04fead645f11703883df99535423d154f9945202e1c5aff49540d9b607
fa43e7c2d9 bitcoin-tx: Avoid treating overflow as OP_0 (MarcoFalke)
fa053c0019 style: Fix whitespace in Parse* functions (MarcoFalke)
fa03dec7e9 refactor: Use C++11 range based for loop in ParseScript (MarcoFalke)
fad55e79ca doc: Fixup ToIntegral docs (MarcoFalke)
Pull request description:
Seems odd to treat integer overflow as `OP_0`, so fix that.
ACKs for top commit:
theStack:
re-ACK fa43e7c2d9
shaavan:
ACK fa43e7c2d9
Tree-SHA512: 1bbe2de62d853badc18d57d169c6e78ddcdff037e5a85357995dead11c8e67a4fe35087e08a181c60753f8ce91058b7fcc06f5b7901afedc78fbacea8bc3ef4f
This addresses issues like the one in #12467, where some of our compiler flags
end up being dropped during the subconfigure of Univalue. Specifically, we're
still using the compiler-default c++ version rather than forcing c++17.
We can drop the need subconfigure completely in favor of a tighter build
integration, where the sources are listed separately from the build recipes,
so that they may be included directly by upstream projects. This is
similar to the way leveldb build integration works in Core.
Core benefits of this approach include:
- Better caching (for ex. ccache and autoconf)
- No need for a slow subconfigure
- Faster autoconf
- No more missing compile flags
- Compile only the objects needed
There are no benefits to Univalue itself that I can think of. These changes
should be a no-op there, and to downstreams as well until they take advantage
of the new sources.mk.
This also removes the option to use an external univalue to avoid similar ABI
issues with mystery binaries.
Co-authored-by: fanquake <fanquake@gmail.com>
7fc487afd1 refactor: use `{Read,Write}BE32` helpers for BIP32 nChild (de)serialization (Sebastian Falbesoner)
Pull request description:
This small refactoring PR replaces manual bit-fiddling (de)serialization of the BIP32 child number (nChild) by the helpers `ReadBE32`/`WriteBE32`. Note that those were first introduced in #4100, almost one year _after_ the BIP32 derivation implementation has been merged (#2829, eb2c9990).
ACKs for top commit:
sipa:
utACK 7fc487afd1
laanwj:
Code review ACK 7fc487afd1
Tree-SHA512: bbe3e411fb0429fa74c8a5705a91f4d6ed704dac9d6623ecb633563f22acf8e21f3189a16f1d0cf1aeedfc56a5b695df54ae51e9577e34eb6d7dc335de2da6de
fa165e9545 Replace stoul with ToIntegral in dbwrapper (MarcoFalke)
Pull request description:
The string is created with `%llu`. See: 7fcf53f7b4/src/leveldb/db/db_impl.cc (L1436-L1437)
So it seems odd to silently accept when parsing: whitespace, a sign character, trailing chars, overflow, ....
Fix that by using the stricter ToIntegral.
ACKs for top commit:
laanwj:
Code review ACK fa165e9545
practicalswift:
cr ACK fa165e9545
theStack:
Code-review ACK fa165e9545
Tree-SHA512: b87f01431ca0b971ff84610022da8679d3c33470b88cfc3f4a337e6e176a0455715588aefd40e8e2bbe7459d902dc89d7bfe34e7fd66755f631cc18dc039fa2f
d54ec27bac qt: Add helper to load font (João Barbosa)
Pull request description:
Originally submitted as https://github.com/bitcoin-core/gui-qml/pull/49.
ACKs for top commit:
hebasto:
re-ACK d54ec27bac
stratospher:
Tested ACK d54ec27. Refactoring the code and defining `loadFont()` in `src/qt/guiutil.cpp` reduces redundant imports of the `QFontDatabase` and is a better design.
shaavan:
ACK d54ec27bac
Tree-SHA512: b156bb6ffb08dd57476f383a29bbb0a1108b62794d430debb77252f7d09df1409a7532b09d17d8836d1c2ab7c126a6618231164b9d0def1b8f361a81ef22d107
44452110f0 [fuzz] Update comment in FillAddrman() (John Newbery)
640476eb0e [fuzz] Make Fill() a free function in fuzz/addrman.cpp (John Newbery)
90ad8ad61a [fuzz] Make RandAddr() a free function in fuzz/addrman.cpp (John Newbery)
491975c596 [fuzz] Pass FuzzedDataProvider& into Fill() in addrman fuzz tests (John Newbery)
56303e382e [fuzz] Create a FastRandomContext in addrman fuzz tests (John Newbery)
Pull request description:
#22974 improved the performance of `CAddrMan::Good()` significantly so that it could be used directly in the fuzz tests, instead of those tests reaching directly into addrman's internal data structures.
This PR continues the work of making the fuzz tests only use `CAddrMan`'s public methods and pulls the `Fill()` and `RandAddr()` methods from `CAddrManDeterministic` into free functions, since they no longer need access to `CAddrMan` internals.
ACKs for top commit:
theuni:
utACK 44452110f0. Agree the failure seems unrelated, looks like some startup race.
mzumsande:
ACK 44452110f0
vasild:
ACK 44452110f0
Tree-SHA512: fcf994e1dedd0012b77f632720b6423d51ceda4eb85c9efe572f2a1150117f9e511114a5206738dd94409137287577f3b01a9998f5237de845410d3d96e7cb7f
43568782c2 External input fund support cleanups (Gregory Sanders)
Pull request description:
Minor cleanups to https://github.com/bitcoin/bitcoin/pull/17211
ACKs for top commit:
achow101:
ACK 43568782c2
meshcollider:
utACK 43568782c2
benthecarman:
ACK 43568782c2
Tree-SHA512: 865f8a3804f8c0027f5393a0539041158166a919378f2c3bc99b936843eee2329372bcc2af888fa62babfa5f6baf4f13d4cfef7b4e26a7265a82a908f9719ad6
ac402e749c util: Conditionalize some syscalls in syscall name table (W. J. van der Laan)
64085b37f8 util: Add __NR_copy_file_range syscall constant for sandbox (W. J. van der Laan)
Pull request description:
Make the new syscall sandbox compilable with kernel 4.4.0.
This defines a further syscall constant `__NR_copy_file_range` to make sure all syscalls used in the profile are available even if not defined in the kernel headers.
Also, make a few syscalls optional in the syscall name table:
- `__NR_pkey_alloc`
- `__NR_pkey_free`
- `__NR_pkey_mprotect`
- `__NR_preadv2`
- `__NR_pwritev2`
ACKs for top commit:
practicalswift:
cr ACK ac402e749c
Tree-SHA512: be6c55bf0a686bcdfad0b80b950d0d7d77a559ac234fc997b47514bdba44865a371c96dd8d34a811ba46424a84f410e23f75485b9b1e69e529b7d40e0b4b91b8
3b613722f6 Add release notes for fee est with replacement txs (Antoine Poinsot)
4556406562 qa: test fee estimation with replacement transactions (Antoine Poinsot)
053415b297 qa: split run_test into smaller parts (Antoine Poinsot)
06c5ce9714 Re-include RBF replacement txs in fee estimation (Antoine Poinsot)
Pull request description:
This effectively reverts #9519.
RBF is now largely in use on the network (signaled for by around 20% of
all transactions on average) and replacement logic is implemented in
most end-user wallets. The rate of replaced transactions is also
expected to rise as fee-bumping techniques are being developed for
pre-signed transaction ("L2") protocols.
ACKs for top commit:
prayank23:
reACK 3b613722f6
Zero-1729:
re-ACK 3b613722f6
benthecarman:
reACK 3b613722f6
glozow:
ACK 3b613722f6
theStack:
re-ACK 3b613722f6🍪
Tree-SHA512: a6146d15c80ff4ba9249314b0ef953a66a15673e61b8f98979642814f1b169b5695e330e3ee069fa9a7e4d1f8aa10e1dcb7f9aa79181cea5a4c4dbcaf5483023
fafff132cf doc: Extract FundTxDoc (MarcoFalke)
Pull request description:
No need to duplicate the documentation for the same field(s) three times.
Fix that by de-duplicating it for the fields: conf_target, estimate_mode, replaceable, and solving_data.
Can be reviewed with `--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space`.
ACKs for top commit:
fanquake:
ACK fafff132cf
Tree-SHA512: 098ddad3904b80b24c9e7b57ca8e807a6ccc3899eac2c9986d71ba3873c2b580bbb95f2fdfbf94b2db02f81c7b0ebf438a90324c23389b7b968ca85ae8475373
fab360aa00 util: Add mremap syscall to AllowAddressSpaceAccess (MarcoFalke)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/23206
ACKs for top commit:
practicalswift:
cr ACK fab360aa00
laanwj:
Code review ACK fab360aa00
fanquake:
ACK fab360aa00 - confirmed that the GUIX build is working with this change:
Tree-SHA512: 9cf808b3e04830e87bca49b27914993929be3c27eb674d89739b8ea5e5c848c87713d638506c1cd2b80b0129c3dff0c488eb240eef3bbf3d7508ece3c934fb54
01bff8f049 qt: Fix WalletControllerActivity progress dialog title (Shashwat)
Pull request description:
Throughout the GUI, the title of the window, tells about the purpose of the window. This was not true for the title of wallet loading wallet.
This PR fixes this issue by renaming the wallet loading window title to 'Open Wallet'
Changes introduced in this PR (Runned Bitcoin-GUI on signet network)
|Master|PR|
|---|---|
|||
ACKs for top commit:
jarolrod:
ACK 01bff8f
hebasto:
ACK 01bff8f049, tested on Linux Mint 20.2 (Qt 5.12.8).
Tree-SHA512: cd21c40752eb1c0afb5ec61b8a40e900bc3aa05749963f7957ece6024e4957f5bb37e0eb4f95aac488f5e08aea51fe13b023b05d8302a08c88dcc6790410ba64
faa5e171e6 RPCConsole: Throw when overflowing size_t type for array indices (MarcoFalke)
Pull request description:
To test:
-> `getblock(getbestblockhash(), 1)[tx][22222222222222222222222222222]`
Before:
<- `868693731dea69a197c13c2cfaa41c9f78fcdeb8ab8e9f8cdf2c9025147ee7d1` (hash of the coinbase tx)
After:
<- `Error: Invalid result query`
ACKs for top commit:
jarolrod:
ACK faa5e171e6
shaavan:
ACK faa5e17
Tree-SHA512: ddff39aae1c15db45928b110a9f1c42eadc5404cdfa89d67ccffc4c6af24091967d43c068ce9e0c1b863cfc4eb5b4f12373a73756a9474f8294e8a44aabc28d8
The WalletControllerActivity progress dialog had title of "Bitcoin-Qt".
The window title for opening wallet window should be "Open Wallet",
for creating wallet window should be "Create Wallet", and for the window
that is displayed when wallets are being loaded at startup should be
"Load Wallets". This PR fixes that.
Make the watch-only icon in the bottom bar enabled by default for a better user interface.
Currently, it's disabled by default with a 50% opacity which makes it hard to see the icon in dark mode.
Put these in `#ifdef` as they are newer syscalls that might not be
defined on all kernels:
__NR_pkey_alloc
__NR_pkey_free
__NR_pkey_mprotect
__NR_preadv2
__NR_pwritev2
Thanks to jamesob for reporting.
The original CAddrMan behaviour (before commit
e6b343d880) was to pick a uniformly
random non-empty bucket, and then pick a random element from that
bucket. That commit, which introduced deterministic placement
of entries in buckets, changed this to picking a uniformly random
non-empty bucket position instead.
This commit reverts the original high-level behavior, but in the
deterministic placement model.
Use a (reference) parameter instead of a data member of
CAddrManDeterministic. This will allow us to make Fill() a free function
in a later commit.
Also remove CAddrManDeterministic.m_fuzzed_data_provider since it's no
longer used.
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>
There is no change in behavior. This just helps prepare for the
transition from the boost::filesystem to the std::filesystem path
implementation.
Co-authored-by: Kiminuo <kiminuo@protonmail.com>
2d0279987e util: Make sure syscall numbers used in profile are defined (W. J. van der Laan)
8289d19ea5 util: Define SECCOMP_RET_KILL_PROCESS if not provided by the headers (W. J. van der Laan)
Pull request description:
Looks like we've broke the GUIX build in #20487. This attempts to fix it:
- Define `__NR_statx` `__NR_getrandom` `__NR_membarrier` as some kernel headers lack them, and it's important to have the same profile independent on what kernel is used for building.
- Define `SECCOMP_RET_KILL_PROCESS` as it isn't defined in the headers.
ACKs for top commit:
practicalswift:
cr ACK 2d0279987e
Tree-SHA512: c264c66f90af76bf364150e44d0a31876c2ef99f05777fcdd098a23f1e80efef43028f54bf9b3dad016110056d303320ed9741b0cb4c6266175fa9d5589b4277
021f86953e [style] Run changed files through clang formatter. (Amiti Uttarwar)
375750387e scripted-diff: Rename CAddrInfo to AddrInfo (Amiti Uttarwar)
dd8f7f2500 scripted-diff: Rename CAddrMan to AddrMan (Amiti Uttarwar)
3c263d3f63 [includes] Fix up included files (Amiti Uttarwar)
29727c2aa1 [doc] Update comments (Amiti Uttarwar)
14f9e000d0 [refactor] Update GetAddr_() function signature (Amiti Uttarwar)
40acd6fc9a [move-only] Move constants to test-only header (Amiti Uttarwar)
7cf41bbb38 [addrman] Change CAddrInfo access (Amiti Uttarwar)
e3f1ea659c [move-only] Move CAddrInfo to test-only header file (Amiti Uttarwar)
7cba9d5618 [net, addrman] Remove external dependencies on CAddrInfo objects (Amiti Uttarwar)
8af5b54f97 [addrman] Introduce CAddrMan::Impl to encapsulate addrman implementation. (Amiti Uttarwar)
f2e5f38f09 [move-only] Match ordering of CAddrMan declarations and definitions (Amiti Uttarwar)
5faa7dd6d8 [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 021f86953e
GeneFerneau:
utACK [021f869](021f86953e)
mzumsande:
ACK 021f86953e
rajarshimaitra:
Concept + Code Review ACK 021f86953e
theuni:
ACK 021f86953e
Tree-SHA512: aa70cb77927a35c85230163c0cf6d3872382d79048b0fb79341493caa46f8e91498cb787d8b06aba4da17b2f921f2230e73f3d66385519794fff86a831b3a71d
Define the following syscall numbers for x86_64, so that the profile
will be the same no matter what kernel is built against, including
kernels that don't have `__NR_statx`:
```c++
#define __NR_statx 332
#define __NR_getrandom 318
#define __NR_membarrier 324
```
44d77d2213 sandbox: add copy_file_range to allowed filesystem syscalls (fanquake)
ee08741c9c sandbox: add newfstatat to allowed filesystem syscalls (fanquake)
Pull request description:
Similar to #23178, this is a follow up to #20487, which has broken running the unit tests for some developers. Fix this by adding `newfstatat` to the list of allowed filesystem related calls.
ACKs for top commit:
achow101:
ACK 44d77d2213
laanwj:
Code review ACK 44d77d2213
practicalswift:
cr ACK 44d77d2213
Tree-SHA512: ce9d1b441ebf25bd2cf290566e05864223c1418dab315c962e1094ad877db5dd9fcab94ab98a46da8b712a8f5f46675d62ca3349215d8df46ec5b3c4d72dbaa6
Display the prevout in transaction inputs when calling getblock level 3
verbosity.
Co-authored-by: Luke Dashjr <luke_github1@dashjr.org>
Co-authored-by: 0xB10C <19157360+0xB10C@users.noreply.github.com>
9d0379cea6 consensus: use <cstdint> over <stdint.h> in amount.h (fanquake)
863e52fe63 consensus: make COIN & MAX_MONEY constexpr (fanquake)
d09071da5b [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 9d0379cea6 🏝
Tree-SHA512: 97fc79262dcb8c00996852a288fee69ddf8398ae2c95700bba5b326f1f38ffcfaf8fa66e29d0cb446d9b3f4e608a96525fae0c2ad9cd531ad98ad2a4a687cd6a
4747da3a5b 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 4747da3a5b
Tree-SHA512: e1c28e323eb4409a46157b7cc0fc29a057ba58d1ee2de268962e2ade28ebd4421b5c2536c64a3af6e9bd3f54016600fec88d016adb49864b63edea51ad838e17
No transaction in the mempool should ever be a coinbase.
Since mempoolDuplicate's backend is the chainstate coins view, it should
always contain the coins available.
UpdateCoins is an unnecessary dependency on validation. All we need to
do is add and remove coins to check inputs. We don't need the extra
logic for checking coinbases and handling TxUndos.
Also remove the wrapper function in validation.h which constructs a
throwaway TxUndo object before calling UpdateCoins because it is now
unused.
Remove variables used for keeping track of mempool transactions for
which we haven't processed the parents yet. Since we're iterating in
topological order now, they're always unused.
No behavior changes.
Before, we're always adding transactions to the "check later" queue if
they have any parents in the mempool. But there's no reason to do this
if all of its inputs are already available from mempoolDuplicate.
Instead, check for inputs, and only mark fDependsWait=true if the
parents haven't been processed yet.
Reduce the amount of "check later" transactions by looking at
ancestors before descendants. Do this by iterating through them in
ascending order by ancestor count. This works because a child will
always have more in-mempool ancestors than its parent.
We should never have any entries in the "check later" queue
after this commit.
fa9d72a794 Remove unused ParseDouble and ParsePrechecks (MarcoFalke)
fa3cd28535 refactor: Remove unused ParsePrechecks from ParseIntegral (MarcoFalke)
Pull request description:
All of the `ParsePrechecks` are already done by `ToIntegral`, so remove them from `ParseIntegral`.
Also:
* Remove redundant `{}`. See https://github.com/bitcoin/bitcoin/pull/20457#discussion_r720116866
* Add missing failing c-string test case
* Add missing failing test cases for non-int32_t integral types
ACKs for top commit:
laanwj:
Code review ACK fa9d72a794, good find on ParseDouble not being used at all, and testing for behavior of embedded NULL characters is always a good thing.
practicalswift:
cr ACK fa9d72a794
Tree-SHA512: 3d654dcaebbf312dd57e54241f9aa6d35b1d1d213c37e4c6b8b9a69bcbe8267a397474a8b86b57740fbdd8e3d03b4cdb6a189a9eb8e05cd38035dab195410aa7
928af61cdb allow send rpc take external inputs and solving data (Andrew Chow)
e39b5a5e7a Tests for funding with external inputs (Andrew Chow)
38f5642ccc allow fundtx rpcs to work with external inputs (Andrew Chow)
d5cfb864ae Allow Coin Selection be able to take external inputs (Andrew Chow)
a00eb388e8 Allow CInputCoin to also be constructed with COutPoint and CTxOut (Andrew Chow)
Pull request description:
Currently `fundrawtransaction` and `walletcreatefundedpsbt` both do not allow external inputs as the wallet does not have the information necessary to estimate their fees.
This PR adds an additional argument to both those RPCs which allows the user to specify solving data. This way, the wallet can use that solving data to estimate the size of those inputs. The solving data can be public keys, scripts, or descriptors.
ACKs for top commit:
prayank23:
reACK 928af61cdb
meshcollider:
Re-utACK 928af61cdb
instagibbs:
crACK 928af61cdb
yanmaani:
utACK 928af61.
Tree-SHA512: bc7a6ef8961a7f4971ea5985d75e2d6dc50c2a90b44c664a1c4b0f1be5c1c97823516358fdaab35771a4701dbefc0862127b1d0d4bfd02b4f20d2befa4434700
