Pass mempool pointer to UnloadBlockIndex/GetCoinsCacheSizeState #19604

MarcoFalke commented at 5:52 AM on July 28, 2020: member

Split out from #19556

Instead of relying on the implicit mempool global, pass a mempool pointer (which can be 0). This helps with testing, code clarity and unlocks the features described in #19556.

MarcoFalke added the label Refactoring on Jul 28, 2020

MarcoFalke added the label Validation on Jul 28, 2020

MarcoFalke force-pushed on Jul 28, 2020

jamesob commented at 2:33 PM on July 28, 2020: member

ACK https://github.com/bitcoin/bitcoin/pull/19604/commits/faabb459fe168d02a5e587000ebfad46b06bf7cc

Will test locally soon.

DrahtBot commented at 5:07 PM on July 28, 2020: member

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#19306 (refactor: Replace RecursiveMutex with Mutex in CTxMemPool by hebasto)
#19271 (validation: Warm coins cache during prevalidation to connect blocks faster by andrewtoth)
#18766 (Disable fee estimation in blocksonly mode by darosior)
#18731 (refactor: Make CCheckQueue RAII-styled by hebasto)
#18710 (Add local thread pool to CCheckQueue by hebasto)

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.

darosior approved

darosior commented at 6:24 PM on July 28, 2020: member

ACK faabb459fe168d02a5e587000ebfad46b06bf7cc

test: Simplify cs_main locks faec851b6e

DrahtBot added the label Needs rebase on Jul 29, 2020

MarcoFalke force-pushed on Jul 29, 2020

MarcoFalke commented at 6:14 AM on July 29, 2020: member

Rebased due to changes in adjacent line, also added unrelated test cleanup commit

DrahtBot removed the label Needs rebase on Jul 29, 2020

in src/qt/test/apptests.cpp:87 in fad551eb7e outdated

  82 | @@ -83,7 +83,8 @@ void AppTests::appTests()
  83 |      // Reset global state to avoid interfering with later tests.
  84 |      LogInstance().DisconnectTestLogger();
  85 |      AbortShutdown();
  86 | -    UnloadBlockIndex();
  87 | +    CTxMemPool dummy;
  88 | +    UnloadBlockIndex(&dummy);

jnewbery commented at 10:18 AM on July 29, 2020:

Can you pass nullptr here?

MarcoFalke commented at 10:31 AM on July 29, 2020:

Thx. Fixed

jnewbery commented at 10:20 AM on July 29, 2020: member

Code review ACK fad551eb7e712e99fb8338eb89ae963b1d12ba3e. One suggested simplification inline.

Pass mempool pointer to UnloadBlockIndex fac674db20

Pass mempool pointer to GetCoinsCacheSizeState fae8c28dae

MarcoFalke force-pushed on Jul 29, 2020

jnewbery commented at 11:44 AM on July 29, 2020: member

code review ACK fae8c28dae747f9c4c6481049742346d18202fc8

darosior approved

darosior commented at 11:45 AM on July 29, 2020: member

Tested ACK fae8c28dae747f9c4c6481049742346d18202fc8

jamesob approved

jamesob commented at 1:40 PM on July 29, 2020: member

ACK fae8c28dae747f9c4c6481049742346d18202fc8 (jamesob/ackr/19604.1.MarcoFalke.pass_mempool_pointer_to)

Built locally and ran unittests.

<details><summary>Show platform data</summary> <p>

Tested on Linux-4.19.0-9-amd64-x86_64-with-glibc2.28

Configured with ./configure LDFLAGS=-L/home/james/src/bitcoin/db4/lib/ CPPFLAGS=-I/home/james/src/bitcoin/db4/include/ CXXFLAGS=-std=c++11 -Wthread-safety-analysis -Werror --enable-wallet --enable-debug --with-daemon CXX=/usr/bin/clang++ CC=/usr/bin/clang --disable-shared --with-pic --enable-benchmark=no --with-bignum=no --enable-module-recovery --disable-jni --no-create --no-recursion

