Age | Commit message (Collapse) | Author |
|
10b7a6d532148f880568c529e61a6d7edc7c91a9 refactor: make txmempool interface use GenTxid (Pieter Wuille)
5c124e17407a5b5824fec062b73a03a1030fa28c refactor: make FindTxForGetData use GenTxid (Pieter Wuille)
a2bfac893549e2d62708d8cda7071b4fe9750a2d refactor: use GenTxid in tx request functions (Pieter Wuille)
e65d115b725640eefb3bfa09786447816f7ca9cc test: request parents of orphan from wtxid relay peer (Anthony Towns)
900d7f6c075fd78e63503f31d267dbc16b3983d9 p2p: enable fetching of orphans from wtxid peers (Pieter Wuille)
9efd86a908cf09d9ddbadd3195f202635117d505 refactor: add GenTxid (=txid or wtxid) type and use it for tx request logic (Pieter Wuille)
d362f19355b36531a4a82094e0259f7f3db500a7 doc: list support for BIP 339 in doc/bips.md (Pieter Wuille)
Pull request description:
This is based on https://github.com/bitcoin/bitcoin/pull/18044#discussion_r450687076.
A new type `GenTxid` is added to protocol.h, which represents a tagged txid-or-wtxid. The tx request logic is updated to use these instead of uint256s, permitting per-announcement distinguishing of txid/wtxid (instead of assuming that everything we want to request from a wtxid peer is wtx). Then the restriction of orphan-parent requesting to non-wtxid peers is lifted.
Also document BIP339 in doc/bips.md.
ACKs for top commit:
jnewbery:
Code review ACK 10b7a6d532148f880568c529e61a6d7edc7c91a9
jonatack:
ACK 10b7a6d532148f880568c529e61a6d7edc7c91a9
ajtowns:
ACK 10b7a6d532148f880568c529e61a6d7edc7c91a9 -- code review. Using gtxid to replace the is_txid_or_wtxid flag for the mempool functions is nice.
naumenkogs:
utACK 10b7a6d
Tree-SHA512: d518d13ffd71f8d2b3c175dc905362a7259689e6022a97a0b4f14f1f9fdd87475cf5af70cb12338d1e5d31b52c12e4faaea436114056a2ae9669cb506240758b
|
|
ae4958be95a1158de9992a8e43ce032d87c74f13 rpc: RPCResult Type of MempoolEntryDescription should be OBJ. If multiple entries are possible, wrapping Type should be OBJ_DYN. fixes #19579 (Chris L)
Pull request description:
If multiple entries are possible, wrapping Type should be OBJ_DYN.
fixes #19579
Top commit has no ACKs.
Tree-SHA512: 59cf9f6e9729a69a867e924d8306e0cd6b70a3d702fc5a4111345874bb1224ee51ac3f70cea61b25cfe6bde7f65cb02528d52acc20dda4eda692eddf34f217e8
|
|
e60ef21b8a0f62ebc2e0bdbb5943fa80cc4d98b2 doc: Clang 8 or later is required with FORCE_USE_SYSTEM_CLANG (fanquake)
Pull request description:
The usage of pragmas within the macOS SDK requires LLVM Clang 8. This is
the same version as our prebuilt Clang, however the minimum is worth noting
here as they may diverge and/or expert users might expect they could use an
earlier version.
If you compile depends using `FORCE_USE_SYSTEM_CLANG=1` and Clang 7 you'll see output like:
```bash
In file included from kernel/qcore_mac_objc.mm:44:
In file included from /bitcoin/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers/System/Library/Frameworks/AppKit.framework/Headers/NSText.h:9:
In file included from /bitcoin/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers/System/Library/Frameworks/AppKit.framework/Headers/NSView.h:19:
In file included from /bitcoin/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers/System/Library/Frameworks/AppKit.framework/Headers/NSResponder.h:10:
/bitcoin/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers/System/Library/Frameworks/AppKit.framework/Headers/NSEvent.h:19:1: error:
expected 'push' or 'pop' after '#pragma clang attribute'
/bitcoin/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers/usr/include/os/availability.h:104:273: note: expanded from macro
'API_UNAVAILABLE_BEGIN'
...__API_UNAVAILABLE_BEGIN5, __API_UNAVAILABLE_BEGIN4, __API_UNAVAILABLE_BEGIN3, __API_UNAVAILABLE_BEGIN2, __API_UNAVAILABLE_BEGIN1, 0)(__VA_A...
^
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.
```
I've got a [godbolt here](https://godbolt.org/z/j6r987) that contains a demo of the issue (based off the macOS SDK). It will compile with Clang 8 but not with Clang 7.
ACKs for top commit:
laanwj:
ACK e60ef21b8a0f62ebc2e0bdbb5943fa80cc4d98b2
Tree-SHA512: affc082a4b28d9cd102a844e10f63f55f0f20a9f840e5d8037a41470ad1ea83edc3075ce262b1d618c59c9b75445e52ddd9ba9f84c5ac3da9ce20681473c47b1
|
|
|
|
|
|
|
|
|
|
Based on a commit by Anthony Towns.
|
|
|
|
|
|
284a969cc082ae3c63ab523f22e71da86ad4ab20 Linter to check commit message formatting (Amir Ghorbanian)
Pull request description:
Write linter to check that commit messages have a new line before the body or no body at all. fixes issue #19091.
ACKs for top commit:
troygiorshev:
ACK 284a969cc082ae3c63ab523f22e71da86ad4ab20 Reviewed, manually tested. Works great!
fjahr:
tested ACK 284a969cc082ae3c63ab523f22e71da86ad4ab20
adamjonas:
utACK 284a969cc082ae3c63ab523f22e71da86ad4ab20
Tree-SHA512: fa278f090780b54e4fa6e2967a62b4c1a4da55d112ec1ad6dd7e1181ac490c5c1af0165524b5781b463fdd6d0f79fd3d95b5160184e6eca432ccff1189f77390
|
|
fae8c28dae747f9c4c6481049742346d18202fc8 Pass mempool pointer to GetCoinsCacheSizeState (MarcoFalke)
fac674db200e6b2d5b32069335fb24e713d7b69f Pass mempool pointer to UnloadBlockIndex (MarcoFalke)
faec851b6eb7e65e28cdcae50d6dc86fafa0f91c test: Simplify cs_main locks (MarcoFalke)
Pull request description:
Split out from #19556
Instead of relying on the implicit mempool global, pass a mempool pointer (which can be `0`). This helps with testing, code clarity and unlocks the features described in #19556.
ACKs for top commit:
jnewbery:
code review ACK fae8c28dae747f9c4c6481049742346d18202fc8
fjahr:
Code review ACK fae8c28dae747f9c4c6481049742346d18202fc8
darosior:
Tested ACK fae8c28dae747f9c4c6481049742346d18202fc8
jamesob:
ACK fae8c28dae747f9c4c6481049742346d18202fc8 ([`jamesob/ackr/19604.1.MarcoFalke.pass_mempool_pointer_to`](https://github.com/jamesob/bitcoin/tree/ackr/19604.1.MarcoFalke.pass_mempool_pointer_to))
Tree-SHA512: fa687518c8cda4a095bdbdfe56e01fae2fb16c13d51efbb1312cd6dc007611fc47f53f475602e4a843e3973c9410e6af5a81d6847bd2399f8262ca7205975728
|
|
8ed9002cd14165f751442f738fbf1fb8a37611b2 refactor: use local argsmanager in CRegTestParams (Ivan Metlushko)
9b20f6682845870d6ac53a01d3166fb83c467e7d scripted-diff: Replace gArgs with local argsman (Ivan Metlushko)
a316e9ce265212a7c6c4ef7922420f6ecba9e7b0 refactor: add unused ArgsManager to replace gArgs (Ivan Metlushko)
Pull request description:
Rationale: reduce use of gArgs to decouple code and simplify future maintenance and easier unit testing.
This PR is continuation of work started in #18926 and #18662
It covers only places that register args in ArgsManager with `AddArgs()` or `AddHiddenArgs()`.
Closes #19511
ACKs for top commit:
MarcoFalke:
ACK 8ed9002cd14165f751442f738fbf1fb8a37611b2 👛
Tree-SHA512: 7e6ba8e8357a48833c71e9c3942a769acb3d93bdcc6748a8ef2b7c4461a2499419b60896abf1d8b6bf8e88ee2590284cdd5da64220243ac22375300bcb8fe3e8
|
|
BerkeleyDatabase
0fcff547d5b47822c13104978fda0c486e596526 walletdb: Ensure that having no database handle is a failure (Andrew Chow)
da039d2a915097c23f2b46e063042409bdc3c4f4 Remove BDB dummy databases (Andrew Chow)
0103d6434ea9d155259b40575008239a3762d6f7 Introduce DummyDatabase and use it in the tests (Andrew Chow)
Pull request description:
In the unit tests, we use a dummy `WalletDatabase` which does nothing and always returns true. This is currently implemented by creating a `BerkeleyDatabase` in dummy mode. This PR instead adds a `DummyDatabase` class which does nothing and never fails for use in the tests. `CreateDummyWalletDatabase` is changed to return this `DummyDatabase` and `BerkeleyDatabase` is cleaned up to remove all of the checks for `IsDummy`.
Based on `WalletDatabase` abstract class introduced in #19334
ACKs for top commit:
instagibbs:
utACK https://github.com/bitcoin/bitcoin/pull/19102/commits/0fcff547d5b47822c13104978fda0c486e596526
MarcoFalke:
crACK 0fcff547d5b47822c13104978fda0c486e596526 🚈
Tree-SHA512: 05fbf32e078753e9a55a05f4c080b6d365b909a2a3a8e571b7e64b59ebbe53da49394f70419cc793192ade79f312f5e0422ca7c261ba81bae5912671c5ff6402
|
|
net processing
c251d710a4c2981c6d52362a9a89db84da3d4a67 p2p, refactoring: use CInv helpers in net_processing.cpp (Jon Atack)
4254cd9f8f2437a916b06db4d925ce4eff8c94b9 p2p: add CInv transaction message helper methods (Jon Atack)
Pull request description:
Following the merge of wtxid relay in #18044, this is the first of three refactoring PRs (this one, #19610, and #19611) with no change in behavior, tightly scoped to ease review, to simplify the net processing code and improve encapsulation:
- add `CInv` transaction message helper methods, defined in the class
- use the new helpers in `net_processing.cpp` to simplify the code and improve encapsulation
Test coverage is provided by the functional p2p tests, notably (from seeing which tests failed when breaking things to test coverage) `p2p_segwit`, `p2p_tx_download`, `p2p_feefilter`, and `p2p_permissions`.
ACKs for top commit:
fjahr:
Code review ACK c251d710a4c2981c6d52362a9a89db84da3d4a67
laanwj:
Code review ACK c251d710a4c2981c6d52362a9a89db84da3d4a67
vasild:
ACK c251d71
theStack:
Code-Review ACK c251d710a4c2981c6d52362a9a89db84da3d4a67
hebasto:
ACK c251d710a4c2981c6d52362a9a89db84da3d4a67, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: ead034b3c9e438909b4c5010c570d7930e69063c114290b051b7cebfd9bd5b19f573218bebe8a521256d32e830797f997adad3d85b4539c64ac5762b698e656d
|
|
78c312c983255e15fc274de2368a2ec13ce81cbf Replace current benchmarking framework with nanobench (Martin Ankerl)
Pull request description:
Replace current benchmarking framework with nanobench
This replaces the current benchmarking framework with nanobench [1], an
MIT licensed single-header benchmarking library, of which I am the
autor. This has in my opinion several advantages, especially on Linux:
* fast: Running all benchmarks takes ~6 seconds instead of 4m13s on
an Intel i7-8700 CPU @ 3.20GHz.
* accurate: I ran e.g. the benchmark for SipHash_32b 10 times and
calculate standard deviation / mean = coefficient of variation:
* 0.57% CV for old benchmarking framework
* 0.20% CV for nanobench
So the benchmark results with nanobench seem to vary less than with
the old framework.
* It automatically determines runtime based on clock precision, no need
to specify number of evaluations.
* measure instructions, cycles, branches, instructions per cycle,
branch misses (only Linux, when performance counters are available)
* output in markdown table format.
* Warn about unstable environment (frequency scaling, turbo, ...)
* For better profiling, it is possible to set the environment variable
NANOBENCH_ENDLESS to force endless running of a particular benchmark
without the need to recompile. This makes it to e.g. run "perf top"
and look at hotspots.
Here is an example copy & pasted from the terminal output:
| ns/byte | byte/s | err% | ins/byte | cyc/byte | IPC | bra/byte | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
| 2.52 | 396,529,415.94 | 0.6% | 25.42 | 8.02 | 3.169 | 0.06 | 0.0% | 0.03 | `bench/crypto_hash.cpp RIPEMD160`
| 1.87 | 535,161,444.83 | 0.3% | 21.36 | 5.95 | 3.589 | 0.06 | 0.0% | 0.02 | `bench/crypto_hash.cpp SHA1`
| 3.22 | 310,344,174.79 | 1.1% | 36.80 | 10.22 | 3.601 | 0.09 | 0.0% | 0.04 | `bench/crypto_hash.cpp SHA256`
| 2.01 | 496,375,796.23 | 0.0% | 18.72 | 6.43 | 2.911 | 0.01 | 1.0% | 0.00 | `bench/crypto_hash.cpp SHA256D64_1024`
| 7.23 | 138,263,519.35 | 0.1% | 82.66 | 23.11 | 3.577 | 1.63 | 0.1% | 0.00 | `bench/crypto_hash.cpp SHA256_32b`
| 3.04 | 328,780,166.40 | 0.3% | 35.82 | 9.69 | 3.696 | 0.03 | 0.0% | 0.03 | `bench/crypto_hash.cpp SHA512`
[1] https://github.com/martinus/nanobench
ACKs for top commit:
laanwj:
ACK 78c312c983255e15fc274de2368a2ec13ce81cbf
Tree-SHA512: 9e18770b18b6f95a7d0105a4a5497d31cf4eb5efe6574f4482f6f1b4c88d7e0946b9a4a1e9e8e6ecbf41a3f2d7571240677dcb45af29a6f0584e89b25f32e49e
|
|
once per UTXO)
82dee87933ed0714976ff4eb9657acfc13c6de84 test: test decodepsbt fee calculation (count input value only once per UTXO) (Sebastian Falbesoner)
Pull request description:
Fixes #19523, adding a simple test to `rpc_psbt.py` that checks that the decodepsbt fee matches the one given by the wallet (`walletcreatefundedpsbt`). This is in particular important for PSBTs with segwit inputs that have both a witness- and a non-witness-UTXO type set.
Example test run after reverting commit 75122780e2c46505d977e24c5612dfa9442ab754 ("Increment input value sum only once per UTXO in decodepsbt"):
```
$ test/functional/rpc_psbt.py
2020-07-26T11:31:44.862000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test__sutcd4y
20.00007580
2020-07-26T11:31:47.073000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/home/honeybadger/buidl/bitcoin_thestack/test/functional/test_framework/test_framework.py", line 118, in main
self.run_test()
File "test/functional/rpc_psbt.py", line 166, in run_test
assert_equal(decoded['fee'], created_psbt['fee'])
File "/home/honeybadger/buidl/bitcoin_thestack/test/functional/test_framework/util.py", line 49, in assert_equal
raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
AssertionError: not(20.00007580 == 0.00007580)
2020-07-26T11:31:47.125000Z TestFramework (INFO): Stopping nodes
......
```
ACKs for top commit:
achow101:
ACK 82dee87933ed0714976ff4eb9657acfc13c6de84
Tree-SHA512: 296b8a701f851d482ef6200c6cbf0cf0257a79a828ac6dbc39b05d8c2d839c6fdb9d3f5a084015295cfa3eac7c11faa2f2d52e619c11627b04c75150eead8330
|
|
2c6a02e0248825e205e6deea4c38409044feb4ab Clean message_count and last_message (Troy Giorshev)
Pull request description:
From #19580
This PR changes comments to clarify the intended usage of `message_count` and `last_message`. Additionally it changes the only usage of `message_count` to use `last_message` instead, bringing the code into alignment with the intended usage.
Note: Now `message_count` is completely unused. However, it is ready to be used (i.e. the supporting code works) and likely will be used in some test in the future.
ACKs for top commit:
jnewbery:
utACK 2c6a02e0248825e205e6deea4c38409044feb4ab
Tree-SHA512: 07c7684c9586de4f845e10d7aac36c1aab9fb56b409949c1c70d5ca705bc3971ca7d5943245a0472def4efd7b4e1c5dad2f713db5ead8fca08404daf4891e98b
|
|
Previously having no database handle could still be considered a success
when BerkeleyDatabase and BerkeleyBatch were used for dummy database
things. With dedicated DummyDatabase and DummyBatch classes now, these
should fail.
|
|
|
|
|
|
74507ce71eb61105fb3ae8460999099234ca7b8b walletdb: Remove BerkeleyBatch friend class from BerkeleyDatabase (Andrew Chow)
00f0041351bcd6ddbab110df1189f79ce011e192 No need to check for duplicate fileids in all dbenvs (Andrew Chow)
d86efab37002841fd059251672e1ec1a977b743f walletdb: Move Db->open to BerkeleyDatabase::Open (Andrew Chow)
4fe4b3bf1b152877677a6115f82aefaf318dd514 walletdb: track database file use as m_refcount within BerkeleyDatabase (Andrew Chow)
65fb8807ac402d1e924fd85969b5837c192bf59f Combine BerkeleyEnvironment::Verify into BerkeleyDatabase::Verify (Andrew Chow)
Pull request description:
`BerkeleyBatch` and `BerkeleyDatabase` are kind of messy. The goal of this is to clean up them up so that they are logically separated.
`BerkeleyBatch` currently handles the creation of the `BerkeleyDatabase`'s `Db` handle. This is instead moved into `BerkeleyDatabase` and is called by `BerkeleyBatch`.
Instead of having `BerkeleyEnvironment` track each database's usage, have `BerkeleyDatabase` track this usage itself with the `m_refcount` variable that is present in `WalletDatabase`.
Lastly, instead of having each `BerkeleyEnvironment` store the fileids of the databases open in it, have a global `g_fileids` to track those fileids. We were already checking fileid uniqueness globally (by checking the fileids in every environment when opening a database) so it's cleaner to do this with a global variable.
All of these changes allow us to make `BerkeleyBatch` and `BerkeleyDatabase` no longer be friend classes.
The diff of this PR is currently the same as in ##18971
Requires #19334
ACKs for top commit:
laanwj:
Code review ACK 74507ce71eb61105fb3ae8460999099234ca7b8b
ryanofsky:
Code review ACK 74507ce71eb61105fb3ae8460999099234ca7b8b. No changes since last review other than rebase
Tree-SHA512: 845d84ee1a470e2bf5d2e2e3d7738183d8ce43ddd06a0bbd57edecf5779b2f55d70728b1b57f5daab0f078650a8d60c3e19dc30b75b36e7aa952ce268399d5f6
|
|
65d0f1a53354fb25c8152ee5b430cf57e6508594 devtools: Add security check for separate_code (Wladimir J. van der Laan)
2e9e6377f10ab1ca5021f6ec964d22161469ba60 build: add -Wl,-z,separate-code to hardening flags (fanquake)
Pull request description:
TLDR: We are generally explicit about the hardening related flags we use,
rather than letting the distro / toolchain decide via their defaults. This PR
adds `-z,separate-code` which has been enabled by default for Linux targets
since binutils 2.31. Ubuntu Bionic (currently used for gitian) ships with
binutils 2.30, so this will enable the option for those builds.
This flag was added to binutils/ld in the 2.30 release,
see commit c11c786f0b45617bb8807ab6a57220d5ff50e414:
> The new "-z separate-code" option will generate separate code LOAD
segment which must be in wholly disjoint pages from any other data.
It was made the default for Linux/x86 targets in the 2.31 release, see commit
f6aec96dce1ddbd8961a3aa8a2925db2021719bb:
> This patch adds --enable-separate-code to ld configure to turn on
-z separate-code by default and enables it by default for Linux/x86.
This avoids mixing code pages with data to improve cache performance
as well as security.
> To reduce x86-64 executable and shared object sizes, the maximum page
size is reduced from 2MB to 4KB when -z separate-code is turned on by
default. Note: -z max-page-size= can be used to set the maximum page
size.
> We compared SPEC CPU 2017 performance before and after this change on
Skylake server. There are no any significant performance changes.
Everything is mostly below +/-1%.
Support was also added to LLVMs lld: https://reviews.llvm.org/D64903, however
there it remains off by default.
There were concerns about an increase in binary size, however in our case, the
difference would seem negligible, given we are shipping a
multi-megabyte binary, which then downloads 100's of GBs of data.
Also note that most recent versions of distros are shipping a new enough version
of binutils that this is available and/or already on by default (assuming the distro
has not turned it off, I haven't checked everywhere):
CentOS 8: 2.30
Debian Buster 2.31.1
Fedora 29: 2.31.1
FreeBSD: 2.33
GNU Guix: 2.33 / 2.34
Ubuntu 18.04: 2.30
Related threads / discussion:
https://bugzilla.redhat.com/show_bug.cgi?id=1623218
The ELF header when building on Debian Buster (where it's already enabled by default in binutils):
```bash
Program Header:
PHDR off 0x0000000000000040 vaddr 0x0000000000000040 paddr 0x0000000000000040 align 2**3
filesz 0x00000000000002a0 memsz 0x00000000000002a0 flags r--
INTERP off 0x00000000000002e0 vaddr 0x00000000000002e0 paddr 0x00000000000002e0 align 2**0
filesz 0x000000000000001c memsz 0x000000000000001c flags r--
LOAD off 0x0000000000000000 vaddr 0x0000000000000000 paddr 0x0000000000000000 align 2**12
filesz 0x0000000000038f10 memsz 0x0000000000038f10 flags r--
LOAD off 0x0000000000039000 vaddr 0x0000000000039000 paddr 0x0000000000039000 align 2**12
filesz 0x00000000006b9389 memsz 0x00000000006b9389 flags r-x
LOAD off 0x00000000006f3000 vaddr 0x00000000006f3000 paddr 0x00000000006f3000 align 2**12
filesz 0x0000000000204847 memsz 0x0000000000204847 flags r--
LOAD off 0x00000000008f7920 vaddr 0x00000000008f8920 paddr 0x00000000008f8920 align 2**12
filesz 0x00000000000183e0 memsz 0x0000000000022fd0 flags rw-
DYNAMIC off 0x000000000090adb0 vaddr 0x000000000090bdb0 paddr 0x000000000090bdb0 align 2**3
filesz 0x0000000000000240 memsz 0x0000000000000240 flags rw-
```
vs when opting out using `-Wl,-z,noseparate-code`:
```bash
Program Header:
PHDR off 0x0000000000000040 vaddr 0x0000000000000040 paddr 0x0000000000000040 align 2**3
filesz 0x0000000000000230 memsz 0x0000000000000230 flags r--
INTERP off 0x0000000000000270 vaddr 0x0000000000000270 paddr 0x0000000000000270 align 2**0
filesz 0x000000000000001c memsz 0x000000000000001c flags r--
LOAD off 0x0000000000000000 vaddr 0x0000000000000000 paddr 0x0000000000000000 align 2**12
filesz 0x00000000008f6a87 memsz 0x00000000008f6a87 flags r-x
LOAD off 0x00000000008f7920 vaddr 0x00000000008f8920 paddr 0x00000000008f8920 align 2**12
filesz 0x00000000000183e0 memsz 0x0000000000022fd0 flags rw-
DYNAMIC off 0x000000000090adb0 vaddr 0x000000000090bdb0 paddr 0x000000000090bdb0 align 2**3
filesz 0x0000000000000240 memsz 0x0000000000000240 flags rw-
```
ACKs for top commit:
laanwj:
ACK 65d0f1a53354fb25c8152ee5b430cf57e6508594
Tree-SHA512: 6e40e434efea8a8e39f6cb244dfd16aaa5a9db5a2ea762a05d1727357b20e33b7e47c1a652ee88490c9d7952a4caa2f992396fb30346239300d37ae123e36d49
|
|
bcfebb6d5511ad4c156868bc799831ace628a225 net: save the network type explicitly in CNetAddr (Vasil Dimov)
100c64a95b518a6a19241aec4058b866a8872d9b net: document `enum Network` (Vasil Dimov)
Pull request description:
(chopped off from https://github.com/bitcoin/bitcoin/pull/19031 to ease review)
Before this change, we would analyze the contents of `CNetAddr::ip[16]`
in order to tell which type is an address. Change this by introducing a
new member `CNetAddr::m_net` that explicitly tells the type of the
address.
This is necessary because in BIP155 we will not be able to tell the
address type by just looking at its raw representation (e.g. both TORv3
and I2P are "seemingly random" 32 bytes).
As a side effect of this change we no longer need to store IPv4
addresses encoded as IPv6 addresses - we can store them in proper 4
bytes (will be done in a separate commit). Also the code gets
somewhat simplified - instead of
`memcmp(ip, pchIPv4, sizeof(pchIPv4)) == 0` we can use
`m_net == NET_IPV4`.
ACKs for top commit:
troygiorshev:
reACK bcfebb6d5511ad4c156868bc799831ace628a225 via `git range-diff master 64897c5 bcfebb6`
jonatack:
re-ACK bcfebb6 per `git diff 662bb25 bcfebb6`, code review, debug build/tests clean, ran bitcoind.
laanwj:
Code review ACK bcfebb6d5511ad4c156868bc799831ace628a225
Tree-SHA512: 9347e2a50feac617a994bfb46a8f77e31c236bde882e4fd4f03eea4766cd5110216f5f3d24dee91d25218bab7f8bb6e1d2d6212a44db9e34594299fd6ff7606b
|
|
|
|
|
|
|
|
-BEGIN VERIFY SCRIPT-
sed -i -e 's/gArgs.Add/argsman.Add/g' `git grep -l "gArgs.Add"`
-END VERIFY SCRIPT-
|
|
|
|
|
|
f19fdd47a6371dcbe0760ef6f3c3c5adb31b1bb4 test: add test for CChainState::ResizeCoinsCaches() (James O'Beirne)
8ac3ef46999ed676ca3775f7b2f461d92f09a542 add ChainstateManager::MaybeRebalanceCaches() (James O'Beirne)
f36aaa6392fdbdac6891d92202d3efeff98754f4 Add CChainState::ResizeCoinsCaches (James O'Beirne)
b223111da2e0e9ceccef75df8a20252b0094b7bc txdb: add CCoinsViewDB::ChangeCacheSize (James O'Beirne)
Pull request description:
This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11):
Parent PR: #15606
Issue: #15605
Specification: https://github.com/jamesob/assumeutxo-docs/tree/master/proposal
---
In the assumeutxo implementation draft (#15056), once a UTXO snapshot is loaded, a new chainstate object is created after initialization. This means that we have to reclaim some of the cache that we've allocated to the original chainstate (per `dbcache=`) to repurpose for the snapshot chainstate.
Furthermore, it makes sense to have different cache allocations depending on which chainstate is more active. While the snapshot chainstate is working to get to the network tip (and the background validation chainstate is idle), it makes sense that the snapshot chainstate should have the majority of cache allocation. And contrariwise once the snapshot has reached network tip, most of the cache should be given to the background validation chainstate.
This set of changes (detailed in the commit messages) allows us to dynamically resize the various coins caches. None of the functionality introduced here is used at the moment, but will be in the next AU PR (which introduces `ActivateSnapshot`).
`ChainstateManager::MaybeRebalanceCaches()` defines the (somewhat normative) cache allocations between the snapshot and background validation chainstates. I'd be interested in feedback if anyone has thoughts on the proportions I've set there.
ACKs for top commit:
ajtowns:
weak utACK f19fdd47a6371dcbe0760ef6f3c3c5adb31b1bb4 -- didn't find any major problems, but not super confident that I didn't miss anything
fjahr:
Code review ACK f19fdd4
ryanofsky:
Code review ACK f19fdd47a6371dcbe0760ef6f3c3c5adb31b1bb4. Only change since last review is constructor cleanup (no change in behavior). I think the suggestions here from ajtowns and others are good, but shouldn't delay merging the PR (and hold up assumeutxo)
Tree-SHA512: fffb7847fb6993dd4a1a41cf11179b211b0b20b7eb5f7cf6266442136bfe9d43b830bbefcafd475bfd4af273f5573500594aa41fff03e0ed5c2a1e8562ff9269
|
|
The usage of pragmas within the macOS SDK requires LLVM Clang 8. This is
the version as our prebuilt Clang, however the minimum is worth noting here
as they may diverge and/or expert users might expect they could use an
earlier version.
If you compile using Clang 7 you'll see output like:
```bash
In file included from kernel/qcore_mac_objc.mm:44:
In file included from /bitcoin/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers/System/Library/Frameworks/AppKit.framework/Headers/NSText.h:9:
In file included from /bitcoin/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers/System/Library/Frameworks/AppKit.framework/Headers/NSView.h:19:
In file included from /bitcoin/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers/System/Library/Frameworks/AppKit.framework/Headers/NSResponder.h:10:
/bitcoin/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers/System/Library/Frameworks/AppKit.framework/Headers/NSEvent.h:19:1: error:
expected 'push' or 'pop' after '#pragma clang attribute'
/bitcoin/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers/usr/include/os/availability.h:104:273: note: expanded from macro
'API_UNAVAILABLE_BEGIN'
...__API_UNAVAILABLE_BEGIN5, __API_UNAVAILABLE_BEGIN4, __API_UNAVAILABLE_BEGIN3, __API_UNAVAILABLE_BEGIN2, __API_UNAVAILABLE_BEGIN1, 0)(__VA_A...
^
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.
```
|
|
fae656144e9ec25a0b610cd5278c1b7c42880097 travis: Re-enable s390x (MarcoFalke)
Pull request description:
According to travis, the issue has been solved. Quote
> I would like to confirm that we have resolved this issue and most of our users are reported that this issue has been resolved on their end as well. Could you please re-check and see if that still exists for you?
ACKs for top commit:
theStack:
ACK https://github.com/bitcoin/bitcoin/pull/19613/commits/fae656144e9ec25a0b610cd5278c1b7c42880097
Tree-SHA512: cf42f96d25474a9dcf0817a049e30e29714731d708f73c40a3042b0c70a71ff08f07dd96a89f0dcd5a50a63a355cf30b3511172a32b8af7d5a2e13ad222a4b49
|
|
|
|
fdf697fe750e99c29f27ca855cb1b30428248a3f ci: Increase CCACHE_SIZE in some builds on Travis (Hennadii Stepanov)
Pull request description:
This PR will decrease build time for the longest Travis builds.
Up to ~20 min improvement with warmed up cache is expected.
Top commit has no ACKs.
Tree-SHA512: ee3a22162d03537be11e60b53de043247f6a65e35e630e0807b758a846b7e05ef2059d18846644aafa3cd5dc7d3e3f56a8ccb94cc71b95227debc9201d3142a2
|
|
5962522fbcb7b72c269fc271ac4463791a103918 depends: bump native_cctools for fixed lto with external clang (Cory Fields)
00d1ba7aaadbe881995ac1497633a012bc487bdd depends: enable lto support for Apple's ld64 (Cory Fields)
Pull request description:
This didn't work for a few reasons (various toolchain compatibility issues) the last time I tested it, but after the last round of bumps it works with no apparent issues.
Note that this does not _enable_ LTO by default in any way, only hooks up the machinery for ```-flto``` to work correctly when specified.
Lines were split for an easier rebase after #17919 is merged.
ACKs for top commit:
fanquake:
ACK 5962522fbcb7b72c269fc271ac4463791a103918. The relevant option upstream is [here](https://github.com/tpoechtrager/cctools-port/blob/master/cctools/m4/llvm.m4#L4).
Tree-SHA512: df2775e74e7bc847e6cef94cb8457d503d6c9e2fdea861e51386fa6ed5a7ba688241db3685561ae1a32f66724c1b3801727252025f00c04b90a3bdc8a4f6f93b
|
|
623f66554d44611485fa14937b6fd4f1dc63b818 Fix .gitignore for src/test/fuzz directory (Hennadii Stepanov)
Pull request description:
On master (31d2b4098a9e4ee9a694ba1ad42829637cbcf3c6):
```
$ git ls-files --ignored --exclude-standard
contrib/init/org.bitcoin.bitcoind.plist
contrib/macdeploy/fancy.plist
src/qt/Makefile
src/qt/test/Makefile
src/test/Makefile
src/test/fuzz/FuzzedDataProvider.h
src/test/fuzz/addition_overflow.cpp
src/test/fuzz/addrdb.cpp
src/test/fuzz/asmap.cpp
src/test/fuzz/asmap_direct.cpp
src/test/fuzz/autofile.cpp
src/test/fuzz/banman.cpp
src/test/fuzz/base_encode_decode.cpp
src/test/fuzz/bech32.cpp
src/test/fuzz/block.cpp
src/test/fuzz/block_header.cpp
src/test/fuzz/blockfilter.cpp
src/test/fuzz/bloom_filter.cpp
src/test/fuzz/buffered_file.cpp
src/test/fuzz/chain.cpp
src/test/fuzz/checkqueue.cpp
src/test/fuzz/coins_view.cpp
src/test/fuzz/crypto.cpp
src/test/fuzz/crypto_aes256.cpp
src/test/fuzz/crypto_aes256cbc.cpp
src/test/fuzz/crypto_chacha20.cpp
src/test/fuzz/crypto_chacha20_poly1305_aead.cpp
src/test/fuzz/crypto_common.cpp
src/test/fuzz/crypto_hkdf_hmac_sha256_l32.cpp
src/test/fuzz/crypto_poly1305.cpp
src/test/fuzz/cuckoocache.cpp
src/test/fuzz/decode_tx.cpp
src/test/fuzz/descriptor_parse.cpp
src/test/fuzz/deserialize.cpp
src/test/fuzz/eval_script.cpp
src/test/fuzz/fee_rate.cpp
src/test/fuzz/fees.cpp
src/test/fuzz/flatfile.cpp
src/test/fuzz/float.cpp
src/test/fuzz/fuzz.cpp
src/test/fuzz/fuzz.h
src/test/fuzz/golomb_rice.cpp
src/test/fuzz/hex.cpp
src/test/fuzz/http_request.cpp
src/test/fuzz/integer.cpp
src/test/fuzz/key.cpp
src/test/fuzz/key_io.cpp
src/test/fuzz/kitchen_sink.cpp
src/test/fuzz/load_external_block_file.cpp
src/test/fuzz/locale.cpp
src/test/fuzz/merkleblock.cpp
src/test/fuzz/message.cpp
src/test/fuzz/multiplication_overflow.cpp
src/test/fuzz/net_permissions.cpp
src/test/fuzz/netaddress.cpp
src/test/fuzz/p2p_transport_deserializer.cpp
src/test/fuzz/parse_hd_keypath.cpp
src/test/fuzz/parse_iso8601.cpp
src/test/fuzz/parse_numbers.cpp
src/test/fuzz/parse_script.cpp
src/test/fuzz/parse_univalue.cpp
src/test/fuzz/policy_estimator.cpp
src/test/fuzz/policy_estimator_io.cpp
src/test/fuzz/pow.cpp
src/test/fuzz/prevector.cpp
src/test/fuzz/primitives_transaction.cpp
src/test/fuzz/process_message.cpp
src/test/fuzz/process_messages.cpp
src/test/fuzz/protocol.cpp
src/test/fuzz/psbt.cpp
src/test/fuzz/random.cpp
src/test/fuzz/rbf.cpp
src/test/fuzz/rolling_bloom_filter.cpp
src/test/fuzz/script.cpp
src/test/fuzz/script_bitcoin_consensus.cpp
src/test/fuzz/script_descriptor_cache.cpp
src/test/fuzz/script_flags.cpp
src/test/fuzz/script_interpreter.cpp
src/test/fuzz/script_ops.cpp
src/test/fuzz/script_sigcache.cpp
src/test/fuzz/script_sign.cpp
src/test/fuzz/scriptnum_ops.cpp
src/test/fuzz/signature_checker.cpp
src/test/fuzz/span.cpp
src/test/fuzz/spanparsing.cpp
src/test/fuzz/string.cpp
src/test/fuzz/strprintf.cpp
src/test/fuzz/system.cpp
src/test/fuzz/timedata.cpp
src/test/fuzz/transaction.cpp
src/test/fuzz/tx_in.cpp
src/test/fuzz/tx_out.cpp
src/test/fuzz/util.h
src/univalue/gen/gen.cpp
test/functional/data/wallets/high_minversion/db.log
test/functional/data/wallets/high_minversion/wallet.dat
```
With this PR:
```
$ git ls-files --ignored --exclude-standard
contrib/init/org.bitcoin.bitcoind.plist
contrib/macdeploy/fancy.plist
src/qt/Makefile
src/qt/test/Makefile
src/test/Makefile
src/univalue/gen/gen.cpp
test/functional/data/wallets/high_minversion/db.log
test/functional/data/wallets/high_minversion/wallet.dat
```
ACKs for top commit:
MarcoFalke:
review ACK 623f66554d44611485fa14937b6fd4f1dc63b818 seems like an improvement when writing new fuzz tests
practicalswift:
ACK 623f66554d44611485fa14937b6fd4f1dc63b818 -- thanks for fixing! ❤️
theStack:
tested ACK 623f66554d44611485fa14937b6fd4f1dc63b818
Tree-SHA512: 16b3854bf4fd8c3096d915a4efc5cbc63d28b18854b051bafee374508dfbb5871ae7dc6f303dbf57469473082d2c3a7df0a8170da22d60d13878544679363b5c
|
|
a8865f8b7215a975fd3dd9d97d7f791ac93ea65d [net processing] Tidy up Misbehaving() (John Newbery)
d15b3afb4cd1b00ad698e6dd19c5861a53e01c42 [net processing] Always supply debug message to Misbehaving() (John Newbery)
634144a1c2a3506fd6285e76f3ce0cbb3648cc69 [net processing] Fixup MaybeDiscourageAndDisconnect() style (John Newbery)
Pull request description:
This PR makes a few minor clean-ups to `Misbehaving()` in preparation to move it out of the cs_main lock.
There are very minor logging changes but otherwise no functional changes.
ACKs for top commit:
troygiorshev:
tACK a8865f8b7215a975fd3dd9d97d7f791ac93ea65d
jonatack:
ACK a8865f8
fjahr:
Code review ACK a8865f8b7215a975fd3dd9d97d7f791ac93ea65d
promag:
Code review ACK a8865f8b7215a975fd3dd9d97d7f791ac93ea65d.
Tree-SHA512: 98fb4f5f76399715545a1ea19290dcebfc8cb4eff72a1d3555dd3de6e184040bb8668c9651dab21db0dfd8e674e53a5977105ef76547146c9f6fa6b4b9d2ba59
|
|
fa5979d12f8c65754e36cdddb9d032ab81fecc3a rpc: Avoid useless mempool query in gettxoutproof (MarcoFalke)
fa1f7f28cbc48675514a01c4ca773f1ba8ea836b rpc: Style fixups in gettxoutproof (MarcoFalke)
Pull request description:
`GetTransaction` implicitly and unconditionally asks the mempool global for a transaction. This is problematic for several reasons:
* `gettxoutproof` is for on-chain txs only and asking the mempool for on-chain txs is confusing and minimally wasteful
* Globals are confusing and make code harder to test with unit tests
Fix both issues by passing in an optional mempool. This also helps with #19556
ACKs for top commit:
hebasto:
re-ACK fa5979d12f8c65754e36cdddb9d032ab81fecc3a
jnewbery:
utACK fa5979d12f8c65754e36cdddb9d032ab81fecc3a
promag:
Code review ACK fa5979d12f8c65754e36cdddb9d032ab81fecc3a.
Tree-SHA512: 048361b82abfcc40481181bd44f70cfc9e97d5d6356549df34bbe30b9de7a0a72d2207a3ad0279b21f06293509b284d8967f58ca7e716263a22b20aa4e7f9c54
|
|
Check that sections are appropriately separated in virtual memory,
based on their (expected) permissions. This checks for missing
-Wl,-z,separate-code and potentially other problems.
Co-authored-by: fanquake <fanquake@gmail.com>
|
|
This flag was added to binutils/ld in the 2.30 release,
see commit c11c786f0b45617bb8807ab6a57220d5ff50e414:
> The new "-z separate-code" option will generate separate code LOAD
segment which must be in wholly disjoint pages from any other data.
It was made the default for Linux/x86 targets in the 2.31 release, see commit
f6aec96dce1ddbd8961a3aa8a2925db2021719bb:
> This patch adds --enable-separate-code to ld configure to turn on
-z separate-code by default and enables it by default for Linux/x86.
This avoids mixing code pages with data to improve cache performance
as well as security.
> To reduce x86-64 executable and shared object sizes, the maximum page
size is reduced from 2MB to 4KB when -z separate-code is turned on by
default. Note: -z max-page-size= can be used to set the maximum page
size.
> We compared SPEC CPU 2017 performance before and after this change on
Skylake server. There are no any significant performance changes.
Everything is mostly below +/-1%.
Support was also added to LLVMs lld: https://reviews.llvm.org/D64903, however
there is remains off by default.
There were concerns about an increase in binary size, however in our case, the
increase (1 page worth of bytes) would seem negligible, given we are shipping a
multi-megabyte binary, which then downloads 100's of GBs of data.
Also note that most recent versions of distros are shipping a new enough version
of binutils that this is available and/or on by default (assuming the distro has
not turned it off, I haven't checked everywhere):
CentOS 8: 2.30
Debian Buster 2.31.1
Fedora 29: 2.31.1
FreeBSD: 2.33
GNU Guix: 2.33 / 2.34
Ubuntu 18.04: 2.30
Related threads / discussion:
https://bugzilla.redhat.com/show_bug.cgi?id=1623218
|
|
Before this change, we would analyze the contents of `CNetAddr::ip[16]`
in order to tell which type is an address. Change this by introducing a
new member `CNetAddr::m_net` that explicitly tells the type of the
address.
This is necessary because in BIP155 we will not be able to tell the
address type by just looking at its raw representation (e.g. both TORv3
and I2P are "seemingly random" 32 bytes).
As a side effect of this change we no longer need to store IPv4
addresses encoded as IPv6 addresses - we can store them in proper 4
bytes (will be done in a separate commit). Also the code gets
somewhat simplified - instead of
`memcmp(ip, pchIPv4, sizeof(pchIPv4)) == 0` we can use
`m_net == NET_IPV4`.
Co-authored-by: Carl Dong <contact@carldong.me>
|
|
7b3851e9473e74043342d414c056c2ef87c2f261 refactor: Drop unused CBufferedFile::Seek() (Hennadii Stepanov)
Pull request description:
ACKs for top commit:
practicalswift:
ACK 7b3851e9473e74043342d414c056c2ef87c2f261 -- deleted code is better than unused untested code:)
MarcoFalke:
ACK 7b3851e9473e74043342d414c056c2ef87c2f261, assuming that removing this should either be correct or result in a compile failure
jonasschnelli:
utACK 7b3851e9473e74043342d414c056c2ef87c2f261
promag:
Code review ACK 7b3851e9473e74043342d414c056c2ef87c2f261.
Tree-SHA512: 7bfd172aa4bbe349855c1303fd9cd58093d66833fefe46bd29081bfcca4ab434b84c6b84e76e94d06b8749a5abe1dc1e184f5189136cd1403d0e5bc25ad6d456
|
|
This commit clarifies the intended usage of message_count and
last_message. Additionally it changes the only usage of message_count
to using last_message instead, bringing the code further along the
intended usage.
|
|
to simplify the code and reach less from it into the CInv class internals
|
|
|
|
|
|
|
|
fa2ae0ac8d43086430a29c73940ad6b1cd129e96 span: Add Span::empty() and use it in script/descriptor (MarcoFalke)
fa8a99258947a9ee3749fa472180542920cd471c Work around memory-aliasing in descriptor ParsePubkey (MarcoFalke)
Pull request description:
While this is not undefined behaviour, the memory aliasing trick is confusing when reading the code. Having `a.size()==0` and then access `a[0]` works in this particular case, but should probably be avoided to harden the code for the future.
ACKs for top commit:
theStack:
re-ACK https://github.com/bitcoin/bitcoin/pull/19508/commits/fa2ae0ac8d43086430a29c73940ad6b1cd129e96
elichai:
ACK fa2ae0ac8d43086430a29c73940ad6b1cd129e96
jonatack:
ACK fa2ae0ac8d43086430a29c73940ad6b1cd129e96
Tree-SHA512: 0ec7b09eef45504973a195923cdf1aa8522117c8e2f69b453e5ce9aa8a7e327c71138518022c32d05133dc99cb861101ed0f60fa891814ee3e9dab3a6fa61a84
|
|
|