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

fix get_account_history may terminate early #168 #347

Merged

Conversation

takaaki7
Copy link
Contributor

@takaaki7 takaaki7 commented Aug 4, 2017

Fix get_account_history and get_account_history_operations don't return 1.11.0 because it is used as sentinel value in this line. (#168)
As stated in #168, fundamentally the operation 1.11.0 shouldn't be used as sentinel value, but just API fix is deployable.

For the current blockchain, 1.11.0 is the first operation of this transaction(cryptofresh) and owner is 1.1090.
I tested following queries return 1.11.0 correctly.

curl -u bytemaster:supersecret --data '{"jsonrpc": "2.0", "params": ["history", "get_account_history", ["1.2.1090", "1.11.0", 30, "1.11.30"]], "method": "call", "id": 10}' http://localhost:8090/rpc
curl -u bytemaster:supersecret --data '{"jsonrpc": "2.0", "params": ["history", "get_account_history_operations", ["1.2.1090", 11, "1.11.50", "1.11.0", 50]], "method": "call", "id": 10}' http://localhost:8090/rpc

Copy link
Member

@abitmore abitmore 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.

@oxarbitrage oxarbitrage self-requested a review August 4, 2017 16:00
@oxarbitrage
Copy link
Member

well done, i tested it and works good, a small change in the commands provided, need to do it as:

curl -u bytemaster:supersecret --data '{"jsonrpc": "2.0", "params": ["history", "get_account_history", ["1.2.1090", "1.11.0", 50, "1.11.30"]], "method": "call", "id": 10}' http://localhost:8090/rpc

in order to get up to 1.11.0, provided commands will end in 1.11.1. test case also works good.

i am merging the pull and closing the issue at #168

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants