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

[RPC] refactor autocombine into specific commands #2248

Merged
merged 4 commits into from
May 7, 2021

Conversation

Fuzzbawls
Copy link
Collaborator

The current autocombine command is limited to whole numbers only, and infringes on our RPC standards by not providing deterministic output even when successfully changing the setting.

This PR addresses the deterministic issue as well as makes the setting information available to getwalletinfo

@Fuzzbawls Fuzzbawls added the RPC label Mar 15, 2021
@Fuzzbawls Fuzzbawls self-assigned this Mar 15, 2021
doc/release-notes.md Outdated Show resolved Hide resolved
@Fuzzbawls Fuzzbawls force-pushed the 2021_rpc-refactor-autocombine branch from 27fcdea to f02f240 Compare March 19, 2021 07:24
@random-zebra random-zebra added this to the 6.0.0 milestone Mar 25, 2021
Copy link

@random-zebra random-zebra 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. Few points.

src/wallet/walletdb.cpp Show resolved Hide resolved
src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved
src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved
src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved
src/wallet/rpcwallet.cpp Show resolved Hide resolved
src/wallet/rpcwallet.cpp Show resolved Hide resolved
@Fuzzbawls Fuzzbawls force-pushed the 2021_rpc-refactor-autocombine branch from f02f240 to 60058d6 Compare April 15, 2021 06:38
@Fuzzbawls
Copy link
Collaborator Author

Rebased and updated to address feedback.

The setter now doesn't allow a threshold value below 1, and also checks to ensure that a threshold value is given if fEnable is True.

I've also added a simple functional test for the two new commands

Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

Code review ACK with a nit.

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved
@Fuzzbawls Fuzzbawls force-pushed the 2021_rpc-refactor-autocombine branch from 60058d6 to 84f9cab Compare April 16, 2021 08:44
random-zebra
random-zebra previously approved these changes Apr 16, 2021
Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

utACK 84f9cab

Introduces `setautocombinethreshold` and `getautocombinethreshold`,
similar to `setstakesplitthreshold`, thus deprecating the now-obsolete
`autocombine` command (to be removed in a future version).
Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

re-utACK 2444de9 after rebase

@random-zebra random-zebra requested a review from furszy May 6, 2021 14:50
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.

utACK 2444de9 and merging..

@furszy furszy merged commit 2e5fcd1 into PIVX-Project:master May 7, 2021
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