Skip to content

fix(ts-sdk): accept TransactionSigner in getPaymentInstruction source_wallet#402

Merged
amilz merged 2 commits intomainfrom
fix/payment-instruction-signer-conflict
Mar 27, 2026
Merged

fix(ts-sdk): accept TransactionSigner in getPaymentInstruction source_wallet#402
amilz merged 2 commits intomainfrom
fix/payment-instruction-signer-conflict

Conversation

@amilz
Copy link
Copy Markdown
Contributor

@amilz amilz commented Mar 26, 2026

Summary

  • getPaymentInstruction previously created a NoopSigner for source_wallet, causing "Multiple distinct signers" errors when callers used a real TransactionSigner for the same address on other instructions (e.g. Codama-generated clients)
  • source_wallet now accepts TransactionSigner | string — when a signer is passed, it's used directly as the transfer authority; when a string is passed, a plain address (no signer) is used
  • Removes all createNoopSigner usage from the payment instruction path
  • Bumps ts-sdk to 0.2.0-beta.7

Closes PRO-1072

Test plan

  • Unit test: combining payment instruction (string source_wallet) with a real signer instruction — no conflict
  • Unit test: passing TransactionSigner as source_wallet preserves signer identity on the instruction
  • Existing tests pass (39/39)
  • Typecheck clean

Open with Devin

…_wallet

Resolves "Multiple distinct signers" error when combining payment
instruction with other instructions that use a real signer for the
same address. source_wallet now accepts TransactionSigner | string.
@amilz amilz requested a review from dev-jodee as a code owner March 26, 2026 21:30
@linear
Copy link
Copy Markdown

linear bot commented Mar 26, 2026

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 26, 2026

📊 TypeScript Coverage Report

Coverage: 33.9%

View detailed report

Coverage artifacts have been uploaded to this workflow run.
View Artifacts

@amilz amilz self-assigned this Mar 26, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 26, 2026

Greptile Summary

This PR fixes a "Multiple distinct signers" error in getPaymentInstruction by allowing source_wallet to accept either a plain address string or a TransactionSigner, replacing the previous hard-coded createNoopSigner that conflicted with real signers for the same address on other instructions (e.g. Codama-generated clients).\n\n- source_wallet type widened from string to TransactionSigner | string in GetPaymentInstructionRequest\n- client.ts uses isTransactionSigner to branch: passes the signer object directly as authority when provided, or uses a plain Address (role 0, no signer) when a string is passed — eliminating the NoopSigner conflict\n- createNoopSigner removed from the production code path entirely\n- Two new unit tests cover both the string-address path (no signer conflict) and the TransactionSigner path (signer identity preserved)\n- Existing test updated to reflect role: 0 (plain address) for the string case\n- Version bumped to 0.2.0-beta.7

Confidence Score: 5/5

Safe to merge — the fix is minimal, correctly scoped, and fully tested with 39/39 existing tests passing and two new targeted tests.

The logic change is small and clearly correct: isTransactionSigner branching replaces createNoopSigner without touching any other codepath. Both new consumer scenarios (string and TransactionSigner) are covered by tests, and the only finding is a trivial unused import in the test file.

No files require special attention; the one unused import in unit.test.ts is non-blocking.

Important Files Changed

Filename Overview
sdks/ts/src/client.ts Core fix: replaces createNoopSigner with a direct isTransactionSigner check. When a TransactionSigner is passed the signer is used directly; when a plain string is passed only the Address is used, eliminating "Multiple distinct signers" conflicts.
sdks/ts/src/types/index.ts Updates source_wallet type from string to `TransactionSigner
sdks/ts/test/unit.test.ts Adds two new test cases covering the string-address and TransactionSigner paths; updates the existing authority-account-meta assertion to expect role: 0. Contains one unused import (createKeyPairSignerFromBytes).
sdks/ts/package.json Version bumped from 0.2.0-beta.6 to 0.2.0-beta.7 to reflect the API change.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["getPaymentInstruction(request)"] --> B{"typeof source_wallet !== 'string'\n&& isTransactionSigner(source_wallet)?"}
    B -- "Yes (TransactionSigner)" --> C["isSigner = true\nwalletAddress = source_wallet.address"]
    B -- "No (string)" --> D["isSigner = false\nwalletAddress = source_wallet as Address"]
    C --> E["assertIsAddress(walletAddress)"]
    D --> E
    E --> F["findAssociatedTokenPda(owner: walletAddress)"]
    F --> G{"isSigner?"}
    G -- "Yes" --> H["authority = source_wallet\n(TransactionSigner, role: 2)"]
    G -- "No" --> I["authority = walletAddress\n(plain Address, role: 0)"]
    H --> J["getTransferInstruction(...)"]
    I --> J
    J --> K["Return payment_instruction"]
Loading

Reviews (1): Last reviewed commit: "chore(ts-sdk): bump version to 0.2.0-bet..." | Re-trigger Greptile

greptile-apps[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

@amilz amilz force-pushed the fix/payment-instruction-signer-conflict branch from 5a01745 to 78318a0 Compare March 26, 2026 21:43
@amilz amilz force-pushed the fix/payment-instruction-signer-conflict branch from 78318a0 to 4acb790 Compare March 26, 2026 21:43
@amilz amilz merged commit 4aba3f9 into main Mar 27, 2026
6 checks passed
@amilz amilz deleted the fix/payment-instruction-signer-conflict branch March 27, 2026 17:55
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.

2 participants