aboutsummaryrefslogtreecommitdiff
path: root/src/bench
AgeCommit message (Collapse)Author
2023-07-21Move block-storage-related logic to ChainstateManagerSuhas Daftuar
Separate the notion of which blocks are stored on disk, and what data is in our block index, from what tip a chainstate might be able to get to. We can use chainstate-agnostic data to determine when to store a block on disk (primarily, an anti-DoS set of criteria) and let the chainstates figure out for themselves when a block is of interest for being a candidate tip. Note: some of the invariants in CheckBlockIndex are modified, but more work is needed (ie to move CheckBlockIndex to ChainstateManager, as most of what CheckBlockIndex is doing is checking the consistency of the block index, which is outside of Chainstate).
2023-07-12Switch all callers from poly1305_auth to Poly1305 classPieter Wuille
This also removes the old poly1305_auth interface, as it no longer serves any function. The new Poly1305 class based interface is more modern and safe.
2023-07-12Extract util::Xor, Add key_offset option, Add benchMarcoFalke
2023-07-07crypto: Implement RFC8439-compatible variant of ChaCha20Pieter Wuille
There are two variants of ChaCha20 in use. The original one uses a 64-bit nonce and a 64-bit block counter, while the one used in RFC8439 uses a 96-bit nonce and 32-bit block counter. This commit changes the interface to use the 96/32 split (but automatically incrementing the first 32-bit part of the nonce when the 32-bit block counter overflows, so to retain compatibility with >256 GiB output). Simultaneously, also merge the SetIV and Seek64 functions, as we almost always call both anyway. Co-authored-by: dhruv <856960+dhruv@users.noreply.github.com>
2023-06-29Merge bitcoin/bitcoin#27978: refactor: Drop unsafe AsBytePtr functionAndrew Chow
7c853619ee9ea17a79f1234b6c7871a45ceadcb9 refactor: Drop unsafe AsBytePtr function (Ryan Ofsky) Pull request description: Replace calls to `AsBytePtr` with calls to `AsBytes` or `reinterpret_cast`. `AsBytePtr` is just a wrapper around `reinterpret_cast`. It accepts any type of pointer as an argument and uses `reinterpret_cast` to cast the argument to a `std::byte` pointer. Despite taking any type of pointer as an argument, it is not useful to call `AsBytePtr` on most types of pointers, because byte representations of most types will be platform specific or undefined. Also, because it is named similarly to the `AsBytes` function, `AsBytePtr` looks safer than it actually is. Both `AsBytes` and `AsBytePtr` call reinterpret_cast internally and may be unsafe to use with certain types, but AsBytes at least has some type checking and can only be called on `Span` objects, while `AsBytePtr` can be called on any pointer argument. The change was motivated by discussion on #27973 and #27927 and is compatible with those PRs ACKs for top commit: jonatack: re-ACK 7c853619ee9ea17a79f1234b6c7871a45ceadcb9 sipa: utACK 7c853619ee9ea17a79f1234b6c7871a45ceadcb9 achow101: ACK 7c853619ee9ea17a79f1234b6c7871a45ceadcb9 Tree-SHA512: 200d858b1d4d579f081a7f9a14d488a99713b4918b4564ac3dd5c18578d927dbd6426e62e02f49f04a3fa73ca02ff7109c495cb0b92bec43c27d9b74e2f95757
2023-06-28refactor: Drop unsafe AsBytePtr functionRyan Ofsky
Replace calls to AsBytePtr with direct calls to AsBytes or reinterpret_cast. AsBytePtr is just a wrapper around reinterpret_cast. It accepts any type of pointer as an argument and uses reinterpret_cast to cast the argument to a std::byte pointer. Despite taking any type of pointer as an argument, it is not useful to call AsBytePtr on most types of pointers, because byte representations of most types will be implmentation-specific. Also, because it is named similarly to the AsBytes function, AsBytePtr looks safer than it actually is. Both AsBytes and AsBytePtr call reinterpret_cast internally and may be unsafe to use with certain types, but AsBytes at least has some type checking and can only be called on Span objects, while AsBytePtr can be called on any pointer argument. Co-authored-by: Pieter Wuille <pieter@wuille.net>
2023-06-28Merge bitcoin/bitcoin#27927: util: Allow std::byte and char Span serializationAndrew Chow
fa38d862358b87219b12bf31236c52f28d9fc5d6 Use only Span{} constructor for byte-like types where possible (MarcoFalke) fa257bc8312b91c2d281f48ca2500d9cba353cc5 util: Allow std::byte and char Span serialization (MarcoFalke) Pull request description: Seems odd to require developers to cast all byte-like spans passed to serialization to `unsigned char`-spans. Fix that by passing and accepting byte-like spans as-is. Finally, add tests and update the code to use just `Span` where possible. ACKs for top commit: sipa: utACK fa38d862358b87219b12bf31236c52f28d9fc5d6 achow101: ACK fa38d862358b87219b12bf31236c52f28d9fc5d6 ryanofsky: Code review ACK fa38d862358b87219b12bf31236c52f28d9fc5d6. This looks great. The second commit really removes a lot of boilerplate and shows why the first commit is useful. Tree-SHA512: 788592d9ff515c3ebe73d48f9ecbb8d239f5b985af86f09974e508cafb0ca6d73a959350295246b4dfb496149bc56330a0b5d659fc434ba6723dbaba0b7a49e5
2023-06-27Merge bitcoin/bitcoin#27334: util: implement `noexcept` move assignment & ↵Andrew Chow
move ctor for `prevector` bfb9291a8661fe5b26c14ed755cfa89d27c37110 util: implement prevector's move ctor & move assignment (Martin Leitner-Ankerl) fffc86f49f4eeb811b8438bc1b7f8d9e05882c6f test: CScriptCheck is used a lot in std::vector, make sure that's efficient (Martin Leitner-Ankerl) 81f67977f543faca2dcc35846f73e2917375ae79 util: prevector's move ctor and move assignment is `noexcept` (Martin Leitner-Ankerl) d380d2877ed45cf1e75a87d822b30e4e1e21e3d4 bench: Add benchmark for prevector usage in std::vector (Martin Leitner-Ankerl) Pull request description: `prevector`'s move assignment and move constructor were not `noexcept`, which makes it inefficient to use inside STL containers like `std::vector`. That's the case e.g. for `CScriptCheck`. This PR adds `noexcept`, and also implements the move assignment & ctor, which makes it quite a bit more efficient to use prevector in an std::vector. The PR also adds a benchmark which grows an `std::vector` by adding `prevector` objects to it. merge-base: | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 6,440.29 | 155,272.42 | 0.2% | 40,713.01 | 20,473.84 | 1.989 | 7,132.01 | 0.2% | 0.44 | `PrevectorFillVectorDirectNontrivial` | 3,213.19 | 311,217.35 | 0.7% | 35,373.01 | 10,214.07 | 3.463 | 6,945.00 | 0.2% | 0.43 | `PrevectorFillVectorDirectTrivial` | 34,749.70 | 28,777.23 | 0.1% | 364,396.05 | 110,521.94 | 3.297 | 78,568.37 | 0.1% | 0.43 | `PrevectorFillVectorIndirectNontrivial` | 32,535.05 | 30,736.09 | 0.4% | 353,823.31 | 103,464.53 | 3.420 | 79,871.80 | 0.2% | 0.40 | `PrevectorFillVectorIndirectTrivial` util: prevector's move ctor and move assignment is `noexcept`: | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 6,603.87 | 151,426.40 | 0.2% | 23,734.01 | 21,009.63 | 1.130 | 2,445.01 | 0.3% | 0.44 | `PrevectorFillVectorDirectNontrivial` | 1,980.93 | 504,813.15 | 0.1% | 13,784.00 | 6,304.32 | 2.186 | 2,258.00 | 0.3% | 0.44 | `PrevectorFillVectorDirectTrivial` | 19,110.54 | 52,327.15 | 0.1% | 139,816.41 | 51,987.72 | 2.689 | 28,512.18 | 0.1% | 0.43 | `PrevectorFillVectorIndirectNontrivial` | 12,334.37 | 81,074.27 | 0.7% | 125,655.12 | 39,253.58 | 3.201 | 27,854.46 | 0.2% | 0.44 | `PrevectorFillVectorIndirectTrivial` util: implement prevector's move ctor & move assignment | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 5,262.66 | 190,018.01 | 0.2% | 20,157.01 | 16,745.26 | 1.204 | 2,445.01 | 0.3% | 0.44 | `PrevectorFillVectorDirectNontrivial` | 1,687.07 | 592,744.35 | 0.2% | 12,742.00 | 5,368.02 | 2.374 | 2,258.00 | 0.3% | 0.44 | `PrevectorFillVectorDirectTrivial` | 17,930.80 | 55,769.95 | 0.1% | 136,237.69 | 47,903.31 | 2.844 | 28,512.02 | 0.2% | 0.42 | `PrevectorFillVectorIndirectNontrivial` | 11,893.75 | 84,077.78 | 0.2% | 126,182.02 | 37,852.91 | 3.333 | 28,152.01 | 0.1% | 0.44 | `PrevectorFillVectorIndirectTrivial` As can be seen, mostly thanks to just `noexcept` the benchmark becomes about 2 times faster because `std::vector` can now use move operations instead of having to fall back to copy everything I had a look at how this change affects the other benchmarks, and they are all pretty much the same, the only noticable difference is `CCheckQueueSpeedPrevectorJob` goes from 364.56ns down to 346.21ns. ACKs for top commit: achow101: ACK bfb9291a8661fe5b26c14ed755cfa89d27c37110 jonatack: > Tested Approach ACK [bfb9291](https://github.com/bitcoin/bitcoin/commit/bfb9291a8661fe5b26c14ed755cfa89d27c37110), ~ACK modulo re-verifying the implementation of the last commit. john-moffett: Approach ACK bfb9291a8661fe5b26c14ed755cfa89d27c37110 theStack: Tested and light code-review ACK bfb9291a8661fe5b26c14ed755cfa89d27c37110 Tree-SHA512: 242995d7cb2f8ebfa73177aa690a505f189d91edeb8cea3e34a41ca507c8cb17c65fe2a4e196fdafc5c6e89b2b2222627bfc9f5c316517de0857b7b5e9c58225
2023-06-27Use only Span{} constructor for byte-like types where possibleMarcoFalke
This removes bloat that is not needed.
2023-06-23Bench test for EllSwift ECDHPieter Wuille
Co-authored-by: Dhruv Mehta <856960+dhruv@users.noreply.github.com>
2023-06-23Bench tests for CKey->EllSwiftdhruv
Co-Authored-By: Pieter Wuille <bitcoin-dev@wuille.net>
2023-06-16bench: disable birth time block skip for wallet_create_tx.cppfurszy
As the benchmarks inside wallet_create_tx.cpp assert the wallet balance at the end, they require all blocks to be scanned by the wallet. So, we need to ensure that no blocks are skipped by the recently added wallet birth time functionality. This just means setting the wallet birthtime to the genesis block time. So the wallet is always older than any new block.
2023-05-30Merge bitcoin/bitcoin#26261: p2p: cleanup `LookupIntern`, `Lookup` and ↵Andrew Chow
`LookupHost` 5c832c3820253affc65c0ed490e26e5b0a4d5c9b p2p, refactor: return `std::optional<CNetAddr>` in `LookupHost` (brunoerg) 34bcdfc6a65de906c65edccdd96fe15219081cd2 p2p, refactor: return vector/optional<CService> in `Lookup` (brunoerg) 7799eb125b7a1146f8251be5d26df574236212a9 p2p, refactor: return `std::vector<CNetAddr>` in `LookupHost` (brunoerg) 5c1774a563dcc237a88df69569cd94fe81e908f7 p2p, refactor: return `std::vector<CNetAddr>` in `LookupIntern` (brunoerg) Pull request description: Continuation of #26078. To improve readability instead of returning a bool and passing stuff by reference, this PR changes: - `LookupHost` to return `std::vector<CNetAddr>` - `LookupHost` to return `std::optional<CNetAddr>` - `Lookup` to return `std::vector<CService>` - `Lookup` to return `std::optional<CService>`. - `LookupIntern` to return `std::vector<CNetAddr>` As discussed in #26078, it would be better to avoid using `optional` in some cases, but for specific `Lookup` and `LookupHost` functions it's necessary to use `optional` to verify if they were able to catch some data from their overloaded function. ACKs for top commit: achow101: ACK 5c832c3820253affc65c0ed490e26e5b0a4d5c9b stickies-v: re-ACK 5c832c3820253affc65c0ed490e26e5b0a4d5c9b - just addressing two nits, no other changes theStack: re-ACK 5c832c3820253affc65c0ed490e26e5b0a4d5c9b Tree-SHA512: ea346fdc54463999646269bd600cd4a1590ef958001d2f0fc2be608ca51e1b4365efccca76dd4972b023e12fcc6e67d226608b0df7beb901bdeadd19948df840
2023-05-30Merge bitcoin/bitcoin#27666: wallet, bench: Move commonly used functions to ↵fanquake
their own file and fix a bug 7379a54ec416c8c0a029cc41835a23d42cb6d800 bench: Remove incorrect LoadWallet call in WalletBalance (Andrew Chow) 846b2fe67ed76a678770d343153acedadfdacd0b tests: Move ADDRESS_BCRT1_UNSPENDABLE to wallet/test/util.h (Andrew Chow) c61d3f02f5122b38ea8bf0029aa9dfbbf38e10d0 tests, bench: Consolidate {Test,Bench}Un/LoadWallet helper (Andrew Chow) Pull request description: I have a few PRs and branches that use these two commits, probably makes sense to split them into a separate PR to be merged sooner. The first commit contains some things that end up being commonly used in new wallet benchmarks. These are moved into `wallet_common.{h/cpp}`. The second commit contains a bugfix for the wallet_balance benchmark where it calls `LoadWallet` in the wrong place. It's unnecessary to call that function in this benchmark. Although this does not cause any issues currently, it ends up causing issues in some PRs and branches that I'm working on. ACKs for top commit: Sjors: utACK 7379a54ec416c8c0a029cc41835a23d42cb6d800 furszy: ACK 7379a54 Tree-SHA512: 47773887a16c69ac7121c699d3446a8c399bd792a6a31714998b7b7a19fea179c6d3b29cb898b04397b2962c1b4120d57009352b8460b8283e188d4cb480c9ba
2023-05-30Merge bitcoin/bitcoin#27636: kernel: Remove util/system from kernel library, ↵fanquake
interface_ui from validation. 7d3b35004b039f2bd606bb46a540de7babdbc41e refactor: Move system from util to common library (TheCharlatan) 7eee356c0a7fefd70c8de21689efa335f52a69ba refactor: Split util::AnyPtr into its own file (TheCharlatan) 44de325d95447498036479c3112ba741caf45bf6 refactor: Split util::insert into its own file (TheCharlatan) 9ec5da36b62276ae22e348f26f88aaf646357d6d refactor: Move ScheduleBatchPriority to its own file (TheCharlatan) f871c69191dfe1331861ebcdbadb6bd47e45c8b1 kernel: Add warning method to notifications (TheCharlatan) 4452707edec91c7d7991f486dd41ef3edb4f7fbf kernel: Add progress method to notifications (TheCharlatan) 84d71457e7250ab25c0a11d1ad1c7657197ffd90 kernel: Add headerTip method to notifications (TheCharlatan) 447761c8228d58f948aae7e73ed079c028cacb97 kernel: Add notification interface (TheCharlatan) Pull request description: This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/27587 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". --- It removes the kernel library's dependency on `util/system` and `interface_ui`. `util/system` contains networking and shell-related code that should not be part of the kernel library. The following pull requests prepared `util/system` for this final step: https://github.com/bitcoin/bitcoin/pull/27419 https://github.com/bitcoin/bitcoin/pull/27254 https://github.com/bitcoin/bitcoin/pull/27238. `interface_ui` defines functions for a more general node interface and has a dependency on `boost/signals2`. After applying the patches from this pull request, the kernel's reliance on boost is down to `boost::multiindex`. The approach implemented here introduces some indirection, which makes the code a bit harder to read. Any suggestions for improving or reworking this pull request to make it more concise, or even reworking it into a more proper interface, are appreciated. ACKs for top commit: MarcoFalke: re-ACK 7d3b35004b039f2bd606bb46a540de7babdbc41e (no change) 🎋 stickies-v: Code Review ACK 7d3b35004b039f2bd606bb46a540de7babdbc41e hebasto: re-ACK 7d3b35004b039f2bd606bb46a540de7babdbc41e, only last two commits dropped since my [recent](https://github.com/bitcoin/bitcoin/pull/27636#pullrequestreview-1435394620) review. Tree-SHA512: c8cfc698dc9d78e20191c444708f2d957501229abe95e5806106d1126fb9c5fbcee686fb55645658c0107ce71f10646f37a2fdf7fde16bbf22cbf1ac885dd08d
2023-05-26p2p, refactor: return `std::optional<CNetAddr>` in `LookupHost`brunoerg
2023-05-26p2p, refactor: return vector/optional<CService> in `Lookup`brunoerg
2023-05-25bench: Remove incorrect LoadWallet call in WalletBalanceAndrew Chow
The WalletBalance benchmarks would incorrectly call LoadWallet after the wallet has been setup. LoadWallet expects to be the first thing that is called and for the CWallet to be in a fresh state. When it is not, it results in bogus pointers which can cause segfaults during this benchmark.
2023-05-25tests: Move ADDRESS_BCRT1_UNSPENDABLE to wallet/test/util.hAndrew Chow
This static address is usable for other wallet tests and benchmarks, so make it available to them.
2023-05-25tests, bench: Consolidate {Test,Bench}Un/LoadWallet helperAndrew Chow
The wallet tests and benchmarks both had helper functions for loading and unloading the wallet for the test that were almost identical. These functions are consolidated and reused.
2023-05-25wallet: skip block scan if block was created before wallet birthdayfurszy
To avoid wasting processing power, we can skip blocks that occurred before the wallet's creation time, since these blocks are guaranteed not to contain any relevant wallet data. This has direct implications (an speed improvement) on the underlying blockchain synchronization process as well. The reason is that the validation interface queue is limited to 10 tasks per time. This means that no more than 10 blocks can be waiting for the wallet(s) to be processed while we are synchronizing the chain (activating the best chain to be more precise). Which can be a bottleneck if blocks arrive and are processed faster from the network than what they are processed by the wallet(s).
2023-05-20refactor: Move system from util to common libraryTheCharlatan
Since the kernel library no longer depends on the system file, move it to the common library instead in accordance to the diagram in doc/design/libraries.md.
2023-05-15Merge bitcoin/bitcoin#26715: Introduce `MockableDatabase` for wallet unit testsfanquake
33e2b82a4fc990253ff77655f437c7aed336bc55 wallet, bench: Remove unused database options from WalletBenchLoading (Andrew Chow) 80ace042d8fece9be50bfef1be64c6e5720e87e6 tests: Modify records directly in wallet ckey loading test (Andrew Chow) b3bb17d5d07f51ac2e501e4a7a3bbcd17144070f tests: Update DuplicateMockDatabase for MockableDatabase (Andrew Chow) f0eecf5e408238c64b77b0a4974ba2b9edb17487 scripted-diff: Replace CreateMockWalletDB with CreateMockableWalletDB (Andrew Chow) 075962bc25a90661612fe4613cd50ea1cae21f52 wallet, tests: Include wallet/test/util.h (Andrew Chow) 14aa4cb1e44f089a6022a2b14a98bca4a7dd9a01 wallet: Move DummyDatabase to salvage (Andrew Chow) f67a385556c60b2e4788a378196a395fca0539f5 wallet, tests: Replace usage of dummy db with mockable db (Andrew Chow) 33c6245ac1ecdfe25b1ee4fd9e93c43393634ae3 Introduce MockableDatabase for wallet unit tests (Andrew Chow) Pull request description: For the wallet's unit tests, we currently use either `DummyDatabase` or memory-only versions of either BDB or SQLite. The tests that use `DummyDatabase` just need a `WalletDatabase` so that the `CWallet` can be constructed, while the tests using the memory-only databases just need a backing data store. There is also a `FailDatabase` that is similar to `DummyDatabase` except it fails be default or can have a configured return value. Having all of these different database types can make it difficult to write tests, particularly tests that work when either BDB or SQLite is disabled. This PR unifies all of these different unit test database classes into a single `MockableDatabase`. Like `DummyDatabase`, most functions do nothing and just return true. Like `FailDatabase`, the return value of some functions can be configured on the fly to test various failure cases. Like the memory-only databases, records can actually be written to the `MockableDatabase` and be retrieved later, but all of this is still held in memory. Using `MockableDatabase` completely removes the need for having BDB or SQLite backed wallets in the unit tests for the tests that are not actually testing specific database behaviors. Because `MockableDatabase`s can be created by each unit test, we can also control what records are stored in the database. Records can be added and removed externally from the typical database modification functions. This will give us greater ability to test failure conditions, particularly those involving corrupted records. Possible alternative to #26644 ACKs for top commit: furszy: ACK 33e2b82 TheCharlatan: ACK 33e2b82a4fc990253ff77655f437c7aed336bc55 Tree-SHA512: c2b09eff9728d063d2d4aea28a0f0e64e40b76483e75dc53f08667df23bd25834d52656cd4eafb02e552db0b9e619cfdb1b1c65b26b5436ee2c971d804768bcc
2023-05-10Merge bitcoin/bitcoin#19690: util: improve FindByte() performanceAndrew Chow
72efc26439da9a1344a19569fb0cab01f82ae7d1 util: improve streams.h:FindByte() performance (Larry Ruane) 604df63f6c70b9692b067777ddb38d946ac0b2fc [bench] add streams findbyte (gzhao408) Pull request description: This PR is strictly a performance improvement; there is no functional change. The `CBufferedFile::FindByte()` method searches for the next occurrence of the given byte in the file. Currently, this is done by explicitly inspecting each byte in turn. This PR takes advantage of `std::find()` to do the same more efficiently, improving its CPU runtime by a factor of about 25 in typical use. ACKs for top commit: achow101: re-ACK 72efc26439da9a1344a19569fb0cab01f82ae7d1 stickies-v: re-ACK 72efc26439da9a1344a19569fb0cab01f82ae7d1 Tree-SHA512: ddf0bff335cc8aa34f911aa4e0558fa77ce35d963d602e4ab1c63090b4a386faf074548daf06ee829c7f2c760d06eed0125cf4c34e981c6129cea1804eb3b719
2023-05-09Merge bitcoin/bitcoin#27491: refactor: Move chain constants to the util libraryfanquake
d168458d1ff987e0d741c75ac1d4b63ae0cfb7e7 scripted-diff: Remove unused chainparamsbase includes (TheCharlatan) e9ee8aaf3acdf6dce2b339916d4c602484570050 Add missing definitions in prep for scripted diff (TheCharlatan) ba8fc7d788932b25864fb260ca14983aa2398c23 refactor: Replace string chain name constants with ChainTypes (TheCharlatan) 401453df419af35957ec711423ac3d93ad512fe8 refactor: Introduce ChainType getters for ArgsManager (TheCharlatan) bfc21c31b2186f7d30fc9a9ca7d6887ab61c6fb9 refactor: Create chaintype files (TheCharlatan) Pull request description: This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/24303 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". It is also a follow up to #26177. It replaces pull request https://github.com/bitcoin/bitcoin/pull/27294, which just moved the constants to a new file, but did not re-declare them as enums. The code move of the chain name constants out of the `chainparamsbase` to their own separate header allows the kernel `chainparams` to no longer include `chainparamsbase`. The `chainparamsbase` contain references to the `ArgsManager` and networking related options that should not belong to the kernel library. Besides this move, the constants are re-declared as enums with helper functions facilitating string conversions. ACKs for top commit: ryanofsky: Code review ACK d168458d1ff987e0d741c75ac1d4b63ae0cfb7e7. Just suggested changes since last review. Tree-SHA512: ac2fbe5cbbab4f52eae1e30af1f16700b6589eb4764c328a151a712adfc37f326cc94a65c385534c57d4bc92cc1a13bf1777d92bc924a20dbb30440e7380b316
2023-05-09scripted-diff: Remove unused chainparamsbase includesTheCharlatan
This is a follow-up to previous commits moving the chain constants out of chainparamsbase. The script removes the chainparamsbase header in all files where it is included, but not used. This is done by filtering against all defined symbols of the header as well as its respective .cpp file. The kernel chainparams now no longer relies on chainparamsbase. -BEGIN VERIFY SCRIPT- sed -i '/#include <chainparamsbase.h>/d' $( git grep -l 'chainparamsbase.h' | xargs grep -L 'CBaseChainParams\|CreateBaseChainParams\|SetupChainParamsBaseOptions\|BaseParams\|SelectBaseParams\|chainparamsbase.cpp' ) -END VERIFY SCRIPT-
2023-05-09refactor: Replace string chain name constants with ChainTypesTheCharlatan
This commit effectively moves the definition of these constants out of the chainparamsbase to their own file. Using the ChainType enums provides better type safety compared to passing around strings. The commit is part of an ongoing effort to decouple the libbitcoinkernel library from the ArgsManager and other functionality that should not be part of the kernel library.
2023-05-08refactor: Remove unused GetTimeMillisMarcoFalke
The function is unused, not type-safe, and does not denote the underlying clock type. So remove it.
2023-05-05util: improve streams.h:FindByte() performanceLarry Ruane
Avoid use of the expensive mod operator (%) when calculating the buffer offset. No functional difference. Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
2023-05-05[bench] add streams findbytegzhao408
2023-05-03wallet, bench: Remove unused database options from WalletBenchLoadingAndrew Chow
2023-05-03tests: Update DuplicateMockDatabase for MockableDatabaseAndrew Chow
2023-05-03scripted-diff: Replace CreateMockWalletDB with CreateMockableWalletDBAndrew Chow
Since we have a mockable wallet database, we don't really need to be using BDB or SQLite's in-memory database capabilities. It doesn't really help us to be using those as we aren't doing anything that requires one type of db over the other, and will just prefer SQLite if it's available. MockableDatabase is suitable for these uses, so use CreateMockableWalletDatabase to use that. -BEGIN VERIFY SCRIPT- sed -i "s/CreateMockWalletDatabase(options)/CreateMockableWalletDatabase()/" $(git grep -l "CreateMockWalletDatabase(options)" -- ":(exclude)src/wallet/walletdb.*") sed -i "s/CreateMockWalletDatabase/CreateMockableWalletDatabase/" $(git grep -l "CreateMockWalletDatabase" -- ":(exclude)src/wallet/walletdb.*") -END VERIFY SCRIPT-
2023-05-03wallet, tests: Include wallet/test/util.hAndrew Chow
This will be needed for the following scripted-diff to work.
2023-05-03wallet, tests: Replace usage of dummy db with mockable dbAndrew Chow
2023-05-02test: Add util to mine invalid blocksMarcoFalke
With the current utils it is only possible to mine valid blocks. This commit adds new util methods to mine invalid blocks.
2023-04-21Merge bitcoin/bitcoin#27419: move-only: Extract common/args from util/systemfanquake
be55f545d53d44fdcf2d8ae802e9eae551d120c6 move-only: Extract common/args and common/config.cpp from util/system (TheCharlatan) Pull request description: This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/24303 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". It is part of a series of patches splitting up the `util/system` files. Its preceding pull request is https://github.com/bitcoin/bitcoin/pull/27254. The pull request contains an extraction of ArgsManager related functions from util/system into their own common/ file. The background of this commit is an ongoing effort to decouple the libbitcoinkernel library from the ArgsManager. The ArgsManager belongs into the common library, since the kernel library should not depend on it. See [doc/design/libraries.md](https://github.com/bitcoin/bitcoin/blob/master/doc/design/libraries.md) for more information on this rationale. ACKs for top commit: MarcoFalke: re-ACK be55f545d53d44fdcf2d8ae802e9eae551d120c6 🚲 ryanofsky: Code review ACK be55f545d53d44fdcf2d8ae802e9eae551d120c6. Just small cleanups since the last review. hebasto: ACK be55f545d53d44fdcf2d8ae802e9eae551d120c6, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 90eb03334af0155b823030b4f2ecf286d35058d700ee2ddbbaa445be19e31eb0fe982656f35bd14ecee3ad2c3d0db3746855cb8f3777eff7253713e42873e111
2023-04-20Merge bitcoin/bitcoin#26720: wallet: coin selection, don't return results ↵Andrew Chow
that exceed the max allowed weight 25ab14712b9d80276016f9fc9bff7fb9c1d09635 refactor: coinselector_tests, unify wallet creation code (furszy) ba9431c505e1590db6103b9632134985cd4704dc test: coverage for bnb max weight (furszy) 5a2bc45ee0b123e461c5191322ed0b43524c3d82 wallet: clean post coin selection max weight filter (furszy) 2d112584e384de10021c64e4700455d71326824e coin selection: BnB, don't return selection if exceeds max allowed tx weight (furszy) d3a1c098e4b5df2ebbae20c6e390c3d783950e93 test: coin selection, add coverage for SRD (furszy) 9d9689e5a657956db8a30829c994600ec7d3098b coin selection: heap-ify SRD, don't return selection if exceeds max tx weight (furszy) 6107ec2229c5f5b4e944a6b10d38010c850094ac coin selection: knapsack, select closest UTXO above target if result exceeds max tx size (furszy) 1284223691127e76135a46d251c52416104f0ff1 wallet: refactor coin selection algos to return util::Result (furszy) Pull request description: Coming from the following comment https://github.com/bitcoin/bitcoin/pull/25729#discussion_r1029324367. The reason why we are adding hundreds of UTXO from different sources when the target amount is covered only by one of them is because only SRD returns a usable result. Context: In the test, we create 1515 UTXOs with 0.033 BTC each, and 1 UTXO with 50 BTC. Then perform Coin Selection to fund 49.5 BTC. As the selection of the 1515 small UTXOs exceeds the max allowed tx size, the expectation here is to receive a selection result that only contain the big UTXO. Which is not happening for the following reason: Knapsack returns a result that exceeds the max allowed transaction size, when it should return the closest utxo above the target, so we fallback to SRD who selects coins randomly up until the target is met. So we end up with a selection result with lot more coins than what is needed. ACKs for top commit: S3RK: ACK 25ab14712b9d80276016f9fc9bff7fb9c1d09635 achow101: ACK 25ab14712b9d80276016f9fc9bff7fb9c1d09635 Xekyo: reACK 25ab14712b9d80276016f9fc9bff7fb9c1d09635 theStack: Code-review ACK 25ab14712b9d80276016f9fc9bff7fb9c1d09635 Tree-SHA512: 2425de4cc479b4db999b3b2e02eb522a2130a06379cca0418672a51c4076971a1d427191173820db76a0f85a8edfff100114e1c38fb3b5dc51598d07cabe1a60
2023-04-20Merge bitcoin/bitcoin#25325: Add pool based memory resourceAndrew Chow
9f947fc3d4b779f017332135323b34e8f216f613 Use PoolAllocator for CCoinsMap (Martin Leitner-Ankerl) 5e4ac5abf54f8e6d6330df0c73119aa0cca4c103 Call ReallocateCache() on each Flush() (Martin Leitner-Ankerl) 1afca6b663bb54022afff193fd9d83856606b189 Add PoolResource fuzzer (Martin Leitner-Ankerl) e19943f049ed8aa4f32a1d8440a9fbf160367f0f Calculate memory usage correctly for unordered_maps that use PoolAllocator (Martin Leitner-Ankerl) b8401c3281978beed6198b2f9782b6a8dd35cbd7 Add pool based memory resource & allocator (Martin Leitner-Ankerl) Pull request description: A memory resource similar to `std::pmr::unsynchronized_pool_resource`, but optimized for node-based containers. The goal is to be able to cache more coins with the same memory usage, and allocate/deallocate faster. This is a reimplementation of #22702. The goal was to implement it in a way that is simpler to review & test * There is now a generic `PoolResource` for allocating/deallocating memory. This has practically the same API as `std::pmr::memory_resource`. (Unfortunately I cannot use std::pmr because libc++ simply doesn't implement that API). * Thanks to sipa there is now a fuzzer for PoolResource! On a fast machine I ran it for ~770 million executions without finding any issue. * The estimation of the correct node size is now gone, PoolResource now has multiple pools and just needs to be created large enough to have space for the unordered_map nodes. I ran benchmarks with #22702, mergebase, and this PR. Frequency locked Intel i7-8700, clang++ 13.0.1 to reindex up to block 690000. ```sh bitcoind -dbcache=5000 -assumevalid=00000000000000000002a23d6df20eecec15b21d32c75833cce28f113de888b7 -reindex-chainstate -printtoconsole=0 -stopatheight=690000 ``` The performance is practically identical with #22702, just 0.4% slower. It's ~21% faster than master: ![Progress in Million Transactions over Time(2)](https://user-images.githubusercontent.com/14386/173288685-91952ade-f304-4825-8bfb-0725a71ca17b.png) ![Size of Cache in MiB over Time](https://user-images.githubusercontent.com/14386/173291421-e6b410be-ac77-479b-ad24-5fafcebf81eb.png) Note that on cache drops mergebase's memory doesnt go so far down because it does not free the `CCoinsMap` bucket array. ![Size of Cache in Million tx over Time(1)](https://user-images.githubusercontent.com/14386/173288703-a80c9c9e-93c8-4a16-9df8-610c89c61cc4.png) ACKs for top commit: LarryRuane: ACK 9f947fc3d4b779f017332135323b34e8f216f613 achow101: re-ACK 9f947fc3d4b779f017332135323b34e8f216f613 john-moffett: ACK 9f947fc3d4b779f017332135323b34e8f216f613 jonatack: re-ACK 9f947fc3d4b779f017332135323b34e8f216f613 Tree-SHA512: 48caf57d1775875a612b54388ef64c53952cd48741cacfe20d89049f2fb35301b5c28e69264b7d659a3ca33d4c714d47bafad6fd547c4075f08b45acc87c0f45
2023-04-20Merge bitcoin/bitcoin#27214: addrman: Enable selecting addresses by networkAndrew Chow
17e705428ddf80c7a7f31fe5430d966cf08a37d6 doc: clarify new_only param for Select function (Amiti Uttarwar) b0010c83a1b4a3d21719cb68e37faf9b1172522a bench: test select for a new table with only one address (Amiti Uttarwar) 9b91aae08579c77d2fd5506804c8e2e0cda0d274 bench: add coverage for addrman select with network parameter (Amiti Uttarwar) 22a4d1489c0678a90c00318203cfce61672f20b7 test: increase coverage of addrman select (without network) (Amiti Uttarwar) a98e542e0c18f7cb2340179631806f14b07430c3 test: add addrman test for special case (Amiti Uttarwar) 5c8b4baff27e0ccd27fda6e915b956d1e8dd7ce2 tests: add addrman_select_by_network test (Amiti Uttarwar) 6b229284fd2209938ee8fdffed4d080395b3aa05 addrman: add functionality to select by network (Amiti Uttarwar) 26c3bf11e2487ed0ac578fb92619c148336003cb scripted-diff: rename local variables to match modern conventions (Amiti Uttarwar) 48806412e2bcd023b78fc05f6c9ce092360d1db1 refactor: consolidate select logic for new and tried tables (Amiti Uttarwar) ca2a9c5f8f14b792a14e81f73b1910a4c8799b93 refactor: generalize select logic (Amiti Uttarwar) 052fbcd5a791855406141e85d32e42e297220fe9 addrman: Introduce helper to generalize looking up an addrman entry (Amiti Uttarwar) 9bf078f66c8f286e1ab5e34b8eeed7d80290a897 refactor: update Select_ function (Amiti Uttarwar) Pull request description: For the full context & motivation of this patch, see #27213 This is joint work with mzumsande. This PR adds functionality to `AddrMan::Select` to enable callers to specify a network they are interested in. Along the way, it refactors the function to deduplicate the logic, updates the local variables to match modern conventions, adds test coverage for both the new and existing `Select` logic, and adds bench tests for the worst case performance of both the new and existing `Select` logic. This functionality is used in the parent PR. ACKs for top commit: vasild: ACK 17e705428ddf80c7a7f31fe5430d966cf08a37d6 brunoerg: re-ACK 17e705428ddf80c7a7f31fe5430d966cf08a37d6 ajtowns: ACK 17e705428ddf80c7a7f31fe5430d966cf08a37d6 mzumsande: Code Review ACK 17e705428ddf80c7a7f31fe5430d966cf08a37d6 Tree-SHA512: e99d1ce0c44a15601a3daa37deeadfc9d26208a92969ecffbea358d57ca951102d759734ccf77eacd38db368da0bf5b6fede3cd900d8a77b3061f4adc54e52d8
2023-04-19move-only: Extract common/args and common/config.cpp from util/systemTheCharlatan
This is an extraction of ArgsManager related functions from util/system into their own common file. Config file related functions are moved to common/config.cpp. The background of this commit is an ongoing effort to decouple the libbitcoinkernel library from the ArgsManager. The ArgsManager belongs into the common library, since the kernel library should not depend on it. See doc/design/libraries.md for more information on this rationale.
2023-04-11Merge bitcoin/bitcoin#26699: wallet, gui: bugfix, getAvailableBalance skips ↵Andrew Chow
selected coins 68eed5df8656bed1be6526b014e58d3123102b03 test,gui: add coverage for PSBT creation on legacy watch-only wallets (furszy) 306aab5bb471904faed325d9f3b38b7e891c7bbb test,gui: decouple widgets and model into a MiniGui struct (furszy) 2f76ac0383904123676f1b4eeba0f772a4c5cb5d test,gui: decouple chain and wallet initialization from test case (furszy) cd98b717398f7b13ace91ea9efac9ce1e60b4d62 gui: 'getAvailableBalance', include watch only balance (furszy) 74eac3a82fc948467d5a15a5af420b36ce8eb04a test: add coverage for 'useAvailableBalance' functionality (furszy) dc1cc1c35995dc09085b3d9270c445b7923fdb51 gui: bugfix, getAvailableBalance skips selected coins (furszy) Pull request description: Fixes https://github.com/bitcoin-core/gui/issues/688 and https://github.com/bitcoin/bitcoin/issues/26687. First Issue Description (https://github.com/bitcoin-core/gui/issues/688): The previous behavior for `getAvailableBalance`, when the coin control had selected coins, was to return the sum of them. Instead, we are currently returning the wallet's available total balance minus the selected coins total amount. Reason: Missed to update the `GetAvailableBalance` function to include the coin control selected coins on #25685. Context: Since #25685 we skip the selected coins inside `AvailableCoins`, the reason is that there is no need to waste resources walking through the entire wallet's txes map just to get coins that could have gotten by just doing a simple `mapWallet.find`). Places Where This Generates Issues (only when the user manually select coins via coin control): 1) The GUI balance check prior the transaction creation process. 2) The GUI "useAvailableBalance" functionality. Note 1: As the GUI uses a balance cache since https://github.com/bitcoin-core/gui/pull/598, this issue does not affect the regular spending process. Only arises when the user manually select coins. Note 2: Added test coverage for the `useAvailableBalance` functionality. ---------------------------------- Second Issue Description (https://github.com/bitcoin/bitcoin/issues/26687): As we are using a cached balance on `WalletModel::getAvailableBalance`, the function needs to include the watch-only available balance for wallets with private keys disabled. ACKs for top commit: Sjors: tACK 68eed5df8656bed1be6526b014e58d3123102b03 achow101: ACK 68eed5df8656bed1be6526b014e58d3123102b03 theStack: ACK 68eed5df8656bed1be6526b014e58d3123102b03 Tree-SHA512: 674f3e050024dabda2ff4a04b9ed3750cf54a040527204c920e1e38bd3d7f5fd4d096e4fd08a0fea84ee6abb5070f022b5c0d450c58fd30202ef05ebfd7af6d3
2023-04-05coin selection: BnB, don't return selection if exceeds max allowed tx weightfurszy
2023-04-03gui: bugfix, getAvailableBalance skips selected coinsfurszy
The previous behavior for getAvailableBalance when coin control has selected coins was to return the sum of them. Instead, we are currently returning the wallet's available total balance minus the selected coins total amount. This turns into a GUI-only issue for the "use available balance" button when the user manually select coins in the send screen. Reason: We missed to update the GetAvailableBalance function to include the coin control selected coins on #25685. Context: Since #25685 we skip the selected coins inside `AvailableCoins`, the reason is that there is no need to traverse the wallet's txes map just to get coins that can directly be fetched by their id.
2023-04-03Merge bitcoin/bitcoin#27254: refactor: Extract util/fs from util/systemfanquake
00e9b97f37e0bdf4c647236838c10b68b7ad5be3 refactor: Move fs.* to util/fs.* (TheCharlatan) 106b46d9d25b5228ef009fbbe6f9a7ae35090d15 Add missing fs.h includes (TheCharlatan) b202b3dd6393b415fa68e18dc49c9431dc6b58b2 Add missing cstddef include in assumptions.h (TheCharlatan) 18fb36367a28819bd5ab402344802796a1248979 refactor: Extract util/fs_helpers from util/system (Ben Woosley) Pull request description: This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/24303 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". This commit was originally authored by empact and is taken from its parent PR #25152. #### Context There is an ongoing effort to decouple the `ArgsManager` used for command line parsing user-provided arguments from the libbitcoinkernel library (https://github.com/bitcoin/bitcoin/pull/25290, https://github.com/bitcoin/bitcoin/pull/25487, https://github.com/bitcoin/bitcoin/pull/25527, https://github.com/bitcoin/bitcoin/pull/25862, https://github.com/bitcoin/bitcoin/pull/26177, and https://github.com/bitcoin/bitcoin/pull/27125). The `ArgsManager` is defined in `system.h`. A similar pull request extracting functionality from `system.h` has been merged in https://github.com/bitcoin/bitcoin/pull/27238. #### Changes Next to providing better code organization, this PR removes some reliance of the tree of libbitcoinkernel header includes on `system.h` (and thus the `ArgsManager` definition) by moving filesystem related functions out of the `system.*` files. There is already a pair of `fs.h` / `fs.cpp` in the top-level `src/` directory. They were not combined with the files introduced here, to keep the patch cleaner and more importantly because they are often included without the utility functions. The new files are therefore named `fs_helpers` and the existing `fs` files are moved into the util directory. Further commits splitting more functionality out of `system.h` are still in #25152 and will be submitted in separate PRs once this PR has been processed. ACKs for top commit: hebasto: ACK 00e9b97f37e0bdf4c647236838c10b68b7ad5be3 Tree-SHA512: 31422f148d14ba3c843b99b1550a6fd77c77f350905ca324f93d4f97b652246bc58fa9696c64d1201979cf88733e40be02d262739bb7d417cf22bf506fdb7666
2023-03-27Merge bitcoin/bitcoin#26642: clang-tidy: Add more `performance-*` checks and ↵fanquake
related fixes 03ec5b6f9ca3af28c9ce25cf2393e28ae852d808 clang-tidy: Exclude `performance-*` checks rather including them (Hennadii Stepanov) 24004372302adfc0e7cb36f8db6830694bf050e9 clang-tidy: Add `performance-type-promotion-in-math-fn` check (Hennadii Stepanov) 7e975e6cf86617346c1d8e2568f74a0252c03857 clang-tidy: Add `performance-inefficient-vector-operation` check (Hennadii Stepanov) 516b75f66ec3ba7495fc028c750937bd66cc9bba clang-tidy: Add `performance-faster-string-find` check (Hennadii Stepanov) Pull request description: ACKs for top commit: martinus: ACK 03ec5b6f9ca3af28c9ce25cf2393e28ae852d808 TheCharlatan: re-ACK [03ec5b6](https://github.com/bitcoin/bitcoin/pull/26642/commits/03ec5b6f9ca3af28c9ce25cf2393e28ae852d808) Tree-SHA512: 2dfa52f9131da88826f32583bfd534a56a998477db9804b7333c0e7ac0b6b36141009755c7163b9f95d0ecbf5c2cb63f8a69ce4b114bb83423faed21b50cec67
2023-03-26clang-tidy: Add `performance-inefficient-vector-operation` checkHennadii Stepanov
https://clang.llvm.org/extra/clang-tidy/checks/performance/inefficient-vector-operation.html
2023-03-26bench: Add benchmark for prevector usage in std::vectorMartin Leitner-Ankerl
2023-03-23Merge bitcoin/bitcoin#26957: bench: update logging benchmarksAndrew Chow
8c47d599b87d6b2d43e7d37ce0aaf4f541535bb9 doc: improve -debuglogfile help to be a bit clearer (jonatack) 20d89d6802b67e2b8c864bedf23673d008e3faab bench: document expected results in logging benchmarks (jonatack) d8deba8c36a42481b1c1e73009d7c9cc2cb25f70 bench: add LogPrintfCategory and LogPrintLevel benchmarks (Jon Atack) 102b2033493f0d61e9763d094cb8a0017f7e3a10 bench: order the logging benchmark code by output (Jon Atack) 4b3fdbf6fe20dc70c8dbdaacce161bc4e76b9c84 bench: update logging benchmark naming for clarity (Jon Atack) 4684aa8733e831d7858d43bec4271d5d026ba183 bench: allow logging benchmarks to be order-independent (Larry Ruane) Pull request description: Update our logging benchmarks for evaluating ongoing work like #25203 and refactoring proposals like #26619 and #26697. - make the logging benchmarks order-independent (Larry Ruane) - add missing benchmarks for the `LogPrintLevel` and `LogPrintfCategory` macros that our logging is migrating to; at some later point it should be feasible to drop some of the previous logging benchmarks - update the logging benchmark naming to be clear which benchmark corresponds to which log macro, and update the ordering to be the same as the output - add clarifying documentation to the logging benchmarks - improve the `-debuglogfile` config option help to be clearer; can be tested by running `./src/bitcoind -help | grep -A4 '\-debuglogfile'` Reviewers can run the logging benchmarks with: ```bash ./src/bench/bench_bitcoin -filter='LogP*.*' ``` ACKs for top commit: LarryRuane: ACK 8c47d599b87d6b2d43e7d37ce0aaf4f541535bb9 martinus: code review & tested ACK 8c47d599b87d6b2d43e7d37ce0aaf4f541535bb9, here are my benchmark results: achow101: ACK 8c47d599b87d6b2d43e7d37ce0aaf4f541535bb9 Tree-SHA512: 705f8720c9ceaf14a1945039c7578a0c17a12215cbc44908099af4ac444561c3f95d833c5a91b325cdd4470737d8a01e2da64db2d542dd7c9a3747fbfdbf213e
2023-03-23Add pool based memory resource & allocatorMartin Leitner-Ankerl
A memory resource similar to std::pmr::unsynchronized_pool_resource, but optimized for node-based containers. Co-Authored-By: Pieter Wuille <pieter@wuille.net>
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-06 19:32:13 +0000