Skip to content

fix: revert PR #247 — multisig deployer merge left main with 292 TS errors#248

Merged
pragmaticAweds merged 1 commit into
mainfrom
fix/main-broken-merge-247
Apr 29, 2026
Merged

fix: revert PR #247 — multisig deployer merge left main with 292 TS errors#248
pragmaticAweds merged 1 commit into
mainfrom
fix/main-broken-merge-247

Conversation

@pragmaticAweds
Copy link
Copy Markdown
Contributor

@pragmaticAweds pragmaticAweds commented Apr 29, 2026

Summary

Reverts the merge of PR #247 (commit 9bdc1d4). The multi-sig deployer merge committed an unresolved 3-way merge: conflict markers were stripped but both sides of the conflict were kept verbatim, leaving duplicated code that fails to compile.

What's broken on main

tsc --noEmit on packages/sdk reports 292 errors in three files:

  • packages/sdk/src/PaymentStreamClient.ts
    • Two import { Client as ContractClient } lines (1 & 5)
    • Two private client: ContractClient declarations (55 & 64)
    • Two constructor(...) definitions
    • A second getStreamHistory method body inserted inside getAllStreamHistory
  • packages/sdk/src/deployer/ContractDeployer.ts
    • Duplicate import { ... } from './types'
    • Duplicate signatures for estimateUploadFee, estimateDeployFee, uploadWasm, deployContract, uploadAndDeploy
    • Stray JSDoc fragments dangling outside method bodies
  • packages/sdk/src/deployer/types.ts
    • SigningCallback declared twice with different signatures ((tx: Transaction) => Transaction vs (txXdr: string) => Promise<string>)
    • Signer declared twice (Keypair | SigningCallback vs Keypair | Keypair[] | SigningCallback)

After this revert: the SDK is back to its pre-#247 baseline (36 type errors, all pre-existing — same as commit dec9919).

Why revert instead of patch

Choosing one signature over the other for every duplicated method is a directional API decision (single-keypair Signer vs multi-sig Deployer/DeployerAccount). That's PR #247's own scope and shouldn't be decided in a hot-fix. PR #247 should be re-opened against a clean main and re-merged with the conflict properly resolved.

Why this matters now

Open PRs (#246, #230, #228, #209, etc.) all conflict against main because of the chimera state in these three files. Merging main into any of them produces nonsensical 3-way merges. Unblocking them requires a clean main first.

Test plan

Summary by CodeRabbit

  • Breaking Changes
    • ContractDeployer methods (estimateUploadFee, estimateDeployFee, uploadWasm, deployContract, uploadAndDeploy) now accept Deployer parameter instead of Signer or Keypair
    • Removed StreamHistoryEvent and StreamEventType type exports from PaymentStreamClient
    • Removed legacy XDR-based signing callback and multi-sig signer support types

…sig-contract-deployer"

This reverts commit 9bdc1d4, reversing
changes made to dec9919.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 29, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 65624f80-fbd3-4d1f-b20a-665eab702b62

📥 Commits

Reviewing files that changed from the base of the PR and between 0261b51 and ac7f01a.

📒 Files selected for processing (5)
  • packages/sdk/src/PaymentStreamClient.ts
  • packages/sdk/src/__tests__/PaymentStreamClient.test.ts
  • packages/sdk/src/deployer/ContractDeployer.ts
  • packages/sdk/src/deployer/types.ts
  • packages/sdk/src/index.ts
💤 Files with no reviewable changes (4)
  • packages/sdk/src/tests/PaymentStreamClient.test.ts
  • packages/sdk/src/index.ts
  • packages/sdk/src/PaymentStreamClient.ts
  • packages/sdk/src/deployer/types.ts

📝 Walkthrough

Walkthrough

The pull request consolidates stream history retrieval responsibility and standardizes contract deployment APIs. PaymentStreamClient removes internal Soroban RPC event parsing and exported event type definitions, delegating history retrieval to a shared helper. ContractDeployer and its type definitions standardize on the Deployer type across all public methods, removing legacy Signer-based signatures and XDR callback support.

Changes

Cohort / File(s) Summary
Stream History Refactoring
packages/sdk/src/PaymentStreamClient.ts, packages/sdk/src/__tests__/PaymentStreamClient.test.ts
Removed internal RPC-based event fetching and parsing logic from PaymentStreamClient.getStreamHistory; delegated to shared getStreamHistory helper. Removed exported types StreamEventType and StreamHistoryEvent. Deleted corresponding test mocks and all test coverage for stream history functionality.
Deployer Type Standardization
packages/sdk/src/deployer/ContractDeployer.ts, packages/sdk/src/deployer/types.ts
Refactored ContractDeployer methods (estimateUploadFee, estimateDeployFee, uploadWasm, deployContract, uploadAndDeploy) to accept Deployer type instead of Signer or `Keypair
Package Exports
packages/sdk/src/index.ts
Adjusted barrel export syntax and removed type re-exports StreamHistoryEvent and StreamEventType from public package surface while retaining client value exports.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~28 minutes

Possibly related PRs

Poem

🐰 History hops to a helper's care,
Deployers now speak in common prayer,
Old XDR dreams fade away,
Standardized paths lead the day! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: reverting PR #247 to fix 292 TypeScript errors caused by a problematic merge with conflict markers.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/main-broken-merge-247

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

@pragmaticAweds pragmaticAweds merged commit 2b00e9d into main Apr 29, 2026
5 of 15 checks passed
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