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

Fix recently introduced PS bugs #2330

Merged
merged 2 commits into from
Oct 11, 2018
Merged

Fix recently introduced PS bugs #2330

merged 2 commits into from
Oct 11, 2018

Conversation

UdjinM6
Copy link

@UdjinM6 UdjinM6 commented Oct 3, 2018

Fixes bugs introduced in #1921 and #2259

Note: e3aef41 should be reverted later ( #2318 is a good place for it, will rebase/fix after this one is merged) but I still want to merge these changes here to have some specific commit in develop with mixing correctly working on 70210 MNs.

@UdjinM6 UdjinM6 added the bug label Oct 3, 2018
@UdjinM6 UdjinM6 added this to the 12.4 milestone Oct 3, 2018
@UdjinM6 UdjinM6 changed the title Fix recent PS bugs Fix recently introduced PS bugs Oct 3, 2018
Copy link

@gladcow gladcow left a comment

Choose a reason for hiding this comment

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

utACK

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

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.

Looks good, one question inline

@@ -35,7 +35,14 @@ bool CDarkSendEntry::AddScriptSig(const CTxIn& txin)

uint256 CDarksendQueue::GetSignatureHash() const
{
return SerializeHash(*this);
// Remove after migration to 70211
{
Copy link

Choose a reason for hiding this comment

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

What is the reason for the braces / limiting scope here?

Copy link
Author

Choose a reason for hiding this comment

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

To make it look really annoying so that we wouldn't forget about removing it later :D

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.

utACK

@UdjinM6 UdjinM6 merged commit 727e940 into dashpay:develop Oct 11, 2018
CryptoCentric pushed a commit to absolute-community/absolute that referenced this pull request May 21, 2019
* backward compatibility fix for dsq

* fix StartNewQueue
@UdjinM6 UdjinM6 deleted the fixps2 branch November 26, 2020 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants