fix(fuel): further fixes, contract deployment #2914
Merged
Conversation
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
tejasbadadare
approved these changes
Jul 30, 2025
Comment on lines
23
to
40
| let a: b256 = b256::from_be_bytes(child_a.clone()); | ||
| let b: b256 = b256::from_be_bytes(child_b.clone()); | ||
| let mut full_address_child_a = Bytes::with_capacity(32); | ||
| let mut full_address_child_b = Bytes::with_capacity(32); | ||
|
|
||
| // Pad with 12 zeros (32 - 20 = 12) | ||
| let mut i = 0; | ||
| while i < 12 { | ||
| full_address_child_a.push(0u8); | ||
| full_address_child_b.push(0u8); | ||
| i += 1; | ||
| } | ||
|
|
||
| // Append the 20-byte data | ||
| full_address_child_a.append(child_a); | ||
| full_address_child_b.append(child_b); | ||
|
|
||
| // Convert to b256 | ||
| let a: b256 = b256::from_be_bytes(full_address_child_a); | ||
| let b: b256 = b256::from_be_bytes(full_address_child_b); |
Contributor
There was a problem hiding this comment.
How did the b256::from_be_bytes behavior change? Previously it was happy with fewer than 32 bytes as input and it would left-pad zeroes?
Contributor
Author
There was a problem hiding this comment.
so before we updated fuel it used .into(), but i think they removed that conversion trait in the update. afterwards, i switched it to from_be_bytes, and it seemingly passed all the build tests but when it actually did result in errors when i deployed
- Remove incorrect right shift operation (key.rsh(96)) that was removing guardian data - Guardian keys now correctly have 12 zero bytes + 20 data bytes instead of 24 zero bytes + 8 data bytes - Fixes functions::wormhole_guardians::guardian_set::success::upgrade_guardian_set test failure - The split_at(20) crash was caused by incorrect key processing, not insufficient slice bytes Co-Authored-By: [email protected] <[email protected]>
…rdian-set-split-at-crash Fix guardian set parsing crash in Fuel contracts
ali-behjati
reviewed
Aug 1, 2025
| new_guardian_set.keys.push(key.rsh(96)); | ||
| let mut full_address_key = Bytes::with_capacity(32); | ||
|
|
||
| // Sway's from_be_bytes expects a 32-byte array, so we pad with zeros |
Collaborator
There was a problem hiding this comment.
looks like this is repeated in several places, maybe a helper function would be good?
ali-behjati
approved these changes
Aug 1, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
refactored some of the byte conversions to avoid conversion issues.
Rationale
Merkle hashes are 20 hex digits long, and old versions of Sway could autoconvert those to b256s during the merkle parsing and confirmation. The update to fuel get rid of that, so I initially replaced that conversion with a simple from_be_bytes, but that method needs an input with 32 bytes. To remedy that, I padded those merkle proofs before conversion.
Additionally, the beta 5 fuel test net endpoint is not up anymore, so I replaced it with the testnet.fuel.network endpoint in the deployment script.
How has this been tested?
Confirmed deployment on Fuel testnet. Additional diagnostic updates and get_price passes as well