Remove unused C++ code not covered by unit tests

practicalswift commented at 10:21 am on March 25, 2017: contributor

in src/rpc/protocol.cpp:118 in 2e2307ba7e outdated

113@@ -114,13 +114,3 @@ bool GetAuthCookie(std::string *cookie_out)
114         *cookie_out = cookie;
115     return true;
116 }
117-
118-void DeleteAuthCookie()

laanwj commented at 10:24 am on March 25, 2017:

Huh, it’s a mistake that this is unused; the auth cookie is supposed to be removed on shutdown.

Apparently I mistakenly removed the call from StopRPC in #5677/40b556d3742a1f65d67e2d4c760d0b13fe8be5b7. Good catch :)

in src/wallet/wallet.cpp:3580 in 2e2307ba7e outdated

3494@@ -3495,22 +3495,6 @@ bool CWallet::LoadDestData(const CTxDestination &dest, const std::string &key, c
3495     return true;
3496 }
3497 
3498-bool CWallet::GetDestData(const CTxDestination &dest, const std::string &key, std::string *value) const

laanwj commented at 10:26 am on March 25, 2017:

This is part of the *DestData API, please don’t remove it just because it’s not used at the moment. Same reasoning as here: #9987 (review)

practicalswift commented at 10:36 am on March 25, 2017:

@laanwj Thanks for reviewing. I see - so basically a test is needed here :-)

in src/net.cpp:2624 in 2e2307ba7e outdated

2597@@ -2616,7 +2598,6 @@ int CConnman::GetBestHeight() const
2598 }
2599 
2600 unsigned int CConnman::GetReceiveFloodSize() const { return nReceiveFloodSize; }
2601-unsigned int CConnman::GetSendBufferSize() const{ return nSendBufferMaxSize; }

laanwj commented at 10:27 am on March 25, 2017:

CConnman is recent code and part of an ongoing effort by @theuni, please discuss with him before removing things.

practicalswift commented at 10:36 am on March 25, 2017:

@laanwj I see. @theuni, would you mind adding a test for this one? :-)

theuni commented at 7:17 pm on March 28, 2017:

@practicalswift Yes, thanks, that’s a good idea.

practicalswift commented at 10:29 am on March 25, 2017: contributor

Trivia: This PR combined with #9987 removes 0.134 % (173 of 128 675 lines of code) of the Bitcoin C++ codebase :-)

0$ git grep "" | egrep '\.(cpp|h):' | wc -l
1128675

in src/chainparamsbase.cpp:107 in 2e2307ba7e outdated

102@@ -103,8 +103,3 @@ std::string ChainNameFromCommandLine()
103         return CBaseChainParams::TESTNET;
104     return CBaseChainParams::MAIN;
105 }
106-
107-bool AreBaseParamsConfigured()

laanwj commented at 10:29 am on March 25, 2017:

ACK on this one, apparently the last use of this went away in #6149/27d760580456d206c5a02ef29797f296f510099c.

in src/key.cpp:134 in 2e2307ba7e outdated

130@@ -131,14 +131,6 @@ void CKey::MakeNewKey(bool fCompressedIn) {
131     fCompressed = fCompressedIn;
132 }
133 
134-bool CKey::SetPrivKey(const CPrivKey &privkey, bool fCompressedIn) {

laanwj commented at 10:33 am on March 25, 2017:

ACK on this one. This seems to have been unused since #5227/36fa4a78acac0ae6bb0e95c6ef78630120a28bdd.

in src/netaddress.cpp:182 in 2e2307ba7e outdated

178@@ -179,12 +179,6 @@ bool CNetAddr::IsLocal() const
179    return false;
180 }
181 
182-bool CNetAddr::IsMulticast() const

laanwj commented at 10:35 am on March 25, 2017:

ACK on this one. Cannot find when this was ever used, and I don’t think we need this now or on the forseeable future.

fanquake commented at 10:56 am on April 2, 2017:

Looks like it was introduced in #735 (5 years ago) and yes possibly never used.

in src/netaddress.cpp:573 in 2e2307ba7e outdated

563@@ -570,11 +564,6 @@ std::string CService::ToString() const
564     return ToStringIPPort();
565 }
566 
567-void CService::SetPort(unsigned short portIn)

laanwj commented at 10:36 am on March 25, 2017:

