Skip to content

Remove unused code #3097

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.

Sign up for GitHub

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

Jump to bottom
Merged
UdjinM6 merged 19 commits into from
Sep 22, 2019
Merged

Remove unused code #3097

UdjinM6 merged 19 commits into from
Sep 22, 2019

Conversation

@PastaPastaPasta
Copy link
Member

@PastaPastaPasta PastaPastaPasta commented Sep 19, 2019 •
edited
Loading

remove quite a bit of unused code in dash specific code to avoid backporting issues

(broken up in a lot of commits in case we want to revert any of these for any reason)

@PastaPastaPasta
remove unused code
7a01ad7 
Signed-off-by: Pasta <[email protected]>
@PastaPastaPasta
remove unused code
6f74a2a 
Signed-off-by: Pasta <[email protected]>
@PastaPastaPasta
remove unused code
2103c65 
Signed-off-by: Pasta <[email protected]>
@PastaPastaPasta
remove unused code
81e4bce 
Signed-off-by: Pasta <[email protected]>
@PastaPastaPasta
remove unused code
2e55e88 
Signed-off-by: Pasta <[email protected]>
@PastaPastaPasta
remove unused code
3d37f73 
Signed-off-by: Pasta <[email protected]>
@PastaPastaPasta
remove unused code
4114816 
Signed-off-by: Pasta <[email protected]>
@PastaPastaPasta
remove unused code
0a01b81 
Signed-off-by: Pasta <[email protected]>
@PastaPastaPasta
remove unused code
9b9185c 
Signed-off-by: Pasta <[email protected]>
@PastaPastaPasta
remove unused code
aee2a22 
Signed-off-by: Pasta <[email protected]>
@PastaPastaPasta
remove unused code
5ea520e 
Signed-off-by: Pasta <[email protected]>
@PastaPastaPasta
remove unused code
2f8cbfd 
Signed-off-by: Pasta <[email protected]>
@PastaPastaPasta
remove unused code
7a430ac 
Signed-off-by: Pasta <[email protected]>
@PastaPastaPasta
remove unused code, maybe need to revert / change the surrounding values
dbdb480 
Signed-off-by: Pasta <[email protected]>
@PastaPastaPasta
remove unused code
7379449 
Signed-off-by: Pasta <[email protected]>
@PastaPastaPasta
remove unused code
6939b6c 
Signed-off-by: Pasta <[email protected]>
@PastaPastaPasta
remove unused code
5ea5d45 
Signed-off-by: Pasta <[email protected]>
@PastaPastaPasta
remove unused code
8ea0bbb 
Signed-off-by: Pasta <[email protected]>
UdjinM6
UdjinM6 requested changes Sep 19, 2019
View reviewed changes
@PastaPastaPasta
Revert "remove unused code"
f2ab1ca 
This reverts commit 8ea0bbb.
@UdjinM6 UdjinM6 added this to the 14.1 milestone Sep 19, 2019
UdjinM6
UdjinM6 approved these changes Sep 19, 2019
View reviewed changes
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK

nmarley
nmarley approved these changes Sep 19, 2019
View reviewed changes
Copy link

@nmarley nmarley left a comment

Choose a reason for hiding this comment

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

Always love to remove cruft! 👍

utACK

codablock
codablock reviewed Sep 22, 2019
View reviewed changes
Copy link

@codablock codablock left a comment

Choose a reason for hiding this comment

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

See inline comments

src/masternode/masternode-meta.h

public:
CMasternodeMetaInfo() {}
CMasternodeMetaInfo(const uint256& _proTxHash) : proTxHash(_proTxHash) {}
Copy link

@codablock codablock Sep 22, 2019

Choose a reason for hiding this comment

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

you remove it here and then re-add it in the next commit?

Copy link
Member Author

@PastaPastaPasta PastaPastaPasta Sep 22, 2019

Choose a reason for hiding this comment

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

