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: Change hash encoding, support passing in code without borsh #152

Merged
merged 4 commits into from
Mar 3, 2025

Conversation

r-near
Copy link
Contributor

@r-near r-near commented Feb 24, 2025

  • Changes encoding of code hashes from base64 to bs58 in the Upgradable plugin
  • Modifies up_staged_code_hash() return type from Option<CryptoHash> to Option<String> and updates all hash generation/comparison code accordingly
  • Tests updated to match the new encoding.
  • Support passing in code directly instead of with Borsh

Closes #150

@r-near r-near requested a review from karim-en February 24, 2025 20:59
@karim-en
Copy link
Collaborator

@r-near can you please bump the rust in seperate PR, because probably this PR will be cherry picked

@r-near r-near changed the title fix: change code hash encoding from base64 to bs58 fix: Change hash encoding, support passing in code without borsh Feb 26, 2025
@r-near
Copy link
Contributor Author

r-near commented Feb 26, 2025

Closes #150

@r-near r-near requested a review from a team February 26, 2025 19:04
@karim-en karim-en requested a review from kiseln February 26, 2025 21:14
@r-near r-near enabled auto-merge (squash) March 1, 2025 09:00
.try_into()
.expect("Code should be converted to CryptoHash")
/// Converts code to a base58-encoded CryptoHash string.
fn convert_code_to_crypto_hash(code: &[u8]) -> String {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not crypto_hash; right now, we are returning CryptoHash. It's not very important, but maybe we could rename it to convert_code_to_bs58_encoded_hash or something like that?

Copy link
Collaborator

Choose a reason for hiding this comment

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

cc @r-near

@r-near r-near merged commit 2d696e0 into master Mar 3, 2025
2 checks passed
@r-near r-near deleted the base58 branch March 3, 2025 16:54
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.

Fix the mess in the upgrade process
4 participants