search

mobilecoinfoundation / mobilecoin

Private payments for mobile devices.
Other
1.16k stars 148 forks source link

Offline Transaction Support #1963

Open jcape opened 2 years ago

jcape commented 2 years ago

We should refactor our existing transaction builder, located in transaction core, so the interactions with the user's private keys (and therefore, spend authority) can live offline, in a custom HSM software and/or a hardware wallet.

The first step is going through the existing TransactionBuilder and identifying the places where we're using our private keys:

When we want to spend an existing TxO, we feed the InputCredentials structure into the TransactionBuilder::add_input() method, which appends it to a Vec of these inputs.

The InputCredentials structure contains a vector of TxOs next to a vector of membership proofs, along with a "real index" into both vectors. The onetime_private_key (which is stored as a RistrettoPrivate, and not it's own type) is generated from the TxO public key, the user's view private key, and the user's spend private key.

Under the reasonable assumption that the onetime_private_key will need to be kept offline---anyone who knows the onetime_private_key can spend the generated TxO, regardless of whether they have the destination wallet's private keys, this seems to indicate that the InputCredentials object will need to be refactored in such a way as to not take the user's private key.

The user's secret material will need to be merged with the TxOs and membership proofs provided to generate the onetime_private_key. This also necessarily means that any further use of the onetime_private_key will also need to happen in the "offline" system.

A reasonable (and small) first step is to refactor the onetime_private_key into it's own concrete newtype wrapper around RistrettoPrivate. In addition to preventing logical confusion errors, doing this without implementing AsRef<RistrettoPrivate> or Deref<RistrettoPrivate> will make any use of OnetimePrivateKey in the codebase into a compiler error, and identify any other code within the transaction builder which needs to live in the HSM.

cbeck88 commented 2 years ago

I think we may have resolved this here: https://github.com/mobilecoinfoundation/mobilecoin/pull/1977

This changed input credentials so that it only required OnetimeKeyDerivateData rather than the one time private key: https://github.com/mobilecoinfoundation/mobilecoin/pull/1977/files#diff-3ca1b5663f720f83940365f886ddfddcc77e79891237a7245515f8c06d32af75R41

The "ring signer" trait either uses the one-time key provided or derives the one-time key using the data and the account keys.

That PR did not do this step:

A reasonable (and small) first step is to refactor the onetime_private_key into it's own concrete newtype wrapper around RistrettoPrivate. In addition to preventing logical confusion errors, doing this without implementing AsRef or Deref will make any use of OnetimePrivateKey in the codebase into a compiler error, and identify any other code within the transaction builder which needs to live in the HSM.

But that could be done in a separate PR. So maybe this ticket should track that work, or maybe we should close it and make a new ticket, for the wrapper type described.