Skip to content

feat(packages): reject btc broadcast when vault is not pending#1415

Open
jonybur wants to merge 1 commit intomainfrom
fix/broadcast-reject-non-pending-vault
Open

feat(packages): reject btc broadcast when vault is not pending#1415
jonybur wants to merge 1 commit intomainfrom
fix/broadcast-reject-non-pending-vault

Conversation

@jonybur
Copy link
Copy Markdown
Contributor

@jonybur jonybur commented Apr 17, 2026

@github-actions
Copy link
Copy Markdown

🔐 Commit Signature Verification

All 1 commit(s) passed verification

Commit Author Signature Key Type Key Check
da95e454b7e3 Jonathan Bursztyn sk-ssh-ed25519

Summary

  • Commits verified: 1
  • Signature check: ✅ All passed
  • Key type enforcement: ✅ All sk-ssh-ed25519

Required key type: sk-ssh-ed25519 (FIDO2 hardware key)

Last verified: 2026-04-17 23:38 UTC

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 17, 2026

Greptile Summary

This PR adds a vault status guard to handleBroadcast in useVaultActions, rejecting BTC broadcast when the vault is not in PENDING state. A corresponding test is added, and baseVault in the test fixture is updated to include status: ContractStatus.PENDING so existing tests continue to pass through the new check.

Confidence Score: 5/5

Safe to merge — the status guard is correct, error handling is consistent, and the test covers the new path.

No P0 or P1 issues found. The logic is simple, the enum reverse-lookup is safe because mapGraphQLStatusToVaultStatus already throws on unknown statuses before this point, and the test properly validates the new behaviour.

No files require special attention.

Important Files Changed

Filename Overview
services/vault/src/hooks/deposit/useVaultActions.ts Adds a vault.status !== ContractStatus.PENDING guard after vault fetch; throws a descriptive error with the status name via numeric enum reverse-lookup — logic is sound and consistent with existing error handling patterns.
services/vault/src/hooks/deposit/tests/useVaultActions.test.ts Adds status: ContractStatus.PENDING to baseVault (required by the new guard), imports ContractStatus from the importOriginal-based mock, and adds a focused test that verifies broadcast is rejected and the error contains EXPIRED.

Sequence Diagram

sequenceDiagram
    participant UI
    participant useVaultActions
    participant fetchVaultById
    participant broadcastPrePeginTransaction

    UI->>useVaultActions: handleBroadcast(params)
    useVaultActions->>fetchVaultById: fetchVaultById(activityId)
    fetchVaultById-->>useVaultActions: vault | null

    alt vault is null
        useVaultActions-->>UI: broadcastError = Vault not found
    else vault.status !== PENDING (NEW GUARD)
        useVaultActions-->>UI: broadcastError = Cannot broadcast: vault is in STATUS state
    else local tx != GraphQL tx
        useVaultActions-->>UI: broadcastError = Transaction mismatch
    else BTC wallet not connected
        useVaultActions-->>UI: broadcastError = BTC wallet not connected
    else all checks pass
        useVaultActions->>broadcastPrePeginTransaction: sign and broadcast
        broadcastPrePeginTransaction-->>useVaultActions: success
        useVaultActions-->>UI: onShowSuccessModal + onRefetchActivities
    end
Loading

Reviews (1): Last reviewed commit: "feat(packages): reject btc broadcast whe..." | Re-trigger Greptile

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