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

Fix: rosetta zero balance accounts #8830

Merged
merged 3 commits into from
Mar 29, 2023
Merged

Fix: rosetta zero balance accounts #8830

merged 3 commits into from
Mar 29, 2023

Conversation

encody
Copy link
Member

@encody encody commented Mar 29, 2023

No description provided.

@encody encody mentioned this pull request Mar 29, 2023
Closed
@wacban wacban requested a review from jakmeier March 29, 2023 09:52
@encody encody marked this pull request as ready for review March 29, 2023 09:53
@encody encody requested a review from a team as a code owner March 29, 2023 09:53
@wacban
Copy link
Contributor

wacban commented Mar 29, 2023

Would it be possible to unit or integration test this change as well? I'm not sure what rosetta rpc endpoint failed exactly but a test that does something along those lines would be cool.

bowenwang1996
bowenwang1996 reviewed Mar 29, 2023
View reviewed changes
Copy link
Collaborator

@bowenwang1996 bowenwang1996 left a comment

Choose a reason for hiding this comment

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

Please add a test so that we can catch similar issues in the future, as Wac suggested

chain/rosetta-rpc/src/utils.rs Show resolved Hide resolved
jakmeier
jakmeier approved these changes Mar 29, 2023
View reviewed changes
Copy link
Contributor

@jakmeier jakmeier left a comment

Choose a reason for hiding this comment

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

As others pointed out, a test would be useful.
But if you need this hotfix now and want to add the test later, I think that's also okay.

@near-bulldozer near-bulldozer bot merged commit ba2ea2a into master Mar 29, 2023
@near-bulldozer near-bulldozer bot deleted the fix/rosetta-zero-balance-accounts branch March 29, 2023 12:05
wacban pushed a commit that referenced this pull request Mar 29, 2023
@encody @near-bulldozer @wacban
Fix: rosetta zero balance accounts (#8830)
e4261b9 
* fix: add zero-balance account check to rosetta

* fix: slightly less magic-numbery

---------

Co-authored-by: near-bulldozer[bot] <73298989+near-bulldozer[bot]@users.noreply.github.com>
@encody
Copy link
Member Author

encody commented Mar 29, 2023

As others pointed out, a test would be useful. But if you need this hotfix now and want to add the test later, I think that's also okay.

See: #8833 for tests, etc.

posvyatokum pushed a commit that referenced this pull request Mar 29, 2023
@encody @near-bulldozer @posvyatokum
Fix: rosetta zero balance accounts (#8830)
d04637d 
* fix: add zero-balance account check to rosetta

* fix: slightly less magic-numbery

---------

Co-authored-by: near-bulldozer[bot] <73298989+near-bulldozer[bot]@users.noreply.github.com>
nikurt pushed a commit to nikurt/nearcore that referenced this pull request Apr 5, 2023
@encody @near-bulldozer @nikurt
Fix: rosetta zero balance accounts (near#8830)
b26552f 
* fix: add zero-balance account check to rosetta

* fix: slightly less magic-numbery

---------

Co-authored-by: near-bulldozer[bot] <73298989+near-bulldozer[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bowenwang1996 bowenwang1996 bowenwang1996 left review comments

@jakmeier jakmeier jakmeier approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@encody @wacban @jakmeier @bowenwang1996