Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use discriminated unions to allow checking for tx broadcast success #1755

Open
aryzing opened this issue Nov 2, 2024 · 1 comment
Open
Assignees
Labels
feature Brand new functionality. New pages, workflows, endpoints, etc.

Comments

@aryzing
Copy link

aryzing commented Nov 2, 2024

Problem

The way the types are currently constructed for TxBroadcastResult,

export type TxBroadcastResult = TxBroadcastResultOk | TxBroadcastResultRejected;

makes it "impossible" to check for success or error with TS. The union only has the txid prop common among all possible types, and therefore it's the only prop TS allows checking:

const res = broadcastTransaction(tx);

console.log(res.txid); // ok

if (res.error) {} // TS Error, prop `error` doesn't exist on all instances of TxBroadcastResult

Solution

Use discriminated unions for success vs error, and for each of the error types. Perhaps something like:

type TxBroadcastResult = // Key `"type"` is the discriminator
  | { type: "success"; txid: string }
  | { type: "error"; detail: ErrorDetail };

type ErrorDetail = // Key `"reason"` is the discriminator
  | {
      reason: "Serialization";
      txid: string;
      // other useful fields
    }
  | {
      reason: "SignatureValidation";
      txid: string;
      // other useful fields
    };
// ... other error types

Additional context

Image

@aryzing aryzing added the feature Brand new functionality. New pages, workflows, endpoints, etc. label Nov 2, 2024
@github-project-automation github-project-automation bot moved this to 🆕 New in DevTools Nov 2, 2024
@smcclellan smcclellan moved this from 🆕 New to 📋 Backlog in DevTools Nov 4, 2024
@janniks
Copy link
Collaborator

janniks commented Nov 14, 2024

Ran into the same issue. Not sure why this wasn't a problem before. The idiomatic solution is to check for error using `'error' in res', but we can think about adding a type discriminator as well (that would however add additional synthetic information that the node isn't responding with)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Brand new functionality. New pages, workflows, endpoints, etc.
Projects
Status: 📋 Backlog
Development

No branches or pull requests

2 participants