refactor: Use HashWriter over legacy CHashWriter #28341

pull maflcko wants to merge 2 commits into bitcoin:master from maflcko:2308-remove-code- changing 4 files +15 −15
  1. maflcko commented at 3:14 pm on August 25, 2023: member

    HashWriter is a slim and less confusing version of CHashWriter, so use it in all places where it compiles.

    This should be correct, if it compiles.

  2. refactor: Use HashWriter over legacy CHashWriter 5555aa2d0d
  3. refactor: Use HashWriter over legacy CHashWriter (via SerializeHash) 99995cfe8d
  4. DrahtBot commented at 3:14 pm on August 25, 2023: contributor

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

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK theuni, TheCharlatan, sipa

    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:

    • #22563 (addrman: treat Tor/I2P/CJDNS as a single group by vasild)

    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.

  5. DrahtBot renamed this:
    refactor: Use HashWriter over legacy CHashWriter
    refactor: Use HashWriter over legacy CHashWriter
    on Aug 25, 2023
  6. DrahtBot added the label Refactoring on Aug 25, 2023
  7. theuni commented at 4:50 pm on August 25, 2023: member

    Concept ACK. Changes look good.

    ~Why not get test/addrman_tests.cpp at the same time?~

  8. theuni approved
  9. theuni commented at 6:49 pm on August 25, 2023: member

    I don’t know if I just missed it when I first looked or if you did some ninja force push, but addrman_tests.cpp is updated here so ignore my previous comment.

    ACK 99995cfe8da6ea2b93a6cd0e0bc84bb34cbb9d8c

  10. maflcko requested review from TheCharlatan on Aug 28, 2023
  11. TheCharlatan approved
  12. TheCharlatan commented at 1:29 pm on August 28, 2023: contributor
    ACK 99995cfe8da6ea2b93a6cd0e0bc84bb34cbb9d8c
  13. sipa commented at 3:03 pm on August 28, 2023: member

    I’m not sure this improves much, unless we can actually get rid of CHashWriter entirely?

    That said, code review ACK 99995cfe8da6ea2b93a6cd0e0bc84bb34cbb9d8c

  14. maflcko commented at 3:09 pm on August 28, 2023: member

    I’m not sure this improves much, unless we can actually get rid of CHashWriter entirely?

    It is still used in CHashVerifier, so it can not be removed right now. But CHashVerifier is removed in #25284, so CHashWriter can be removed after that as well.

  15. fanquake merged this on Aug 28, 2023
  16. fanquake closed this on Aug 28, 2023

  17. maflcko deleted the branch on Aug 28, 2023
  18. Frank-GER referenced this in commit 44354d0438 on Sep 8, 2023
  19. Fabcien referenced this in commit 78cbb32b9c on Jul 16, 2024
  20. bitcoin locked this on Aug 27, 2024

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-17 15:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me