test, refactor: Embedded ASmap selected preparatory work #33026

1. 
   fjahr commented at 9:55 pm on July 20, 2025: contributor

   This contains four commits from #28792 that can be easily reviewed and merged independently. I hope splitting this change off can make this part move a bit faster and reduce frequency of needed rebases for #28792.

   The commits in order:

   • Add additional unit test vectors to the asmap interpreter (written by sipa). This helps to ensure that the further refactors in #28792 don’t change behavior.
   • Modernizes the logging in util/asmap.cpp, I added this while touching the rest of the file all over anyway.
   • Fixes the behavior of the GetPathArg function when an path arg is given “1” (for example -asmap=1), which would be interpreted as a path but should be interpreted as using the are without a parameter which activates the fallback. There is no explicit test added here because this is tested implicitly in the tests added in #28792.
   • Adds an AutoFile::size helper function
2. 
   DrahtBot commented at 9:55 pm on July 20, 2025: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33026.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	sipa, achow101
   Stale ACK	nervana21

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #31868 ([IBD] specialize block serialization by l0rinc)
   • #29641 (scripted-diff: Use LogInfo over LogPrintf [WIP, NOMERGE, DRAFT] by maflcko)
   • #28792 (Embed default ASMap as binary dump header file by fjahr)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

3. 
   nervana21 commented at 4:02 am on July 24, 2025: contributor

   tACK 942d95d

   I thoroughly reviewed and understood all code changes. I ran the tests locally and they all passed.

4. fanquake referenced this in commit 16f7b43b68 on Jul 24, 2025
5. in src/util/asmap.cpp:203 in 942d95d3c6 outdated

   199@@ -200,13 +200,11 @@ std::vector<bool> DecodeAsmap(fs::path path)
   200     FILE *filestr = fsbridge::fopen(path, "rb");
   201     AutoFile file{filestr};
   202     if (file.IsNull()) {
   203-        LogPrintf("Failed to open asmap file from disk\n");
   

   ---

   ---

   

   fanquake commented at 10:22 am on July 24, 2025:

   In 9481c77420845316e825855ac589aa8a10bff057: Should drop the redundant \n on all lines.

   ---

   

   fjahr commented at 1:44 pm on July 24, 2025:

   done
6. in src/common/args.cpp:276 in 942d95d3c6 outdated

   272@@ -273,7 +273,7 @@ fs::path ArgsManager::GetPathArg(std::string arg, const fs::path& default_value)
   273 {
   274     if (IsArgNegated(arg)) return fs::path{};
   275     std::string path_str = GetArg(arg, "");
   276-    if (path_str.empty()) return default_value;
   277+    if (path_str.empty() || path_str == "1") return default_value;
   

   ---

   ---

   

   fanquake commented at 10:25 am on July 24, 2025:

   In 74932c62f9627a2eee634b1dd16fe2582c258a74: Seems a bit awkaward to add an (undocumented) special case here for asmap? Can’t we just fail if the value given isn’t a valid path, or stop supporting this behaviour? cc @ryanofsky

   ---

   

   fjahr commented at 1:30 pm on July 24, 2025:

   I can drop this is other reviewers agree that it’s confusing. I personally think this is the more intuitive behavior for path args independently of asmap because our bool args all work with both -foo and -foo=1 afaik. If the user wants to use -asmap (or any path arg) as a bool then I think they would expect it to work the same as the other bool params. Fwiw, I discovered this because I wrote a functional test that covered -asmap=1 and I actually expected it work, only after it didn’t I added this.
7. fjahr force-pushed on Jul 24, 2025
8. DrahtBot added the label Needs rebase on Jul 25, 2025
9. tests: add unit test vectors for asmap interpreter 5a63a5a1fd
10. refactor: Modernize logging in util/asmap.cpp 70826bd4e7
11. common: Use fallback in GetPathArg if the path is 1

    Otherwise -asmap=1, for example, would be interpreted as a path "1".

    054d79934f
12. refactor: Add AutoFile::size 734737b593
13. fjahr force-pushed on Jul 25, 2025
14. DrahtBot removed the label Needs rebase on Jul 25, 2025
15. DrahtBot added the label CI failed on Jul 26, 2025
16. DrahtBot removed the label CI failed on Jul 26, 2025
17. 
    sipa commented at 3:30 pm on August 8, 2025: member

    utACK 734737b5930df7cebab83cf0dbe5fd390143f2be
18. 
    achow101 commented at 8:27 pm on August 27, 2025: member

    ACK 734737b5930df7cebab83cf0dbe5fd390143f2be

Contributors
fjahr DrahtBot nervana21 fanquake sipa achow101