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

[INDEX] Fix address balance #691

Merged
merged 2 commits into from
Jun 7, 2020

Conversation

aguycalled
Copy link
Member

Fixes #538. Without this PR the balance was not correctly accounted for addresses used as both cold staking and legacy spending addresses.

@mxaddict
Copy link
Contributor

mxaddict commented Jun 4, 2020

This was fast :P

@mxaddict
Copy link
Contributor

mxaddict commented Jun 4, 2020

I can't test it since I don't have a full node with indexes

Do we have a scenario I can test on testnet instead?

@mxaddict
Copy link
Contributor

mxaddict commented Jun 4, 2020

@sakdeniz can you give a test scenario?

@navbuilder
Copy link

A new build of af06dc7 has completed succesfully!
Binaries available at https://build.nav.community/binaries/fix-addressbalanceindex

@mxaddict
Copy link
Contributor

mxaddict commented Jun 4, 2020

Tested on devnet with the scenario given via discord, working for me on Ubuntu 18.04

@chasingkirkjufell
Copy link
Contributor

@sakdeniz reported the issue still exists and hopefully he can provide steps to replicate.

@mxaddict
Copy link
Contributor

mxaddict commented Jun 5, 2020

Yeah, I could not for the life of me replicate on devnet.

@mxaddict
Copy link
Contributor

mxaddict commented Jun 5, 2020

Was finally able to replicate the issue:

getaddressbalance '{"addresses":["mxSedhHhcyQqry3ZLRTXeEvgz7J8TMwB8Y"]}'
{
  "balance": 20649890000,
  "received": 32949670000
}
"Confirmed","Date","Type","Label","Address","Amount (NAV)","ID"
"true","2020-06-05T18:27:39","Sent to","","mujGmsuuL7JurRNqmFBAifWYDNm2UmmZKH","-106.49890000","f8903e17378b671b6c4bfc673961f0c4ef51d4ca6dceb4fb28e6360597a1456b"
"true","2020-06-05T18:25:20","Staked","","mxSedhHhcyQqry3ZLRTXeEvgz7J8TMwB8Y","2.00000000","d8b7f07b01fd3eae9ed53d012b36d66383d0115f1c7b270644014a70b6f5f8fa"
"true","2020-06-05T18:23:44","Staked","","mxSedhHhcyQqry3ZLRTXeEvgz7J8TMwB8Y","2.00000000","ab4455c9105ca0e20e2a844d8637ac41295b1594f9ce8611a07a16a05bdfaa78"
"false","2020-06-05T18:22:40","Orphan","","mxSedhHhcyQqry3ZLRTXeEvgz7J8TMwB8Y","2.00000000","df2f761ecc74dce5c4168c7f2200d5f6a5ef16d55ea6c04a21b06a79701bc4b5"
"true","2020-06-05T18:19:01","Received with","","mxSedhHhcyQqry3ZLRTXeEvgz7J8TMwB8Y","100.00000000","89b0321ab6383c2a1eec606d692256c0bc87fd0248ddfd9e3c1f5e534c32d3e1"
"true","2020-06-05T18:18:37","Sent to","","mujGmsuuL7JurRNqmFBAifWYDNm2UmmZKH","-6.50110000","3af9eb7986c1559117fb70a1dc5a391c5814cc8ce7d5f3978fb02ab29c4b1e01"
"true","2020-06-05T18:17:20","Staked","","mxSedhHhcyQqry3ZLRTXeEvgz7J8TMwB8Y","2.00000000","8f6c69d9f9d6371f93592db14128b0ba5ae184ceb94046b3afb5512759e3175f"
"false","2020-06-05T18:16:48","Orphan","","mxSedhHhcyQqry3ZLRTXeEvgz7J8TMwB8Y","2.00000000","fc75e1f775d16cbb87c57e53d88e1f05c2fbba03f1721b610364af5719be4458"
"true","2020-06-05T18:15:41","Received with","","mxSedhHhcyQqry3ZLRTXeEvgz7J8TMwB8Y","7.00000000","77e614f6e93c449f5300f98d1b1a5a98ec9d41901f9fd42bcb999edb5e39e3cb"

This wallet should have zero balance, but getaddressbalance call still returns a value.

It seems it's counting "change" coins as part of the balance.

@navbuilder
Copy link

A new build of f56da3c has completed succesfully!
Binaries available at https://build.nav.community/binaries/fix-addressbalanceindex

Copy link
Contributor

@mxaddict mxaddict left a comment

Choose a reason for hiding this comment

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

Checked the same scenario I did before, and it's now getting the correct ending balance for my test scenario.

Copy link
Contributor

@chasingkirkjufell chasingkirkjufell left a comment

Choose a reason for hiding this comment

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

Replicated the balance issue with sending coins from A to a cold-staking address A (spending) + B, then back to A. The amount was count twice.
Confirm the fix with this new PR with Gitian build on Windows 10.

@mxaddict
Copy link
Contributor

mxaddict commented Jun 7, 2020

Should we merge?

@mxaddict mxaddict merged commit 74d643a into navcoin:master Jun 7, 2020
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.

getaddressbalance shows incorrect balance
5 participants