aboutsummaryrefslogtreecommitdiff
path: root/test/lint
AgeCommit message (Collapse)Author
2023-05-31Add public Boost headers explicitlyHennadii Stepanov
2023-05-29lint: stop ignoring LIEF importsfanquake
Type stubs are now available as of 0.13.0. See https://github.com/lief-project/LIEF/issues/650.
2023-05-26Merge bitcoin/bitcoin#27302: init: Error if ignored bitcoin.conf file is foundfanquake
eefe56967b4eb4b5144325cde4f40fc1cbde3e65 bugfix: Fix incorrect debug.log config file path (Ryan Ofsky) 3746f00be1b732a04976fc70cbb0661f97bbbd99 init: Error if ignored bitcoin.conf file is found (Ryan Ofsky) 398c3719b02197ad92fded20f6ff83b364747297 lint: Fix lint-format-strings false positives when format specifiers have argument positions (Ryan Ofsky) Pull request description: Show an error on startup if a bitcoin datadir that is being used contains a `bitcoin.conf` file that is ignored. There are two cases where this could happen: - One case reported in [#27246 (comment)](https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1470006043) happens when a `bitcoin.conf` file in the default datadir (e.g. `$HOME/.bitcoin/bitcoin.conf`) has a `datadir=/path` line that sets different datadir containing a second `bitcoin.conf` file. Currently the second `bitcoin.conf` file is ignored with no warning. - Another way this could happen is if a `-conf=` command line argument points to a configuration file with a `datadir=/path` line and that path contains a `bitcoin.conf` file, which is currently ignored. This change only adds an error message and doesn't change anything about way settings are applied. It also doesn't trigger errors if there are redundant `-datadir` or `-conf` settings pointing at the same configuration file, only if they are pointing at different files and one file is being ignored. ACKs for top commit: pinheadmz: re-ACK eefe56967b4eb4b5144325cde4f40fc1cbde3e65 willcl-ark: re-ACK eefe56967b TheCharlatan: ACK eefe56967b4eb4b5144325cde4f40fc1cbde3e65 Tree-SHA512: 939a98a4b271b5263d64a2df3054c56fcde94784edf6f010d78693a371c38aa03138ae9cebb026b6164bbd898d8fd0845a61a454fd996e328fd7bcf51c580c2b
2023-05-15add `lief` to `spelling.ignore-words`brunoerg
2023-04-21lint: Fix lint-format-strings false positives when format specifiers have ↵Ryan Ofsky
argument positions Do not error on valid format specifications like strprintf("arg2=%2$s arg1=%1$s arg2=%2$s", arg1, arg2); Needed to avoid lint error in upcoming commit: https://cirrus-ci.com/task/4755032734695424?logs=lint#L221 Additionally tested with python -m doctest test/lint/run-lint-format-strings.py
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-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-14Respect and update FILES_ARGS in test/lint/lint-python.pyPieter Wuille
2023-04-14Disable Python lint in src/secp256k1Pieter Wuille
2023-03-27fuzz: Remove legacy int parse fuzz testsMarcoFalke
2023-02-28Extend bilingual_str support for tinyformatRyan Ofsky
Previous bilingual_str tinyformat::format accepted bilingual format strings, but not bilingual arguments. Extend it to accept both. This is useful when embedding one translated string inside another translated string, for example: `strprintf(_("Error: %s"), message)` which would fail previously if `message` was a bilingual_str.
2023-02-22Merge bitcoin/bitcoin#25867: lint: enable E722 do not use bare exceptfanquake
61bb4e783b3acc62b121a228f6b14c2462e23315 lint: enable E722 do not use bare except (Leonardo Lazzaro) Pull request description: Improve test code and enable E722 lint check. If you want to catch all exceptions that signal program errors, use except Exception: (bare except is equivalent to except BaseException:). Reference: https://peps.python.org/pep-0008/#programming-recommendations ACKs for top commit: MarcoFalke: lgtm ACK 61bb4e783b3acc62b121a228f6b14c2462e23315 Tree-SHA512: c7497769d5745fa02c78a20f4a0e555d8d3996d64af6faf1ce28e22ac1d8be415b98e967294679007b7bda2a9fd04031a9d140b24201e00257ceadeb5c5d7665
2023-02-18lint: enable E722 do not use bare exceptLeonardo Lazzaro
2023-02-15script: remove out-of-date snprintf TODOJon Atack
that was resolved in PR27036 "test: Remove last uses of snprintf and simplify" and while here, fix up 2 words in docs to make the spelling linter green again.
2023-02-03Remove use of snprintf and simplifyJohn Moffett
One test case uses snprintf to convert an int to a string. Change it to use ToString (which uses a locale-independent version of std::to_string). Also remove unnecessary parts of StringContentsSerializer.
2023-01-19Merge bitcoin/bitcoin#26906: test: add an easy way to run linters locallyMarcoFalke
b68e5a7feff3e93027e75da0cd9a590fef99aac1 lint: specify the right commit range when running locally (James O'Beirne) dff7ed5732c01b3ba47ef9e9ddb687d073d7739d test: add an easy way to run linters locally (James O'Beirne) Pull request description: Adds a Dockerfile configuration ~~(originally written mostly by fanquake)~~ that allows straightforward running of linters with compatible versions locally. This removes a ton of annoyance when trying to appease CI, because many of the linter versions are quite old and difficult to maintain locally. I realize that people may not be thrilled to add more ancillary tooling to the repo, but I think this makes a lot of sense given the linter versions listed in this container configuration are dictated by this repo (within the CI configuration), so having these things live in two separate places is a recipe for version mismatches. Eventually we can likely just use this container on CI directly to avoid any chance of inconsistencies between local dev experience and CI. ACKs for top commit: aureleoules: ACK b68e5a7feff3e93027e75da0cd9a590fef99aac1 stickies-v: ACK b68e5a7fe john-moffett: ACK b68e5a7feff3e93027e75da0cd9a590fef99aac1 Tree-SHA512: 7ef7a5dae023d81fdb6296d5d92dfa074ee321c7993e607c9f014d0f21c91558611aa00fc3ce1edc7b5e68371aea0d27fa1931291a79bb867a6c783bb536775f
2023-01-18test: add an easy way to run linters locallyJames O'Beirne
Adds a Dockerfile configuration that allows straightforward running of linters with compatible versions locally. This removes a ton of annoyance when trying to appease CI, because many of the linter versions are quite old and difficult to maintain locally. I realize that people may not be thrilled to more ancillary tooling to the repo, but I think this makes a lot of sense given the linter versions listed in this container configuration are dictated by this repo (within the CI configuration), so having these things live in two separate places is a recipe for version mismatches. Eventually we can likely just use this container on CI directly to avoid any chance of inconsistencies between local dev experience and CI.
2023-01-18scripted-diff: Use new python 3.7 keywordsMarcoFalke
-BEGIN VERIFY SCRIPT- sed -i 's/universal_newlines/text/g' $(git grep -l universal_newlines) -END VERIFY SCRIPT-
2023-01-05Merge bitcoin/bitcoin#26598: contrib: remove builder keysMarcoFalke
e6864fa157d75d8ae2b2f56620b019bde2355a24 contrib: remove builder keys (fanquake) Pull request description: This has been superseded by adding a builder-keys/ directory in guix.sigs, where the presence of keys, and validity of signatures is checked. Preventing issues like missing keys or invalid signatures. New (or exisiting) Guix builders can add their key in the next PR they open adding attestations. Related to issues like #26566, #26563. Also follows up with the comment here: https://github.com/bitcoin/bitcoin/pull/26565#issuecomment-1326053939. ACKs for top commit: hebasto: ACK e6864fa157d75d8ae2b2f56620b019bde2355a24, modulo s/update/remove/ in the PR tittle. Tree-SHA512: 095b4cf12ed0baeaf0ee7b8edcb3e2647e9c0f812e8fd63915ddb454f81dacc9c2d2b409de2773b7adb5ff643893d614d8aad1bc44c26da648e1bbbe19e11e05
2023-01-03Use same Python executable for subprocesses as for all-lint.pyKristaps Kaupe
2022-12-24scripted-diff: Bump copyright headersHennadii Stepanov
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT- Commits of previous years: - 2021: f47dda2c58b5d8d623e0e7ff4e74bc352dfa83d7 - 2020: fa0074e2d82928016a43ca408717154a1c70a4db - 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
2022-12-20Merge bitcoin/bitcoin#26721: test, lint: add `crypted` to `ignore-words`fanquake
a4defcdd57918f3693902cc40a54d0e64d716d50 test, lint: add `crypted` to `ignore-words` (brunoerg) Pull request description: Fixes #26719 "Crypted" is used in some comments at `walletload_tests` because it refers to `DBKeys::CRYPTED_KEY`, it's not necessary a mistake. Obs: I can change the approach (changing `walletload_tests` comments to use `encrypted` word instead of adding it to the `ignore_words`) if reviewers think it makes more sense. ACKs for top commit: achow101: ACK a4defcdd57918f3693902cc40a54d0e64d716d50 Tree-SHA512: 49f38eed30ffb0fda4e792566591c3455629379619eb9a5c4240c5b00e14cd27ba1faa36337192233752e642f0998373b86fcb8ca586508bbf15900d68b17950
2022-12-19contrib: remove builder keysfanquake
This has been superseded by adding a builder-keys/ directory in guix.sigs, where the presence of keys, and validity of signatures is checked. Preventing issues like missing keys or invalid signatures. New (or exisiting) Guix builders can add their key in the next PR they open adding attestations.
2022-12-18test, lint: add `crypted` to `ignore-words`brunoerg
2022-12-17test: Improve `check-doc.py` patternHennadii Stepanov
2022-11-28Merge bitcoin/bitcoin#26588: ci: Skip COMMIT_RANGE if no CIRRUS_PRMarcoFalke
fad1c55301b9f2d091d3b0d8a75ff522ce8dae5a lint: Skip COMMIT_RANGE if no CIRRUS_PR (MarcoFalke) Pull request description: It doesn't make sense to run this for non-PRs, because: * There are known whitespace "violations" in previous commits, so the lint may fail * Once the changes are merged, it is too late to fix them up (force pushes are illegal) * It isn't possible to determine which commits to run on if there is no reference branch (target branch of the pull request) Moreover, the test fails on non-master: * https://github.com/bitcoin/bitcoin/runs/8664441400 Fix all issues by skipping it. ACKs for top commit: hebasto: ACK fad1c55301b9f2d091d3b0d8a75ff522ce8dae5a, also tested in my personal Cirrus account. Tree-SHA512: be15f00e2b2a9069583833545883e0e5968a33d2455dad59e6fb47c1102b4dd16ef932e9ba945e29e9d941e6c17bd531a02c66b0491097801be6bda476875537
2022-11-28lint: Skip COMMIT_RANGE if no CIRRUS_PRMarcoFalke
2022-11-16refactor: Move `CTxMemPoolEntry` class to its own moduleHennadii Stepanov
This change nukes the policy/fees->mempool circular dependency. Easy to review using `diff --color-moved=dimmed-zebra`.
2022-10-13Merge bitcoin/bitcoin#25667: assumeutxo: snapshot initializationAndrew Chow
bf9597606166323158bbf631137b82d41f39334f doc: add note about snapshot chainstate init (James O'Beirne) e4d799528696c5ede38c257afaffd367917e0de8 test: add testcases for snapshot initialization (James O'Beirne) cced4e7336d93a2dc88e4a61c49941887766bd72 test: move-only-ish: factor out LoadVerifyActivateChainstate() (James O'Beirne) 51fc9241c08a00f1f407f1534853a5cddbbc0a23 test: allow on-disk coins and block tree dbs in tests (James O'Beirne) 3c361391b8f5971eb3c7b620aa7ad9b437cc515e test: add reset_chainstate parameter for snapshot unittests (James O'Beirne) 00b357c215ed900145bd770525a341ba0ed9c027 validation: add ResetChainstates() (James O'Beirne) 3a29dfbfb2c16a50d854f6f81428a68aa9180509 move-only: test: make snapshot chainstate setup reusable (James O'Beirne) 8153bd9247dad3982d54488bcdb3960470315290 blockmanager: avoid undefined behavior during FlushBlockFile (James O'Beirne) ad67ff377c2b271cb4683da2fb25fd295557f731 validation: remove snapshot datadirs upon validation failure (James O'Beirne) 34d159033106cc595cfa852695610bfe419c989c add utilities for deleting on-disk leveldb data (James O'Beirne) 252abd1e8bc5cdf4368ad55e827a873240535b28 init: add utxo snapshot detection (James O'Beirne) f9f1735f139b6a1f1c7fea50717ff90dc4ba2bce validation: rename snapshot chainstate dir (James O'Beirne) d14bebf100aaaa25c7558eeed8b5c536da99885f db: add StoragePath to CDBWrapper/CCoinsViewDB (James O'Beirne) Pull request description: This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11) (parent PR: https://github.com/bitcoin/bitcoin/pull/15606) --- Half of the replacement for #24232. The original PR grew larger than expected throughout the review process. This change adds the ability to initialize a snapshot-based chainstate during init if one is detected on disk. This is of course unused as of now (aside from in unittests) given that we haven't yet enabled actually loading snapshots. Don't be scared! There are some big move-only commits in here. Accompanying changes include: - moving the snapshot coinsdb directory from being called `chainstate_[base blockhash]` to `chainstate_snapshot`, since we only support one snapshot in use at a time. This simplifies some logic, but it necessitates writing that base blockhash out to a file within the coinsdb dir. See [discussion here](https://github.com/bitcoin/bitcoin/pull/24232#discussion_r832762880). - adding a simple fix in `FlushBlockFile()` that avoids a crash when attemping to flush to disk before `LoadBlockIndexDB()` is called, which happens when calling `MaybeRebalanceCaches()` during multiple chainstate init. - improving the unittest to allow testing with on-disk chainstates - necessary to test a simulated restart and re-initialization. ACKs for top commit: naumenkogs: utACK bf9597606166323158bbf631137b82d41f39334f ariard: Code Review ACK bf9597606 ryanofsky: Code review ACK bf9597606166323158bbf631137b82d41f39334f. Changes since last review: rebasing, switching from CAutoFile to AutoFile, adding comments, switching from BOOST_CHECK to Assert in test util, using chainman.GetMutex() in tests, destroying one ChainstateManager before creating a new one in tests fjahr: utACK bf9597606166323158bbf631137b82d41f39334f aureleoules: ACK bf9597606166323158bbf631137b82d41f39334f Tree-SHA512: 15ae75caf19f8d12a12d2647c52897904d27b265a7af6b4ae7b858592eeadb8f9da6c2394b6baebec90adc28742c053e3eb506119577dae7c1e722ebb3b7bcc0
2022-09-15Fix issues identified by codespell 2.2.1 and update ignored wordsJon Atack
and also fix spelling in test/lint/lint-locale-dependence.py not caught by the spelling linter and fix up a paragraph we are touching here in test/README.md.
2022-09-13validation: rename snapshot chainstate dirJames O'Beirne
This changes the snapshot's leveldb chainstate dir name from `chainstate_[blockhash]` to `chainstate_snapshot`. This simplifies later logic that loads snapshot data, and enforces the limitation of a single snapshot at any given time. Since we still need to persis the blockhash of the base block, we write that out to a file (`chainstate_snapshot/base_blockhash`) for later use during initialization, so that we can reinitialize the snapshot chainstate. Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
2022-08-08refactor: Drop `boost/algorithm/string/replace.hpp` dependencyHennadii Stepanov
2022-07-18indexes, refactor: Pass Chain interface instead of CChainState class to indexesRyan Ofsky
Passing abstract Chain interface will let indexes run in separate processes. This commit does not change behavior in any way.
2022-07-15Move {Load,Dump}Mempool to kernel namespaceCarl Dong
Also: 1. Add the newly introduced kernel/mempool_persist.cpp to IWYU CI script 2. Add chrono mapping for iwyu
2022-06-15refactor: cleanups post unsubtree'ing univaluefanquake
Mostly changes to remove src/univalue exceptions from the various linters, and the required code changes to make them happy. As well as minor doc changes.
2022-06-14Merge bitcoin/bitcoin#25306: logging: add LogPrintfCategory to log ↵laanwj
unconditionally with category ecff20db286e2f5d3afe32cfaae72de69d34d23c logging: use LogPrintfCategory rather than a manual category (Jon Atack) eb8aab759fb15824a5dd3004e689d0eb5b884a32 logging: add LogPrintfCategory to log unconditionally with category (Jon Atack) Pull request description: These are the next two commits from #25203. - Add `LogPrintfCategory` to log unconditionally while prefixing the output with the passed category name. Add documentation and a unit test, and update the `lint-logs.py` and `lint-format-strings.py` scripts. - Replace the log messages that manually print a category, with `LogPrintfCategory`. In upcoming commits, it will likely be used in many other cases, such as to replace `LogPrintf` where it makes sense. ACKs for top commit: klementtan: Code Review ACK ecff20db286e2f5d3afe32cfaae72de69d34d23c laanwj: Code review ACK ecff20db286e2f5d3afe32cfaae72de69d34d23c brunoerg: ACK ecff20db286e2f5d3afe32cfaae72de69d34d23c Tree-SHA512: ad3a82835254f7606efcd14b88f3d9072f1eb9b25db1321ed38ef6a4ec60efd555d78f5e19d93736f2f8500251d06f8beee9d694a153f24bf5cce3590a2a45a5
2022-06-09Merge bitcoin/bitcoin#25307: doc: fix typo in kernel/context.h and add ↵fanquake
`desig` to ignore-words d575413fb8f8569803ed0050c15be003ef50951c doc: add `desig` to ignore-words (brunoerg) c06cc41ddbac3611d5c7826a2a3f2cbae9aeea12 doc: fix typo in kernel/context.h (brunoerg) Pull request description: This PR fixes a typo in `kernel/context.h` (libary => library) and add `desig` to ignore-words since it's a valid word, see: https://github.com/bitcoin/bitcoin/blob/b9416c3847cd347238a9d75d949327f69e187d79/src/net.cpp#L1105-L1117 ACKs for top commit: fanquake: ACK d575413fb8f8569803ed0050c15be003ef50951c Tree-SHA512: 2d548c737b8184d0243445c7503f3f68256ecb0970bd834d52de099de3cd8c8b9c140e2b77d55e2542fbd45b1d21cbdee639f5b2ef8138c37b8b72e5211029c3
2022-06-09doc: add `desig` to ignore-wordsbrunoerg
2022-06-08logging: add LogPrintfCategory to log unconditionally with categoryJon Atack
prefixing the output with the passed category name. - add documentation - add a unit test - update lint-logs.py - update lint-format-strings.py
2022-06-07Add LogPrintLevel to lint-format-strings, drop LogPrint-vs-LogPrintf section ↵Jon Atack
in dev notes that was added in 2015 by commit b8c06ef40 in PR 7003, as that potential issue would now be caught by the test/lint/lint-format-strings.py script run by the CI
2022-06-07test: clean up all-lint.pyMartin Leitner-Ankerl
Removed th check against __file__ which is not necessary any more after the rename to all-lint.py. Changed glob to find only `lint-*.py` scripts.
2022-06-07test: rename lint-all.py to all-lint.pyMartin Leitner-Ankerl
That way it is impossible for the script to call itself.
2022-05-31Merge bitcoin/bitcoin#25200: doc: Fix spelling errors identified by ↵MacroFake
codespell in comments f565b2836d5efeb6f7c16d0fac813b06fa4d41e4 Fixup option name in bench message (Ben Woosley) bf209ac7a732394c3a54d6d1e3fb43f180ac1bb8 doc: Fix spelling errors identified by codespell in coments (Ben Woosley) Pull request description: From the output [here](https://cirrus-ci.com/task/5275612980969472?logs=lint#L849): ``` src/qt/test/addressbooktests.cpp:185: wilcard ==> wildcard src/qt/test/addressbooktests.cpp:191: wilcard ==> wildcard src/test/miniscript_tests.cpp:227: nd ==> and, 2nd src/test/versionbits_tests.cpp:260: everytime ==> every time src/util/time.h:89: precicion ==> precision src/util/time.h:90: precicion ==> precision ^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/spelling.ignore-words.txt ``` ~~I left the 'nd' in miniscript_tests as-is, as it's valid miniscript, and I'm wary of whitelisting it.~~ ACKs for top commit: dunxen: ACK f565b28 Tree-SHA512: 501a426c5f6f9761e2c8f980d5d955611428a827321888f53e0ae9526b0fecd43f9d1fa845fc70ae2489d77be6dc0b5b371dff55c5146f4b39ed874f4a1ea917
2022-05-26scripts and tools: update lint-logs.py to detect LogPrintLevel()Jon Atack
and add WalletLogPrintf() (already detected) to the lint-logs.py suggestion Co-authored-by: laanwj <126646+laanwj@users.noreply.github.com>
2022-05-25doc: Fix spelling errors identified by codespell in comentsBen Woosley
From the output here: src/qt/test/addressbooktests.cpp:185: wilcard ==> wildcard src/qt/test/addressbooktests.cpp:191: wilcard ==> wildcard src/test/miniscript_tests.cpp:227: nd ==> and, 2nd src/test/versionbits_tests.cpp:260: everytime ==> every time src/util/time.h:89: precicion ==> precision src/util/time.h:90: precicion ==> precision ^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/spelling.ignore-words.txt https://cirrus-ci.com/task/5275612980969472?logs=lint#L849 I added 'nd' to the spelling.ignored-words.txt, as it's valid miniscript.
2022-05-24Merge bitcoin/bitcoin#24410: [kernel 2a/n] Split hashing/index ↵laanwj
`GetUTXOStats` codepaths, decouple from `coinstatsindex` 664a14ba7ccb40aa82d35a59831acd35db1897a6 coinstats: Move GetUTXOStats to rpc/blockchain (Carl Dong) f1006875665ffe8ff5da8185effe25b860743b4e kernel: Use ComputeUTXOStats in validation (Carl Dong) faa52387e8e4856445b1cfc9b5e072ce8f690f36 style-only: Rearrange using decls after scripted-diff (Carl Dong) f329a9298c06ffe74b9e9fbc07bfe6d282fef9cb scripted-diff: Move src/kernel/coinstats to kernel:: (Carl Dong) 0e54456f0498e52131f8ae0c76b4dfe25f45b076 Use only kernel/coinstats.h in index/coinstatsindex.h (Carl Dong) 80970985c965f79b8c376c8a922497e385445dd8 coinstats: Split node/coinstats.h to kernel/coinstats.h (Carl Dong) 35f73ce4b2efd7341fe55f77b334f27ad8aad090 coinstats: Move hasher codepath to kernel/coinstats (Carl Dong) b7634fe02b6b030f5d62502c73db84ba9a276640 Move logic from LookupUTXOStatsWithIndex to CoinStatsIndex::LookUpStats (Carl Dong) 1352e410a5b84070279ff28399083cb3ab278593 coinstats: Separate hasher/index lookup codepaths (Carl Dong) 524463daf6a10b20a4e20116a68101a684929eda coinstats: Return purely out-param CCoinsStats (Carl Dong) 46eb9fc56a296a2acea10ec7e5bf7b1827f73c45 coinstats: Extract index_requested in-member to in-param (Carl Dong) a789f3f2b878e1236f8e043a8bb1ffb1afc1b673 coinstats: Extract hash_type in-member to in-param (Carl Dong) 102294898d708b7adc0150aba8e500a4aa19bc1c includes: Remove rpc/util.h -> node/coinstats.h (Carl Dong) 0848db9c35d9eae4d68cbdbef68c337656f3c906 fuzz: Remove useless GetUTXOStats fuzz case (Carl Dong) 52b1939993771d0a8a718ca1667241872de8241a kernel: Remove unnecessary blockfilter{index,}.cpp (Carl Dong) Pull request description: Part of: #24303 Depends on: #24322 The `GetUTXOStats` function has 2 codepaths: - One which queries the `CoinStatsIndex` for the UTXO hash - One which actually performs the hashing For `libbitcoinkernel`, the only place where we call `GetUTXOStats` is in `PopulateAndValidateSnapshots`, which uses the `SHA256D` hash, and is therefore unable to use the `CoinStatsIndex` since that only provides `MuHash` hashes. Not that I think indices necessarily belong in `libbitcoinkernel` anyway. This PR separates these 2 aforementioned codepaths of `GetUTXOStats`, uses the hashing codepath in `PopulateAndValidateSnapshots`, and removes the need to link in `index/coinstatsindex.cpp` and `node/coinstats.cpp`. ----- Logistically, this PR: - Extracts out the `index_requested` and `hash_type` members of `CoinStats`, which served as "in-params" to `GetUTXOStats` embedded within the `CoinStats` struct. This allows `CoinStats` to only consist of "out-param" members, and be returned by `GetUTXOStats` without needing to be an "in-out" param - Introduce the purely virtual `UTXOHashers` class, with 3 implementations: `SHA256DHasher`, `MuHashHasher`, and `NullHasher`. These replace the existing template-based polymorphism. - Split `GetUTXOStats` into: - `CalculateUTXOStatsWithHasher(UTXOHasher&, ...)`, and - `LookupUTXOStatsWithIndex(CoinStatsIndex&, ...)` - Use `CalculateUTXOStatsWithHasher` directly where appropriate (`src/validation.cpp` and `src/fuzz`) - Move `GetUTXOStats` to `rpc/blockchain`, which is the only place that depends on `GetUTXOStats`'s weird fallback behaviour - Move `LookupUTXOStatsWithIndex` to `index/coinstatsindex` Code organization: - `src/` - `kernel/` → only contains the hashing codepath - `coinstats.cpp` → hashing codepath implementations - `coinstats.h` → header for `kernel/coinstats.cpp` - `index/` → only contains the index codepath - `coinstatsindex.cpp` → index codepath implementations - `coinstatsindex.h` - `validation.cpp` → only uses the hashing codepath - `rpc/blockchain.cpp` → uses both the hashing and index codepath, old `GetUTXOStats` fallback logic moved here as static - `test/fuzz/coins_view.cpp` → only uses the hashing codepath TODOs: - [x] Commit messages could be fleshed out more Would love any feedback! ACKs for top commit: laanwj: Code review ACK 664a14ba7ccb40aa82d35a59831acd35db1897a6 Tree-SHA512: 18722c7bd279174d2d1881fec33ea04a9b261aae1c12e998cf434ef297d8ded47de69c526c8033a2ba7abc93ba3d2ff5faf4ce05e8888c725c31cf885ce3ef73
2022-05-23kernel: Use ComputeUTXOStats in validationCarl Dong
This is the "fruit of our labor" for this patchset. ChainstateManager::PopulateAndValidateSnapshot can now directly call ComputeUTXOStats(...). Our consensus engine is now fully decoupled from all indices. See the src/Makefile.am for some satisfying removals.
2022-05-23Use only kernel/coinstats.h in index/coinstatsindex.hCarl Dong
Removes a circular dependency, horray!
2022-05-23Merge bitcoin/bitcoin#25015: test: Use permissions from git in lint-files.pyMacroFake
908fb7e2ec37fe68675d38dbfee4df9f861bb2b5 test: Use permissions from git in `lint-files.py` (laanwj) 48d2e80a7479a44b0ab09e87542c8cb7a8f72223 test: Don't use shell=True in `lint-files.py` (laanwj) Pull request description: Improvements to the `lint-files.py` script: - Avoid use of `shell=True`. - Check the permissions in git's metadata instead of in the filesystem. This stops the umask or filesystem from interfering. It's also more efficient as it only needs a single call to `git ls-files`. (what triggered this change was `File "..." contains a shebang line, but has the file permission 775 instead of the expected executable permission 755.` errors running the script locally). ACKs for top commit: vincenzopalazzo: re-tACK https://github.com/bitcoin/bitcoin/commit/908fb7e2ec37fe68675d38dbfee4df9f861bb2b5 Tree-SHA512: 2eaf868c55a9c3508b12658a5b3ac429963fd0551e645332d0ac54be56fefccee95115e4667386df24b46b545593cb0d0bf8c6cecab73f9cb19d37ddf704c614
2022-05-23test: Use permissions from git in `lint-files.py`laanwj
Instead of using permissions from the local file system, which might depend on the umask, directly check the permissions from git's metadata.