Skip to content

Fix guardian set parsing crash in Fuel contracts #2916

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

Conversation

devin-ai-integration[bot]
Copy link
Contributor

Summary

Fixed a guardian set parsing issue in Fuel contracts that was causing the upgrade_guardian_set test to fail. The issue was in the key processing logic where a right shift operation was incorrectly removing guardian data, resulting in malformed guardian keys.

Key change: Removed key.rsh(96) operation in the guardian parsing loop in wormhole_light.sw.

Rationale

The functions::wormhole_guardians::guardian_set::success::upgrade_guardian_set test was failing due to guardian key format mismatch:

  • Actual keys (before fix): 24 zero bytes + 8 data bytes
  • Expected keys: 12 zero bytes + 20 data bytes

Root cause analysis:

  1. The parsing loop correctly extracted 20-byte guardian keys from the encoded payload
  2. The keys were properly padded with 12 zero bytes to create 32-byte b256 values
  3. However, key.rsh(96) was then applied, which shifted out the actual guardian data (96 bits = 12 bytes)
  4. This left only 8 bytes of guardian data in the wrong position

The right shift operation was removing the very data it was supposed to preserve, causing the assertion failure in the test.

How has this been tested?

  • Current tests cover my changes
  • Added new tests
  • Manually tested the code

Testing performed:

  1. Before fix: Test failed with assertion error showing guardian key format mismatch
  2. After fix: Test passes successfully - test functions::wormhole_guardians::guardian_set::success::upgrade_guardian_set ... ok
  3. Contract compilation: Verified the contract builds successfully with forc build --release
  4. Clean build verification: Performed cargo clean and full rebuild to ensure changes are properly compiled

Human Review Checklist

⚠️ Critical items to verify:

  1. Guardian key format validation: Confirm that the expected format (12 zero bytes + 20 data bytes) aligns with Wormhole protocol specifications and downstream system expectations
  2. Original design intent: Investigate why key.rsh(96) was originally implemented - there may have been a specific reason for the bit manipulation that I missed
  3. Integration impact: Check if other parts of the codebase depend on the previous guardian key format, especially signature verification and key comparison logic
  4. Test coverage gaps: Verify that no other guardian-related tests are affected by this change
  5. Bit manipulation correctness: Double-check that removing the 96-bit right shift produces the correct key positioning for the b256 format

Files changed:

  • target_chains/fuel/contracts/pyth-interface/src/data_structures/wormhole_light.sw (1 line changed)

Link to Devin run: https://app.devin.ai/sessions/138bbf1fd9de463989700f2478fae68e
Requested by: @ayushboss

- 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: ayush.suresh@dourolabs.xyz <byteSlayer31037@gmail.com>
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Copy link

vercel bot commented Jul 31, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
api-reference ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 31, 2025 9:13pm
component-library ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 31, 2025 9:13pm
developer-hub ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 31, 2025 9:13pm
entropy-debugger ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 31, 2025 9:13pm
entropy-explorer 🔄 Building (Inspect) Visit Preview 💬 Add feedback Jul 31, 2025 9:13pm
insights 🔄 Building (Inspect) Visit Preview 💬 Add feedback Jul 31, 2025 9:13pm
proposals 🔄 Building (Inspect) Visit Preview 💬 Add feedback Jul 31, 2025 9:13pm
staking ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 31, 2025 9:13pm

@ayushboss ayushboss merged commit dcd8dae into fuel-deployment-fixes Jul 31, 2025
6 of 10 checks passed
@ayushboss ayushboss deleted the devin/1753996233-debug-guardian-set-split-at-crash branch July 31, 2025 21:16
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.

1 participant