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

Query ViewState with with non-existing account id and invalid prefix does not return error as expected #2793

Closed
khorolets opened this issue Jun 4, 2020 · 8 comments · Fixed by #6341
Assignees
Labels
A-RPC Area: rpc C-bug Category: This is a bug P-low Priority: low T-public-interfaces Team: issues relevant to the public interfaces team

Comments

@khorolets
Copy link
Member

khorolets commented Jun 4, 2020

Describe the bug
Calling query method view_state with account_id that doesn't exist returns result with data instead of error. But it might be intentional /cc @frol

To Reproduce
I'm including rust test test_view_state_non_existing_account_invalid_prefix to reproduce it. #2740

Expected behavior
I expect to receive response with error key instead of result

Screenshots
If applicable, add screenshots to help explain your problem.

Version (please complete the following information):

  • nearcore master
  • rust version rustc 1.45.0-nightly (a74d1862d 2020-05-14)
  • testnet

Additional context
it is found while implementing #2422

@frol frol added C-bug Category: This is a bug A-RPC Area: rpc labels Jun 4, 2020
@frol frol changed the title [jsonrpc] query ViewState with with non-existing account id and invalid prefix does not return error as expected Query ViewState with with non-existing account id and invalid prefix does not return error as expected Jun 4, 2020
@frol
Copy link
Collaborator

frol commented Jun 4, 2020

It might be by design, so it is yet to be decided if we want to return Not Found (I guess, it is nice to have it this way).

@frol frol self-assigned this Jun 10, 2020
@frol frol added the P-low Priority: low label Jun 10, 2020
@stale
Copy link

stale bot commented Jul 1, 2021

This issue has been automatically marked as stale because it has not had recent activity in the last 2 months.
It will be closed in 7 days if no further activity occurs.
Thank you for your contributions.

@stale stale bot added the S-stale label Jul 1, 2021
@bowenwang1996 bowenwang1996 added the T-public-interfaces Team: issues relevant to the public interfaces team label Jul 2, 2021
@stale stale bot removed the S-stale label Jul 2, 2021
@bowenwang1996
Copy link
Collaborator

@frol @khorolets what is the status of this issue?

@frol
Copy link
Collaborator

frol commented Aug 9, 2021

@miraclx I believe with the recent strictly-typed AccountId we addressed this issue. Can you try removing the #[ignore] attribute from the tests in chain/jsonrpc/tests/rpc_query.rs? We might be able to close a ton of issues now!

@miraclx
Copy link
Contributor

miraclx commented Aug 12, 2021

Indeed! We go from

this:

test test_view_state_non_existing_account_invalid_prefix ... FAILED

failures:

---- test_view_state_non_existing_account_invalid_prefix stdout ----
thread 'test_view_state_non_existing_account_invalid_prefix' panicked at 'queried view account for not exsiting account, but received success instead of error', chain/jsonrpc/tests/rpc_query.rs:659:9

to this:

account_id: "\u{0}\u{0}\u{0}\u{0}\u{0}\u{4}\u{0}\u{0}\u{0}\u{8}\u{0}\u{0}\u{0}\u{0}\u{0}eeeeeeeeeeeeeeeeeeeeeeeeeeeee".parse().unwrap(),

test test_view_state_non_existing_account_invalid_prefix ... FAILED

failures:

---- test_view_state_non_existing_account_invalid_prefix stdout ----
thread 'test_view_state_non_existing_account_invalid_prefix' panicked at 'called `Result::unwrap()` on an `Err` value: ParseAccountError(Invalid, "\u{0}\u{0}\u{0}\u{0}\u{0}\u{4}\u{0}\u{0}\u{0}\u{8}\u{0}\u{0}\u{0}\u{0}\u{0}eeeeeeeeeeeeeeeeeeeeeeeeeeeee")', chain/jsonrpc/tests/rpc_query.rs:656:148

Yay for early validation. We can proceed to resolve these issues. I can take this one.

@frol
Copy link
Collaborator

frol commented Aug 12, 2021

Yay for early validation. We can proceed to resolve these issues. I can take this one.

Great! Please, take it!

@miraclx miraclx self-assigned this Aug 17, 2021
@stale
Copy link

stale bot commented Nov 15, 2021

This issue has been automatically marked as stale because it has not had recent activity in the last 2 months.
It will be closed in 7 days if no further activity occurs.
Thank you for your contributions.

@stale stale bot added the S-stale label Nov 15, 2021
@frol frol removed the S-stale label Nov 18, 2021
@stale
Copy link

stale bot commented Feb 16, 2022

This issue has been automatically marked as stale because it has not had recent activity in the last 2 months.
It will be closed in 7 days if no further activity occurs.
Thank you for your contributions.

@stale stale bot added the S-stale label Feb 16, 2022
@miraclx miraclx removed the S-stale label Feb 16, 2022
@miraclx miraclx linked a pull request Feb 23, 2022 that will close this issue
near-bulldozer bot pushed a commit that referenced this issue Feb 23, 2022
Since introducing the strictly typed `AccountId`, the following issues are non-existent:

- #2790
- #2791
- #2792
- #2793

In the case of #2789, as pointed out in #2789 (comment), we have `integration-tests` replacing this unit test.

https://github.com/near/nearcore/blob/2677f00918b7425beafce562d4fdff67692a3a3f/integration-tests/src/tests/nearcore/rpc_nodes.rs#L309-L345

And finally, #2800 seems to work properly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-RPC Area: rpc C-bug Category: This is a bug P-low Priority: low T-public-interfaces Team: issues relevant to the public interfaces team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants