Fix deserialization of squads txs with lookup tables

[ngundotra]

Description

Fixes application crash when tx inspector provided a squads transaction account with a program ID in a lookup table.

Example: explorer.solana.com/tx/inspector?squadsTx=D6zTKhuJdvU4aPcgnJrXhaL3AP54AGQKVaiQkikH7fwH

Type of change

• Bug fix
• New feature
• Protocol integration
• Documentation update
• Other (please describe):

Screenshots

Testing

Added test

Related Issues

Checklist

• My code follows the project's style guidelines
• I have added tests that prove my fix/feature works
• All tests pass locally and in CI
• I have updated documentation as needed
• CI/CD checks pass
• I have included screenshots for protocol screens (if applicable)

Additional Notes

---

Important

Fix crash in InspectorPage.tsx by correctly handling Squads transactions with program IDs in lookup tables.

• Bug Fix:
  • Fix crash in InspectorPage.tsx when deserializing Squads transactions with program IDs in lookup tables.
  • Adjust convertVaultTransactionToMessage to handle addressTableLookups and compiledInstructions correctly.
  • Modify intoTransactionInstructionFromVersionedMessage in utils.ts to correctly resolve program IDs from addressTableLookups.
• Testing:
  • Add test in InspectorPage.spec.tsx to verify handling of Squads transactions with lookup tables.
  • Ensure tests cover both normal and lookup table scenarios.

^{This description was created by for 4901666. It will automatically update as commits are pushed.}

[vercel]

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

Name	Status	Preview	Comments	Updated (UTC)
explorer	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 19, 2025 9:22pm

[ellipsis-dev]

❌ Changes requested. Reviewed everything up to 0172cd9 in 3 minutes and 7 seconds

More details

• Looked at 226 lines of code in 4 files
• Skipped 0 files when reviewing.
• Skipped posting 10 drafted comments based on config settings.

1. app/components/inspector/InspectorPage.tsx:208

• Draft comment:
  When converting a VaultTransaction to a MessageV0, you left commented code regarding pushing lookup table accounts into accountKeys. Consider removing these commented lines or adding a comment explaining why they are not needed.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
  While it's generally good practice to remove commented code, this comment is more of a code cleanup suggestion rather than pointing out a functional issue. The code works correctly without this commented section. The comment doesn't indicate any actual problems with the implementation.
  The comment could be valid since commented code can make maintenance more difficult. However, sometimes commented code is kept intentionally for historical context or future reference.
  While code cleanliness is important, this comment is not critical enough to warrant inclusion in a PR review. The commented code isn't causing any functional issues.
  This comment should be removed as it's more of a minor cleanup suggestion rather than identifying a significant issue that needs to be addressed.

2. app/components/inspector/InspectorPage.tsx:218

• Draft comment:
  Switching from Message to MessageV0 is appropriate for handling lookup tables, but double-check that the conversion of instruction data (using Buffer.from instead of bs58 encoding) is safe and doesn’t break any consumers expecting a base58 string.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
  The comment asks the PR author to double-check the conversion of instruction data, which violates the rule against asking the author to confirm or double-check things. It does not provide a specific suggestion or ask for a test to be written. Therefore, it should be removed.

3. app/components/inspector/InstructionsSection.tsx:41

• Draft comment:
  The fallback for the AnchorDetailsCard now displays programName as 'Unknown Program' even when the anchor program exists. Verify that this change is intentional and doesn’t confuse users who expect 'Anchor Program' to be shown.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
  The comment asks to "verify" the change is intentional, which violates our rules about not asking for verification. Additionally, this seems like a UI text change that was likely intentional - the new text "Unknown Program" is actually more accurate since we're in an error state where we couldn't properly render the program details. The old text "Anchor Program" could have been misleading since we know it's an anchor program but failed to render it properly.
  Maybe this text change could confuse users who are used to the old behavior. The comment does raise a valid point about user experience.
  While user experience is important, this is ultimately just a UI text change that was likely intentional, and the new text is arguably more accurate. The comment's main ask is for verification, which we should avoid.
  Delete this comment. It asks for verification of an intentional UI text change, and the new text actually seems more accurate for the error state it represents.

4. app/components/inspector/__tests__/InspectorPage.spec.tsx:152

