Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Return unopened accounts #4066

Merged

Conversation

RickiNano
Copy link
Contributor

@RickiNano RickiNano commented Jan 21, 2023

Fix for issue #4064

accounts_balances RPC was changed in 580c3c1
That also changed the response of unopened accounts into an error message
The response of accounts_balances should mimic account_balance and this PR makes accounts_balances return the actual balance values of unopened accounts the way it did before 580c3c1
Invalid accounts will still return an error

@RickiNano RickiNano changed the title Bugfix/return unopened accounts Return unopened accounts Jan 21, 2023
pwojcikdev
pwojcikdev previously approved these changes Jan 22, 2023
Copy link
Contributor

@pwojcikdev pwojcikdev left a comment

Choose a reason for hiding this comment

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

It doesn't make sense to return an error if one of the accounts is not opened yet, it's unfortunate it wasn't caught during testing. Thanks for the PR!

@RickiNano
Copy link
Contributor Author

I targeted this PR against the develop branch but maybe v24 should get a patched version as well?

@pwojcikdev
Copy link
Contributor

When it gets merged into develop it can be cherry picked into v24 branch, so it's fine.

nano/node/json_handler.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@pwojcikdev pwojcikdev left a comment

Choose a reason for hiding this comment

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

Looks good

Copy link
Contributor

@Exxenoz Exxenoz left a comment

Choose a reason for hiding this comment

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

👍

@dsiganos dsiganos merged commit 0812626 into nanocurrency:develop Jan 25, 2023
dsiganos added a commit to dsiganos/nano-node that referenced this pull request Jan 25, 2023
* Remove account check error and return unopened account balance

* Update unit tests for accounts_balances

* Remove unneeded account_info variable

* Improve the comments a little

* Unit test case for unopened account with receivables

Co-authored-by: Dimitrios Siganos <dimitris@siganos.org>
dsiganos added a commit that referenced this pull request Jan 25, 2023
* Remove account check error and return unopened account balance

* Update unit tests for accounts_balances

* Remove unneeded account_info variable

* Improve the comments a little

* Unit test case for unopened account with receivables

Co-authored-by: Dimitrios Siganos <dimitris@siganos.org>
dsiganos added a commit that referenced this pull request Jan 27, 2023
* Remove account check error and return unopened account balance

* Update unit tests for accounts_balances

* Remove unneeded account_info variable

* Improve the comments a little

* Unit test case for unopened account with receivables

Co-authored-by: Dimitrios Siganos <dimitris@siganos.org>
@thsfs thsfs added documentation This item indicates the need for or supplies updated or expanded documentation rpc Changes related to Remote Procedure Calls labels Mar 1, 2023
@thsfs
Copy link
Contributor

thsfs commented May 19, 2023

Documentation changes for V24/V25: nanocurrency/nano-docs#670

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
documentation This item indicates the need for or supplies updated or expanded documentation rpc Changes related to Remote Procedure Calls
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants