Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Timelocked addresses BIP #993

Open
chris-belcher opened this issue Aug 19, 2021 · 8 comments
Open

Timelocked addresses BIP #993

chris-belcher opened this issue Aug 19, 2021 · 8 comments
Labels
protocol related to the communication between maker / taker

Comments

@chris-belcher
Copy link
Contributor

chris-belcher commented Aug 19, 2021

Someone should write a BIP that documents the timelocked addresses used in joinmarket fidelity bonds.

Since we use the BIP39 seed phrase standard, this BIP would join the family of BIP44 / BIP49 / BIP84 standards.

Such a BIP might be useful to allow other wallets to be able to recover joinmarket fidelity bonds addresses. Also, if joinmarket ever gets the ability to use fidelity bonds stored in cold storage, then those fidelity bonds could be stored on a hardware wallet (although users should make sure they don't dox themselves to the hardware wallet's address lookup servers or any electrum servers)

@kristapsk
Copy link
Member

I could do this.

@chris-belcher
Copy link
Contributor Author

chris-belcher commented Mar 20, 2022

Right now one of the things I'm working on is adding fidelity bonds to Teleport (the coinswap project). I want it to use the same format so that the same fidelity bonds can be used for both Teleport and JoinMarket. Since I'm currently delving again into the details I may as well write this BIP. I've already found most of the information and implemented some of the routines in rust.

@chris-belcher
Copy link
Contributor Author

chris-belcher commented Apr 26, 2022

I've been working on a BIP for this, and I found something less-than-ideal which would be nice to fix as soon as we can. If we fixed this issue then it becomes a lot easier for devs of other wallets (hardware wallets, electrum, sparrow, etc) to implement this BIP, and that's good for users because it means its more likely for more wallets to support recovery of their fidelity bonds.

This change involves a backward-compatible protocol change between takers and makers, so we should move fast to get the code out there as soon as possible. The sooner takers are running this code, the less likely it is for old non-upgraded takers to still be around when people start using fidelity bonds in cold storage.

As a recap; the cert_msg is a small bit of data that gets signed by the private key of the timelocked address. Other implementations of this BIP must be able to do this in order to be useful for holding fidelity bonds in cold storage.

Right now the cert_msg involves a public key in binary. For example:

'fidelity-bond-cert|\x03\xcb\x17,\xc3\xc6\x117F\xbc\x99\x8a\xdf\xeejJ\xcde\x92\xa1V\xabX\x9f8\xac\x86\xd8U~.R\x11|1'

It would be really nice if that public key was in ascii instead, then the whole message would be entirely ascii, for example:

'fidelity-bond-cert|03cb172cc3c6113746bc998adfee6a4acd6592a156ab589f38ac86d8557e2e5211|1'

Other wallets who might implement this BIP already support signing a message with a private key. If the cert_msg was entirely ascii then it would allow the user to copypaste it into the "Sign Message" interface. This would make it much much easier for other wallet devs to implement our BIP, because they can just reuse the Sign Message interface that they already have. If we keep using the binary version then it cant be copypasted and the other wallet devs have to make a special "Sign Fidelity Bond Cert Message" interface just for us.

Edit: I wrote code which does this: #1256

chris-belcher added a commit that referenced this issue Apr 26, 2022
Previously the `cert_msg` in the fidelity bond protocol involves a
binary public key. This is additional unnecessary complexity given
that we want other wallets to also implement the fidelity bond
protocol (see issue #993 for full discussion). So its good to move
to a certificate message which is entirely ascii.

This commit has ascii certficiate messages also be accepted as valid
along with the old certificate messages.
@AdamISZ AdamISZ added the protocol related to the communication between maker / taker label Apr 27, 2022
@AdamISZ
Copy link
Member

AdamISZ commented Apr 27, 2022

'fidelity-bond-cert|03cb172cc3c6113746bc998adfee6a4acd6592a156ab589f38ac86d8557e2e5211|1'

It does seem like there's a lot of possibilities here.

Did you look into how TLS/PKI does this? Afair X509/DER/ASN.1 is a bit of a dumpster fire (well ASN.1 at least), so certainly not suggesting that mess (see X509), but as a general principle might we want to:

  • Have as compact an encoding as possible
  • Use fields with compact types (rather than ascii names) - individual bytes and/or bytes with bit flags, that allow for future extensions
  • Create an encoding that naturally fits a 'certification/verification path' (not sure the right name), or even conceivably a tree, like how in PKI they allow you to trace to a root cert along multiple hops, not only 2
  • Do we have to consider the concept of revocation here? For example a user choosing to repurpose a FB utxo from one usage to another (I really don't know, just throwing it out there)

There's also the really tricky question of 'domain separation tags' - something pretty common in cryptography nowadays afaik, but a good concrete example of it is the tagging used in BIP340/341. I find this pretty slippery conceptually - is reuse of a FB in different domains possible, and should it be possible (different questions, I guess).

I could imagine all these issues being raised for a proposed standard, and probably several other important points that I haven't considered at all ... but then that's the entire point of a BIP process I guess :)

Re: the exact suggestion you're making here, it seems reasonable to make the entire binary string be encoded ascii, if I understand your point, that makes it one consistent encoding so less confusing/messy for other implementers? It is a bit longer though, which is a negative.

@chris-belcher
Copy link
Contributor Author

chris-belcher commented Apr 28, 2022

Re: the exact suggestion you're making here, it seems reasonable to make the entire binary string be encoded ascii, if I understand your point, that makes it one consistent encoding so less confusing/messy for other implementers? It is a bit longer though, which is a negative.

The message I'm talking about is just the form taken before sending to the verify() function. It's not sent over the wire anywhere, so it's length doesn't matter.

But yes, the intention is to make the standard be as easy as possible for other implementers to add.

All those other suggestions you write about require another protocol change, which I was hoping to avoid as much as possible. because protocol changes are hard. This BIP is more about documenting the situation as it is, warts and all, rather than designing a new one.

@AdamISZ
Copy link
Member

AdamISZ commented Apr 28, 2022

The message I'm talking about is just the form taken before sending to the verify() function. It's not sent over the wire anywhere, so it's length doesn't matter.

Oops, my bad. I've just gone back and read the code, I'd forgotten that this cert_msg is not part of the published proof (i.e. not part of the serialization of FidelityBondProof, but implicit and reconstructable). So yeah absolutely no issue there.

@chris-belcher
Copy link
Contributor Author

@chris-belcher
Copy link
Contributor Author

This guy found a way to reduce the size of the redeemscript by one opcode: https://gist.github.com/chris-belcher/7257763cedcc014de2cd4239857cd36e?permalink_comment_id=4151609#gistcomment-4151609 It's too late to change now though, whoops! It's not too big of a deal though, as its only one byte, which is 0.25 vbytes as its witness data, and people only make timelock address spends once every few months or years.

takinbo pushed a commit to takinbo/joinmarket-clientserver that referenced this issue May 27, 2022
Previously the `cert_msg` in the fidelity bond protocol involves a
binary public key. This is additional unnecessary complexity given
that we want other wallets to also implement the fidelity bond
protocol (see issue JoinMarket-Org#993 for full discussion). So its good to move
to a certificate message which is entirely ascii.

This commit has ascii certficiate messages also be accepted as valid
along with the old certificate messages.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
protocol related to the communication between maker / taker
Projects
None yet
Development

No branches or pull requests

3 participants