search

JoinMarket-Org / joinmarket-clientserver

Bitcoin CoinJoin implementation with incentive structure to convince people to take part
GNU General Public License v3.0
731 stars 179 forks source link

Timelocked addresses BIP #993

Open chris-belcher opened 3 years ago

chris-belcher commented 3 years ago

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 commented 3 years ago

I could do this.

chris-belcher commented 2 years ago

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 commented 2 years ago

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: https://github.com/JoinMarket-Org/joinmarket-clientserver/pull/1256

AdamISZ commented 2 years ago

'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:

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

BIP proposal: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-May/020389.html

chris-belcher commented 2 years ago

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.