stacks.js icon indicating copy to clipboard operation
stacks.js copied to clipboard

Use discriminated unions to allow checking for tx broadcast success

Open aryzing opened this issue 1 year ago • 1 comments

Problem

The way the types are currently constructed for TxBroadcastResult,

https://github.com/hirosystems/stacks.js/blob/50399dc0a4f9abf30f072b0bceb7888d68d948ec/packages/transactions/src/types.ts#L156

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 avatar Nov 02 '24 12:11 aryzing

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)

janniks avatar Nov 14 '24 15:11 janniks