bitcoindevkit / bitcoindevkit/bitcoin-ffi

`Txid` type is currently kind of a footgun

Open
#20 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
13
Forks
11
PR merge metrics
No merged PRs in 30d

Description

After trying to use the `Txid` type in the testing of [this pr]( https://github.com/bitcoindevkit/bdk-ffi/pull/598), 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.

```rust
pub fn get_tx(&self, txid: Txid) -> Result>, EsploraError> {
let tx_opt = self.0.get_tx(&txid)?;
Ok(tx_opt.map(|inner| Arc::new(Transaction::from(inner))))
}
```

```kotlin
// 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.

```kotlin
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
```txt
[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!

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by tracing the Txid typedef and its conversion at methods such as get_tx, then compare the proposed Custom typedef with a Result against a proper Txid constructor. Done means invalid Txid values produce an error that target languages can catch instead of an uncaught panic.

Written by the indexing model from the issue text.

Assessment

Tech stack
kotlin, rust
Domain
api
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.