Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove unused code #4183

Merged
merged 6 commits into from May 25, 2014
Merged

remove unused code #4183

merged 6 commits into from May 25, 2014

Conversation

kdomanski
Copy link
Contributor

The title is self-explanatory,
Obviously, you might want to keep some of those, so I'm waiting for feedback.

@laanwj
Copy link
Member

laanwj commented May 14, 2014

Did some digging using git's pickaxe...

ACK on removing:

  • CWallet::AddReserveKey : Was never used at all, added by @sipa back in 2011 in 30ab2c9
  • CTransaction::IsNewerThan: Was used by the transaction replacement code which was removed in 98c7c8f
  • LogException: Hasn't been used for as long as Satoshi was still here
  • LookupHostNumeric: Is simply a wrapper for LookupHost(_1, _2, _3, false), no need for this we can pass a boolean if we need this functionality.
  • CBlockIndex::GetMedianTime: was used in CWalletTx::GetTxTime until 471426f, in 2011

NACK on removing:

  • CWallet::GetDestData: part of the *DestData API, which is planned to be used more extensively by the GUI
  • CNetAddr::IsMulticast: part of basic 'what is this address' functionality - though it's doubtful we will use multicast, we might want to detect these addresses for error robustness or such.
  • CDBEnv::RemoveDb: doubtful that we'll ever allow removing wallets - but for API symmetry it makes sense to keep this function
  • CService::SetPort: API symmetry, run-time configurability
  • UnregisterWallet: it's probably a bug that this isn't used - and will certainly be needed if someone ever implements multi-wallet support

Not sure:

  • CPubKey::VerifyCompact: counterpart to CKey::SignCompact which is used, but seems that we use RecoverCompact instead in verifymessage()

@kdomanski
Copy link
Contributor Author

Dropped the removal of functions under "NACK on removing" and "Not sure". Rebased against master.

@BitcoinPullTester
Copy link

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/f40dbeedde3a6a73c13b0241258cf23248db1de1 for binaries and test log.
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

@laanwj laanwj merged commit f40dbee into bitcoin:master May 25, 2014
laanwj added a commit that referenced this pull request May 25, 2014
f40dbee remove CPubKey::VerifyCompact( ) which is never used (Kamil Domanski)
28b6c1d remove GetMedianTime( ) which is never used (Kamil Domanski)
5bd4adc remove LookupHostNumeric( ) which is never used (Kamil Domanski)
595f691 remove LogException( ) which is never used (Kamil Domanski)
f4057cb remove CTransaction::IsNewerThan which is never used (Kamil Domanski)
0e31e56 remove CWallet::AddReserveKey which is never used (Kamil Domanski)
@kdomanski kdomanski deleted the 2014_05_dead_code branch May 25, 2014 14:22
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants