test: #29007 follow ups #29636

1. 
   0xB10C commented at 4:00 pm on March 12, 2024: contributor

   A few, small follow-ups to #29007. See commit messages for details.
2. 
   DrahtBot commented at 4:00 pm on March 12, 2024: contributor

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

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	stratospher, mzumsande, maflcko

   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:

   • #29641 (scripted-diff: Use LogInfo/LogDebug over LogPrintf/LogPrint by maflcko)

   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. DrahtBot added the label Tests on Mar 12, 2024
4. 
   0xB10C commented at 4:05 pm on March 12, 2024: contributor

   Since addpeeraddress can currently fail to add an address to the tried table but will report { success: true }, it makes sense to get that fixed with #28998 first before this PR, especially the first commit, is ready for review. The other two commits are minor and can wait.
5. 0xB10C force-pushed on Mar 13, 2024
6. fanquake requested review from stratospher on Mar 20, 2024
7. fanquake requested review from mzumsande on Mar 20, 2024
8. test: check that addrman seeding is successful

   The addpeeraddress calls can fail due to collisions. As we are using a
   deteministic addrman, they won't fail with the current bucket/position
   calculation. However, if the calculation is changed, they might collide
   and fail silently causing tests using `seed_addrman()` to fail.
   
   Assert that the addpeeraddress calls are successful.

   89b84ea91a
9. addrman: drop /*deterministic=*/ comment

   Just having deterministic is enough. See https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1488241966

   3047c3e3a9
10. init: clarify -test error

    See https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1469388717

    9a44a20fb7
11. in src/addrdb.cpp:208 in 777fb3d920 outdated

    203@@ -204,15 +204,15 @@ util::Result<std::unique_ptr<AddrMan>> LoadAddrman(const NetGroupManager& netgro
    204         LogPrintf("Loaded %i addresses from peers.dat  %dms\n", addrman->Size(), Ticks<std::chrono::milliseconds>(SteadyClock::now() - start));
    205     } catch (const DbNotFoundError&) {
    206         // Addrman can be in an inconsistent state after failure, reset it
    207-        addrman = std::make_unique<AddrMan>(netgroupman, /*deterministic=*/deterministic, /*consistency_check_ratio=*/check_addrman);
    208+        addrman = std::make_unique<AddrMan>(netgroupman, deterministic, /*consistency_check_ratio=*/check_addrman);
    209         LogPrintf("Creating peers.dat because the file was not found (%s)\n", fs::quoted(fs::PathToString(path_addr)));
    

    ---

    ---

    

    maflcko commented at 7:03 am on March 21, 2024:

    nit (feel free to ignore): If you want, you can also replace LogPrintf with LogInfo for clarity, while touching this file.

    ---

    

    0xB10C commented at 2:43 pm on March 23, 2024:

    I’m not doing this for now as I think this is slightly out-of-scope of this PR even if I touch the file.
12. 0xB10C force-pushed on Mar 23, 2024
13. 
    0xB10C commented at 2:44 pm on March 23, 2024: contributor

    With #28998 merged, this PR is now ready-for-review. I’ve also rebased it.
14. 0xB10C marked this as ready for review on Mar 23, 2024
15. 
    stratospher commented at 12:20 pm on March 24, 2024: contributor

    tested ACK 9a44a20.

    you can also include the check in test/functional/feature_asmap.py since addpeeraddress + deterministic addrman is present there too.

16. 
    mzumsande commented at 8:12 pm on March 24, 2024: contributor

    Code Review ACK 9a44a20fb790f3be5d5d5d8f5d0f48aac633b2a4
17. 
    maflcko commented at 7:02 am on March 25, 2024: member

    lgtm ACK 9a44a20fb790f3be5d5d5d8f5d0f48aac633b2a4
18. fanquake merged this on Mar 25, 2024
19. fanquake closed this on Mar 25, 2024

    ---

20. 0xB10C deleted the branch on Mar 25, 2024
21. bitcoin locked this on Mar 25, 2025

Contributors
0xB10C DrahtBot maflcko stratospher mzumsande

Labels
Tests