aboutsummaryrefslogtreecommitdiff
path: root/src/test
AgeCommit message (Collapse)Author
2023-10-02net: advertise NODE_P2P_V2 if CLI arg -v2transport is onPieter Wuille
Co-authored-by: Dhruv Mehta <856960+dhruv@users.noreply.github.com>
2023-10-02Merge bitcoin/bitcoin#27596: assumeutxo (2)Andrew Chow
edbed31066e3674ba52b8c093ab235625527f383 chainparams: add signet assumeutxo param at height 160_000 (Sjors Provoost) b8cafe38713cbf10d15459042f7f911bcc1b1e4e chainparams: add testnet assumeutxo param at height 2_500_000 (Sjors Provoost) 99839bbfa7110c7abf22e587ae2f72c9c57d3c85 doc: add note about confusing HaveTxsDownloaded name (James O'Beirne) 7ee46a755f1d57ce9d51975d3b54dc9ac3d08d52 contrib: add script to demo/test assumeutxo (James O'Beirne) 42cae39356fd20d521aaf99aff1ed85856f3c9f3 test: add feature_assumeutxo functional test (James O'Beirne) 0f64bac6030334d798ae205cd7af4bf248feddd9 rpc: add getchainstates (James O'Beirne) bb0585779472962f40d9cdd9c6532132850d371c refuse to activate a UTXO snapshot if mempool not empty (James O'Beirne) ce585a9a158476b0ad3296477b922e79f308e795 rpc: add loadtxoutset (James O'Beirne) 62ac519e718eb7a31dca1102a96ba219fbc7f95d validation: do not activate snapshot if behind active chain (James O'Beirne) 9511fb3616b7bbe1d0d2f54a45ea0a650ba0367b validation: assumeutxo: swap m_mempool on snapshot activation (James O'Beirne) 7fcd21544a333ffdf1910b65c573579860be6a36 blockstorage: segment normal/assumedvalid blockfiles (James O'Beirne) 4c3b8ca35c2e4a441264749bb312df2bd054b5b8 validation: populate nChainTx value for assumedvalid chainstates (James O'Beirne) 49ef778158c43859946a592e11ec34fe1b93a5b6 test: adjust chainstate tests to use recognized snapshot base (James O'Beirne) 1019c399825b0d512c1fd751c376d46fed4992b9 validation: pruning for multiple chainstates (James O'Beirne) 373cf91531b84bfdd06fdf8abf4dca228029ce6b validation: indexing changes for assumeutxo (James O'Beirne) 1fffdd76a1bca908f55d73b64983655b14cf7432 net_processing: validationinterface: ignore some events for bg chain (James O'Beirne) fbe0a7d7ca680358237b6c2369b3fd2b43221113 wallet: validationinterface: only handle active chain notifications (James O'Beirne) f073917a9e7ba423643dcae0339776470b628f65 validationinterface: only send zmq notifications for active (James O'Beirne) 4d8f4dcb450d31e4847804e62bf91545b949fa14 validation: pass ChainstateRole for validationinterface calls (James O'Beirne) 1e59acdf17309f567c370885f0cf02605e2baa58 validation: only call UpdatedBlockTip for active chainstate (James O'Beirne) c6af23c5179cc383f8e6c275373af8d11e6a989f validation: add ChainstateRole (James O'Beirne) 9f2318c76cc6986d48e13831cf5bd8dab194fdf4 validation: MaybeRebalanceCaches when chain leaves IBD (James O'Beirne) 434495a8c1496ca23fe35b84499f3daf668d76b8 chainparams: add blockhash to AssumeutxoData (James O'Beirne) c711ca186f8d8a28810be0beedcb615ddcf93163 assumeutxo: remove snapshot during -reindex{-chainstate} (James O'Beirne) c93ef43e4fd4fbc1263cdc9e98ae5856830fe89e bugfix: correct is_snapshot_cs in VerifyDB (James O'Beirne) b73d3bbd23220857bf17cbb6401275bf58013b72 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: https://github.com/Sjors/bitcoin/commit/24deb2022b822f22fba9fcbee201e37a83225eb2 - 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 edbed31066e3674ba52b8c093ab235625527f383 Tree-SHA512: 6086fb9a38dc7df85fedc76b30084dd8154617a2a91e89a84fb41326d34ef8e7d7ea593107afba01369093bf8cc91770621d98f0ea42a5b3b99db868d2f14dc2
2023-10-02rpc: getrawaddrman for addrman entries0xb10c
Exposing address manager table entries in a hidden RPC allows to introspect addrman tables in tests and during development. As response JSON object the following FORMAT1 is choosen: { "table": { "<bucket>/<position>": { "address": "..", "port": .., ... }, "<bucket>/<position>": { "address": "..", "port": .., ... }, "<bucket>/<position>": { "address": "..", "port": .., ... }, ... } } An alternative would be FORMAT2 { "table": { "bucket": { "position": { "address": "..", "port": .., ... }, "position": { "address": "..", "port": .., ... }, .. }, "bucket": { "position": { "address": "..", "port": .., ... }, .. }, } } FORMAT1 and FORMAT2 have different encodings for the location of the address in the address manager. While FORMAT2 might be easier to process for downstream tools, it also mimics internal addrman mappings, which might change at some point. Users not interested in the address location can ignore the location key. They don't have to adapt to a new RPC response format, when the internal addrman layout changes. Additionally, FORMAT1 is also slightly easier to to iterate in downstream tools. The RPC response-building implemenation complexcity is lower with FORMAT1 as we can more easily build a "<bucket>/<position>" key than a multiple "bucket" objects with multiple "position" objects (FORMAT2).
2023-10-02Merge bitcoin/bitcoin#28508: refactor: Remove SER_GETHASH, hard-code client ↵fanquake
version in CKeyPool serialize fac29a0ab19fda457b55d7a0a37c5cd3d9680f82 Remove SER_GETHASH, hard-code client version in CKeyPool serialize (MarcoFalke) fa72f09d6ff8ee204f331a69d3f5e825223c9e11 Remove CHashWriter type (MarcoFalke) fa4a9c0f4334678fb80358ead667807bf2a0a153 Remove unused GetType() from OverrideStream, CVectorWriter, SpanReader (MarcoFalke) Pull request description: Removes a bunch of redundant, dead or duplicate code. Uses the idea from and finishes the idea https://github.com/bitcoin/bitcoin/pull/28428 by theuni ACKs for top commit: ajtowns: ACK fac29a0ab19fda457b55d7a0a37c5cd3d9680f82 kevkevinpal: added one nit but otherwise ACK [fac29a0](https://github.com/bitcoin/bitcoin/pull/28508/commits/fac29a0ab19fda457b55d7a0a37c5cd3d9680f82) Tree-SHA512: cc805e2f38e73869a6691fdb5da09fa48524506b87fc93f05d32c336ad3033425a2d7608e317decd3141fde3f084403b8de280396c0c39132336fe0f7510af9e
2023-10-02[txpackages] IsChildWithParentsTree()glozow
Many edge cases exist when parents in a child-with-parents package can spend each other. However, this pattern should also be uncommon in normal use cases.
2023-09-30rpc: add getchainstatesJames O'Beirne
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2023-09-30rpc: add loadtxoutsetJames O'Beirne
Co-authored-by: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
2023-09-30validation: do not activate snapshot if behind active chainJames O'Beirne
Most easily reviewed with git show --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2023-09-30validation: assumeutxo: swap m_mempool on snapshot activationJames O'Beirne
Otherwise we will not receive transactions during background sync until restart.
2023-09-30test: adjust chainstate tests to use recognized snapshot baseJames O'Beirne
In future commits, loading the block index while making use of a snapshot is contingent on the snapshot being recognized by chainparams. Ensure all existing unittests that use snapshots use a recognized snapshot (at height 110). Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2023-09-30validation: pass ChainstateRole for validationinterface callsJames O'Beirne
This allows consumers to decide how to handle events from background or assumedvalid chainstates.
2023-09-30chainparams: add blockhash to AssumeutxoDataJames O'Beirne
This allows us to reference assumeutxo configuration by blockhash as well as height; this is helpful in future changes when we want to reference assumeutxo configurations before the block index is loaded.
2023-09-29Merge bitcoin/bitcoin#28525: net: Drop v2 garbage authentication packetAndrew Chow
e3720bca398820038b3e97f467adb2c45ef9ef5f net: Simplify v2 recv logic by decoupling AAD from state machine (Tim Ruffing) b0f5175c044df956c0f07f540706d457c4912856 net: Drop v2 garbage authentication packet (Tim Ruffing) Pull request description: Note that this is a breaking change, see also https://github.com/bitcoin/bips/pull/1498 The benefit is a simpler implementation: - The protocol state machine does not need separate states for garbage authentication and version phases. - The special case of "ignoring the ignore bit" is removed. - The freedom to choose the contents of the garbage authentication packet is removed. This simplifies testing. ACKs for top commit: naumenkogs: ACK e3720bca398820038b3e97f467adb2c45ef9ef5f sipa: ACK e3720bca398820038b3e97f467adb2c45ef9ef5f. Re-ran the v2 transport fuzzer overnight. ajtowns: ACK e3720bca398820038b3e97f467adb2c45ef9ef5f - simpler and more flexible, nice achow101: ACK e3720bca398820038b3e97f467adb2c45ef9ef5f Sjors: utACK e3720bca398820038b3e97f467adb2c45ef9ef5f theStack: Code-review ACK e3720bca398820038b3e97f467adb2c45ef9ef5f Tree-SHA512: 16600ed868c8a346828de075c4072e37cf86440751d08ab099fe8b58ff71d8b371a90397d6a4247096796db68794275e7e0403f218859567d04838e0411dadd6
2023-09-28Merge bitcoin/bitcoin#28450: Add package evaluation fuzzerglozow
262ab8ef7860d43cebc9d04721e3a075b4edf06e Add package evaluation fuzzer (Greg Sanders) Pull request description: This fuzzer target caught the issue in https://github.com/bitcoin/bitcoin/pull/28251 within 5 minutes on master branch, and an additional issue which I've applied a preliminary patch to cover. Fuzzer target does the following: 1) Picks mempool confgs, including max package size, count, mempool size, etc 2) Generates 1 to 26 transactions with arbitrary coins/fees, the first N-1 spending only confirmed outpoints 3) Nth transaction, if >1, sweeps all unconfirmed outpoints in mempool 4) If N==1, it may submit it through single-tx submission path, to allow for more interesting topologies 5) Otherwise submits through package submission interface 6) Repeat 1-5 a few hundred times per mempool instance In other words, it ends up building chains of txns in the mempool using parents-and-children packages, which is currently the topology supported on master. The test itself is a direct rip of tx_pool.cpp, with a number of assertions removed because they were failing for unknown reasons, likely due to the notification changes of single tx submission to package, which is used to track addition/removal of transactions in the test. I'll continue working on re-adding these assertions for further invariant testing. ACKs for top commit: murchandamus: ACK 262ab8ef7860d43cebc9d04721e3a075b4edf06e glozow: reACK 262ab8ef7860d43cebc9d04721e3a075b4edf06e dergoegge: tACK 262ab8ef7860d43cebc9d04721e3a075b4edf06e Tree-SHA512: 190784777d0f2361b051b3271db8f79b7927e3cab88596d2c30e556da721510bd17f6cc96f6bb03403bbf0589ad3f799fa54e63c1b2bd92a2084485b5e3e96a5
2023-09-27Add package evaluation fuzzerGreg Sanders
2023-09-27net: Drop v2 garbage authentication packetTim Ruffing
See also https://github.com/bitcoin/bips/pull/1498 The benefit is a simpler implementation: - The protocol state machine does not need separate states for garbage authentication and version phases. - The special case of "ignoring the ignore bit" is removed. - The freedom to choose the contents of the garbage authentication packet is removed. This simplifies testing.
2023-09-26Merge bitcoin/bitcoin#28483: refactor: Return CAutoFile from ↵fanquake
BlockManager::Open*File() fa56c421be04af846f479c30749b17e6663ab418 Return CAutoFile from BlockManager::Open*File() (MarcoFalke) 9999b89cd37fb2a23c5ebcd91d9cb31d69375933 Make BufferedFile to be a CAutoFile wrapper (MarcoFalke) fa389d902fbf5fac19fba8c5d0c5e0a25f15ca63 refactor: Drop unused fclose() from BufferedFile (MarcoFalke) Pull request description: This is required for https://github.com/bitcoin/bitcoin/pull/28052, but makes sense on its own, because offloading logic to `CAutoFile` instead of re-implementing it allows to delete code and complexity. ACKs for top commit: TheCharlatan: Re-ACK fa56c421be04af846f479c30749b17e6663ab418 willcl-ark: tACK fa56c421be Tree-SHA512: fe4638f3a6bd3f9d968cfb9ae3259c9d6cd278fe2912cbc90289851311c8c781099db4c160e775960975c4739098d9af801a8d2d12603f371f8edfe134d8f85a
2023-09-23Merge bitcoin/bitcoin#28385: [refactor] rewrite ↵fanquake
DisconnectedBlockTransactions to not use boost 4313c77400eb8eaa8586db39a7e29a861772ea80 make DisconnectedBlockTransactions responsible for its own memory management (glozow) cf5f1faa037e9a40a5029cc7dd4ee61454b62466 MOVEONLY: DisconnectedBlockTransactions to its own file (glozow) 2765d6f3434c101fe2d46e9313e540aa680fbd77 rewrite DisconnectedBlockTransactions as a list + map (glozow) 79ce9f0aa46de8ff742be83fd6f68eab40e073ec add std::list to memusage (glozow) 59a35a7398f5bcb3e3805d1e4f363e4c2fb336b3 [bench] DisconnectedBlockTransactions (glozow) 925bb723ca71aa76380b769d8926c7c2ad9bbb7b [refactor] batch-add transactions to DisconnectedBlockTransactions (glozow) Pull request description: Motivation - I think it's preferable to use stdlib data structures instead of depending on boost if we can achieve the same thing. - Also see #28335 for further context/motivation. This PR simplifies that one. Things done in this PR: - Add a bench for `DisconnectedBlockTransactions` where we reorg and the new chain has {100%, 90%, 10%} of the same transactions. AFAIU in practice, it's usually close to 100%. - Rewrite `DisconnectedBlockTransactions` as a `std::list` + `unordered_map` instead of a boost multi index container. - On my machine, the bench suggests the performance is very similar. - Move `DisconnectedBlockTransactions` from txmempool.h to its own kernel/disconnected_transactions.h. This struct isn't used by txmempool and doesn't have much to do with txmempool. My guess is that it's been living there for convenience since the boost includes are there. ACKs for top commit: ismaelsadeeq: Tested ACK 4313c77400eb8eaa8586db39a7e29a861772ea80 stickies-v: ACK 4313c77400eb8eaa8586db39a7e29a861772ea80 TheCharlatan: ACK 4313c77400eb8eaa8586db39a7e29a861772ea80 Tree-SHA512: 273c80866bf3acd39b2a039dc082b7719d2d82e0940e1eb6c402f1c0992e997256722b85c7e310c9811238a770cfbdeb122ea4babbc23835d17128f214a1ef9e
2023-09-21Merge bitcoin/bitcoin#28471: Fix virtual size limit enforcement in ↵Andrew Chow
transaction package context eb8f58f5e4a249d55338304099e8238896d2316d Add functional test to catch too large vsize packages (Greg Sanders) 1a579f9d01256b0b2570168496d129a8b2009b35 Handle over-sized (in virtual bytes) packages with no in-mempool ancestors (Greg Sanders) bc013fe8e3d0bae2ab766a8248219aa3c9e344df Bugfix: Pass correct virtual size to CheckPackageLimits (Luke Dashjr) 533660c58ad5a218671a9bc1537299b1d67bb55d Replace MAX_PACKAGE_SIZE with MAX_PACKAGE_WEIGHT to avoid vbyte confusion (Greg Sanders) Pull request description: (Alternative) Minimal subset of https://github.com/bitcoin/bitcoin/pull/28345 to: 1) Replace MAX_PACKAGE_SIZE with MAX_PACKAGE_WEIGHT which accounts for additional WU necessary to not exclude default chain limit transactions that would have been accepted individually. Avoids sigops vbyte confusion. 2) pass correct vsize to chain limit evaluations in package context 3) stop overly-large packages that have no existing mempool ancestors (also a bugfix by itself if someone sets non-standard chain limits) This should fix the known issues while not blocking additional refactoring later. ACKs for top commit: achow101: ACK eb8f58f5e4a249d55338304099e8238896d2316d ariard: Re-Code ACK eb8f58f5e glozow: reACK eb8f58f5e4a249d55338304099e8238896d2316d Tree-SHA512: 1b5cca1a526207e25d387fcc29a776a3198c3a013dc2b35c6275b9d5a64db2476c154ebf52e3a1aed0b9924c75613f21a946577aa760de28cadf0c9c7f68dc39
2023-09-20Merge bitcoin/bitcoin#27511: rpc: Add test-only RPC getaddrmaninfo for ↵Andrew Chow
new/tried table address count 28bac81a346c0b68273fa73af924f7096cb3f41d test: add functional test for getaddrmaninfo (stratospher) c8eb8dae51039aa1938e7040001a149210e87275 rpc: Introduce getaddrmaninfo for count of addresses stored in new/tried table (stratospher) Pull request description: implements https://github.com/bitcoin/bitcoin/issues/26907. split off from #26988 to keep RPC, CLI discussions separate. This PR introduces a new RPC `getaddrmaninfo`which returns the count of addresses in the new/tried table of a node's addrman broken down by network type. This would be useful for users who want to see the distribution of addresses from different networks across new/tried table in the addrman. ```jsx $ getaddrmaninfo Result: { (json object) json object with network type as keys "network" : { (json object) The network (ipv4, ipv6, onion, i2p, cjdns) "new" : n, (numeric) number of addresses in new table "tried" : n, (numeric) number of addresses in tried table "total" : n (numeric) total number of addresses in both new/tried tables from a network }, ... } ``` ### additional context from [original PR](https://github.com/bitcoin/bitcoin/pull/26988) 1. network coverage tests were skipped because there’s a small chance that addresses from different networks could hash to the same bucket and cause count of different network addresses in the tests to fail. see https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1137596851. 2. #26988 uses this RPC in -addrinfo CLI. Slight preference for keeping the RPC hidden since this info will mostly be useful to only super users. see https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1173964808. ACKs for top commit: 0xB10C: ACK 28bac81a346c0b68273fa73af924f7096cb3f41d willcl-ark: reACK 28bac81a346c0b68273fa73af924f7096cb3f41d achow101: ACK 28bac81a346c0b68273fa73af924f7096cb3f41d brunoerg: reACK 28bac81a346c0b68273fa73af924f7096cb3f41d theStack: Code-review ACK 28bac81a346c0b68273fa73af924f7096cb3f41d Tree-SHA512: 346390167e1ebed7ca5c79328ea452633736aff8b7feefea77460e04d4489059334ae78a3f757f32f5fb7827b309d7186bebab3c3760b3dfb016d564a647371a
2023-09-20Replace MAX_PACKAGE_SIZE with MAX_PACKAGE_WEIGHT to avoid vbyte confusionGreg Sanders
While allowing submitted packages to be slightly larger than what may be allowed in the mempool to allow simpler reasoning about contextual-less checks vs chain limits.
2023-09-20Merge bitcoin/bitcoin#28470: fuzz: Rework addr fuzzingfanquake
fad52baf1e9bf9d55a300922e73d3bc3169a8843 fuzz: Rework addr fuzzing (MarcoFalke) fa5b6d29ee90911271d4304a6f39c38743a84f33 fuzz: Drop unused params from serialize helpers (MarcoFalke) Pull request description: Some minor fixups to addr fuzzing ACKs for top commit: dergoegge: utACK fad52baf1e9bf9d55a300922e73d3bc3169a8843 Tree-SHA512: 6a2b07fb1a65cf855d5e7c0a52bfcb81d46dbc5d4b3e72cef359987cbd28dbfeb2fc54f210e9737cb131b40ac5f88a90e9af284e441e0b37196121590bbaf015
2023-09-19Merge bitcoin/bitcoin#28246: wallet: Use CTxDestination in CRecipient ↵fanquake
instead of just scriptPubKey ad0c469d98c51931b98b7fd937c6ac3eeaed024e wallet: Use CTxDestination in CRecipient rather than scriptPubKey (Andrew Chow) 07d3bdf4ebc06825ea24ab6f7c87aef6a22238c6 Add PubKeyDestination for P2PK scripts (Andrew Chow) 1a98a51c666e9ae77364115775ec2e0ba984e8e0 Allow CNoDestination to represent a raw script (Andrew Chow) 8dd067088d41f021b357d7db5fa5f0a9f61edddc Make WitnessUnknown members private (Andrew Chow) Pull request description: For silent payments, we want to provide a `SilentPaymentsDestination` to be used as the recipient, which requires `CRecipient` to use something other than just the `scriptPubKey` as we cannot know the output script for a silent payment prior to transaction creation. `CTxDestination` seems like the obvious place to add a `SilentPaymentsDestination` as it is our internal representation of an address. In order to still allow paying to arbitrary scriptPubKeys (e.g. for data carrier outputs, or the user hand crafted a raw transaction that they have given to `fundrawtransaction`), `CNoDestination` is changed to contain raw scripts. Additionally, P2PK scripts are now interpreted as a new `PubKeyDestination` rather than `PKHash`. This results in some things that would have given an address for P2PK scripts to no longer do so. This is arguably more correct. `ExtractDestination`'s behavior is slightly changed for the above. It now returns `true` for those destinations that have addresses, so P2PK scripts now result in `false`. Even though it returns false for `CNoDestination`, the script will now be included in that `CNoDestination`. Builds on #28244 ACKs for top commit: josibake: ACK https://github.com/bitcoin/bitcoin/pull/28246/commits/ad0c469d98c51931b98b7fd937c6ac3eeaed024e Tree-SHA512: ef3f8f3c7284779d9806c77c85b21caf910a79a1f7e7f1b51abcc0d7e074f14e00abf30f625a13075e41d94dad6202c10ddff462c0ee74c2ca4aab585b145a52
2023-09-19Remove CHashWriter typeMarcoFalke
The type is only ever set, but never read via GetType(), so remove it. Also, remove SerializeHash to avoid silent merge conflicts and use the already existing GetHash() boilerplate consistently.
2023-09-19Remove unused GetType() from OverrideStream, CVectorWriter, SpanReaderMarcoFalke
GetType() is never called, so it is completely unused and can be removed.
2023-09-19fuzz: Add missing PROVIDE_FUZZ_MAIN_FUNCTION guard to __AFL_FUZZ_INITMarcoFalke
2023-09-19rpc: Introduce getaddrmaninfo for count of addresses stored in new/tried tablestratospher
2023-09-16Merge bitcoin/bitcoin#28489: tests: fix incorrect assumption in v2transport_testfanquake
3f4e1bb9ae5ee43da9503da37b9894037d613c6d tests: fix incorrect assumption in v2transport_test (Pieter Wuille) Pull request description: One part of the current `v2transport_test` introduced in #28196 assumes that if a bit gets modified in a message, failure should instantly be detected after sending that message. This is not correct in case the length descriptor is modified, as that may cause the receiver to need more data first. Fix this by sending more messages until failure actually occurs. Discovered in https://github.com/bitcoin/bitcoin/pull/27495#issuecomment-1719934041. ACKs for top commit: theStack: ACK 3f4e1bb9ae5ee43da9503da37b9894037d613c6d Tree-SHA512: faa90bf91996cbaaef62d764e746cb222eaf6796316b0d0e13709e528750b7c0ef09172f7fecfe814dbb8c136c5259f65ca1ac79318e6768a0bfc4e626a63249
2023-09-15Merge bitcoin/bitcoin#28473: refactor: Serialization parameter cleanupsfanquake
fb6a2ab63e310d8b600352ef41aab6dafccfbff0 scripted-diff: use SER_PARAMS_OPFUNC (Anthony Towns) 5e5c8f86b60a8018e8801fb44bbe56ce97d9deef serialize: add SER_PARAMS_OPFUNC (Anthony Towns) 33203f59b482bddfe0bbe7d497cb8731ce8334a4 serialize: specify type for ParamsWrapper not ref (Anthony Towns) bf147bfffa1afb11721f30e83eec1fa829f64d5f serialize: move ser_action functions out of global namespace (Anthony Towns) Pull request description: Cleanups after #25284: * ser_action namespacing - https://github.com/bitcoin/bitcoin/pull/25284#discussion_r1316189977 * make reference implicit - https://github.com/bitcoin/bitcoin/pull/25284#discussion_r1316277030 * function notation - https://github.com/bitcoin/bitcoin/pull/25284#issuecomment-1710714821 ACKs for top commit: MarcoFalke: lgtm ACK fb6a2ab63e310d8b600352ef41aab6dafccfbff0 💨 TheCharlatan: ACK fb6a2ab63e310d8b600352ef41aab6dafccfbff0 Tree-SHA512: aacca2ee9cfec360ade6b394606e13d1dfe05bc29c5fbdd48a4e6992bd420312d4ed0d32218d95c560646af326e9977728dc2e759990636298e326947f6f9526
2023-09-15Return CAutoFile from BlockManager::Open*File()MarcoFalke
This is a refactor.
2023-09-15Make BufferedFile to be a CAutoFile wrapperMarcoFalke
This refactor allows to forward some calls to the underlying CAutoFile, instead of re-implementing the logic in the buffered file.
2023-09-15refactor: Drop unused fclose() from BufferedFileMarcoFalke
This was only explicitly used in the tests, where it can be replaced by wrapping the original raw file pointer into a CAutoFile on creation and then calling CAutoFile::fclose(). Also, it was used in LoadExternalBlockFile(), where it can also be replaced by the (implicit call to the) CAutoFile destructor after wrapping the original raw file pointer in a CAutoFile.
2023-09-15tests: fix incorrect assumption in v2transport_testPieter Wuille
2023-09-15Merge bitcoin/bitcoin#28480: fuzz: Don't use afl++ deferred forkserver modefanquake
508d05f8a7b511dd53f543df8899813487eb03e5 [fuzz] Don't use afl++ deferred forkserver mode (dergoegge) Pull request description: Fixes #28469 This makes our afl++ harness essentially behave like libFuzzer, with the exception that the whole program does fully reset every 100000 iterations. 100000 is somewhat arbitrary and we could also go with `std::numeric_limits<unsigned in>::max()` but a smaller limit does allow for the occasional reset to counter act some amount of instability in the fuzzing loop (e.g. non-determinism, statefulness). It's a bit of a shame to do this just for the targets whose initial state can't be forked (e.g. threads) because other targets do benefit from not having to redo the state setup. An alternative would be https://github.com/bitcoin/bitcoin/issues/28469#issuecomment-1717526774: ``` If the goal is to be maximally performant, the fork would need to happen for each fuzz target specifically. I guess it can be achieved by wrapping __AFL_INIT(); into a helper function and then require all fuzz target initialize() to call it? ``` ACKs for top commit: MarcoFalke: lgtm ACK 508d05f8a7b511dd53f543df8899813487eb03e5 Tree-SHA512: d9fe94e2e3198795f8fb58f67eb383531a534bcd4ec75a1f0ae6ccb5531863dbc09800bb7d77536417745c4c8bc49a4f84dcc959918b27d4997a270eeacb0e7e
2023-09-15Merge bitcoin/bitcoin#28452: Do not use std::vector = {} to release memoryfanquake
3fcd7fc7ff563bdc0e2bba66b4cbe72d898c876e Do not use std::vector = {} to release memory (Pieter Wuille) Pull request description: It appears that invoking `v = {};` for an `std::vector<...> v` is equivalent to `v.clear()`, which does not release its allocated memory. There are a number of places in the codebase where it appears to be used for that purpose however (mostly written by me). Replace those with `std::vector<...>{}.swap(v);` (using a helper function `ClearShrink` in util/vector.h). To explain what is going on: `v = {...};` is equivalent in general to `v.operator=({...});`. For many types, the `{}` is converted to the type of `v`, and then assigned to `v` - which for `std::vector` would ordinarily have the effect of clearing its memory (constructing a new empty vector, and then move-assigning it to `v`). However, since `std::vector<T>` has an `operator=(std::initializer_list<T>)` defined, it has precedence (since no implicit conversion is needed), and with an empty list, that is equivalent to `clear()`. I did consider using `v = std::vector<T>{};` as replacement for `v = {};` instances where memory releasing is desired, but it appears that it does not actually work universally either. `V{}.swap(v);` does. ACKs for top commit: ajtowns: utACK 3fcd7fc7ff563bdc0e2bba66b4cbe72d898c876e stickies-v: ACK 3fcd7fc7ff563bdc0e2bba66b4cbe72d898c876e theStack: Code-review ACK 3fcd7fc7ff563bdc0e2bba66b4cbe72d898c876e Tree-SHA512: 6148558126ec3c8cfd6daee167ec1c67b360cf1dff2cbc132bd71768337cf9bc4dda3e5a9cf7da4f7457d2123288eeba77dd78f3a17fa2cfd9c6758262950cc5
2023-09-14Merge bitcoin/bitcoin#26152: Bump unconfirmed ancestor transactions to ↵Andrew Chow
target feerate f18f9ef4d31c70e2d71ab90a24511692821418c3 Amend bumpfee for inputs with overlapping ancestry (Murch) 2e35e944dab09eff30952233f8dfc0b12c4553d5 Bump unconfirmed parent txs to target feerate (Murch) 3e3e05241128f68cf12f73ee06ff997395643885 coinselection: Move GetSelectionWaste into SelectionResult (Andrew Chow) c57889da6650715f3e1153b6104bbdae15fcac90 [node] interface to get bump fees (glozow) c24851be945b2a633ee44ed3c8a501eee5580b62 Make MiniMinerMempoolEntry fields private (Murch) ac6030e4d8f7d578cd4a8593f41189efca548064 Remove unused imports (Murch) d2f90c31ef3b8dee5a3e0804ecc62fa1cfec7cd5 Fix calculation of ancestor set feerates in test (Murch) a1f7d986e0211e54e21a1d4a570e5f15294dca72 Match tx names to index in miniminer overlap test (Murch) Pull request description: Includes some commits to address follow-ups from #27021: https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1554675156 Reduces the effective value of unconfirmed UTXOs by the fees necessary to bump their ancestor transactions to the same feerate. While the individual UTXOs always account for their full ancestry before coin-selection, we can correct potential overestimates with a second pass where we establish the ancestry and bump fee for the whole input set collectively. Fixes #9645 Fixes #9864 Fixes #15553 ACKs for top commit: S3RK: ACK f18f9ef4d31c70e2d71ab90a24511692821418c3 ismaelsadeeq: ACK f18f9ef4d31c70e2d71ab90a24511692821418c3 achow101: ACK f18f9ef4d31c70e2d71ab90a24511692821418c3 brunoerg: crACK f18f9ef4d31c70e2d71ab90a24511692821418c3 t-bast: ACK https://github.com/bitcoin/bitcoin/pull/26152/commits/f18f9ef4d31c70e2d71ab90a24511692821418c3, I reviewed the latest changes and run e2e tests against eclair, everything looks good :+1: Tree-SHA512: b65180c4243b1f9d13c311ada7a1c9f2f055d530d6c533b78c2068b50b8c29ac1321e89e85675b15515760d4f1b653ebd9da77b37c7be52d9bc565a3538f0aa6
2023-09-14Merge bitcoin/bitcoin#27850: test: Add unit & functional test coverage for ↵Andrew Chow
blockstore de8f9123afbecc3b4f59fa80af8148bc865d0588 test: cover read-only blockstore (Matthew Zipkin) 5c2185b3b624ce87320ec16412f98ab591a5860c ci: enable chattr +i capability inside containers (Matthew Zipkin) e573f2420244c583e218f51cd0d3a3cac6731003 unit test: add coverage for BlockManager (Matthew Zipkin) Pull request description: This PR adds unit and functional tests to cover the behavior described in #2039. In particular, that bitcoind will crash on startup if a reindex is requested but the `blk` files are read-only. Eventually this behavior can be updated with https://github.com/bitcoin/bitcoin/pull/27039. This PR just commits the test coverage from #27039 as suggested in https://github.com/bitcoin/bitcoin/pull/27039#issuecomment-1584915782 ACKs for top commit: jonatack: ACK de8f9123afbecc3b4f59fa80af8148bc865d0588 modulo suggestions in https://github.com/bitcoin/bitcoin/pull/27850#discussion_r1319010039, tested on macOS, but not on Linux for the Linux-related change in the last push achow101: ACK de8f9123afbecc3b4f59fa80af8148bc865d0588 MarcoFalke: lgtm ACK de8f9123afbecc3b4f59fa80af8148bc865d0588 📶 Tree-SHA512: b9bd684035dcea11c901b649fc39f397a2155a9a8459f3348e67947e387e45312fddeccb52981aef486f8a31deebb5356a7901c1bb94b78f82c24192a369af73
2023-09-14[fuzz] Don't use afl++ deferred forkserver modedergoegge
Deferring the forkserver initialization doesn't make sense for some of our targets since they involve state that can't be forked (e.g. threads). We therefore remove the use of __AFL_INIT entirely. We also increase the __AFL_LOOP count to 100000. Our fuzz targets are meant to all be deterministic and stateless therefore this should be fine.
2023-09-14Merge bitcoin/bitcoin#28460: fuzz: Use afl++ shared-memory fuzzingfanquake
97e2e1d641016cd7b74848b9560e3771f092c1ea [fuzz] Use afl++ shared-memory fuzzing (dergoegge) Pull request description: Using shared-memory is faster than reading from stdin, see https://github.com/AFLplusplus/AFLplusplus/blob/7d2122e0596132f9344a5d0896020ebc79cd33db/instrumentation/README.persistent_mode.md ACKs for top commit: MarcoFalke: review ACK 97e2e1d641016cd7b74848b9560e3771f092c1ea Tree-SHA512: 7e71b5f84835e41531c19ee959be2426da245869757de8e5dd1c730ae83ead650e2ef75f4d594d7965f661821a4ffbd27be84d3ce623702991501b34a8d02fc3
2023-09-14Merge bitcoin/bitcoin#28423: kernel: Remove protocol.h/netaddress.h/compat.h ↵fanquake
from kernel headers d5067651991f3e6daf456ba13c7036ddc4545352 [refactor] Remove compat.h from kernel headers (TheCharlatan) 36193af47c8dcff53e59498c416b85b59e0d0f91 [refactor] Remove netaddress.h from kernel headers (TheCharlatan) 2b08c55f01996e0b05763f05eac50b83ba9d5a8e [refactor] Add CChainParams member to CConnman (TheCharlatan) f0d1d8b35c3aa9f2f923f74e3dbbf1e5ece4cd2f [refactor] Add missing includes for next commit (TheCharlatan) 534b314a7401d44f51aabd4565f97be9ee411740 kernel: Move MessageStartChars to its own file (TheCharlatan) 9be330b654cfbd792620295f3867f592059d6a7a [refactor] Define MessageStartChars as std::array (TheCharlatan) 37e2b011136ca1cf00dfb9e575d12f0d035a6a2c [refactor] Allow std::array<std::byte, N> in serialize.h (MarcoFalke) Pull request description: This removes the non-consensus critical `protocol.h` and `netaddress.h` headers from the kernel headers. With this patch, they are no longer required to include in order to use the libbitcoinkernel library. This also allows for the removal of the `compat.h` header from the kernel headers. As an added future benefit it also reduces the number of of kernel headers that include the platform specific `bitcoin-config.h`. For those interested, the currently required kernel headers can be inspected visually with the [sourcetrail](https://github.com/CoatiSoftware/Sourcetrail) tool by looking at the required includes of `bitcoin-chainstate.cpp`. --- This is part of the [libbitcoinkernel project](https://github.com/bitcoin/bitcoin/issues/27587), namely its stage 1 step 3: Decouple most non-consensus headers from libbitcoinkernel. ACKs for top commit: stickies-v: re-ACK d506765 hebasto: ACK d5067651991f3e6daf456ba13c7036ddc4545352. ajtowns: utACK d5067651991f3e6daf456ba13c7036ddc4545352 MarcoFalke: lgtm ACK d5067651991f3e6daf456ba13c7036ddc4545352 🍛 Tree-SHA512: 6f90ea510a302c2927e84d16900e89997c39b8ff3ce9d4effeb8a134bd29cc52bd9e81e51aaa11f7496bad00025b78a58b88c5a9e0bb3f4ebbe9a76309215fb7
2023-09-14Merge bitcoin/bitcoin#28458: refactor: Remove unused GetType() from ↵fanquake
CBufferedFile and CAutoFile fa19c914f7fe7be127c0fb330b41ff7c091f40aa scripted-diff: Rename CBufferedFile to BufferedFile (MarcoFalke) fa2f2413b87f5fc1e5c92bf510beebdcd0091714 Remove unused GetType() from CBufferedFile and CAutoFile (MarcoFalke) 5c2b3cd4b856f1bb536daaf7f576b1b1b42293ca dbwrapper: Use DataStream for batch operations (TheCharlatan) Pull request description: This refactor is required for https://github.com/bitcoin/bitcoin/pull/28052 and https://github.com/bitcoin/bitcoin/pull/28451 Thus, split it out. ACKs for top commit: ajtowns: utACK fa19c914f7fe7be127c0fb330b41ff7c091f40aa TheCharlatan: ACK fa19c914f7fe7be127c0fb330b41ff7c091f40aa Tree-SHA512: d9c232324702512e45fd73ec3e3170f1e8a8c8f9c49cb613a6b693a9f83358914155527ace2517a2cd626a0cedcada56ef70a2a7812edafb1888fd6765eebba2
2023-09-14scripted-diff: use SER_PARAMS_OPFUNCAnthony Towns
-BEGIN VERIFY SCRIPT- sed -i 's/WithParams(\(CAddress::V[12]_[A-Z]*\) *, */\1(/g' $(git grep -l 'WithParams' src/) sed -i 's/WithParams(\(CNetAddr::V[12]\) *, */\1(/g' $(git grep -l 'WithParams' src/) sed -i 's@\(CNetAddr::V1.CService{}.*\) //@\1 //@' src/test/util/net.cpp -END VERIFY SCRIPT-
2023-09-13Fix calculation of ancestor set feerates in testMurch
Follow-up from #27021. Also included is an ASCII art visualization of the test’s transaction topology by theStack. Co-authored-by: theStack <sebastian.falbesoner@gmail.com>
2023-09-13Match tx names to index in miniminer overlap testMurch
Follow-up from #27021: In the prior commit, the vector started counting at 0, but the transaction names started with 1. This commit matches the names to the transactions’ vector indices for better readability. Co-authored-by: theStack <sebastian.falbesoner@gmail.com>
2023-09-13Merge bitcoin/bitcoin#28251: validation: fix coins disappearing mid-package ↵fanquake
evaluation 32c1dd1ad65af0ad4d36a56d2ca32a8481237e68 [test] mempool coins disappearing mid-package evaluation (glozow) a67f460c3fd1c7eb8070623666d887eefccff0d6 [refactor] split setup in mempool_limit test (glozow) d08696120e3647b4c2cd0ae8d6e57dea12418b7c [test framework] add ability to spend only confirmed utxos (glozow) 3ea71feb11c261f002ed918f91f3434fd8a23589 [validation] don't LimitMempoolSize in any subpackage submissions (glozow) d227b7234cd4cfd7c593ffcf8e2f24573d1ebea5 [validation] return correct result when already-in-mempool tx gets evicted (glozow) 9698b81828ff98820fa49c83ca364063233374c6 [refactor] back-fill results in AcceptPackage (glozow) 8ad7ad33929ee846a55a43c55732be0cb8973060 [validation] make PackageMempoolAcceptResult members mutable (glozow) 03b87c11ca0705e1d6147b90da33ce555f9f41c8 [validation] add AcceptSubPackage to delegate Accept* calls and clean up m_view (glozow) 3f01a3dab1c4ee37fd4093b6a0a3b622f53e231d [CCoinsViewMemPool] track non-base coins and allow Reset (glozow) 7d7f7a1189432b1b6245ba25df572229870567cb [policy] check for duplicate txids in package (glozow) Pull request description: While we are evaluating a package, we split it into "subpackages" for evaluation (currently subpackages all have size 1 except the last one). If a subpackage has size 1, we may add a tx to mempool and call `LimitMempoolSize()`, which evicts transactions if the mempool gets full. We handle the case where the just-submitted transaction is evicted immediately, but we don't handle the case in which a transaction from a previous subpackage (either just submitted or already in mempool) is evicted. Mainly, since the coins created by the evicted transaction are cached in `m_view`, we don't realize the UTXO has disappeared until `CheckInputsFromMempoolAndCache` asserts that they exist. Also, the returned `PackageMempoolAcceptResult` reports that the transaction is in mempool even though it isn't anymore. Fix this by not calling `LimitMempoolSize()` until the very end, and editing the results map with "mempool full" if things fall out. Pointed out by instagibbs in https://github.com/bitcoin/bitcoin/commit/faeed687e5cde5e32750d93818dd1d4add837f24 on top of the v3 PR. ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/28251/commits/32c1dd1ad65af0ad4d36a56d2ca32a8481237e68 Tree-SHA512: 61e7f69db4712e5e5bfa27d037ab66bdd97f1bf60a8d9ffb96adb1f0609af012c810d681102ee5c7baec7b5fe8cb7c304a60c63ccc445d00d86a2b7f0e7ddb90
2023-09-13[validation] add AcceptSubPackage to delegate Accept* calls and clean up m_viewglozow
(1) Call AcceptSingleTransaction when there is only 1 transaction in the subpackage. This avoids calling PackageMempoolChecks() which enforces rules that don't need to be applied for a single transaction, i.e. disabling CPFP carve out. There is a slight change in the error type returned, as shown in the txpackage_tests change. When a transaction is the last one left in the package and its fee is too low, this returns a PCKG_TX instead of PCKG_POLICY. This interface is clearer; "package-fee-too-low" for 1 transaction would be a bit misleading. (2) Clean up m_view and m_viewmempool so that coins created in this sub-package evaluation are not available for other sub-package evaluations. The contents of the mempool may change, so coins that are available now might not be later.
2023-09-13[policy] check for duplicate txids in packageglozow
Duplicates of normal transactions would be found by looking for conflicting inputs, but this doesn't catch identical empty transactions. These wouldn't be valid but exiting early is good and AcceptPackage's result sanity checks assume non-duplicate transactions.
2023-09-13fuzz: Rework addr fuzzingMarcoFalke
* Replace ConsumeDeserializationParams with V1, because V2 is unconditionally checked as well. * Also fuzz CAddress::Format::Disk in the address_deserialize fuzz target.
2023-09-13fuzz: Drop unused params from serialize helpersMarcoFalke
With the ser-type and ser-version going away, it seems unlikely that there is need for them in the future, so just remove them.
2023-09-13make DisconnectedBlockTransactions responsible for its own memory managementglozow
Co-authored-by: Cory Fields <cory-nospam-@coryfields.com>
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-20 05:52:27 +0000