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

[MoveOnly] Remove zPIV code from main.cpp #609

Merged
merged 1 commit into from May 22, 2018

Conversation

presstab
Copy link

No description provided.

@ghost ghost assigned presstab May 11, 2018
@ghost ghost added the review label May 11, 2018
@Fuzzbawls Fuzzbawls changed the title Remove zPIV code from main.cpp [MoveOnly] Remove zPIV code from main.cpp May 12, 2018
@Fuzzbawls
Copy link
Collaborator

Concept ACK.

I was already looking at moving the -reindexzerocoin code in init elsewhere

Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

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

build warning and some minor nits.

src/zpivchain.h Outdated

class CBlock;
class CBigNum;
class CMintMeta;
Copy link
Collaborator

Choose a reason for hiding this comment

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

build warning here:

./zpivchain.h:16:1: warning: class 'CMintMeta' was previously declared as a struct [-Wmismatched-tags]
class CMintMeta;
^
./primitives/zerocoin.h:16:8: note: previous use is here
struct CMintMeta
       ^
./zpivchain.h:16:1: note: did you mean struct here?
class CMintMeta;
^~~~~
struct
1 warning generated.

src/zpivchain.h Outdated
#define PIVX_ZPIVCHAIN_H

#include <list>
#include <string>
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: list c++ standard includes after in-tree includes

src/zpivchain.h Outdated
// Copyright (c) 2018 The PIVX developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#ifndef PIVX_ZPIVCHAIN_H
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: have an empty line between the copyright header and the #ifndef

// Copyright (c) 2018 The PIVX developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include "zpivchain.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: have an empty line between the copyright header and the first #include

@presstab
Copy link
Author

@Fuzzbawls thanks for the quick review. Styling changes have been fixed and pushed.

@Warrows
Copy link

Warrows commented May 15, 2018

I've been testing this before the changes proposed by fuzzbawls and ended up getting deadlocks crashes. The issue was appearing upon unlocking the wallet on a Debian 9 x64. I was unable to reproduce the issue on Windows so far.
I didn't have time to investigate further and I'm not sure I'll have it before next week. I'll try to test with the latest changes ASAP though.

@Fuzzbawls
Copy link
Collaborator

ACK 873ef19

@Warrows any further update on your deadlock issue? if it isn't directly related to this code move, then this should be good to go.

@Warrows
Copy link

Warrows commented May 18, 2018

I won't be able to test further before Monday. I don't know how the issue could be related but it appeared with this pr.

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.

Seems like the deadlock is unrelated to this PR. Sorry for the delay, that's a ACK from me.

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 and merging...

@Mrs-X Mrs-X merged commit 873ef19 into PIVX-Project:master May 22, 2018
Mrs-X added a commit that referenced this pull request May 22, 2018
873ef19 Remove zPIV code from main.cpp (presstab)

Tree-SHA512: 480c3dc43c25de7223468842e15f6b2f4ac4cfcef5e920d4b69dcd489cc924fe89ad262f37233c040246e291de791abdc733beb5e6e7dda6bc511b7399a0178a
@ghost ghost removed the review label May 22, 2018
Fuzzbawls pushed a commit to Fuzzbawls/PIVX that referenced this pull request Jul 6, 2018
@Fuzzbawls Fuzzbawls added this to the 3.1.1 milestone Jul 9, 2018
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.

None yet

4 participants