Must have realized it was actually used and maybe did a soft reset instead of a hard reset, imo it's mostly fine since this will all be squashed into one merge

Copy link

@codablock codablock Sep 22, 2019

Choose a reason for hiding this comment

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

True, ignore my comment 👍

src/masternode/masternode-sync.cpp
void CMasternodeSync::Fail()
{
nTimeLastFailure = GetTime();
nCurrentAsset = MASTERNODE_SYNC_FAILED;
Copy link

@codablock codablock Sep 22, 2019

Choose a reason for hiding this comment

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

Now that whole MASTERNODE_SYNC_FAILED state got impossible to get into, so all the code that handles the state can be removed as well. I'm wondering...can sync really not fail anymore? @UdjinM6

Copy link

@UdjinM6 UdjinM6 Sep 22, 2019

Choose a reason for hiding this comment

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

Well, governance objects/votes are still off-chain and smth might go wrong there... but we have no way of figuring it out so, technically, it's not going to fail :)

@nmarley
Copy link

nmarley commented Sep 22, 2019

@codablock Not sure why I can't comment on your last comment, but...

As this is the last commit in this PR, I'd prefer to remove the whole commit instead of reverting it afterwards.

Won't we be squashing it anyway? Shouldn't matter if that's the case, right?

codablock
codablock approved these changes Sep 22, 2019
View reviewed changes
Copy link

@codablock codablock left a comment

Choose a reason for hiding this comment

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

utACK

@UdjinM6 UdjinM6 merged commit 35f079c into dashpay:develop Sep 22, 2019
@PastaPastaPasta PastaPastaPasta deleted the remove-unused-ps-code branch September 23, 2019 01:27
@xdustinface xdustinface mentioned this pull request Sep 7, 2020
Merged
panleone pushed a commit to panleone/PIVX that referenced this pull request Nov 15, 2024
@PastaPastaPasta @panleone
cherry-pick dashpay#3097
cf138e0
Fuzzbawls added a commit to PIVX-Project/PIVX that referenced this pull request Jan 11, 2025
@Fuzzbawls
Merge #2951: Dash LLMQ backports part 5
08eb5bb 
a85b450 Merge pull request dashpay#3399 from codablock/pr_speedups2 (Alexander Block)
136f900 cherry-pick dashpay#2833 (Alexander Block)
d942439 Merge pull request dashpay#3389 from codablock/pr_concentrated_recovery (Alexander Block)
36790d2 cherry pick dashpay#3368 (Author Alexander Block)
dac01a9 cherry-pick dashpay#2780 (Alexander Block)
39d0ed9 cherry-pick dashpay#3367 (Alexander Block)
5084bbf Allow re-signing of IS locks when performing retroactive signing (dashpay#3219) (Alexander Block)
802c006 Only track last seen time instead of first and last seen time (dashpay#3165) (Alexander Block)
479b64b Avoid propagating InstantSend related old recovered sigs (dashpay#3145) (Alexander Block)
27fa2af cherry-pick dashpay#3117 (Pasta)
cf138e0 cherry-pick dashpay#3097 (Pasta)
23b140e Introduce getbestchainlock rpc and fix llmq-is-cl-conflicts.py (dashpay#3094) (UdjinM6)

Pull request description:

  as usual each commit backports a different PR

ACKs for top commit: a85b450
  Duddino:
    utACK a85b450
  Liquid369:
    uTACK a85b450
  Fuzzbawls:
    ACK a85b450

Tree-SHA512: e9024d180888d8a6cc300ba9df74fc15929e3ade1773e5d312bd8cc93f6c9fd3898c5bf2d14672abf4faba576575c33936708e6e1dfd01a393479d264d3f2c57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@UdjinM6 UdjinM6 UdjinM6 approved these changes

+2 more reviewers

@nmarley nmarley nmarley approved these changes

@codablock codablock codablock approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

15

Development

Successfully merging this pull request may close these issues.

4 participants

@PastaPastaPasta @nmarley @codablock @UdjinM6