Fix/issue 32 wasm hash type#308
Merged
hman38705 merged 2 commits intosolutions-plug:mainfrom Mar 27, 2026
Merged
Conversation
- Add e.deployer().require_auth() as the first check in initialize() - Prevents front-running attacks where an attacker monitors deployment and races to call initialize() with their own admin address - Only the deployer's signed transaction can pass the auth check - Add test_initialize_rejects_non_deployer to verify enforcement Closes solutions-plug#28
- Change ConfigKey::UpgradeRejectedAt payload from String to BytesN<32> - Update require_no_upgrade_collision, get/set/clear_upgrade_rejected_at helpers to accept &BytesN<32> instead of &String - Remove wasm_hash.is_empty() guard (BytesN<32> is always 32 bytes) - Update 6 test usages from String::from_str hashes to BytesN::from_array PendingUpgrade.wasm_hash was already BytesN<32> in types.rs; this fixes the remaining String references in governance.rs that caused a type mismatch at the host upgrade call site. Closes solutions-plug#32
|
@Fidelis900 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Title: fix: use BytesN<32> for WASM hash in PendingUpgrade struct
Body:
Summary
Fixes #32 — Resolve WASM Hash Type Mismatch in Governance
PendingUpgrade.wasm_hash was already typed as BytesN<32> in types.rs, but the governance module's internal helpers (
require_no_upgrade_collision, get/set/clear_upgrade_rejected_at) and ConfigKey::UpgradeRejectedAt still used String.
This caused a type mismatch that would fail at the host upgrade call site since
e.deployer().update_current_contract_wasm() requires BytesN<32>.
Changes
to accept &BytesN<32> instead of &String
doesn't compile
patterns
Why this matters
The Soroban host's update_current_contract_wasm function requires a BytesN<32> argument. Storing the hash as String in
the cooldown/rejection tracking keys meant the type flowing through the upgrade lifecycle was inconsistent, and would
panic or fail to compile when the stored value was retrieved and passed to the host function.
closes #140