Clean up FC

[pmconrad]

User Story As a developer I want to have the FC library cleaned up in order to get rid of broken/obsolete/unused code and reduce maintenance effort.

Much of the code in FC predates C++-11 and modern Boost. Both C++-11 and Boost have standardized or at least well-proven replacements for much of FC. We should get rid of all code in FC that is either unused by the core, or for which standard and/or Boost replacements exist.

Impacts Describe which portion(s) of BitShares Core may be impacted by your request. Please tick at least one box.

• [ ] API (the application programming interface)
• [ ] Build (the build process or something prior to compiled code)
• [ ] CLI (the command line wallet)
• [ ] Deployment (the deployment process after building such as Docker, Travis, etc.)
• [ ] DEX (the Decentralized EXchange, market engine, etc.)
• [ ] P2P (the peer-to-peer network for transaction/block propagation)
• [x] Performance (system or user efficiency, etc.)
• [ ] Protocol (the blockchain logic, consensus, validation, etc.)
• [x] Security (the security of system or user data, etc.)
• [ ] UX (the User Experience)
• [x] Other (please add below) Code quality

CORE TEAM TASK LIST

• [ ] Evaluate / Prioritize Feature Request
• [ ] Refine User Stories / Requirements
• [ ] Define Test Cases
• [ ] Design / Develop Solution
• [ ] Perform QA/Testing
• [ ] Update Documentation

[nathanielhourt]

I've been wanting to knock out some warnings coming from FC as well; is it alright if I just tag those commits onto this issue?

[jmjatlanta]

CMakeModules/FindBoost.cmake does not seem to work on my Windows machine (very recent cmake and boost versions). If I remove it (thereby using the default one that comes with cmake), everything works.

Building bitshares-core works fine even with the existence of libraries/fc/CMakeModules/FindBoost.cmake, but it may be that cmake uses its own instead of the one buried in our repository.

My guess is that if FindBoost.cmake was removed from our fc repository, older versions of cmake or boost may break. Hence, I do not want to remove it without knowing why it is there.

[pmconrad]

> git log -- CMakeModules/FindBoost.cmake
commit 1a461c08a577fc7ff5f0f744ad958bddee2c51f4
Author: vogel76 <[email protected]>
Date:   Mon Feb 17 12:03:57 2014 +0100

    [BW]: [Fix] Added local version of FindBoost.cmake to avoid building problems on Ubuntu which doesn't use patched version of cmake in the distro cmake package. Here is point to cmake bugreport:
    http://public.kitware.com/Bug/view.php?id=14720

Response in http://public.kitware.com/Bug/view.php?id=14720 :

Please try with CMake 2.8.11 or higher, preferably the latest release.

Confirmed to work by followup posters.

I guess it's ok to remove if we put CMake 2.8.11 or higher into build requirements.

[pmconrad]

Question: do we want to retain the ability to switch EC crypto between slow openssl and the fast secp256k1, or can EC openssl support be removed?

[nathanielhourt]

Question: do we want to retain the ability to switch EC crypto between slow openssl and the fast secp256k1, or can EC openssl support be removed?

If we remove it, do we still need openssl at all? Or can it be dropped as a dependency?

[pmconrad]

OpenSSL will still be required. (Even the secp256k1-based uses it in some places, due to lack of certain primitives in that library.) OpenSSL is also used for symmetric encryption and hashing for example.

[nathanielhourt]

Right, OK. So does it buy us anything to drop support for using openssl for EC signatures? If not, I suppose it makes sense to keep it.

[pmconrad]

It's a simplification. If the code isn't used there's no need to keep (and maintain) it.

[nathanielhourt]

Fair point. Go ahead and remove it, then :)

Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away. [1]

[pmconrad]

Future ideas:

• replace fc::optional with boost::optional or std::optional (C++17)
• replace fc::thread with boost::fiber - this would be a major undertaking and should be combined with attempts to allow parallel read access

[abitmore]

Please remove fc::uint160_t, because it's not really a traditional integer, for example, it doesn't have an operator+().

[pmconrad]

Merged cleanups with #1789 . Still potential for more, so please keep this open.

[abitmore]

Since we no longer support boost 1.57 nor lower versions, we can cleanup code like this:

#if BOOST_VERSION >= 105600
...
#else
...
#endif