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

[GUI] MasternodeWidget-Wizard bugfixes #1389

Merged
merged 5 commits into from Mar 10, 2020

Conversation

random-zebra
Copy link

This PR fixes some issue with the masternode GUI interaction.

1) MasternodeWizard: lock collateral output after MN creation.
The masternode configuration file is only read during init, and then the COutPoints relative to the collateral of each entry is locked for spending. Thus, after creating a MN controller with the wizard, the collateral remains unlocked until the wallet is restarted, and therefore could be potentially spent or staked.

2) Enforce required depth for mn collaterals
The masternode collateral transaction requires at least 15 confirmations before the mn broadcast can be considered valid. Properly inform the user after creating a controller with the wizard, and when trying to start a masternode not yet confirmed.

3) MasternodeWidget: unlock collateral output coin after MN deletion
Mirrors (1). Unlock the collateral utxo when the masternode is deleted, otherwise it remains locked until wallet restart.

4) MasterNodeWizardDialog: fix RegTestNet default port
RegTest was using the fixed port value of TestNet instead of the default 51476.

@random-zebra random-zebra added this to the 4.1.0 milestone Mar 8, 2020
@random-zebra random-zebra self-assigned this Mar 8, 2020
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.

Concept ACK, would like to address my styling comment though to have more easily readable commit diffs

src/qt/pivx/masternodeswidget.cpp Show resolved Hide resolved
Notify the user about the required confirmations
(MASTERNODE_MIN_CONFIRMATIONS) after masternode creation and when trying
to start too early.
Consider MNModel::WAS_COLLATERAL_ACCEPTED only when the tx has the
required depth.
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.

utACK 2be6f7b

Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

code ACK 2be6f7b.

@furszy furszy merged commit f80888b into PIVX-Project:master Mar 10, 2020
akshaynexus added a commit to ZENZO-Ecosystem/ZENZO-Core that referenced this pull request Mar 13, 2020
2be6f7b [GUI] MasterNodeWizardDialog: fix RegTestNet default port 51476 (random-zebra)
68a92f9 [GUI] MasternodeWidget: unlock collateral output coin after MN deletion (random-zebra)
bdb13c8 [GUI][Model] Enforce required depth for mn collaterals (random-zebra)
1c197d2 [GUI] MasternodeWizard: lock collateral output coin after MN creation (random-zebra)
d56a8f8 [Trivial] Styling: spaces and brackets (random-zebra)

Pull request description:

  This PR fixes some issue with the masternode GUI interaction.

  **1)**  __MasternodeWizard: lock collateral output after MN creation.__
  The masternode configuration file is only read during init, and then the `COutPoint`s relative to the collateral of each entry is locked for spending. Thus, after creating a MN controller with the wizard, the collateral remains unlocked until the wallet is restarted, and therefore could be potentially spent or staked.

  **2)** __Enforce required depth for mn collaterals__
  The masternode collateral transaction requires at least 15 confirmations before the mn broadcast can be considered valid. Properly inform the user after creating a controller with the wizard, and when trying to start a masternode not yet confirmed.

  **3)** __MasternodeWidget: unlock collateral output coin after MN deletion__
  Mirrors (1). Unlock the collateral utxo when the masternode is deleted, otherwise it remains locked until wallet restart.

  **4)** __MasterNodeWizardDialog: fix RegTestNet default port__
  RegTest was using the fixed port value of TestNet instead of the default 51476.

ACKs for top commit:
  Fuzzbawls:
    utACK 2be6f7b
  furszy:
    code ACK 2be6f7b.

Tree-SHA512: a0d9a51b548dbf4d808ebdbd6ad696fc05f9d59c8768dafc7985e7753351ec1094c975c80c5b4cf7b788c45344432d5dfc241cca089236cdce904d1429db9608
akshaynexus added a commit to ZENZO-Ecosystem/ZENZO-Core that referenced this pull request Mar 15, 2020
2be6f7b [GUI] MasterNodeWizardDialog: fix RegTestNet default port 51476 (random-zebra)
68a92f9 [GUI] MasternodeWidget: unlock collateral output coin after MN deletion (random-zebra)
bdb13c8 [GUI][Model] Enforce required depth for mn collaterals (random-zebra)
1c197d2 [GUI] MasternodeWizard: lock collateral output coin after MN creation (random-zebra)
d56a8f8 [Trivial] Styling: spaces and brackets (random-zebra)

Pull request description:

  This PR fixes some issue with the masternode GUI interaction.

  **1)**  __MasternodeWizard: lock collateral output after MN creation.__
  The masternode configuration file is only read during init, and then the `COutPoint`s relative to the collateral of each entry is locked for spending. Thus, after creating a MN controller with the wizard, the collateral remains unlocked until the wallet is restarted, and therefore could be potentially spent or staked.

  **2)** __Enforce required depth for mn collaterals__
  The masternode collateral transaction requires at least 15 confirmations before the mn broadcast can be considered valid. Properly inform the user after creating a controller with the wizard, and when trying to start a masternode not yet confirmed.

  **3)** __MasternodeWidget: unlock collateral output coin after MN deletion__
  Mirrors (1). Unlock the collateral utxo when the masternode is deleted, otherwise it remains locked until wallet restart.

  **4)** __MasterNodeWizardDialog: fix RegTestNet default port__
  RegTest was using the fixed port value of TestNet instead of the default 51476.

ACKs for top commit:
  Fuzzbawls:
    utACK 2be6f7b
  furszy:
    code ACK 2be6f7b.

Tree-SHA512: a0d9a51b548dbf4d808ebdbd6ad696fc05f9d59c8768dafc7985e7753351ec1094c975c80c5b4cf7b788c45344432d5dfc241cca089236cdce904d1429db9608
furszy added a commit that referenced this pull request Apr 11, 2020
…ollateral

7830c17 [BUG] Prevent StartAll from starting mns with immature collateral (random-zebra)

Pull request description:

  Extends the fix of #1389 (point 2) also to `startAll` (not only `startAlias`).

  Additional ref: #1226

ACKs for top commit:
  Fuzzbawls:
    utACK 7830c17
  furszy:
    utACK 7830c17

Tree-SHA512: 74798a075aab71d27b3e65827e625eb876ddc7d0a0a6a641864aa5bfa8bc720ee8200020dc497b03e436b6e4df25bef09955397dbdc7f267cdbd3f5781b08223
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

3 participants