• Draft comment:
  The new test for Squads transactions with lookup tables sets different expected values for fee payer and account list count. Make sure these expectations match the updated logic in transaction deserialization for lookups.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
  The comment is asking the author to ensure that the expectations in the test match the updated logic. This falls under asking the author to double-check things, which is not allowed according to the rules.

5. app/components/inspector/utils.ts:91

• Draft comment:
  The logic to obtain programId from the lookup table (if index exceeds the static keys) is specific for Squads vault transactions. Ensure this special case does not inadvertently affect normal transactions that should not have outer program IDs.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

6. app/components/inspector/InspectorPage.tsx:218

• Draft comment:
  Conversion now creates a MessageV0 with explicit mapping of addressTableLookups and compiledInstructions. Please confirm that the header fields and conversion (e.g. using Buffer.from for instruction data) match the expected MessageV0 format for squads transactions.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

7. app/components/inspector/InstructionsSection.tsx:41

• Draft comment:
  The change to derive programId from the transactionInstruction (via intoTransactionInstructionFromVersionedMessage) correctly handles lookup table cases. This replaces the previous staticAccountKeys access and appears to resolve dynamic program ID extraction.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

8. app/components/inspector/__tests__/InspectorPage.spec.tsx:156

• Draft comment:
  The new test for squads transactions with lookup tables checks for the fee payer value ('62gRsAdA6dcbf4Frjp7YRFLpFgdGu8emAACcnnREX3L3') and account list size. Please ensure these hardcoded values accurately reflect the deserialized message structure.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

9. app/components/inspector/utils.ts:104

• Draft comment:
  Using Buffer.from(data) for the compiledInstruction's data is acceptable if the input data is not already a Buffer. Please double-check that the type of instruction.data is consistent to avoid redundant conversions.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

10. app/components/inspector/__tests__/InspectorPage.spec.tsx:23

• Draft comment:
  Typo detected: The comment on line 23 has an extra apostrophe at the end ('ASwDJP5mzxV1dfov2eQz5WAVEy833nwK17VLcjsrZsZf''). Please remove the trailing apostrophe for clarity.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50%
  While the comment is technically correct about the typo, it's an extremely minor issue in a comment that doesn't affect functionality. The transaction ID is also used elsewhere in the code without the extra apostrophe, so it's clear what the correct ID is. Comments about typos in comments are generally too trivial to be worth raising in a PR review.
  The extra apostrophe could potentially cause confusion if someone tries to copy-paste the transaction ID from the comment. It's a small but real issue.
  While copy-paste is a concern, the transaction ID appears correctly formatted elsewhere in the file (line 18), and developers would be more likely to copy from the actual code than from a comment.
  This comment, while technically correct, is too trivial to be worth keeping in a PR review. It addresses a minor typo in a comment that doesn't impact functionality.

Workflow ID: wflow_hyT60RCjZY7sZcLs

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on 4901666 in 1 minute and 4 seconds

More details

• Looked at 52 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 5 drafted comments based on config settings.

1. app/components/inspector/InspectorPage.tsx:215

• Draft comment:
  Good fix: Reintroducing the writableIndexes mapping in addressTableLookups ensures lookup table data is properly deserialized. Verify that both indexes arrays meet expected types.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

2. app/components/inspector/InspectorPage.tsx:232

• Draft comment:
  Reordering of properties (placing staticAccountKeys after recentBlockhash) appears intentional. Confirm that MessageV0 constructor accepts keys in this order.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

3. app/components/inspector/InspectorPage.tsx:209

• Draft comment:
  Removal of obsolete commented-out code is good for clarity. Ensure no critical logic was inadvertently removed.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

4. app/components/inspector/InspectorPage.tsx:11

• Draft comment:
  Removed the unused import 'Message' from '@solana/web3.js'. This is a clean-up that avoids importing unnecessary members.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

5. app/components/inspector/InspectorPage.tsx:215

• Draft comment:
  In the conversion function, the lookup table mapping now explicitly converts both 'readonlyIndexes' and 'writableIndexes' via Array.from, and the duplicate compiledInstructions mapping was removed. This should correctly handle squads transactions using lookup tables. Verify that staticAccountKeys remains correct and that no required lookup key is inadvertently omitted.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
  The comment is informative and asks the PR author to verify that staticAccountKeys remains correct and that no required lookup key is inadvertently omitted. This violates the rule against asking the author to verify or ensure correctness.

Workflow ID: wflow_U9K0w3i7zncaKtbV

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.