82dee87933 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 75122780e2 ("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 82dee87933
Tree-SHA512: 296b8a701f851d482ef6200c6cbf0cf0257a79a828ac6dbc39b05d8c2d839c6fdb9d3f5a084015295cfa3eac7c11faa2f2d52e619c11627b04c75150eead8330
2c6a02e024 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 2c6a02e024
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.
74507ce71e walletdb: Remove BerkeleyBatch friend class from BerkeleyDatabase (Andrew Chow)
00f0041351 No need to check for duplicate fileids in all dbenvs (Andrew Chow)
d86efab370 walletdb: Move Db->open to BerkeleyDatabase::Open (Andrew Chow)
4fe4b3bf1b walletdb: track database file use as m_refcount within BerkeleyDatabase (Andrew Chow)
65fb8807ac 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 74507ce71e
ryanofsky:
Code review ACK 74507ce71e. No changes since last review other than rebase
Tree-SHA512: 845d84ee1a470e2bf5d2e2e3d7738183d8ce43ddd06a0bbd57edecf5779b2f55d70728b1b57f5daab0f078650a8d60c3e19dc30b75b36e7aa952ce268399d5f6
65d0f1a533 devtools: Add security check for separate_code (Wladimir J. van der Laan)
2e9e6377f1 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 65d0f1a533
Tree-SHA512: 6e40e434efea8a8e39f6cb244dfd16aaa5a9db5a2ea762a05d1727357b20e33b7e47c1a652ee88490c9d7952a4caa2f992396fb30346239300d37ae123e36d49
bcfebb6d55 net: save the network type explicitly in CNetAddr (Vasil Dimov)
100c64a95b 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 bcfebb6d55 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 bcfebb6d55
Tree-SHA512: 9347e2a50feac617a994bfb46a8f77e31c236bde882e4fd4f03eea4766cd5110216f5f3d24dee91d25218bab7f8bb6e1d2d6212a44db9e34594299fd6ff7606b
f19fdd47a6 test: add test for CChainState::ResizeCoinsCaches() (James O'Beirne)
8ac3ef4699 add ChainstateManager::MaybeRebalanceCaches() (James O'Beirne)
f36aaa6392 Add CChainState::ResizeCoinsCaches (James O'Beirne)
b223111da2 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 f19fdd47a6 -- 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 f19fdd47a6. 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.
```
fae656144e 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 fae656144e
Tree-SHA512: cf42f96d25474a9dcf0817a049e30e29714731d708f73c40a3042b0c70a71ff08f07dd96a89f0dcd5a50a63a355cf30b3511172a32b8af7d5a2e13ad222a4b49
Rather than just using it on Linux and NetBSD, use `fdatasync()` based
on whether it's available. i.e it is available in newer versions
of FreeBSD (11.1 and later).
This also aligns our code more closely with what is being done in leveldb.
Was pointed out by Luke in #19430.
Closes: #19559
While #19559 has been fixed upstream, it makes sense to not only
recommend using `CC_FOR_BUILD`here until the fix is pulled in as
part of our next libsecp update, but after discussing with Cory,
he suggested we should be setting this on OpenBSD (which still has
the an ancient GCC) regardless.
fdf697fe75 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
5962522fbc depends: bump native_cctools for fixed lto with external clang (Cory Fields)
00d1ba7aaa 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 5962522fbc. The relevant option upstream is [here](https://github.com/tpoechtrager/cctools-port/blob/master/cctools/m4/llvm.m4#L4).
Tree-SHA512: df2775e74e7bc847e6cef94cb8457d503d6c9e2fdea861e51386fa6ed5a7ba688241db3685561ae1a32f66724c1b3801727252025f00c04b90a3bdc8a4f6f93b
a8865f8b72 [net processing] Tidy up Misbehaving() (John Newbery)
d15b3afb4c [net processing] Always supply debug message to Misbehaving() (John Newbery)
634144a1c2 [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 a8865f8b72
jonatack:
ACK a8865f8
fjahr:
Code review ACK a8865f8b72
promag:
Code review ACK a8865f8b72.
Tree-SHA512: 98fb4f5f76399715545a1ea19290dcebfc8cb4eff72a1d3555dd3de6e184040bb8668c9651dab21db0dfd8e674e53a5977105ef76547146c9f6fa6b4b9d2ba59
fa5979d12f rpc: Avoid useless mempool query in gettxoutproof (MarcoFalke)
fa1f7f28cb 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 fa5979d12f
jnewbery:
utACK fa5979d12f
promag:
Code review ACK fa5979d12f.
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>
7b3851e947 refactor: Drop unused CBufferedFile::Seek() (Hennadii Stepanov)
Pull request description:
ACKs for top commit:
practicalswift:
ACK 7b3851e947 -- deleted code is better than unused untested code:)
MarcoFalke:
ACK 7b3851e947, assuming that removing this should either be correct or result in a compile failure
jonasschnelli:
utACK 7b3851e947
promag:
Code review ACK 7b3851e947.
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.
fa2ae0ac8d span: Add Span::empty() and use it in script/descriptor (MarcoFalke)
fa8a992589 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 fa2ae0ac8d
elichai:
ACK fa2ae0ac8d
jonatack:
ACK fa2ae0ac8d
Tree-SHA512: 0ec7b09eef45504973a195923cdf1aa8522117c8e2f69b453e5ce9aa8a7e327c71138518022c32d05133dc99cb861101ed0f60fa891814ee3e9dab3a6fa61a84
Checks that the RPC decodepsbt calculates the fee correctly, in particular for
PSBTs with segwit inputs that have both a witness- and a non-witness-UTXO type
set. Before commit 75122780e2 ("Increment input
value sum only once per UTXO in decodepsbt") the values for those inputs were
double counted.
- Make const things const.
- Replace conditional return with assert.
- Don't log the peer's IP address.
- Log the name Misbehaving directly instead of relying on __func__.
Misbehaving() could optionally take a debug string for printing to the
log file. Make this mandatory and always provide the string.
A couple of additional minor changes:
- remove the unnecessary forward declaration of Misbehaving()
- don't include the nodeid or newline in the passed debug message.
Misbehaving() adds these itself.
655b195747 [net processing] Continue SendMessages processing if not disconnecting peer (John Newbery)
a49781e56d [net processing] Only call MaybeDiscourageAndDisconnect from SendMessages (John Newbery)
a1d5a428a2 [net processing] Fix bad indentation in SendMessages() (John Newbery)
1a1c23f8d4 [net processing] Change cs_main TRY_LOCK to LOCK in SendMessages() (John Newbery)
Pull request description:
The motivation for this PR is to reduce the scope of cs_main locking in misbehavior logic. It is the first set of commits from a larger branch to move the misbehavior data out of CNodeState and into a new struct that doesn't take cs_main.
There are some very minor behavior changes in this branch, such as:
- Not checking for discouragement/disconnect in `ProcessMessages()` (and instead relying on the following check in `SendMessages()`)
- Checking for discouragement/disconnect as the first action in `SendMessages()` (and not doing ping message sending first)
- Continuing through `SendMessages()` if `MaybeDiscourageAndDisconnect()` doesn't disconnect the peer (rather than dropping out of `SendMessages()`
ACKs for top commit:
jonatack:
re-ACK 655b195 per `git range-diff 505b4ed f54af5e 655b195`, code/commit messages review, a bit of code history, and debug build.
MarcoFalke:
ACK 655b195747 only some style-nits 🚁
promag:
Code review ACK 655b195747.
ariard:
Code Review ACK 655b195
Tree-SHA512: fd6d7bc6bb789f5fb7771fb6a45f61a8faba32af93b766554f562144f9631d15c9cc849a383e71743ef73e610b4ee14853666f6fbf08a3ae35176d48c76c65d3
ef3d4ce4c3 build: call AC_PATH_TOOL for dsymutil in macOS cross-compile (fanquake)
Pull request description:
While testing #19530 I noticed that we couldn't call [`dsymutil`](https://www.llvm.org/docs/CommandGuide/dsymutil.html) after LTO:
```bash
../libtool: line 10643: x86_64-apple-darwin16-dsymutil: command not found
```
This updates configure to call `AC_PATH_TOOL` so that we end up with the
full path to dsymutil, similar to `otool` and `install_name_tool`, ie:
`/bitcoin/depends/x86_64-apple-darwin16/share/../native/bin/x86_64-apple-darwin16-dsymutil`.
ACKs for top commit:
laanwj:
Code review ACK ef3d4ce4c3
theuni:
ACK ef3d4ce4c3.
Tree-SHA512: e4fa93e7f9f7945289143dfe2a6645ad8ee7f3bee0793412b3509901a30566d6f952e3b39e0e525a54f8dbd0c480f8da70fc6cb80b07800d11b0c6071fbb7466
