refactor: remove unused methods in classes `CDBIterator,CDBWrapper,CCoinsViewDBCursor` #25438

pull theStack wants to merge 2 commits into bitcoin:master from theStack:202206-txdb-refactor_remove_unused_getvaluesize_method changing 3 files +0 −28

theStack commented at 2:21 PM on June 21, 2022: member

The GetValueSize methods haven't been used since the chainstate db cache has been switched from per-tx to per-txout model years ago (PR #10195, commit d342424301013ec47dc146a4beb49d5c9319d80a). The CompactRange is unused since the txindex migration code was removed (PR #22626, commit https://github.com/bitcoin/bitcoin/commit/fa20f815a9cb438c5ab61e97a453612ddd8b21b5).

refactor: remove unused methods `{CDBIterator,CCoinsViewDBCursor}::GetValueSize()`

These methods haven't been used since the chainstate db cache has been
switched from per-tx to per-txout model years ago (PR #10195, commit
d342424301013ec47dc146a4beb49d5c9319d80a).

fb38c6e21f

DrahtBot added the label Refactoring on Jun 21, 2022
DrahtBot added the label UTXO Db and Indexes on Jun 21, 2022
w0xlt approved
w0xlt commented at 5:15 PM on June 21, 2022: contributor

Code Review ACK https://github.com/bitcoin/bitcoin/pull/25438/commits/fb38c6e21f064e23b63a46d15adb873029463cff
Riahiamirreza approved
laanwj commented at 6:02 PM on June 22, 2022: member

Code review ACK fb38c6e21f064e23b63a46d15adb873029463cff
furszy approved
furszy commented at 6:32 PM on June 22, 2022: member

ACK fb38c6e2
furszy commented at 6:40 PM on June 22, 2022: member

Btw, checking at this dbwrapper.h file, isn't CompactRange unused as well?
laanwj commented at 8:35 AM on June 23, 2022: member

Btw, checking at this dbwrapper.h file, isn't CompactRange unused as well?

Yes. It seems like it! A direct call to leveldb's CompactRange is used in CDBWrapper::CDBWrapper in -forcecompactdb handling. But our own function is never used. The last use went away in #22626.

refactor: remove unused method `CDBWrapper::CompactRange`

This method hasn't been used since the txindex migration code has been
removed (PR #22626, commit fa20f815a9cb438c5ab61e97a453612ddd8b21b5).

Co-authored-by: furszy <matiasfurszyfer@protonmail.com>

e4b4db5610

theStack renamed this:
~~refactor: remove unused methods `{CDBIterator,CCoinsViewDBCursor}::GetValueSize()`~~
refactor: remove unused methods in classes `CDBIterator,CCoinsViewDBCursor`
on Jun 23, 2022
theStack commented at 12:09 PM on June 23, 2022: member

@furszy @laanwj: Thanks a lot, good catch! Added another commit and updated the PR title / description accordingly.
fanquake approved
fanquake commented at 12:10 PM on June 23, 2022: member

ACK e4b4db561049c97c956e5b856713dcf63ac3e2f0
theStack renamed this:
~~refactor: remove unused methods in classes `CDBIterator,CCoinsViewDBCursor`~~
refactor: remove unused methods in classes `CDBIterator,CDBWrapper,CCoinsViewDBCursor`
on Jun 23, 2022
furszy approved
furszy commented at 12:31 PM on June 23, 2022: member

re-ACK e4b4db56
MarcoFalke commented at 12:52 PM on June 23, 2022: member

Clear may be unused as well? https://marcofalke.github.io/btc_cov/total.coverage/src/dbwrapper.h.gcov.html#66
theStack commented at 1:12 PM on June 23, 2022: member
Clear may be unused as well? https://marcofalke.github.io/btc_cov/total.coverage/src/dbwrapper.h.gcov.html#66

Seems like it is used in CCoinsViewDB::BatchWrite (https://github.com/bitcoin/bitcoin/blob/01e9e2d1ca7fc08d65663b398c71ecec6a01f686/src/txdb.cpp#L154):
```
txdb.cpp:154:19: error: no member named 'Clear' in 'CDBBatch'
            batch.Clear();
            ~~~~~ ^
1 error generated.
gmake[2]: *** [Makefile:10066: libbitcoin_node_a-txdb.o] Error 1
```
DrahtBot commented at 3:02 PM on June 23, 2022: 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:
- #25296 (Add DataStream without ser-type and ser-version and use it where possible by MarcoFalke)
- #24232 (assumeutxo: add init and completion logic by jamesob)
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.
laanwj approved
laanwj commented at 5:51 PM on June 23, 2022: member

Code review ACK e4b4db561049c97c956e5b856713dcf63ac3e2f0
MarcoFalke merged this on Jun 24, 2022
MarcoFalke closed this on Jun 24, 2022
theStack deleted the branch on Jun 24, 2022
sidhujag referenced this in commit e1fc0f67a5 on Jun 27, 2022
DrahtBot locked this on Jun 24, 2023

Contributors

Labels

Refactoring UTXO Db and Indexes

Linked (view graph)

#10195 Switch chainstate db and cache to per-txout model #22626 Remove txindex migration code

refactor: remove unused methods in classes `CDBIterator,CDBWrapper,CCoinsViewDBCursor` #25438

Conflicts