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

[Bug] nStakeSplitThreshold: division by zero #1343

Merged

Conversation

random-zebra
Copy link

@random-zebra random-zebra commented Feb 19, 2020

RPC should accept only values between 1 and 999999 (as per setstakesplitthreshold help) but the input provided was only checked against the upper bound.
Setting a 0 value would result in a crash (division-by-zero) at the first stake.

Now prevent the wallet from splitting the stake if the threshold is set to zero.

Future work:
There is no reason why the split threshold should be an integer.
Make it CAmount and enable decimal values for it.
Also use JSONRPCError(RPC_INVALID_PARAMETER) instead of runtime errors when the input is not valid.

Use a zero nStakeSplitThreshold value to disable stake-split feature
@random-zebra random-zebra added RPC Bug Wallet Block Generation Mining/Staking related issues labels Feb 19, 2020
@random-zebra random-zebra added this to the 4.1.0 milestone Feb 19, 2020
@random-zebra random-zebra self-assigned this Feb 19, 2020
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.

Good catch, utACK 4829b35

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 4829b35

@CaveSpectre11
Copy link

utACK 4829b35

@random-zebra
Copy link
Author

Merging...
Release notes will be added in a subsequent PR.

@random-zebra random-zebra merged commit 735d8dd into PIVX-Project:master Feb 20, 2020
furszy added a commit that referenced this pull request Mar 1, 2020
6f05f01 [Doc] changes to Stake-Split threshold and related RPC (random-zebra)
c9ec304 [Wallet][RPC][GUI] nStakeSplitThreshold as CAmount (random-zebra)

Pull request description:

  Follow-up on #1343
  - Wallet: save/use nStakeSplitThreshold as CAmount
  - RPC: setstakesplitthreshold takes decimal values
  - RPC: getstakesplitthreshold returns a double
  - GUI: widget in settingsoptions changed to QDoubleSpinBox
  - GUI: QSetting saved as double.

ACKs for top commit:
  Fuzzbawls:
    ACK 6f05f01
  furszy:
    Left a tiny correction for later, the rest is working fine. ACK 6f05f01

Tree-SHA512: dc4d6a1a55c6ce11a6fb5a6a2c78e1fe4ea0905fb3ca1d3d085984533a820eea960caec919b61cf73f1868ffac926f4d0b5eff2cf27248194d297d8c0cc10326
@random-zebra random-zebra deleted the 2020_nStakeSplitThreshold_0 branch September 24, 2020 00:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Block Generation Mining/Staking related issues Bug RPC Wallet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants