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

Txid type is currently kind of a footgun #20

Open
thunderbiscuit opened this issue Sep 27, 2024 · 0 comments
Open

Txid type is currently kind of a footgun #20

thunderbiscuit opened this issue Sep 27, 2024 · 0 comments

Comments

@thunderbiscuit
Copy link
Member

thunderbiscuit commented Sep 27, 2024

After trying to use the Txid type in the testing of this pr, I realized a few things I had not yet internalized.

  1. The Txid type is just a typealias on String until you use it in a method that requires an actual Txid. At that point it attempts the conversion, and panics if it can't do it.
  2. This panic is not catchable by the target languages.

Example

If you use a valid Txid, the new type works perfectly, and allows type-safe API on a method like get_tx(txid: Txid). Does the conversion, provides it to the Esplora client in this case, and everything is dandy.

pub fn get_tx(&self, txid: Txid) -> Result<Option<Arc<Transaction>>, EsploraError> {
    let tx_opt = self.0.get_tx(&txid)?;
    Ok(tx_opt.map(|inner| Arc::new(Transaction::from(inner))))
}
// This works
val txid: Txid = "9d0a83df4a556d61338ff18cbc20bf49a3c732805e404a66e25e1579271597a9"
val transaction: Transaction? = esploraClient.getTx(txid)
println("Transaction: ${transaction?.computeTxid()}")

But if your Txid is not valid, you'll get a panic which is not thrown on the JVM side, and therefore cannot be caught.

This crashes your test/app
try {
    val txid: Txid = "imatxidiswear"
    val transaction: Transaction? = esploraClient.getTx(txid)
    println("Transaction: ${transaction?.computeTxid()}")
} catch (t: Throwable) {
    println("Panic: $t") // you'll never get here
}

I just realized this a few hours ago and haven't looked at how/whether we could solve it using the

[Custom]
typedef string Txid;

construction with a Result or if the only way is to have a proper constructor on the Txid type. I'll take a look at this next week!

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

No branches or pull requests

1 participant