pcsc-rust icon indicating copy to clipboard operation
pcsc-rust copied to clipboard

Allow a transaction to own the underlying card

Open nwalfield opened this issue 3 years ago • 6 comments
trafficstars

Imagine that you have a small virtual machine, which is implemented as a state machine. It might be driven by a program like:

open SERIAL_NUMBER
begin-transaction
read-do 0x63
end-transaction

The state machine's state can't easily hold both the card and the transaction, because Rust doesn't like self-referential data structures. To work around this, the transaction could own the card.

nwalfield avatar Feb 16 '22 16:02 nwalfield

Note: the provided patch changes the API. As such, it would be necessary to bump the version to 3.0.

nwalfield avatar Feb 16 '22 16:02 nwalfield

For more context, please see: https://gitlab.com/hkos/openpgp-card/-/issues/17

nwalfield avatar Feb 16 '22 16:02 nwalfield

Hi @nwalfield, I've been thinking about this issue. Let me lay it out chronologically...

  1. I completely understand the problem.
  2. First I tried to think of ways to make it work (with things like Pin and whatnot), but I am not a Rust expert and it's probably too complicated right now.
  3. Then I started to acquiesce to your idea of having an owned transaction variant.
  4. I would like to avoid a breaking change, and the Borrow stuff is again too complicated for my taste, so I think it would be better to just have card.transaction_owned which returns a new type TransactionOwned.
  5. But this is starting to get really inelegant...

So my final thought is: your use case requires sufficient flexibility that the Rust borrow checker is just not worth dealing with. I think, let's keep Transaction as is, for the straightforward lexical-scope cases where the Rust borrow checker provides nice static guarantees, but expose card.begin_transaction()/card.end_transaction() functions (which take &self and don't return anything) for the more complicated cases. PCSC does check that the API is used correctly dynamically so there is unsafety here. We should just make it clear in the docs of these functions that transaction() is a better alternative, when it's possible to use it.

What do you think?

bluetech avatar Mar 05 '22 10:03 bluetech

Hi @bluetech,

Thanks for following up.

I like option 4: having pssc::Card::transaction_owned take ownership of the pcsc::Card and return a TransactionOwned would indeed solve the problem and it wouldn't cause an API break. Happily Transaction doesn't expose must functionality so there will be at most minimal redundancy.

Option 6 (your final thought), I find worrying, because it is less type safe. Option 4 means that the API prevents the user from attempting to start a second transaction at compile time, because transaction_owned takes ownership of the pcsc::Card. Option 6's begin_transaction, however, just takes (and presumably doesn't hold) a reference, so that could only be caught at runtime.

I don't think option 4 adds much complexity. If you agree, I'll implement it.

Thanks in advance!

nwalfield avatar Mar 09 '22 13:03 nwalfield

I agree that the static safety provided by Transaction is great. But TransactionOwned which takes ownership of the card feels wrong to me, and is also a bit tricky to use since the user must make sure not to rely on Drop, and Rust currently doesn't support "turning Drop off" (linear types).

I think that option 6 is the best way forward, since it allows users when Transaction is not viable to build their own abstraction. In your state machine for example it seems natural for the state machine to control the dynamic lifetime of the transaction rather than the Rust type itself.

bluetech avatar Mar 13 '22 11:03 bluetech

If the owner of the transaction calls drop (implicitly or explicitly) without getting the owned Card back, either it is a bug or intentional. If it is a bug, then the bug will be caught at compile time, because the compiler will complain that the Card has been moved when the developer tries to use the Card again. That's a clear benefit over option 6.

It would make sense to provide option 6, if option 6 is more expressive. I'm having trouble envisioning a case where option 6 can be used to do something useful, which option 4 can't do. In other words, it seems to me that option 4 can do everything that option 6 can, but provides more safety. Do you have something in mind?

nwalfield avatar Mar 13 '22 15:03 nwalfield