fix: attach signers to Solana sweep transaction#70
Merged
Conversation
signTransactionMessageWithSigners was producing unsigned transactions because only plain Address strings were in the instruction accounts — no actual signer objects were attached. Use addSignersToTransactionMessage to bind both newSigner and oldSigner before signing.
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/solana-sweep.ts`:
- Around line 249-255: The file fails CI due to Prettier formatting; run the
formatter and commit the changes (e.g., run npx prettier --write) to normalize
spacing/line breaks around the addSignersToTransactionMessage and
signTransactionMessageWithSigners call site; after formatting, verify the
txMessageWithSigners variable and adjacent comments remain unchanged and then
stage and push the formatted file to clear the CI warning.
ℹ️ Review info
Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a53da588-c199-4a9a-bf6e-2165cf4f53cd
📒 Files selected for processing (1)
src/solana-sweep.ts
Comment on lines
+249
to
+255
| // Attach both signers so signTransactionMessageWithSigners can find them | ||
| const txMessageWithSigners = addSignersToTransactionMessage( | ||
| [newSigner, oldSigner], | ||
| txMessage, | ||
| ); | ||
|
|
||
| const signedTx = await signTransactionMessageWithSigners(txMessageWithSigners); |
There was a problem hiding this comment.
CI warning: format this file with Prettier.
The pipeline reports formatting drift in this file. Please run npx prettier --write src/solana-sweep.ts to clear CI.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/solana-sweep.ts` around lines 249 - 255, The file fails CI due to
Prettier formatting; run the formatter and commit the changes (e.g., run npx
prettier --write) to normalize spacing/line breaks around the
addSignersToTransactionMessage and signTransactionMessageWithSigners call site;
after formatting, verify the txMessageWithSigners variable and adjacent comments
remain unchanged and then stage and push the formatted file to clear the CI
warning.
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
1 participant
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
signTransactionMessageWithSignerswas producing unsigned transactions because only plainAddressstrings were used in instruction account metas — no signer objects were attachedaddSignersToTransactionMessage([newSigner, oldSigner], txMessage)before signing so both the fee payer (new wallet) and token transfer authority (old wallet) are properly bound/wallet migrate-solanaTest plan
/wallet migrate-solanawith a user that has USDC in the old (legacy secp256k1) walletSummary by CodeRabbit