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.

Sign up for GitHub

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

Jump to bottom

Exclude spent outputs from remote staking balance #1000

Merged
merged 6 commits into from
Apr 17, 2019
Merged

Exclude spent outputs from remote staking balance #1000

merged 6 commits into from
Apr 17, 2019

Conversation

dsaveliev
Copy link
Member

Fixes #843

At the moment, WalletExtension::GetRemoteStakingBalance counts every output in remote staking coinbase and shows incorrect data in getwalletinfo response.
This change excludes spent outputs from remote staking balance.

Signed-off-by: Dmitry Saveliev dima@thirdhash.com

@dsaveliev
Ignore spent transaction in WalletExtension::GetRemoteStakingBalance
1cb0b13 
Signed-off-by: Dmitry Saveliev <dima@thirdhash.com>
@dsaveliev dsaveliev added the bug A problem of existing functionality label Apr 16, 2019
@dsaveliev dsaveliev added this to the 0.2 milestone Apr 16, 2019
@dsaveliev dsaveliev requested review from cmihai, Nizametdinov and a team April 16, 2019 23:03
@dsaveliev dsaveliev self-assigned this Apr 16, 2019
@dsaveliev
Don't count spent outputs in GetRemoteStakingBalance
fdfee06 
Signed-off-by: Dmitry Saveliev <dima@thirdhash.com>
@dsaveliev dsaveliev force-pushed the fix-remote-staking-balance branch from 4ff26fc to fdfee06 Compare April 17, 2019 07:52
Nizametdinov
Nizametdinov reviewed Apr 17, 2019
View reviewed changes
src/esperanza/walletextension.cpp Outdated Show resolved Hide resolved
src/esperanza/walletextension.cpp Outdated Show resolved Hide resolved
@dsaveliev
Fix nits
ec8785e 
Signed-off-by: Dmitry Saveliev <dima@thirdhash.com>
@dsaveliev
Check actual balance unchanged
2535fb2 
Signed-off-by: Dmitry Saveliev <dima@thirdhash.com>
@dsaveliev
Add a test case for remote stake spending
b9058ad 
Signed-off-by: Dmitry Saveliev <dima@thirdhash.com>
@dsaveliev dsaveliev force-pushed the fix-remote-staking-balance branch from dcc362a to b9058ad Compare April 17, 2019 12:58
Nizametdinov
Nizametdinov reviewed Apr 17, 2019
View reviewed changes
Copy link
Member

@Nizametdinov Nizametdinov left a comment

Choose a reason for hiding this comment

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

LGTM except for few small nits.

test/functional/feature_remote_staking.py Outdated Show resolved Hide resolved
test/functional/feature_remote_staking.py Outdated Show resolved Hide resolved
test/functional/feature_remote_staking.py Outdated Show resolved Hide resolved
test/functional/feature_remote_staking.py Outdated Show resolved Hide resolved
test/functional/feature_remote_staking.py Outdated Show resolved Hide resolved
test/functional/test_framework/blocktools.py Outdated Show resolved Hide resolved
test/functional/test_framework/blocktools.py Outdated Show resolved Hide resolved
@dsaveliev
Fix nits
9223184 
Signed-off-by: Dmitry Saveliev <dima@thirdhash.com>
@dsaveliev dsaveliev force-pushed the fix-remote-staking-balance branch from 316e45d to 9223184 Compare April 17, 2019 15:42
Nizametdinov
Nizametdinov approved these changes Apr 17, 2019
View reviewed changes
Copy link
Member

@Nizametdinov Nizametdinov left a comment

Choose a reason for hiding this comment

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

utACK 9223184

cmihai
cmihai approved these changes Apr 17, 2019
View reviewed changes
Copy link
Member

@cmihai cmihai left a comment

Choose a reason for hiding this comment

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

utACK 9223184

@dsaveliev dsaveliev merged commit d3d1250 into dtr-org:master Apr 17, 2019
@dsaveliev dsaveliev deleted the fix-remote-staking-balance branch April 17, 2019 16:25
@thothd thothd modified the milestones: 0.2, 0.1 Apr 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@frolosofsky frolosofsky frolosofsky left review comments

@Nizametdinov Nizametdinov Nizametdinov approved these changes

@cmihai cmihai cmihai approved these changes

Assignees

@dsaveliev dsaveliev

Labels
bug A problem of existing functionality wallet
Projects
None yet
Milestone
0.1
Development

Successfully merging this pull request may close these issues.

Remote staking balance is calculated incorrectly
5 participants
@dsaveliev @cmihai @Nizametdinov @frolosofsky @thothd