Skip to content

Conversation

@Fuzzbawls
Copy link
Collaborator

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.

@Fuzzbawls Fuzzbawls added this to the Future milestone Feb 28, 2019
@ghost ghost assigned Fuzzbawls Feb 28, 2019
@ghost ghost added the review label Feb 28, 2019
@Fuzzbawls Fuzzbawls requested a review from Mrs-X March 3, 2019 11:36
Copy link

@blondfrogs blondfrogs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

@Fuzzbawls Fuzzbawls force-pushed the 2019_obfuscation-removal branch from 035e9e6 to 3f1a071 Compare April 1, 2019 06:58
@Fuzzbawls
Copy link
Collaborator Author

Rebased

Copy link

@Mrs-X Mrs-X left a 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...

@Fuzzbawls
Copy link
Collaborator Author

@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).

Fuzzbawls added 6 commits May 14, 2019 20:58
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.
@Fuzzbawls Fuzzbawls force-pushed the 2019_obfuscation-removal branch from 3f1a071 to fe67706 Compare May 15, 2019 04:13
@Fuzzbawls Fuzzbawls changed the title [WIP] [Refactor] Remove stale obfuscation code [Refactor] Remove stale obfuscation code May 15, 2019
@Fuzzbawls
Copy link
Collaborator Author

Rebased again now that #830 has been merged

Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK fe67706

@Fuzzbawls Fuzzbawls requested a review from Mrs-X May 18, 2019 05:42
@Fuzzbawls Fuzzbawls modified the milestones: Future, 3.3.0 Jun 4, 2019
@Warrows Warrows self-requested a review June 9, 2019 21:17
Copy link

@Warrows Warrows left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@Fuzzbawls Fuzzbawls merged commit fe67706 into PIVX-Project:master Jun 18, 2019
Fuzzbawls added a commit that referenced this pull request Jun 18, 2019
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
lyricidal added a commit to TheArcadiaGroup/DAPS that referenced this pull request Nov 6, 2019
This is a small removal from a larger PR: PIVX-Project/PIVX#824
lyricidal added a commit to TheArcadiaGroup/DAPS that referenced this pull request Nov 22, 2019
This is a small removal from a larger PR: PIVX-Project/PIVX#824
lyricidal added a commit to TheArcadiaGroup/DAPS that referenced this pull request Nov 24, 2019
* 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
lyricidal added a commit to TheArcadiaGroup/DAPS that referenced this pull request Nov 24, 2019
* [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
@Fuzzbawls Fuzzbawls deleted the 2019_obfuscation-removal branch January 9, 2020 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants