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

Revert "json: remove deprecated old style path+data request format" #7797

Merged
merged 3 commits into from
Oct 10, 2022

Conversation

mina86
Copy link
Contributor

@mina86 mina86 commented Oct 10, 2022

Turns out there still are users of the old query format. Bring it back.

This reverts commit be8a1ea.

Fixes: #7769

…ear#7601)"

Turns out there still are users of the old query format.  Bring it back.

This reverts commit be8a1ea.
@mina86 mina86 added P-high Priority: High S-automerge labels Oct 10, 2022
@mina86 mina86 requested a review from a team as a code owner October 10, 2022 18:23
@mina86 mina86 requested a review from akhi3030 October 10, 2022 18:23
@mina86
Copy link
Contributor Author

mina86 commented Oct 10, 2022

@posvyatokum, we’ll need another rc with this cherry picked.

/// Deprecated. If you want to decode a CryptoHash, use CryptoHash::from_str.
/// For anything else, don’t use base58. This is still here because of
/// deprecated RPC query format in RpcQueryRequest::parse.
pub fn from_base58(s: &str) -> Result<Vec<u8>, Box<dyn std::error::Error + Send + Sync>> {
Copy link
Contributor

Choose a reason for hiding this comment

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

In a follow-up, lets move this closed to the use-site, so that we don't have to mark it as deprecated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Counter argument though is that it would add direct bs58 dependency in chain/jsonrpc which we’ll forget to remove once from_base58 is removed.

@near-bulldozer near-bulldozer bot merged commit 7d9c73c into near:master Oct 10, 2022
@mina86 mina86 deleted the e branch October 10, 2022 20:01
@posvyatokum
Copy link
Member

@mina86 Testnet will switch to new protocol version in the next epoch. If everything goes well, I imagine doing a release a day after that, so on Wednesday. If something breaks with protocol upgrade, the release date will be determined by that bugfix. Is that timeline ok or is this more urgent?

@mina86
Copy link
Contributor Author

mina86 commented Oct 10, 2022

Sure. Most importantly is that this revert gets into mainnet release.

nikurt pushed a commit that referenced this pull request Nov 9, 2022
…7797)

Turns out there still are users of the old query format.  Bring it back.

This reverts commit be8a1ea.

Fixes: #7769
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-high Priority: High
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Query doesn't work after upgrade v1.30.0-rc.1
3 participants