starknet-rs icon indicating copy to clipboard operation
starknet-rs copied to clipboard

Unify `pub enum TransactionStatus` variants

Open tcoratger opened this issue 1 year ago • 1 comments

At the moment, we have multiple TransactionStatus enum variants around the codebase:

https://github.com/xJonathanLEI/starknet-rs/blob/4dbc1692d4f295de803484181e07803201528922/starknet-core/src/types/mod.rs#L178-L183

https://github.com/xJonathanLEI/starknet-rs/blob/4dbc1692d4f295de803484181e07803201528922/starknet-core/src/types/codegen.rs#L1536-L1545

https://github.com/xJonathanLEI/starknet-rs/blob/4dbc1692d4f295de803484181e07803201528922/starknet-providers/src/sequencer/models/transaction_receipt.rs#L35-L51

I think this should be unified, probably in core to have a single variant for this purpose.

tcoratger avatar Jun 18 '24 14:06 tcoratger

In an ideal world there should probably just be one. However:

  1. The sequencer-specific type exists because the sequencer apparently indeed returns more variants than what's seen on JSON-RPC. If we unify everything into a single type we will have to include those sequencer-specific variants, which means someone trying doing a match expression would have to deal with these extra variants when using JSON-RPC (as they're supposed to).
  2. The TransactionStatus type is the idiomatic wrapper for the finality_status and execution_status fields. It combines these 2 fields so that users who match can directly access the execution status without an unnecessary unwrap. This is different from SequencerTransactionStatus, which is pretty much just a C-style enum.
  3. On the other hand, SequencerTransactionStatus is also necessary as it's needed to support NoTraceAvailableErrorData, which only gives you the finality status, so it cannot be replaced by TransactionStatus.

I don't like it either, but I don't really see a way out of this mess.

xJonathanLEI avatar Jun 20 '24 00:06 xJonathanLEI

As explained there doesn't seem to be an apparent clean solution. Closing this for now.

xJonathanLEI avatar Oct 21 '24 14:10 xJonathanLEI