Compiled with /usr/bin/ccache /usr/bin/clang++ -std=c++11 -mavx -mavx2 -std=c++11 -Wthread-safety-analysis -Werror -O0 -g3 -ftrapv  -Wstack-protector -fstack-protector-all -msse4 -msha -msse4.1 -msse4.2  i

Compiler version: clang version 7.0.1-8 (tags/RELEASE_701/final)

</p></details>

<details><summary>Show signature data</summary> <p>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK fae8c28dae747f9c4c6481049742346d18202fc8 ([`jamesob/ackr/19604.1.MarcoFalke.pass_mempool_pointer_to`](https://github.com/jamesob/bitcoin/tree/ackr/19604.1.MarcoFalke.pass_mempool_pointer_to))

Built locally and ran unittests.

<details><summary>Show platform data</summary>
<p>

Tested on Linux-4.19.0-9-amd64-x86_64-with-glibc2.28

Configured with ./configure LDFLAGS=-L/home/james/src/bitcoin/db4/lib/ CPPFLAGS=-I/home/james/src/bitcoin/db4/include/ CXXFLAGS=-std=c++11 -Wthread-safety-analysis -Werror --enable-wallet --enable-debug --with-daemon CXX=/usr/bin/clang++ CC=/usr/bin/clang --disable-shared --with-pic --enable-benchmark=no --with-bignum=no --enable-module-recovery --disable-jni --no-create --no-recursion

Compiled with /usr/bin/ccache /usr/bin/clang++ -std=c++11 -mavx -mavx2 -std=c++11 -Wthread-safety-analysis -Werror -O0 -g3 -ftrapv -Wstack-protector -fstack-protector-all -msse4 -msha -msse4.1 -msse4.2 i

Compiler version: clang version 7.0.1-8 (tags/RELEASE_701/final)


</p></details>
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEpm2+LMqPsWj2rAwWyrywS1A+5mgFAl8hfCAACgkQyrywS1A+
5mgfqg//fT5WYUt3TfjmMu+QR1mi5y8XAi+H0YQ9Jxmt3PZI7NdEwe/WNENO+s1J
nBw7Z+NI5F+u0pEB2s517aUQBVUjL0W9yyRQ3Y2m2fsObjMbH2dPR2RWn72Hheub
dis1vfJpbp3Jzz6dWQzGd5E19HNN+3oJCgZ3HQbjm9+yPuKfajQuQ4KXhly47eHZ
DDANRUHuTJITYxb8wge++xNiFaG0hTdhbBX01A5cCg20XUDx6DiCzEt61thBlYGF
1Zp5ddRCDbER1pLn7K1U0Lg1b5tVnOVu6kO0G9cMXNYnHlVOKlBupVOvKFZZKIQO
Pl8lXMc0cLUcCBWsSzkw2xLVSiRVPwaQ9bFo/zlPZUNRwxdx6kUrTPXpYS8sZmgu
qdykwMcqmyGrS9SDr5IwiZC37gfuRPNnF24cCatUQqfFWsvnrnGwkkjmINS1SPM9
slWMc7gZFQ2u+T46NsnhpmsJ8AmjZ6muaYyn+o5SQIZnWxRKNmGH8I73qKPoo1K8
B4Mu31BeFsS5s9sXjBt9DmRIceWH1YXblOJp2j4lJ55fYxAsMm5/f22k41fGlDa1
KtJ31YbXCqc1Hh4SrNlAucsmHONI7xGVs5ABrouveInj2PMascWRSZKKdk8MlD7l
YN+zp5MykAp8/eot5qdj8W9qv7+dGuh7N9V3nyNtdUZWBeyZTDk=
=zZ7M
-----END PGP SIGNATURE-----

</p></details>

fjahr commented at 1:48 PM on July 30, 2020: member

Code review ACK fae8c28dae747f9c4c6481049742346d18202fc8

MarcoFalke merged this on Jul 30, 2020

MarcoFalke closed this on Jul 30, 2020

Fabcien referenced this in commit 881afbd7aa on Jul 8, 2021

Fabcien referenced this in commit eb27969520 on Jul 8, 2021

Fabcien referenced this in commit af4f4249e6 on Jul 8, 2021

DrahtBot locked this on Feb 15, 2022

MarcoFalke deleted the branch on Aug 14, 2022