Move-only: bloom to src/common #23137

pull MarcoFalke wants to merge 3 commits into bitcoin:master from MarcoFalke:2109-srcCommon changing 11 files +17 −17

MarcoFalke commented at 3:25 PM on September 29, 2021: member

To avoid having all files at the top level ./src directory, start moving them to their respective sub directory according to #15732.

bloom currently depends on libconsensus (CTransaction, CScript, ...) and it is currently located in the libcommon. Thus, move it to src/common/. (libutil in src/util/ is for stuff that doesn't depend on libconsensus).
MarcoFalke commented at 3:26 PM on September 29, 2021: member

See also #22951 (which has a minor conflict in one adjacent line).
DrahtBot added the label Build system on Sep 29, 2021
DrahtBot added the label P2P on Sep 29, 2021
DrahtBot added the label Refactoring on Sep 29, 2021
vincenzopalazzo approved
vincenzopalazzo commented at 7:51 PM on September 29, 2021: none

Concept ACK https://github.com/bitcoin/bitcoin/pull/23137/commits/fa762d0cb13a12cb8e569816d1b4516405942925
MarcoFalke removed the label P2P on Sep 30, 2021
practicalswift commented at 8:22 AM on September 30, 2021: contributor

Concept ACK
naumenkogs commented at 10:18 AM on September 30, 2021: member

Concept ACK
DrahtBot commented at 2:44 PM on September 30, 2021: contributor



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



Conflicts

No conflicts as of last run.
DrahtBot cross-referenced this on Sep 30, 2021 from issue consensus: move amount.h into consensus by fanquake
DrahtBot added the label Needs rebase on Oct 5, 2021
refactor: Remove unused MakeUCharSpan fac303c504

scripted-diff: Move bloom to src/common

-BEGIN VERIFY SCRIPT-
 # Move to directory
 mkdir                src/common
 git mv src/bloom.cpp src/common/
 git mv src/bloom.h   src/common/

 # Replace occurrences
 sed -i 's|\<bloom\.cpp\>|common/bloom.cpp|g' $(git grep -l 'bloom.cpp')
 sed -i 's|\<bloom\.h\>|common/bloom.h|g'     $(git grep -l 'bloom.h')
 sed -i 's|BITCOIN_BLOOM_H|BITCOIN_COMMON_BLOOM_H|g' $(git grep -l 'BLOOM_H')
-END VERIFY SCRIPT-

fa1e5de2db

style: Sort fa2d611bed
MarcoFalke force-pushed on Oct 5, 2021
DrahtBot removed the label Needs rebase on Oct 5, 2021
theStack approved
theStack commented at 10:07 AM on October 10, 2021: contributor

Code-review ACK fa2d611bedc2a755dcf84a82699c70b57b903cf6

TIL how to move files via scripted-diff (IIRC I tried once and failed, probably used "mv" rather than "git mv"...)
in src/common/bloom.cpp:85 in fa2d611bed
```
  81 | @@ -82,7 +82,7 @@ bool CBloomFilter::contains(const COutPoint& outpoint) const
  82 |  {
  83 |      CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
  84 |      stream << outpoint;
  85 | -    return contains(MakeUCharSpan(stream));
  86 | +    return contains(stream);
```
hebasto commented at 7:31 PM on October 10, 2021:

fac303c504ab19b863fddc7a0093068fee9d4ef3

Why MakeUCharSpan became unused? What has been changed since 2ba4ddf31d27bebc144b3729479967b40bbe0b6a from #23115?

MarcoFalke commented at 6:37 AM on October 11, 2021:

There was simply no reason to add it. cc @fanquake
hebasto commented at 7:31 PM on October 10, 2021: member

Concept ACK.
MarcoFalke requested review from ryanofsky on Oct 11, 2021
MarcoFalke requested review from fanquake on Oct 20, 2021
ryanofsky approved
ryanofsky commented at 12:45 PM on October 20, 2021: contributor

Code review ACK fa2d611bedc2a755dcf84a82699c70b57b903cf6
fanquake approved
fanquake commented at 2:25 AM on October 21, 2021: member

ACK fa2d611bedc2a755dcf84a82699c70b57b903cf6 - source shuffle starts now.
fanquake merged this on Oct 21, 2021
fanquake closed this on Oct 21, 2021
MarcoFalke deleted the branch on Oct 21, 2021
sidhujag referenced this in commit 5c4f8b30ab on Oct 21, 2021
hebasto cross-referenced this on Oct 27, 2021 from issue refactor: Use C++17 std::clamp by hebasto
bitcoin locked this on Oct 30, 2022