Setting ports seems to be reasonably useful part of the API, unless all of CService is made immutable I’d suggest keeping this.

in src/wallet/db.cpp:469 in 2e2307ba7e outdated

462@@ -463,15 +463,6 @@ void CDBEnv::CloseDb(const std::string& strFile)
463     }
464 }
465 
466-bool CDBEnv::RemoveDb(const std::string& strFile)

laanwj commented at 10:43 am on March 25, 2017:

Not 100% sure about this one. We don’t use it, and haven’t used it for a long time. From what I see it was introduced in eed1785f701be93ac2464e854c2a7de1f748ef84 (pull# unknown) and never used. Would deleting wallets be something we’d ever support? (e.g. as part of multiwallet). It is an extremely dangerous thing to do, on the other hand for watch-only wallets it’d be useful.

practicalswift commented at 10:46 am on March 25, 2017:

@laanwj I’d vote for removing it until someone finds a need for it. Reasoning: such a dangerous operation should be tested, and also YAGNI :-)

fanquake added the label Refactoring on Mar 25, 2017

practicalswift force-pushed on Mar 28, 2017

practicalswift commented at 7:46 am on March 28, 2017: contributor

@laanwj I’ve now addressed your review items. Please let me know if any further adjustments are needed :-)

in src/rpc/server.cpp:325 in 1f8e7dcb9c outdated

321@@ -322,6 +322,7 @@ void StopRPC()
322 {
323     LogPrint("rpc", "Stopping RPC\n");
324     deadlineTimers.clear();
325+    DeleteAuthCookie();

fanquake commented at 10:51 am on April 2, 2017:

I think this could/should be broken out into it’s own PR. It’s wasn’t obvious right away why this was being added.

laanwj commented at 1:37 pm on April 2, 2017:

At least it’s a separate commit in this PR. But splitting it to a separate PR would make sense, yes.

practicalswift force-pushed on Apr 2, 2017

practicalswift commented at 1:55 pm on April 2, 2017: contributor

@fanquake @laanwj Good points! Re-introduction of auth cookie removal on shutdown has been moved to #10139 :-)

practicalswift commented at 9:32 pm on April 11, 2017: contributor

Let me know if there are any suggestions on further tweaks needed for this PR :-)

fanquake commented at 10:43 am on April 26, 2017: member

This needs a rebase.

Remove unused C++ code not covered by unit tests b51aaf1c42

practicalswift force-pushed on Apr 26, 2017

practicalswift commented at 7:23 pm on April 26, 2017: contributor

@fanquake Thanks for the notification. Now rebased!

Let me know if anything needs to be changed in this PR.

paveljanik commented at 7:46 pm on April 26, 2017: contributor

ACK https://github.com/bitcoin/bitcoin/pull/10075/commits/b51aaf1c42d9936ddbe8ec48a0d9b675221ab7ba

laanwj merged this on Apr 27, 2017

laanwj closed this on Apr 27, 2017

laanwj referenced this in commit 4c924011f5 on Apr 27, 2017

PastaPastaPasta referenced this in commit a791a177bc on Jun 10, 2019

PastaPastaPasta referenced this in commit b486af36bb on Jun 10, 2019

PastaPastaPasta referenced this in commit f88d81f096 on Jun 11, 2019

PastaPastaPasta referenced this in commit 852333ec4b on Jun 11, 2019

PastaPastaPasta referenced this in commit dff1ca4a6d on Jun 12, 2019

PastaPastaPasta referenced this in commit b00dc6b78d on Jun 14, 2019

PastaPastaPasta referenced this in commit 9ca0944eea on Jun 14, 2019

PastaPastaPasta referenced this in commit ed68d2ca12 on Jun 14, 2019

PastaPastaPasta referenced this in commit f805c80a81 on Jun 15, 2019

PastaPastaPasta referenced this in commit 2d44cd85c8 on Jun 19, 2019

barrystyle referenced this in commit 4916db3b59 on Jan 22, 2020

practicalswift deleted the branch on Apr 10, 2021

random-zebra referenced this in commit 2d50b6e3b9 on Jun 9, 2021

furszy referenced this in commit 1e2b5336fe on Sep 25, 2021

DrahtBot locked this on Aug 16, 2022

Remove unused C++ code not covered by unit tests #10075