e47c6c7656 Reset settings.json when GUI options are reset (Ryan Ofsky)
99ccc02b65 Add release notes about unified bitcoin-qt and bitcoind persistent settings (Ryan Ofsky)
504b06b1de Migrate -lang setting from QSettings to settings.json (Ryan Ofsky)
9a016a3c07 Migrate -prune setting from QSettings to settings.json (Ryan Ofsky)
f067e19433 Migrate -proxy and -onion settings from QSettings to settings.json (Ryan Ofsky)
a09e3b7cf2 Migrate -listen and -server settings from QSettings to settings.json (Ryan Ofsky)
d2ada6e635 Migrate -upnp and -natpmp settings from QSettings to settings.json (Ryan Ofsky)
1dc4fc29c1 Migrate -spendzeroconfchange and -signer settings from QSettings to settings.json (Ryan Ofsky)
a7ef6d5975 Migrate -par setting from QSettings to settings.json (Ryan Ofsky)
284f339de6 Migrate -dbcache setting from QSettings to settings.json (Ryan Ofsky)
Pull request description:
If a setting like pruning, port mapping, or a network proxy is enabled in the GUI, it will now be stored in the bitcoin persistent setting file in the datadir and shared with bitcoind, instead of being stored as Qt settings which end up in the the windows registry or platform specific config files and are ignored by bitcoind.
This PR has been split off from bitcoin/bitcoin#15936 so some review of these commits previously took place in that PR.
ACKs for top commit:
furszy:
Code review ACK e47c6c76
hebasto:
ACK e47c6c7656
Tree-SHA512: 076ea7c7efe67805b4a357113bfe1643dce364d0032774106de59566a0ed5771d57a5923920085e03d686beb34b98114bd278555dfdf8bb7af0b778b0f35b7d2
1f653dc262 qt, wallet, refactor: Make `WalletModel::sendCoins()` return `void` (Hennadii Stepanov)
Pull request description:
Currently, the `WalletModel::sendCoins()` function always returns the same value.
Also dead and noop (calling `processSendCoinsReturn(OK)`) code has been removed.
The other `return` statements have been removed from the `WalletModel::sendCoins()` function in bitcoin/bitcoin#17154 and bitcoin/bitcoin#17165.
ACKs for top commit:
kristapsk:
cr ACK 1f653dc262
furszy:
Code review ACK 1f653dc2
shaavan:
Code Review ACK 1f653dc262
w0xlt:
Code Review ACK 1f653dc262
Tree-SHA512: 2b59495a7fc10b4de30fcc63fc3af92d50406e16031112eb72494736dce193ac1fbac0802623496cf81edcd16766e1647d9c4f3a607b3eb84cc50e273b999c04
fa7a711a30 test: Fix out-of-range port collisions (MacroFake)
Pull request description:
Otherwise the test will fail if two tests running in parallel use the same port. See https://github.com/bitcoin/bitcoin/pull/25096#discussion_r892558783 and https://github.com/bitcoin/bitcoin/pull/25312
ACKs for top commit:
dergoegge:
ACK fa7a711a30 - This gets rid of some rather arbitrary choices for ports in some of our functional tests that can cause port collisions across test runs, resulting in intermittent failures.
Tree-SHA512: ac73da8a498230b992ab12e1ee3c4ff3d868cd63c00d2c71537d156cb7c8f8be8598ec574646b17c5a44ae3ac5bb54bf29d300f054a36cec6f6ce8054a0da0a4
ce893c0497 doc: Update developer notes (Anthony Towns)
d2852917ee sync.h: Imply negative assertions when calling LOCK (Anthony Towns)
bba87c0553 scripted-diff: Convert global Mutexes to GlobalMutexes (Anthony Towns)
a559509a0b sync.h: Add GlobalMutex type (Anthony Towns)
be6aa72f9f qt/clientmodel: thread safety annotation for m_cached_tip_mutex (Anthony Towns)
f24bd45b37 net_processing: thread safety annotation for m_tx_relay_mutex (Anthony Towns)
Pull request description:
This changes `LOCK(mutex)` for non-global, non-recursive mutexes to be annotated with the negative capability for the mutex it refers to, to prevent . clang applies negative capabilities recursively, so this helps avoid forgetting to annotate functions.
This can't reasonably be used for globals, because clang would require every function to be annotated with `EXCLUSIVE_LOCKS_REQUIRED(!g_mutex)` for each global mutex; so this introduces a trivial `GlobalMutex` subclass of `Mutex`, and reduces the annotations for both `GlobalMutex` to `LOCKS_EXCLUDED` which only catches trivial errors (eg (`LOCK(x); LOCK(x);`).
ACKs for top commit:
MarcoFalke:
review ACK ce893c0497🐦
hebasto:
ACK ce893c0497
Tree-SHA512: 5c35e8c7677ce3d994a7e3774f4344adad496223a51b3a1d1d3b5f20684b2e1d5cff688eb3fbc8d33e1b9940dfa76e515f9434e21de6f3ce3c935e29a319f529
ea54ba2f42 [test] Fix port collisions caused by p2p_getaddr_caching.py (dergoegge)
f9682e75ac [test_framework] Set PortSeed.n directly after initialising params (dergoegge)
Pull request description:
This PR fixes the issue mentioned [here](https://github.com/bitcoin/bitcoin/pull/25096#discussion_r892558783), to avoid port collisions between nodes spun up by the test framework.
Top commit has no ACKs.
Tree-SHA512: ec9159f0af90db636f7889d664c24e1430cf2bcb3c02a9ab2dcfe531b2a4d18f6e3a0f8ba73071bdf2f7db518df9d5d86a9cd06695e67644d20fe4515fac32b7
d575413fb8 doc: add `desig` to ignore-words (brunoerg)
c06cc41ddb doc: fix typo in kernel/context.h (brunoerg)
Pull request description:
This PR fixes a typo in `kernel/context.h` (libary => library) and add `desig` to ignore-words since it's a valid word, see:
b9416c3847/src/net.cpp (L1105-L1117)
ACKs for top commit:
fanquake:
ACK d575413fb8
Tree-SHA512: 2d548c737b8184d0243445c7503f3f68256ecb0970bd834d52de099de3cd8c8b9c140e2b77d55e2542fbd45b1d21cbdee639f5b2ef8138c37b8b72e5211029c3
8888bd43c1 Remove redundant nLastTry check (MarcoFalke)
00001e57fe Remove redundant nTime checks (MarcoFalke)
Pull request description:
Split out from https://github.com/bitcoin/bitcoin/pull/24697 because it makes sense on its own.
ACKs for top commit:
dergoegge:
re-ACK 8888bd43c1
naumenkogs:
utACK 8888bd43c1
Tree-SHA512: 32c6cde1c71e943c76b7991c2c24caf29ae467ab4ea2d758483a0cee64625190d1a833b468e8eab1f834beeb2c365af96552c14b05270f08cf63790e0707581d
fa74b63c01 test: Fix wait_for_debug_log UnicodeDecodeError (MacroFake)
Pull request description:
Fix the intermittent `UnicodeDecodeError` when the debug log is truncated on an (multi-byte) unicode character by treating everything as bytes.
Also, remove the `ignore_case` option and the`re.search+re.escape` wrap. All of this is unused and doesn't exist on raw byte strings.
Fixes https://github.com/bitcoin/bitcoin/issues/24575
ACKs for top commit:
jonatack:
ACK fa74b63c01
brunoerg:
ACK fa74b63c01
Tree-SHA512: c67c9355073e784fa8d9d48b8e79ff0c98f5ae9cd4d704ad12a76d2604733946054bc74b8ab346aa2184db23d740b85c8c13eb892d76cba92e42ebfd73f2f1bf
06e18e0b53 build: use BOOST_MULTI_INDEX_ENABLE_SAFE_MODE when debugging (fanquake)
Pull request description:
Use of this macro enables precondition checks for iterators and functions of the library. It's use is recommended in debug builds. See https://www.boost.org/doc/libs/1_78_0/libs/multi_index/doc/tutorial/debug.html for more info.
There is also a `BOOST_MULTI_INDEX_ENABLE_INVARIANT_CHECKING` macro:
> When this mode is in effect, all public functions of Boost.MultiIndex will perform post-execution tests aimed at ensuring that the basic internal invariants of the data structures managed are preserved.
ACKs for top commit:
laanwj:
Concept and code review ACK 06e18e0b53
Tree-SHA512: 7ee489eccda81c7dbca9210af6d3007d5b2c704b645139d2714c077af157789dd9478c29d0d212e210e96686ea83713aaf3d458e879122b3cde64f3e3e3789d2
292828cd77 [test] Test addr cache for multiple onion binds (dergoegge)
3382905bef [net] Seed addr cache randomizer with port from binding address (dergoegge)
f10e80b6e4 [net] Use ConnectedThroughNetwork() instead of GetNetwork() to seed addr cache randomizer (dergoegge)
Pull request description:
The addr cache id randomizer is currently supposed to be seeded with the network of the inbound connection and the local socket (only the address is used not the port): a8098f2cef/src/net.cpp (L2800-L2804)
For inbound onion connections `CNode::addr.GetNetwork()` returns `NET_UNROUTABLE` and `CNode::addrBind` is set to `127.0.0.1:<onion bind port>`. This results in the same addr cache for all inbound connections on 127.0.0.1 binds.
To avoid the same addr cache across all onion and other 127.0.0.1 binds, we should seed the addr cache randomizer with the correct network for inbound onion connections (using `CNode::ConnectedThroughNetwork()`) as well as the port of `CNode::addrBind`.
ACKs for top commit:
sipa:
utACK 292828cd77
mzumsande:
Code Review ACK 292828cd77
naumenkogs:
utACK 292828cd77
Tree-SHA512: d0be13bab6bc121c2926d4b168687f6c2ed4ce0c9dd19be71eb4886adeba8afc3daacdc4e232a0ba3b03a89d69b618abc5595b69abd1ad0c476d825bc6ea1f9f
433b525694 Add LogPrintLevel to lint-format-strings, drop LogPrint-vs-LogPrintf section in dev notes (Jon Atack)
Pull request description:
added by #7003 in 2015, as that potential issue would now be caught by the `test/lint/lint-format-strings.py` script run by the CI.
ACKs for top commit:
MarcoFalke:
cr ACK 433b525694
w0xlt:
ACK 433b525694
Tree-SHA512: 91a2ac76689ed4f1f638e07c16d2ec8952fb013cc8bb896780fbd9333abd084281ce99afdc9de715d07a9abb4dce5dd67edf5e347aff466c6ef339ccc4158679
57fb37c275 wallet: CommitTransaction, remove extra wtx lookup and add exception for a possible db write error. (furszy)
Pull request description:
Two points for `CWallet::CommitTransaction`:
1) The extra wtx lookup:
As we are calling to `AddToWallet` first, which returns the recently added/updated wtx pointer, there is no need to look up the wtx again few lines later. We can just use it.
2) The db write error:
`AddToWallet` can only return a nullptr if the db write fails, which inside `CommitTransaction` translates to an exception throw cause. We expect everywhere that `CommitTransaction` always succeed.
------------------------------------------------
Extra note:
This finding generated another working path for me :)
It starts with the following question: why are we returning a nullptr from `AddToWallet` if the db write failed without removing the recently added transaction from the wallet's map?..
Can led to a wallet invalid state where the inputs of this new transaction are not marked dirty, while the transaction that spends them still exist on the in-memory wallet tx map.
-- I'm writing it here to gather some feedback first and not forget it, will create a follow-up PR in the coming days 🚜 --
ACKs for top commit:
achow101:
ACK 57fb37c275
jonatack:
ACK 57fb37c
ryanofsky:
Code review ACK 57fb37c275. Seems like a clear improvement. Better to fail earlier with a better error message if the failure is going to happen anyway
Tree-SHA512: 80e59c01852cfbbc70a5de1a1c2c59b5e572f9eaa08c2175112cb515256e63fa04c7942f92a513b620d6b06e66392029ebe8902287c456efdbee58a7a5ae42da
687addaf13 test: add BIP-125 rule 5 testcase with default mempool (James O'Beirne)
6120e8e287 test: allow passing sequence through create_self_transfer_multi (James O'Beirne)
Pull request description:
Currently, we only test rule 5 of BIP-125 (replacement transactions cannot evict more than 100 transactions) by changing default mempool parameters to allow for more descendants. The current test works on a single transaction graph that has over 100 descendants.
This patch adds a test to exercise rule 5 using the default mempool parameters. The case is a little more sophisticated: instead of working on a single transaction graph, it uses a replacement transaction to "unite" several UTXOs which join independent transaction graphs. The total number of transactions in these graphs sum to more than the max allowable replacement.
I think the difference in transaction topology makes this a worthwhile testcase to have, setting aside the fact that this testcase works without having to use atypical mempool params.
See also: [relevant discussion from IRC](https://www.erisian.com.au/bitcoin-core-dev/log-2022-05-27.html#l-126)
ACKs for top commit:
laanwj:
Code review ACK 687addaf13
LarryRuane:
ACK 687addaf13
Tree-SHA512: e589aeaf9d6f137d546b7809f8795d6f6043d87b15e97c2efe85b42ce8b49d977ee7d79440c542ca4b0b5ca2de527488029841a1ffc0d96c5771897df4b3f324
that was added in 2015 by commit b8c06ef40 in PR 7003, as that potential issue
would now be caught by the test/lint/lint-format-strings.py script run by the CI
d40550d725 scripted-diff: remove duplicate categories from LogPrint output (Jon Atack)
Pull request description:
This is the first commit from #25203.
- Scripted-diff: de-duplicate logging category output for the tor, i2p, net, zmq, and prune messages (e.g. where I found duplicates), as these category prefixes are now printed automatically since #24464
examples before
```
[tor] tor: Successfully connected!
[i2p] I2P: Creating SAM session with 127.0.0.1:7656
[zmq] zmq: Initialize notification interface
[net] net: enabling extra block-relay-only peers
```
after
```
[tor] Successfully connected!
[i2p] Creating SAM session with 127.0.0.1:7656
[zmq] Initialize notification interface
[net] enabling extra block-relay-only peers
```
ACKs for top commit:
klementtan:
crACK d40550d725
MarcoFalke:
cr ACK d40550d725
Tree-SHA512: 63b799f2f899f0597981dd1acb91ef4439cd00b257a9eb19d67c4ce2c4dc72a95ac5761cb78f2a19090a10be74f23ea1db6929ed942ba0d008b4be563f0d5e7e
fa4068b4e2 Move minRelayTxFee to policy/settings (MacroFake)
Pull request description:
Seems a bit confusing to put policy stuff into validation, so fix that.
Also fix includes via `iwyu`.
ACKs for top commit:
ariard:
ACK fa4068b, the includes move compiles well locally.
ryanofsky:
Code review ACK fa4068b4e2. Make sense to move the global variable to policy/settings and the default constant to policy/policy. Ariard points out other constants that could be moved, which seems fine, but it seems like moving the global variable to be with other related global variables is more significant.
Tree-SHA512: adf9619002610d1877f3aef0a9e6115fc4c2ad64135a3e5100824c650b560c47f47ac28894c6214a50a7888355252a9f6f7cec98c23a771a1964160ef1ca77de
f26a496dfd test: clean up all-lint.py (Martin Leitner-Ankerl)
64d72c4c87 test: rename lint-all.py to all-lint.py (Martin Leitner-Ankerl)
Pull request description:
When running `./test/lint/lint-all.py`, the script runs all tests but
also calls itself because the comparison with `__file__` doesn't work.
Comparing resolved paths gives reliable comparison, and lint-all.py doesn't call itself any more
ACKs for top commit:
laanwj:
Code review ACK f26a496dfd
Tree-SHA512: b44abdd685f7b48a6a9f48e96d97138b635c31c1c7ab543cb5636b5f49690ccd56fa6fec01ae7fcc16af01a613372ee77632f70c32059919b373aa8051953791
e593ae07c4 Bugfix: RPC/blockchain: pruneblockchain: Return the height of the actual last pruned block (Luke Dashjr)
Pull request description:
From 0.14 (2017 Mar) until before 0.19 (2019 Nov), the height of the last block pruned was returned, subject to a bug if there were blocks left unpruned due to sharing files with later blocks.
In #15991, this was "fixed" to the current implementation, introducing a new bug: now, it returns the first *unpruned* block.
Since the user provides the parameter as a block to include in pruning, it makes more sense to fix the behaviour to match the documentation.
~~(Additionally, the description of "pruneheight" in getblockchaininfo is fixed to be technically correct)~~
ACKs for top commit:
fjahr:
utACK e593ae07c4
ryanofsky:
Code review ACK e593ae07c4. Just rebased since last review. Maybe some of the original reviewers of #15991 will want to take a look at this to correct the mistake that was introduced there!
Tree-SHA512: c2d511df80682d57260aae8af1665f9d7eaed16448f185f4c9f23c78fa9b8289a02053da7a0b83643fef57610d601ea63b59ff39661a51f4827f1eb27cc30594
3a9b9bb38e test: ensure createmultisig and addmultisigaddress are not returning any warning for expected cases (brunoerg)
eaf6f630c0 rpc: fix inappropriate warning for address type p2sh-segwit in createmultisig and addmultisigaddress (brunoerg)
Pull request description:
Fixes#25127
If there are any uncompressed keys when calling `AddAndGetMultisigDestination`, it will just default to a legacy address regardless of the chosen `address_type`. So, #23113 added a warnings field which will warn the user why their address format is different.
However, when creating a multisig (p2sh-segwit), it is returning an inappropriate warning, because when getting the output type from destination (`OutputTypeFromDestination`), it returns `ScriptHash` for both legacy and `P2SH_SEGWIT`. So, since `P2SH_SEGWIT` is different from `ScriptHash`, it returns the warning:
192d639a6b/src/rpc/output_script.cpp (L166-L169)
So, to avoid this mistake I changed `OutputTypeFromDestination` to `descriptor->GetOutputType()` to get the appropriate output type.
ACKs for top commit:
jonatack:
ACK 3a9b9bb38e
laanwj:
Code review ACK 3a9b9bb38e
Tree-SHA512: 49f717479c2b8906277e7591ddd4747f7961c2d5c77494b5124045de9036a4277d46b9ad99279d51f0c4484284c445f1e1d3c55c49bbf0716741bad426a89369
c47944f4e9 ci: Reuse some configure options in "ARM64 Android APK" task (Hennadii Stepanov)
7739438811 ci, android: Update NDK up to r23c (Hennadii Stepanov)
ca0c3e5077 ci, android: Update Command-line Tools from 2.1 up to 7.0 (Hennadii Stepanov)
8790da3c1e ci: Drop unneeded packages in "ARM64 Android APK" task (Hennadii Stepanov)
Pull request description:
This PR improves the "ARM64 Android APK" CI task in the following ways:
- dropped packages that are not required to be installed
- updated Android Command-line Tools and Android NDK to make the CI environment closer to the default one, which is provided by Android Studio
ACKs for top commit:
icota:
utACK c47944f4e9
Tree-SHA512: 45f5aba41007a502ae90333272370fd559c48a27d573896c449b3e436c5cf2b6440408381e4d20eb53104426ade26d3a9014c09dcdf3257ec897a537095efa4f
d1b7bcbca2 qt: Drop no longer supported Android architecture (Hennadii Stepanov)
Pull request description:
The `i686-linux-android` arch support has been dropped since bitcoin/bitcoin#23744.
ACKs for top commit:
katesalazar:
ACK d1b7bcbca2
icota:
utACK d1b7bcbca2
prusnak:
Approach ACK d1b7bcbca2
Tree-SHA512: 13689ec8c63c92b9a52a3c25edc35536b8e51ff583f57c45b168515f928d020d6bb85d03db9efd8d5efd57b944dfd313a89f5ff8a52f99982ccc8d9671f6e7a9
f3a50c9dfe miniscript: rename IsSane and IsSaneSubexpression to prevent misuse (Antoine Poinsot)
c5fe5163dc miniscript: nit: don't return after assert(false) (Antoine Poinsot)
7bbaca9d8d miniscript: explicit the threshold size computation in multi() (Antoine Poinsot)
8323e4249d miniscript: add an OpCode typedef for readability (Antoine Poinsot)
7a549c6c59 miniscript: mark nodes with duplicate keys as insane (Antoine Poinsot)
8c0f8bf7bc fuzz: add a Miniscript target for string representation roundtripping (Antoine Poinsot)
be34d5077b fuzz: rename and improve the Miniscript Script roundtrip target (Antoine Poinsot)
7eb70f0ac0 miniscript: tiny doc fixups (Antoine Poinsot)
5cea85f12c miniscript: split ValidSatisfactions from IsSane (Antoine Poinsot)
a0f064dc14 miniscript: introduce a CheckTimeLocksMix helper (Antoine Poinsot)
ed45ee3882 miniscript: use optional instead of bool/outarg (Antoine Poinsot)
1ab8d89fd1 miniscript: make equality operator non-recursive (Antoine Poinsot)
5922c662c0 scripted-diff: miniscript: rename 'nodetype' variables to 'fragment' (Antoine Poinsot)
c5f65db0f0 miniscript: remove a workaround for a GCC 4.8 bug (Antoine Poinsot)
Pull request description:
The Miniscript repository and the Miniscript integration PR here have been a moving target for the past months, and some final cleanups were done there that were not included here. I initially intended to add some small followup commits to #24148 but i think there are enough of them to be worth a followup PR on its own.
Some parts of the code did not change since it was initially written in 2019, and the code could use some modernization. (Use std::optional instead of out args, remove old compiler workarounds).
We refactored the helpers to be more meaningful, and also did some renaming. A new fuzz target was also added and both were merged in a single file. 2 more will be added in #24149 that will be contained in this file too.
The only behaviour change in this PR is to rule out Miniscript with duplicate keys from sane Miniscripts. In a P2WSH context, signatures can be rebounded (Miniscript does not use CODESEPARATOR) and it's reasonable to assume that reusing keys across the Script drops the malleability guarantees.
It was previously assumed such Miniscript would never exist in the first place since a compiler should never create them. We finally agreed that if one were to exist (say, written by hand or from a buggy compiler) it would be very confusing if an imported Miniscript descriptor (after #24148) with duplicate keys was deemed sane (ie, "safe to use") by Bitcoin Core. We now check for duplicate keys in the constructor.
This is (still) joint work with Pieter Wuille. (Actually he entirely authored the cleanups and code modernization.)
ACKs for top commit:
sipa:
utACK f3a50c9dfe (with the caveat that a lot of it is my own code)
sanket1729:
code review ACK f3a50c9dfe. Did not review the fuzz tests.
Tree-SHA512: c043325e4936fe25e8ece4266b46119e000c6745f88cea530fed1edf01c80f03ee6f9edc83b6e9d42ca01688d184bad16bfd967c5bb8037744e726993adf3deb
d87784ac87 kernel: SanityChecks: Return an error struct (Carl Dong)
265d6393bf Move init::SanityCheck to kernel::SanityCheck (Carl Dong)
fed085a1a4 init: Initialize globals with kernel::Context's life (Carl Dong)
7d03feef81 kernel: Introduce empty and unused kernel::Context (Carl Dong)
eeb4fc20c5 test: Use Set/UnsetGlobals in BasicTestingSetup (Carl Dong)
Pull request description:
The full `init/common.cpp` is dependent on things like ArgsManager (which we wish to remove from libbitcoinkernel in the future) and sanity checks. These aren't necessary for libbitcoinkernel so we only extract the portion that is necessary (namely `init::{Set,Unset}Globals()`.
ACKs for top commit:
theuni:
ACK d87784ac87
vasild:
ACK d87784ac87
Tree-SHA512: cd6b4923ea1865001b5f0caed9a4ff99c198d22bf74154d935dc09a47fda22ebe585ec912398cea69f722454ed1dbb4898faab5a2d02fb4c5e719c5c8d71a3f9
From 0.14 (2017 Mar) until before 0.19 (2019 Nov), the height of the last
block pruned was returned, subject to a bug if there were blocks left unpruned
due to sharing files with later blocks.
In #15991, this was "fixed" to the current implementation, introducing a new
bug: now, it returns the first *unpruned* block.
Since the user provides the parameter as a block to include in pruning, it
makes more sense to fix the behaviour to match the documentation.
3a171f742c logging: fix logging empty threadname (klementtan)
Pull request description:
Currently, `leveldb` background thread does not have a thread name and as a result, an empty thread name is logged.
This PR fixes this by logging thread name as `"unknown"` if the thread name is empty
On master:
```txt
2022-06-02T14:30:38Z [] [leveldb:debug] Generated table #281@0: 1862 keys, 138303 bytes
```
On this PR:
```txt
2022-06-02T14:30:38Z [unknown] [leveldb:debug] Generated table #281@0: 1862 keys, 138303 bytes
```
ACKs for top commit:
laanwj:
Code review ACK 3a171f742c
hebasto:
ACK 3a171f742c
Tree-SHA512: 0af0fa5c4ddd3640c6dab9595fe9d97f74d0e0f4b41287a6630cf8ac5a21240250e0659ec4ac5a561e888d522f5304bf627104de2aba0fd0a86c1222de0897c2
48262a00f5 Add functional test for block sync from inbound peers (Suhas Daftuar)
0569b5c4bb Sync chain more readily from inbound peers during IBD (Suhas Daftuar)
Pull request description:
When in IBD, if the honest chain is only known by inbound peers, then we must
eventually sync from them in order to learn it. This change allows us to
perform initial headers sync and fetch blocks from inbound peers, if we have no
blocks in flight.
The restriction on having no blocks in flight means that we will naturally
throttle our block downloads to any such inbound peers that we may be
downloading from, until we leave IBD. This is a tradeoff between preferring
outbound peers for most of our block download, versus making sure we always
eventually will get blocks we need that are only known by inbound peers even
during IBD, as otherwise we may be stuck in IBD indefinitely (which could have
cascading failure on the network, if a large fraction of the network managed to
get stuck in IBD).
Note that the test in the second commit fails on master, without the first commit.
ACKs for top commit:
ajtowns:
ACK 48262a00f5
sipa:
ACK 48262a00f5
Tree-SHA512: ffad3a05fa9a32a92226843c9128f52c275e8d51930fde7368badc340227f2ed680561c4c9f2937b4e3bd722474464849ec9b624f912f5e380ce98d71b55764d
1bace0cfee test: check `replaceable` mismatch error in `createrawtransaction` RPC (Sebastian Falbesoner)
Pull request description:
This PR adds missing test coverage for the createrawtransaction RPC, in the case that the `replaceable` parameter is set, but the passed transaction doesn't signal RBF (i.e. no input's nSequence is < 0xffffffe):
1c7ef0abd1/src/rpc/rawtransaction_util.cpp (L135-L137)
ACKs for top commit:
laanwj:
Code review ACK 1bace0cfee
brunoerg:
crACK 1bace0cfee
furszy:
Code review ACK 1bace0cf
Tree-SHA512: e6390401c8026c782643e3de7be56ea1745736b7e4c8886186d75c353c301b57afdabb631b9d8e2770386e4d7a59ac5fba1f380b9a5a21296512ca4515b35baa
