Skip to content

feat(sdk-core): depositToVault with getVaultConfig and Concrete BTC support#9211

Merged
venkateshv1266 merged 1 commit into
masterfrom
venkatesh/defi-279-deposittovault-getvaultconfig
Jul 13, 2026
Merged

feat(sdk-core): depositToVault with getVaultConfig and Concrete BTC support#9211
venkateshv1266 merged 1 commit into
masterfrom
venkatesh/defi-279-deposittovault-getvaultconfig

Conversation

@venkateshv1266

Copy link
Copy Markdown
Contributor

Summary

  • Add VaultProvider enum (Morpho, ConcreteBtccx) and named result types ConcreteDepositResult / MorphoDepositResult; update DepositResult union to use them
  • Add getVaultConfig to DefiVault and IDefiVault, proxying GET /api/defi-service/v1/vaults/:id
  • Add protocol dispatch in depositToVault: routes ConcreteBtccxdepositToConcreteVault, MorphodepositToMorphoVault, else throws unsupported provider error
  • depositToConcreteVault: single sendMany with type: 'defi-deposit' and defiParams; no recipients (WP resolves escrow destination server-side)
  • Add defiParams: t.unknown to BuildParams whitelist and optional defiParams to SendManyOptions; bump @bitgo/public-types to ^6.39.0
  • Unit tests: getVaultConfig happy path + error cases, Concrete dispatch (single sendMany, pendingApproval extraction, passphrase forwarding), Morpho regression guard

Test plan

  • yarn unit-test -- --grep "DefiVault" in modules/sdk-core: all passing
  • yarn build in modules/sdk-core: no TypeScript errors
  • Morpho existing tests are green (regression guard)
  • Concrete path: depositToVault with a concrete_btccx vault returns { pendingApprovalId, state }
  • Unsupported provider throws descriptive error

CLOSES TICKET: DEFI-279

@linear-code

linear-code Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

DEFI-279

@venkateshv1266 venkateshv1266 force-pushed the venkatesh/defi-279-deposittovault-getvaultconfig branch 4 times, most recently from 61f2d24 to e9261f7 Compare July 8, 2026 17:23
@venkateshv1266 venkateshv1266 requested a deployment to breaking-changes-override July 8, 2026 17:33 — with GitHub Actions Waiting
Comment thread modules/sdk-core/src/bitgo/defi/defiVault.ts
@nvjsr

nvjsr commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

CI is failing

@venkateshv1266 venkateshv1266 marked this pull request as ready for review July 9, 2026 09:59
@venkateshv1266 venkateshv1266 requested review from a team as code owners July 9, 2026 09:59

@OttoAllmendinger OttoAllmendinger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why are we still not using typed routes?

@venkateshv1266 venkateshv1266 marked this pull request as draft July 9, 2026 15:00
@venkateshv1266 venkateshv1266 force-pushed the venkatesh/defi-279-deposittovault-getvaultconfig branch from e9261f7 to 68c4ab7 Compare July 9, 2026 18:42
@venkateshv1266 venkateshv1266 temporarily deployed to breaking-changes-override July 9, 2026 18:52 — with GitHub Actions Inactive
@venkateshv1266 venkateshv1266 marked this pull request as ready for review July 9, 2026 18:57
@venkateshv1266 venkateshv1266 requested a deployment to breaking-changes-override July 9, 2026 22:23 — with GitHub Actions Waiting
Comment thread modules/sdk-core/src/bitgo/defi/defiVault.ts Outdated
@venkateshv1266 venkateshv1266 force-pushed the venkatesh/defi-279-deposittovault-getvaultconfig branch from 68c4ab7 to c298e1b Compare July 10, 2026 11:58
@venkateshv1266 venkateshv1266 requested a deployment to breaking-changes-override July 10, 2026 12:07 — with GitHub Actions Waiting
@venkateshv1266 venkateshv1266 force-pushed the venkatesh/defi-279-deposittovault-getvaultconfig branch from c298e1b to cf025ce Compare July 10, 2026 12:54
@venkateshv1266 venkateshv1266 deployed to breaking-changes-override July 10, 2026 13:02 — with GitHub Actions Active
@vmccarty

Copy link
Copy Markdown
Contributor

Breaking change linter override approve. Re:

Breaking change linter override approved. Re:

None of the 98 findings is the removal of a required, client-facing, in-production response field. So the premise ("each was intentionally broken elsewhere and got a DevEx override") doesn't map 1:1 — most aren't breaks at all. But every real change traces to a deliberate, reviewed, released public-types PR with a Linear ticket, and several were explicitly coordinated with the owning service:

Irys revert → #396 (SC-5765/6/7), owner-driven
DeFi clientIdempotencyKey/restructure → #389 (CGD-1400), #414 (CGD-2007, body: "Unblocks CGD-1821 (wallet-platform) and CGD-1822 (BitGoJS)")
Canton choiceArgument optional → v6.26.0
goStaking optionality → v6.29.0
The reason there's no override "in the public-types repo" is exactly the gap you identified: public-types has no breaking-change CI, and the consuming service (wallet-platform, which owns signtxtss/sweep) drove these via tickets rather than through a spec-diff gate.

Recommendation
Both PRs (9232 → 6.41.0; 9211 → ^6.39.0, same range, subset of the same findings) are safe to override and approve on client-impact grounds — the breaking-change gate is misfiring on oneOf reindexing, not detecting real contract breaks. I'd note in the DevEx override request: "All findings are positional oneOf artifacts in the signtxtss/sweep intent union from additive public-types changes 6.22→6.41 plus the intentional Irys revert (#396); verified surviving fields still present; no required client-facing field removed."

Two things I can do next if useful:

Pull the literal override/approval paper trail from wallet-platform (owner of these endpoints) to attach as corroboration — I'd need to add that repo to the session.
For the CI/pipeline fix: the concrete bug is the positional-index diff on large unions. The durable fix is a name/discriminator-aware oneOf comparison (match variants by intentType literal, not array position) in BitGo/static-analysis's breaking-changes rule — I can scope that out.
Want me to grab the wallet-platform override records, or start on the static-analysis fix?

- Add DefiVault class with depositToVault (Morpho + Concrete paths),
  resumeDeposit, getVaultConfig, getOperation, listOperations
- getVaultConfig uses decodeWithCodec(GetVaultResponse) for runtime
  validation of the defi-service API response
- VaultProtocol/GetVaultResponse imported from @bitgo/public-types;
  removes local VaultProvider/VaultConfig/ConcreteVaultConfig duplicates
- Expose wallet.defi getter on Wallet class
- Add defiParams to SendManyOptions and BuildParams whitelist
- Add DEFI- to commitlint issuePrefixes

TICKET: DEFI-279
@venkateshv1266 venkateshv1266 force-pushed the venkatesh/defi-279-deposittovault-getvaultconfig branch from cf025ce to 47e9153 Compare July 11, 2026 05:33
@venkateshv1266

Copy link
Copy Markdown
Contributor Author

Removed the bump commit (chore(sdk-core): bump @bitgo/public-types to 6.40.1) since a newer version was already merged to master, which was causing merge conflicts. Rebased the branch onto latest master — now only the feat commit remains.

@Marzooqa Marzooqa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Codeowners should be updated

@venkateshv1266 venkateshv1266 merged commit 4aa0484 into master Jul 13, 2026
24 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.

5 participants