fa6b053b5c mempool: persist with XOR (MarcoFalke)
Pull request description:
Currently the `mempool.dat` file stores data received from remote peers as-is. This may be problematic when a program other than Bitcoin Core tries to interpret them by accident. For example, an anti-virus program or other program may scan the file and move it into quarantine, or delete it, or corrupt it.
While the local wallet is expected to re-submit any pending transactions, unrelated transactions may be missing from the mempool after a restart. This may cause fee estimates to be off, or may cause block relay to be slower.
Fix this, similar to https://github.com/bitcoin/bitcoin/pull/6650, by rolling a random XOR pattern over the dat file when writing or reading it.
Obviously this can only protect against programs that accidentally and unintentionally are trying to mess with the dat file. Any program that intentionally wants to mess with the dat file can still trivially do so.
ACKs for top commit:
achow101:
re-ACK fa6b053b5c
glozow:
reACK fa6b053b5c
ismaelsadeeq:
ACK fa6b053b5c
Tree-SHA512: ded2ce3d81bc944b828263534e3178a1e45a914fe8e024f4a14c6561a73e301820944ecc75dd704b3d4221a7a3a5c0597ccab79546250c1197609ee981fe324e
3b70f7b615 doc: fix broken doc/design/multiprocess.md links after #24352 (Ryan Ofsky)
6d43aad742 span: Make Span template deduction guides work in SFINAE context (Ryan Ofsky)
8062c3bdb9 util: Add ArgsManager SetConfigFilePath method (Ryan Ofsky)
441d00c60f interfaces: Rename CalculateBumpFees methods to be compatible with capn'proto (Ryan Ofsky)
156f49d682 interfaces: Change getUnspentOutput return type to avoid multiprocess segfault (Ryan Ofsky)
4978754c00 interfaces: Add schedulerMockForward method so mockscheduler RPC can work across processes (Ryan Ofsky)
924327eaf3 interfaces: Fix const virtual method that breaks multiprocess support (Ryan Ofsky)
82a379eca8 streams: Add SpanReader ignore method (Russell Yanofsky)
Pull request description:
This is a collection of small changes to interfaces and code which were needed as part of multiprocess PR #10102, but have been moved here to make that PR smaller.
All of these changes are refactoring changes which do not affect behavior of current code
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/issues/28722).
ACKs for top commit:
achow101:
ACK 3b70f7b615
naumenkogs:
ACK 3b70f7b615
maflcko:
re-ACK 3b70f7b615🎆
Tree-SHA512: 2368772b887056ad8a9f84c299cfde76ba45943770e3b5353130580900afa9611302195b899ced7b6e303b11f053ff204cae7c28ff4e12c55562fcc81119ba4c
3c208cc05e Add offline signing tutorial (Brandon Odiwuor)
Pull request description:
This PR adds offline signing tutorial. Fixes https://github.com/bitcoin/bitcoin/issues/9492
Although there currently exists tutorials on external-signer and on multisig implemented on #24519 . The external-signer tutorial assumes a connected device and the multisig tutorial is only for multisig transactions and does not include using an offline wallet
- The tutorial uses signet(instead of regtest) to be as close as possible to mainnet
ACKs for top commit:
achow101:
ACK 3c208cc05e
willcl-ark:
ACK 3c208cc05e
pinheadmz:
ACK 3c208cc05e
Zero-1729:
ACK 3c208cc05e
Tree-SHA512: c1686043d9e9ed440e78d219a6b18d58d62efd05bdd535e74194d8cc2db0a91e94c6c619106453120a137e47220cf3ab27af3214e861f4e5cc419a73a8704dd6
91d0888921 sync: unpublish LocksHeld() which is used only in sync.cpp (Vasil Dimov)
3df37e0c78 doc: clarify that LOCK() does AssertLockNotHeld() internally (Vasil Dimov)
Pull request description:
Constructs like
```cpp
AssertLockNotHeld(m);
LOCK(m);
```
are equivalent to (almost, modulo some logging differences, see below)
```cpp
LOCK(m);
```
for non-recursive mutexes, so it is ok to omit `AssertLockNotHeld()` in such cases. Requests to do the former keep coming during review process. `developer-notes.md` explicitly states "Combine annotations in function declarations with run-time asserts in function definitions", but that seems to be too strong or unclear. `LOCK()` is also a run-time assert in this case.
Also remove `LocksHeld()` from the public interface in `sync.h` since it is only used in `sync.cpp`.
ACKs for top commit:
achow101:
ACK 91d0888921
hebasto:
ACK 91d0888921, I have reviewed the code and it looks OK.
Tree-SHA512: c4b7ef2c0bfeb28d1c4f55f497810f629873137e02f5a92137c02cb1ff603ac76473dcd2171e594491494a5cb87b8c0c803e06b86f190d4acb231791e28e802d
fa25e8b0a1 doc: Recommend lint image build on every call (MarcoFalke)
faf70c1f33 Bump python minimum version to 3.9 (MarcoFalke)
fa8996b930 ci: Bump i686_multiprocess.sh to latest Ubuntu LTS (MarcoFalke)
Pull request description:
All supported operating systems ship with python 3.9 (or later), so bumping the minimum should not cause any issues. A bump will allow new code to use new python 3.9 features.
For reference:
* https://packages.debian.org/bullseye/python3
* https://packages.ubuntu.com/focal/python3.9
* FreeBSD 12/13 also ships with 3.9
* CentOS-like 8/9 also ships with 3.9 (and 3.11)
* OpenSuse Leap also ships with 3.9 (and 3.11) https://software.opensuse.org/package/python311-base
This is for Bitcoin Core 27.0 in 2024 (next year), not the soon upcoming 26.0 next month.
ACKs for top commit:
Sjors:
ACK fa25e8b0a1
jamesob:
ACK fa25e8b0a1 ([`jamesob/ackr/28211.1.MarcoFalke.bump_python_minimum_supp`](https://github.com/jamesob/bitcoin/tree/ackr/28211.1.MarcoFalke.bump_python_minimum_supp))
Tree-SHA512: 86c9f6ac4b5ba94a62ee6a6062dd48a8295d8611a39cdb5829f4f0dbc77aaa1a51edccc7a99275bf699143ad3a6fe826de426d413e5a465e3b0e82b86d10c32e
4bfaad4eca chainparams, assumeutxo: Fix signet txoutset hash (Fabian Jahr)
a503cd0f0b chainparams, assumeutxo: Fix testnet txoutset hash (Fabian Jahr)
f6213929c5 assumeutxo: Check deserialized coins for out of range values (Fabian Jahr)
66865446a7 docs: Add release notes for #28685 (Fabian Jahr)
cb0336817e scripted-diff: Rename hash_serialized_2 to hash_serialized_3 (Fabian Jahr)
351370a1d2 coinstats: Fix hash_serialized2 calculation (Fabian Jahr)
Pull request description:
Closes#28675
The last commit demonstrates that theStack's analysis [here](https://github.com/bitcoin/bitcoin/issues/28675#issuecomment-1770389468) seems to be correct. There will be more changes needed for the rest of the test suite but the `feature_assumeutxo.py` with my additional tests pass.
ACKs for top commit:
achow101:
ACK 4bfaad4eca
theStack:
Code-review ACK 4bfaad4eca
ryanofsky:
Code review ACK 4bfaad4eca
Tree-SHA512: 2f6abc92b282f7c5da46391803cf0804d13978d191d541f2509b532c538abccd0a081e46cda23d80d47206a05fa2b5d41b7ab246e6a263db7a7461d6292116ef
03f82087f6 doc: assumeutxo prune and index notes (Sjors Provoost)
Pull request description:
Based on recent comments on #27596.
ACKs for top commit:
pablomartin4btc:
re ACK 03f82087f6
ryanofsky:
ACK 03f82087f6. Nice changes, these seem like very helpful notes
Tree-SHA512: fe651b49f4d667400a3655899f27a96dd1eaf67cf9215fb35db5f44fb8c0313e7d541518be6791fec93392df24b909793f3886adb808e53228ed2a291165639d
1b672eb766 doc: add release note for #27460 (new `importmempool` RPC) (Sebastian Falbesoner)
Pull request description:
This PR adds a missing release note for #27460.
ACKs for top commit:
glozow:
ACK 1b672eb766
Tree-SHA512: 89deadbfd6779e6eb19801c9fe7459a9876b920d44e09df102774c1eb8b3c0716462613dc99d1711eda4bd959ea61595b33f4528424ac02cf1af6cb4e5f1f0e9
5b878be742 [doc] add release note for submitpackage (glozow)
7a9bb2a2a5 [rpc] allow submitpackage to be called outside of regtest (glozow)
5b9087a9a7 [rpc] require package to be a tree in submitpackage (glozow)
e32ba1599c [txpackages] IsChildWithParentsTree() (glozow)
b4f28cc345 [doc] parent pay for child in aggregate CheckFeeRate (glozow)
Pull request description:
Permit (restricted topology) submitpackage RPC outside of regtest. Suggested in https://github.com/bitcoin/bitcoin/pull/26933#issuecomment-1510851570
This RPC should be safe but still experimental - interface may change, not all features (e.g. package RBF) are implemented, etc. If a miner wants to expose this to people, they can effectively use "package relay" before the p2p changes are implemented. However, please note **this is not package relay**; transactions submitted this way will not relay to other nodes if the feerates are below their mempool min fee. Users should put this behind some kind of rate limit or permissions.
ACKs for top commit:
instagibbs:
ACK 5b878be742
achow101:
ACK 5b878be742
dergoegge:
Code review ACK 5b878be742
ajtowns:
ACK 5b878be742
ariard:
Code Review ACK 5b878be742. Though didn’t manually test the PR.
Tree-SHA512: 610365c0b2ffcccd55dedd1151879c82de1027e3319712bcb11d54f2467afaae4d05dca5f4b25f03354c80845fef538d3938b958174dda8b14c10670537a6524
3d420d8f28 Add instructions for headerssync-params.py to release-process.md (Pieter Wuille)
53d7d35b58 Update parameters in headerssync.cpp (Pieter Wuille)
7899402cff Add headerssync-params.py script to the repository (Pieter Wuille)
Pull request description:
Builds upon #25946, as it incorporates changes based on the selected values there.
This adds the headerssync tuning parameters optimization script from https://gist.github.com/sipa/016ae445c132cdf65a2791534dfb7ae1 to the repository, updates the parameters based on its output, and adds release process instructions for doing this update in the future.
A few considerations:
* It would be a bit cleaner to have these parameters be part of `CChainParams`, but due to the nature of the approach, it really only applies to chains with unforgeable proof-of-work, which we really can only reasonably expect from mainnet, so I think it's fine to keep them local to `headerssync.cpp`. Keeping them as compile-time evaluatable constants also has a (likely negligible) performance impact (avoiding runtime modulo operations).
* If we want to make sure the chainparams and headerssync params don't go out of date, it could be possible to run the script in CI, and and possibly even have the parameters be generated automatically at build time. I think that's overkill for how unfrequently these need to change, and running the script has non-trivial cost (~minutes in the normal python interpreter).
* A viable alternative is just leaving this out-of-repo entirely, and just do ad-hoc updating from time to time. Having it in the repo and release notes does make sure it's not forgotten, though adds a cost to contributors/maintainers who follow the process.
ACKs for top commit:
ajtowns:
reACK 3d420d8f28
Tree-SHA512: 03188301c20423c72c1cbd008ccce89b93e2898edcbeecc561b2928a0d64e9a829ab0744dc3b017c23de8b02f3c107ae31e694302d3931f4dc3540e184de1963
fa6e6a3f03 doc: Remove confusing assert linter (MarcoFalke)
Pull request description:
The `assert()` documentation and linter are redundant and confusing:
* The source code already refuses to compile with `assert()` disabled.
* They violate the assumptions about `Assert()`, which *requires* side effects.
* The existing linter doesn't enforce the guideline, only checking for `++` and `--` side effects.
Fix all issues by removing the docs and the linter. See also https://github.com/bitcoin/bitcoin/pull/26684#discussion_r1287370102
Going forward everyone is free to use whatever code in this regard they think is the easiest to read. Also, everyone is still free to share style-nits, if they think it is a good use of their time and of the pull request author. Finally, the author is still free to dismiss or ignore this style-nit, or any other style-nit.
ACKs for top commit:
hebasto:
ACK fa6e6a3f03, I have reviewed the code and it looks OK.
theStack:
ACK fa6e6a3f03
Tree-SHA512: 686738d71e1316cc95e5d3f71869b55a02bfb137c795cc0875057f4410e564bc8eff03c985a2087b007fb08fc84551c7da1e8b30c7a9c3f2b14e5e44a5970236
edbed31066 chainparams: add signet assumeutxo param at height 160_000 (Sjors Provoost)
b8cafe3871 chainparams: add testnet assumeutxo param at height 2_500_000 (Sjors Provoost)
99839bbfa7 doc: add note about confusing HaveTxsDownloaded name (James O'Beirne)
7ee46a755f contrib: add script to demo/test assumeutxo (James O'Beirne)
42cae39356 test: add feature_assumeutxo functional test (James O'Beirne)
0f64bac603 rpc: add getchainstates (James O'Beirne)
bb05857794 refuse to activate a UTXO snapshot if mempool not empty (James O'Beirne)
ce585a9a15 rpc: add loadtxoutset (James O'Beirne)
62ac519e71 validation: do not activate snapshot if behind active chain (James O'Beirne)
9511fb3616 validation: assumeutxo: swap m_mempool on snapshot activation (James O'Beirne)
7fcd21544a blockstorage: segment normal/assumedvalid blockfiles (James O'Beirne)
4c3b8ca35c validation: populate nChainTx value for assumedvalid chainstates (James O'Beirne)
49ef778158 test: adjust chainstate tests to use recognized snapshot base (James O'Beirne)
1019c39982 validation: pruning for multiple chainstates (James O'Beirne)
373cf91531 validation: indexing changes for assumeutxo (James O'Beirne)
1fffdd76a1 net_processing: validationinterface: ignore some events for bg chain (James O'Beirne)
fbe0a7d7ca wallet: validationinterface: only handle active chain notifications (James O'Beirne)
f073917a9e validationinterface: only send zmq notifications for active (James O'Beirne)
4d8f4dcb45 validation: pass ChainstateRole for validationinterface calls (James O'Beirne)
1e59acdf17 validation: only call UpdatedBlockTip for active chainstate (James O'Beirne)
c6af23c517 validation: add ChainstateRole (James O'Beirne)
9f2318c76c validation: MaybeRebalanceCaches when chain leaves IBD (James O'Beirne)
434495a8c1 chainparams: add blockhash to AssumeutxoData (James O'Beirne)
c711ca186f assumeutxo: remove snapshot during -reindex{-chainstate} (James O'Beirne)
c93ef43e4f bugfix: correct is_snapshot_cs in VerifyDB (James O'Beirne)
b73d3bbd23 net_processing: Request assumeutxo background chain blocks (Suhas Daftuar)
Pull request description:
- Background and FAQ: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal
- Prior progress/project: https://github.com/bitcoin/bitcoin/projects/11
- Replaces https://github.com/bitcoin/bitcoin/pull/15606, which was closed due to Github slowness. Original description and commentary can be found there.
---
This changeset finishes the first phase of the assumeutxo project. It makes UTXO snapshots loadable via RPC (`loadtxoutset`) and adds `assumeutxo` parameters to chainparams. It contains all the remaining changes necessary to both use an assumedvalid snapshot chainstate and do a full validation sync in the background.
This may look like a lot to review, but note that
- ~200 lines are a (non-essential) demo shell script
- Many lines are functional test, documentation, and relatively dilute RPC code.
So it shouldn't be as burdensome to review as the linecount might suggest.
- **P2P**: minor changes are made to `init.cpp` and `net_processing.cpp` to make simultaneous IBD across multiple chainstates work.
- **Pruning**: implement correct pruning behavior when using a background chainstate
- **Blockfile separation**: to prevent "fragmentation" in blockfile storage, have background chainstates use separate blockfiles from active snapshot chainstates to avoid interleaving heights and impairing pruning.
- **Indexing**: some `CValidationInterface` events are given with an additional parameter, ChainstateRole, and all indexers ignore events from ChainstateRole::ASSUMEDVALID so that indexation only happens sequentially.
- Have `-reindex` properly wipe snapshot chainstates.
- **RPC**: introduce RPC commands `loadtxoutset` and (hidden) `getchainstates`.
- **Release docs & first assumeutxo commitment**: add notes and a particular assumeutxo hash value for first AU-enabled release.
- This will complete the project and allow use of UTXO snapshots for faster node bootstrap.
The next phase, if it were to be pursued, would be coming up with a way to distribute the UTXO snapshots over the P2P network.
---
### UTXO snapshots
Create your own with `./contrib/devtools/utxo_snapshot.sh`, e.g.
```shell
./contrib/devtools/utxo_snapshot.sh 788000 utxo.dat ./src/bitcoin-cli -datadir=$(pwd)/testdata`)
```
or use the pre-generated ones listed below.
- Testnet: **2'500'000** (Sjors):
- torrent: `magnet:?xt=urn:btih:511e09f4bf853aefab00de5c070b1e031f0ecbe9&dn=utxo-testnet-2500000.dat&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969`
- sha256: `79db4b025448cc0ac388d8589a28eab02de53055d181e34eb47391717aa16388`
- Signet: **160'000** (Sjors):
- torrent: `magnet:?xt=urn:btih:9da986cb27b3980ea7fd06b21e199b148d486880&dn=utxo-signet-160000.dat&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969`
- sha256: `eeeca845385ba91e84ef58c09d38f98f246a24feadaad57fe1e5874f3f92ef8c`
- Mainnet: **800'000** (Sjors):
- Note: this needs the following commit cherry-picked in: 24deb2022b
- torrent: `magnet:?xt=urn:btih:50ee955bef37f5ec3e5b0df4cf0288af3d715a2e&dn=utxo-800000.dat&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969`
### Testing
#### For fun (~5min)
If you want to do a quick test, you can run `./contrib/devtools/test_utxo_snapshots.sh` and follow the instructions. This is mostly obviated by the functional tests, though.
#### For real (longer)
If you'd like to experience a real usage of assumeutxo, you can do that too.
I've cut a new snapshot at height 788'000 (http://img.jameso.be/utxo-788000.dat - but you can do it yourself with `./contrib/devtools/utxo_snapshot.sh` if you want). Download that, and then create a datadir for testing:
```sh
$ cd ~/src/bitcoin # or whatever
# get the snapshot
$ curl http://img.jameso.be/utxo-788000.dat > utxo-788000.dat
# you'll want to do this if you like copy/pasting
$ export AU_DATADIR=/home/${USER}/au-test # or wherever
$ mkdir ${AU_DATADIR}
$ vim ${AU_DATADIR}/bitcoin.conf
dbcache=8000 # or, you know, something high
blockfilterindex=1
coinstatsindex=1
prune=3000
logthreadnames=1
```
Obtain this branch, build it, and then start bitcoind:
```sh
$ git remote add jamesob https://github.com/jamesob/bitcoin
$ git fetch jamesob assumeutxo
$ git checkout jamesob/assumeutxo
$ ./configure $conf_args && make # (whatever you like to do here)
# start 'er up and watch the logs
$ ./src/bitcoind -datadir=${AU_DATADIR}
```
Then, in some other window, load the snapshot
```sh
$ ./src/bitcoin-cli -datadir=${AU_DATADIR} loadtxoutset $(pwd)/utxo-788000.dat
```
You'll see some log messages about headers retrieval and waiting to see the snapshot in the headers chain. Once you get the full headers chain, you'll spend a decent amount of time (~10min) loading the snapshot, checking it, and flushing it to disk. After all that happens, you should be syncing to tip in pretty short order, and you'll see the occasional `[background validation]` log message go by.
In yet another window, you can check out chainstate status with
```sh
$ ./src/bitcoin-cli -datadir=${AU_DATADIR} getchainstates
```
as well as usual favorites like `getblockchaininfo`.
ACKs for top commit:
achow101:
ACK edbed31066
Tree-SHA512: 6086fb9a38dc7df85fedc76b30084dd8154617a2a91e89a84fb41326d34ef8e7d7ea593107afba01369093bf8cc91770621d98f0ea42a5b3b99db868d2f14dc2
Introduces ChainstateManager::GetPruneRange().
The prune budget is split evenly between the number of chainstates,
however the prune budget may be exceeded if the resulting shares are
beneath `MIN_DISK_SPACE_FOR_BLOCK_FILES`.
a99e9e655a doc: add release note (ismaelsadeeq)
2b4edf889a test: check `descriptorprocesspsbt` return hex encoded tx (ismaelsadeeq)
c405207a18 rpc: `descriptorprocesspsbt` return hex encoded tx (ismaelsadeeq)
Pull request description:
Coming from [#28414 comment](https://github.com/bitcoin/bitcoin/pull/28414#pullrequestreview-1618684391) Same thing also for `descriptorprocesspsbt`.
Before this PR `descriptorprocesspsbt` returns a boolean `complete` which indicates that the psbt is final, users then have to call `finalizepsbt` to get the hex encoded network transaction.
In this PR if the psbt is complete the return object also has the hex encoded network transaction ready for broadcast with `sendrawtransaction`.
This save users calling `finalizepsbt` with the descriptor, if it is already complete.
ACKs for top commit:
achow101:
ACK a99e9e655a
pinheadmz:
ACK a99e9e655a
ishaanam:
ACK a99e9e655a
Tree-SHA512: c3f1b1391d4df05216c463127cd593f8703840430a99febb54890bc66fadabf9d9530860605f347ec54c1694019173247a0e7a9eb879d3cbb420f9e8d9839b75
Andrew Chow