-
Notifications
You must be signed in to change notification settings - Fork 725
[Refactor] Remove stale obfuscation code #824
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
[Refactor] Remove stale obfuscation code #824
Conversation
blondfrogs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK
035e9e6 to
3f1a071
Compare
|
Rebased |
Mrs-X
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK, but I'd like to see this running on testnet first because of the changes in the signature part.
Last time (or was it second last time) we messed with signatures wasn't exactly smooth if I remember correctly...
|
@Mrs-X the combined diff is a bit messy, have a look at each individual commit's diff and you'll see that this is pretty much entirely code deletion for whole functions (that haven't been used for some time now). |
obfuscationconfig(.h/.cpp) are unused and not needed at all anymore.
This function statically returns `false`, no need to have anything else here.
These two variables have been meaningless since zPIV's introduction... Nuke them from orbit!
these functions were used back when obfuscation existed and are no longer called anywhere in the source tree.
3f1a071 to
fe67706
Compare
|
Rebased again now that #830 has been merged |
random-zebra
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK fe67706
Warrows
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
fe67706 Remove more useless obfuscation code (Fuzzbawls) eed0a1d Remove more useless obfuscation code (Fuzzbawls) 3bb5c76 Remove unused functions in wallet.cpp (Fuzzbawls) 166b2d3 Remove nAnonymizePivxAmount and nLiquidityProvider (Fuzzbawls) 886d806 Remove unused code in DoAutomaticDenominating (Fuzzbawls) 742a7b1 [Qt] Remove unused obfuscationconfig (Fuzzbawls) Pull request description: Removal of "dead" code surrounding the defunct obfuscation feature. Obfuscation as a usable feature was removed in November 2017, but much of it's underlaying code remained. This PR aims to clean up and remove the now unnecessary code. Marking as [WIP] for the time being. ACKs for commit fe6770: random-zebra: ACK fe67706 Tree-SHA512: 46ab7d035aa899677d1337c6a58def25e452c24db038b769e9e6f980ac43e03a34af968114facadcd62ec93eacf41de0a385f60d91630a0ac8425cd385d9a513
This is a small removal from a larger PR: PIVX-Project/PIVX#824
This is a small removal from a larger PR: PIVX-Project/PIVX#824
* depends: allow CC/CXX to be overridden during configure bitcoin/bitcoin@0d00fd5 * depends: Enable unicode support on bdb for Windows bitcoin/bitcoin@5bb0164 * Scripts and tools: increased timeout downloading DOWNLOAD_CONNECT_TIMEOUT changed from 10 to 30 because some file start only after 15 sec (see below). Fetching boost_1_64_0.tar.bz2 from https://dl.bintray.com/boostorg/release/1.64.0/source/ % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 0 0 0 0 0 0 0 0 --:--:-- 0:00:15 --:--:-- 0 100 76.7M 100 76.7M 0 0 1863k 0 0:00:42 0:00:42 --:--:-- 8136k /home/gitianuser/bitcoin/depends/work/download/boost-1_64_0/boost_1_64_0.tar.bz2.temp: OK bitcoin/bitcoin@e7a7245 * depends: Remove unused Qt 4 dependencies bitcoin/bitcoin@7177e09 * depends: qt: avoid system harfbuzz and bz2 We may eventually want to break out harfbuzz and build it in depends, but for now just ensure that runtime dependencies don't depend on whether or not harfbuzz was present on the builder. bitcoin/bitcoin@f149e31 * build: Remove illegal spacing in darwin.mk bitcoin/bitcoin@63c74d2 * depends: expat 2.2.6 bitcoin/bitcoin@095e765 * depends: Preprocessing doesn't care about deps bitcoin/bitcoin@80f0e05 * depends: Add commands for each package for each stage bitcoin/bitcoin@6d44c5e * depends: native_protobuf: avoid system zlib bitcoin/bitcoin@19a0c4a * depends: tar: Always extract as yourself For normal users, --no-same-owner is default, but not so for root, where it is assumed that root can change ownership willy-nilly. This is not the case for privilege-limited container environments where we gaslight the process into thinking it's root. bitcoin/bitcoin@89bee1b * depends: qt: Don't hardcode pwd path Let a man use his builtins if he wants to! Also, removes the unnecessary assumption that pwd lives under /bin/pwd. bitcoin/bitcoin@f7696e6 * depends: switch to secure download of all dependencies Some dependency sources were downloaded via http, even though https (SSL/TLS) options are available. Even if we potentially check the integrity of the downloaded files via hash comparison, we should make use of this additional security layer. bdb.mk fontconfig.mk freetype.mk libX11.mk libXau.mk libXext.mk libxcb.mk native_cctools.mk native_cdrkit.mk xcb_proto.mk xextproto.mk xproto.mk xtrans.mk zlib.mk miniupnp was switched to official project mirror with SSL support bitcoin/bitcoin@d8bc47f * depends: Build secondary deps statically. Secondary dependencies don't need to be shared. bitcoin/bitcoin@1420928 * [Build] compile/link winshutdownmonitor.cpp on Windows only The file `qt/winshutdownmonitor.cpp` is for Windows only. Excluding it on other systems prevents a linker warning about missing symbols at compile time. * [QT] cleanup, remove old trading dialog form * [Wallet] Remove (explicitely) unused tx comparator We remove the '==' and '!=' operators from CMutableTransaction. These comparators are never explicitely used in our code. * Fix compile error - unused code This is a small removal from a larger PR: PIVX-Project/PIVX#824 * Resize About screen to properly fit DAPS * Disable getstakingstatus, remove staking status from getinfo (avoid future conflicts) * Bump version to v1.0.4 * a more sanitized check for received transaction: check commitment whenever receive a tranaction b62a4f0 * customize transaction selection and fees to speed up transaction processing * [Qt] Remove Growl support [Qt] Remove Growl support * [Refactor] Remove 'boost::lexical_cast<> * Clean up apparent locking issues These locks address compiler warnings when compiling with clang's `-Wthread-safety-analysis` * [Refactoring] Replace BOOST FOREACH with for : -BEGIN VERIFY SCRIPT- sed -i 's/BOOST_FOREACH *(\(.*\),/for (\1 :/' ./src/*.h ./src/*.cpp ./src/*/*.h ./src/*/*.cpp ./src/*/*/*.h ./src/*/*/*.cpp ; -END VERIFY SCRIPT- * Bump version to v1.0.4.1 * Update tests to fix compile * Comment out erroring tests (can be restored in future) * Fix Multisig test Somehow this one was reverted to the monosig test, restored previous fix by @pvc2104 * Cleanup clang's range loop analysis warnings * [Qt] Add a security warning to the debug console's default output. This adds a generic warning message to the debug console's initial output, mentioning that the commands available should not be used without prior understanding of what they do. Also fixes the `clear` key combo displayed when on macOS. * Increase LevelDB max_open_files unless on 32-bit Unix This change significantly increases IBD performance by increasing the amount of the UTXO index that can remain in memory. To ensure this doesn't cause problems in the future, a static_assert on the LevelDB version has been added, which must be updated by anyone upgrading LevelDB. * Stop using dummy strings in clientversion This changes the "dummy" GIT_COMMIT_ID and GIT_COMMIT_DATE strings used in archive packages to use live values instead of static placeholders. Useful when compiling in an isolated/base environment with an archive produced using make dist from a fully connected environment. * [Wallet] Add missing variable to 2 AvailableCoins() calls This is mostly depreciated code since the removal of coinjoin style privacy (Obfuscation), but the missing boolean variable in these two calls was causing compiler warnings on newer versions of gcc for always evaluating to true. The use of a static `false` was taken from upstream DASH. * Add masternode's pubkey to listmasternodes RPC Add the masternode's public key (used to broadcast masternode messages) to listmasternodes RPC. I could not find another convenient way to get a masternode's public key (which is public information needed to validate masternode messages) from the RPC calls. * [RPC] Fix verifychain verifychain only accepts a single (optional) argument instead of two, and we need to capture that argument properly. * Remove the no longer supported obfuscation RPC command * Remove time-based version disconnect Now that the time has passed for the tiered disconnect, we can make it a blanket ban of older versions, regardless of timestamp. * DAPS-1284 increase relay transaction buffer size to 50kb * DAPS-1258 Linux binaries in the publicly downloadable ZIPs should already be executable Add zip package to OSLibs Use .zip instead of tar.xz Exclude PoA Miner, packaged by itself in another .zip Adjust filename to dapscoin-poa-minerd-$BUILD_TARGET.zip * DAPS-942 copy only secret key for 2fa * Bump version to v1.0.4.2 * DAPS-1260 - Qt > Tools: Typo in description of Wallet Repair tab Fix typo * DAPS-761 From PIVX: [RPC]: Fix RPCTimerInterface ordering issue "when running the wallet with -server and using walletpassphrase over the RPC server (not the GUI console), the wallet doesn't lock again when the timeout expires, it stays unlocked indefinitely..." * [RPC] fix typo in 'unlockwallet' help * DAPS-1077 Rescan transactions after using mnemonic recovery phrase * DAPS-1248 From PIVX: [Wallet] fix bug with fWalletUnlockAnonymizeOnly flag setting This fixes an issue where the flag could be inappropriately changed. * Bump version to v1.0.4.3 * DAPS-1290: QT: Icon for the 'block-explorer' menu item is missing - Restore explorer icon - Remove 2 unused icons * DAPS-1305 from PIVX: Improve addrman Select() performance when buckets are nearly empty Simple backport of bitcoin#6530 PIVX-Project/PIVX#798 Earlier, when there are only a few entries in the addrman tables, Select() could spin for long periods of time (~200ms), resulting in high CPU usage (up to 40% here). This reduces it to around 0.3ms (0.5% CPU). (We have half of this commit for some reason, second half added) * DAPS-1247 fix getchaintips crash issue fix getchaintips crash issue * DAPS-761 From PIVX: Remove deprecated throw() build warnings in leveldb function signatures "Using throw() specifications in function signatures is not only not required in C++, it is considered deprecated for [various reasons] https://stackoverflow.com/questions/1055387/throw-keyword-in-functions-signature). It is not implemented by any of the common C++ compilers. The usage is also inconsistent with the rest of the source code." * DAPS-1282 Fix gettransaction showing enormous transaction fees fix gettransaction showing enormous transaction fees * Fix taskbar menu spacing for Sync KeyImage * Update Auto consolidate tooltip (even though it is hidden) * DAPS-761 From PIVX: Remove Redundant Declarations that cause compiler warnings This removes redundant function declarations in the following files: main.h * DAPS-1077/1309 opening wallet in v1.0.4.3 take long time (5 -10 min) cause of re-scanning fix rescanning everytime issue * DAPS-761 From PIVX: Hide orphans [Not hooked up yet] This is currently not hooked up, but prep for hiding orpans Still requires a checkbox in the UI on our newer Settings page vs the older Options Dialog * Daps 1242 rescanwallettransactions * Bump version to v1.0.4.4 * Fix compile error
* [TRIAL] fix multisig wallet not sync * fix compilation errors * genesis to restart devnet * reduce PoW blocks to 200 * fix compilation error * remove staking check in sendcoin diaglo * add RPC to daemon to co-sign * update multisig * Don't allow resizing of Co-Signers window * DAPS-1275 #comment disable auto-reconsolidation feature * fix bug with connection port * DAPS-1330 - Multisig [commit by commit] (#967) * depends: allow CC/CXX to be overridden during configure bitcoin/bitcoin@0d00fd5 * depends: Enable unicode support on bdb for Windows bitcoin/bitcoin@5bb0164 * Scripts and tools: increased timeout downloading DOWNLOAD_CONNECT_TIMEOUT changed from 10 to 30 because some file start only after 15 sec (see below). Fetching boost_1_64_0.tar.bz2 from https://dl.bintray.com/boostorg/release/1.64.0/source/ % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 0 0 0 0 0 0 0 0 --:--:-- 0:00:15 --:--:-- 0 100 76.7M 100 76.7M 0 0 1863k 0 0:00:42 0:00:42 --:--:-- 8136k /home/gitianuser/bitcoin/depends/work/download/boost-1_64_0/boost_1_64_0.tar.bz2.temp: OK bitcoin/bitcoin@e7a7245 * depends: Remove unused Qt 4 dependencies bitcoin/bitcoin@7177e09 * depends: qt: avoid system harfbuzz and bz2 We may eventually want to break out harfbuzz and build it in depends, but for now just ensure that runtime dependencies don't depend on whether or not harfbuzz was present on the builder. bitcoin/bitcoin@f149e31 * build: Remove illegal spacing in darwin.mk bitcoin/bitcoin@63c74d2 * depends: expat 2.2.6 bitcoin/bitcoin@095e765 * depends: Preprocessing doesn't care about deps bitcoin/bitcoin@80f0e05 * depends: Add commands for each package for each stage bitcoin/bitcoin@6d44c5e * depends: native_protobuf: avoid system zlib bitcoin/bitcoin@19a0c4a * depends: tar: Always extract as yourself For normal users, --no-same-owner is default, but not so for root, where it is assumed that root can change ownership willy-nilly. This is not the case for privilege-limited container environments where we gaslight the process into thinking it's root. bitcoin/bitcoin@89bee1b * depends: qt: Don't hardcode pwd path Let a man use his builtins if he wants to! Also, removes the unnecessary assumption that pwd lives under /bin/pwd. bitcoin/bitcoin@f7696e6 * depends: switch to secure download of all dependencies Some dependency sources were downloaded via http, even though https (SSL/TLS) options are available. Even if we potentially check the integrity of the downloaded files via hash comparison, we should make use of this additional security layer. bdb.mk fontconfig.mk freetype.mk libX11.mk libXau.mk libXext.mk libxcb.mk native_cctools.mk native_cdrkit.mk xcb_proto.mk xextproto.mk xproto.mk xtrans.mk zlib.mk miniupnp was switched to official project mirror with SSL support bitcoin/bitcoin@d8bc47f * depends: Build secondary deps statically. Secondary dependencies don't need to be shared. bitcoin/bitcoin@1420928 * [Build] compile/link winshutdownmonitor.cpp on Windows only The file `qt/winshutdownmonitor.cpp` is for Windows only. Excluding it on other systems prevents a linker warning about missing symbols at compile time. * [QT] cleanup, remove old trading dialog form * [Wallet] Remove (explicitely) unused tx comparator We remove the '==' and '!=' operators from CMutableTransaction. These comparators are never explicitely used in our code. * Fix compile error - unused code This is a small removal from a larger PR: PIVX-Project/PIVX#824 * Resize About screen to properly fit DAPS * Disable getstakingstatus, remove staking status from getinfo (avoid future conflicts) * Bump version to v1.0.4 * a more sanitized check for received transaction: check commitment whenever receive a tranaction b62a4f0 * customize transaction selection and fees to speed up transaction processing * [Qt] Remove Growl support [Qt] Remove Growl support * [Refactor] Remove 'boost::lexical_cast<> * Clean up apparent locking issues These locks address compiler warnings when compiling with clang's `-Wthread-safety-analysis` * [Refactoring] Replace BOOST FOREACH with for : -BEGIN VERIFY SCRIPT- sed -i 's/BOOST_FOREACH *(\(.*\),/for (\1 :/' ./src/*.h ./src/*.cpp ./src/*/*.h ./src/*/*.cpp ./src/*/*/*.h ./src/*/*/*.cpp ; -END VERIFY SCRIPT- * Bump version to v1.0.4.1 * Update tests to fix compile * Comment out erroring tests (can be restored in future) * Fix Multisig test Somehow this one was reverted to the monosig test, restored previous fix by @pvc2104 * Cleanup clang's range loop analysis warnings * [Qt] Add a security warning to the debug console's default output. This adds a generic warning message to the debug console's initial output, mentioning that the commands available should not be used without prior understanding of what they do. Also fixes the `clear` key combo displayed when on macOS. * Increase LevelDB max_open_files unless on 32-bit Unix This change significantly increases IBD performance by increasing the amount of the UTXO index that can remain in memory. To ensure this doesn't cause problems in the future, a static_assert on the LevelDB version has been added, which must be updated by anyone upgrading LevelDB. * Stop using dummy strings in clientversion This changes the "dummy" GIT_COMMIT_ID and GIT_COMMIT_DATE strings used in archive packages to use live values instead of static placeholders. Useful when compiling in an isolated/base environment with an archive produced using make dist from a fully connected environment. * [Wallet] Add missing variable to 2 AvailableCoins() calls This is mostly depreciated code since the removal of coinjoin style privacy (Obfuscation), but the missing boolean variable in these two calls was causing compiler warnings on newer versions of gcc for always evaluating to true. The use of a static `false` was taken from upstream DASH. * Add masternode's pubkey to listmasternodes RPC Add the masternode's public key (used to broadcast masternode messages) to listmasternodes RPC. I could not find another convenient way to get a masternode's public key (which is public information needed to validate masternode messages) from the RPC calls. * [RPC] Fix verifychain verifychain only accepts a single (optional) argument instead of two, and we need to capture that argument properly. * Remove the no longer supported obfuscation RPC command * Remove time-based version disconnect Now that the time has passed for the tiered disconnect, we can make it a blanket ban of older versions, regardless of timestamp. * DAPS-1284 increase relay transaction buffer size to 50kb * DAPS-1258 Linux binaries in the publicly downloadable ZIPs should already be executable Add zip package to OSLibs Use .zip instead of tar.xz Exclude PoA Miner, packaged by itself in another .zip Adjust filename to dapscoin-poa-minerd-$BUILD_TARGET.zip * DAPS-942 copy only secret key for 2fa * Bump version to v1.0.4.2 * DAPS-1260 - Qt > Tools: Typo in description of Wallet Repair tab Fix typo * DAPS-761 From PIVX: [RPC]: Fix RPCTimerInterface ordering issue "when running the wallet with -server and using walletpassphrase over the RPC server (not the GUI console), the wallet doesn't lock again when the timeout expires, it stays unlocked indefinitely..." * [RPC] fix typo in 'unlockwallet' help * DAPS-1077 Rescan transactions after using mnemonic recovery phrase * DAPS-1248 From PIVX: [Wallet] fix bug with fWalletUnlockAnonymizeOnly flag setting This fixes an issue where the flag could be inappropriately changed. * Bump version to v1.0.4.3 * DAPS-1290: QT: Icon for the 'block-explorer' menu item is missing - Restore explorer icon - Remove 2 unused icons * DAPS-1305 from PIVX: Improve addrman Select() performance when buckets are nearly empty Simple backport of bitcoin#6530 PIVX-Project/PIVX#798 Earlier, when there are only a few entries in the addrman tables, Select() could spin for long periods of time (~200ms), resulting in high CPU usage (up to 40% here). This reduces it to around 0.3ms (0.5% CPU). (We have half of this commit for some reason, second half added) * DAPS-1247 fix getchaintips crash issue fix getchaintips crash issue * DAPS-761 From PIVX: Remove deprecated throw() build warnings in leveldb function signatures "Using throw() specifications in function signatures is not only not required in C++, it is considered deprecated for [various reasons] https://stackoverflow.com/questions/1055387/throw-keyword-in-functions-signature). It is not implemented by any of the common C++ compilers. The usage is also inconsistent with the rest of the source code." * DAPS-1282 Fix gettransaction showing enormous transaction fees fix gettransaction showing enormous transaction fees * Fix taskbar menu spacing for Sync KeyImage * Update Auto consolidate tooltip (even though it is hidden) * DAPS-761 From PIVX: Remove Redundant Declarations that cause compiler warnings This removes redundant function declarations in the following files: main.h * DAPS-1077/1309 opening wallet in v1.0.4.3 take long time (5 -10 min) cause of re-scanning fix rescanning everytime issue * DAPS-761 From PIVX: Hide orphans [Not hooked up yet] This is currently not hooked up, but prep for hiding orpans Still requires a checkbox in the UI on our newer Settings page vs the older Options Dialog * Daps 1242 rescanwallettransactions * Bump version to v1.0.4.4 * Fix compile error
Removal of "dead" code surrounding the defunct obfuscation feature.
Obfuscation as a usable feature was removed in November 2017, but much of it's underlaying code remained. This PR aims to clean up and remove the now unnecessary code.
Marking as [WIP] for the time being.