search

boltlabs-inc / zeekoe

Zero-knowledge layer-2 payment channels
MIT License
24 stars 1 forks source link

Add verified revocation pair #328

Closed indomitableSwan closed 2 years ago

indomitableSwan commented 2 years ago

This PR implements the downstream changes necessary to incorporate verified revocation pairs into zkabacus (https://github.com/boltlabs-inc/libzkchannels-crypto/pull/178).

There are now two types of database insertions in the pub trait QueryMerchant for revocation info, namely insert_revocation_lock and insert_revocation_pair; these are built using the existing logic in insert_revocation, which has been moved out of the pub trait. @yaymukund is that the best way to handle this change?

Additionally, the customer now sends a verified revocation pair to the merchant, instead of a lock and a secret.

Aside: I'm not familiar with async/await, so I'm not sure I handled those parts properly.

indomitableSwan commented 2 years ago

There are now two types of database insertions in the pub trait QueryMerchant for revocation info, namely insert_revocation_lock and insert_revocation_pair; these are built using the existing logic in insert_revocation, which has been moved out of the pub trait. @yaymukund is that the best way to handle this change?

I recommend keeping the original database API and instead splitting up the RevocationPair at the callsite, for a couple reasons:

  • insert_revocation_pair() is unlikely to be invoked many times, so adding logic to its invocation doesn't result in much repetition (well, no repetition right now, because there's exactly one invocation!)
  • The DB interface is likely to change when we add support for PostgreSQL. It would be nice to avoid adding logic there.

I had your suggested approach originally, but after discussion with @marsella, switched to this approach. Why do you say that insert_revocation_pair() is unlikely to be invoked many times?

Aside: I'm not familiar with async/await, so I'm not sure I handled those parts properly.

Your use of async/await looks good to me!

Good!

yaymukund commented 2 years ago

Oops, I misspoke. I meant it's unlikely to be invoked from many places. There's presently only one place we call it. Do you anticipate us invoking insert_revocation_pair() from another part of the code?

indomitableSwan commented 2 years ago

Oops, I misspoke. I meant it's unlikely to be invoked from many places. There's presently only one place we call it. Do you anticipate us invoking insert_revocation_pair() from another part of the code?

No, I don't.

Going back to original comment, I am still unclear which approach is more desirable. Given that when you insert a revocation lock and a secret, the meaningful object is really the full pair, I have a preference for keeping insert_revocation_lock and insert_revocation_pair.