bips icon indicating copy to clipboard operation
bips copied to clipboard

bip-0046: Address Scheme for Timelocked Fidelity Bonds

Open theborakompanioni opened this issue 1 year ago • 9 comments

This is a continuation of @chris-belcher's PR#1341 titled "Derivation scheme for timelocked address fidelity bond based accounts", as Chris will quite possibly not be able to progress this any further. Please see comments in the PR, especially https://github.com/bitcoin/bips/pull/1341#issuecomment-2112742401:

It will probably be the easiest to open a new PR that supersedes this one.

TBD (from https://github.com/bitcoin/bips/pull/1341#pullrequestreview-2055394826):
  • [x] Assign BIP number (https://github.com/bitcoin/bips/pull/1341#issuecomment-2112743358)
  • [x] Copyright section
  • [x] Backwards Compatibility section
  • [x] Rationale section

As soon as all missing points are addressed, #1341 can be closed and this PR un-drafted. Happy for any and all feedback, particularly when it comes to finalizing it. :pray:

theborakompanioni avatar May 23 '24 12:05 theborakompanioni

@murchandamus Addressed every raised point and will mark this as ready for review soon. It would be nice to know if everything has been addressed in a satisfactory manner and if this–after taking additional feedback into account–meets the official standard and guidelines. I hope I managed to keep it as simple and straightforward as possible, with minimal changes to the original document and in the best interest of @chris-belcher.

Thank you again for your support.

Edit: As the CI checks fail, should I create the table row entry in the readme file myself?

theborakompanioni avatar May 27 '24 08:05 theborakompanioni

Ping @kristapsk @AdamISZ @openoms. Would you possibly be so kind as to read the document once again and see if there are no obvious mistakes or if something has been overlooked? :pray:

theborakompanioni avatar May 28 '24 09:05 theborakompanioni

Hi @theborakompanioni , thanks for this effort.

It also defines how to sign fidelity bond certificates, which are needed when using fidelity bonds that are stored offline.

I don't think this should be in the document. See:

You've convinced me that specifying the exact form of the fidelity bond certificate is a bad idea. I'll leave it more general, saying just that wallets should be able to do SignMessage using the timelocked privkey. And I'll leave the example signature in the test vectors.

(etc.) from here.

Now, to be clear, I think this has already been addressed in an edit to the BIP language. Notice it has a section on Signing Messages which only refers to general principles, not to a specific format for a certificate. So my point here is only that the phrase "it defines how to sign fidelity bond certificates" should be removed. Instead perhaps "It also gives advice to wallet developers on how to use fidelity bonds to sign over messages, such as certificates".

It would be useful to be able to keep the private keys of fidelity bonds in cold storage. This would allow the sybil resistance of a system to increase without hot wallet risk.

(minor): Language/stylistic nit: I don't believe these standard defining documents should have "would" type statements; that's about proposals, not definitions. So here for example you'd write "It is very useful to be able ...", "This allows ..".

\x18Bitcoin Signed Message:\n

(very minor): is there another more unambiguous way to write the starting byte?

I have a more general comment which I think might be very important in practice, but I'm not sure how to address:

the document defines two things specifically, as it has to: first, the BIP32 derivation and second, the time intervals.

  • For the first point, would it be more natural to choose a specific account? (See BIP44), than a third branch (2) at the lowest level, deviating from the BIP32 standard structure of external/internal (deposit/change)? My question here is somewhat academic, this has been in place in Joinmarket for a few years. Most wallet developers wouldn't struggle with it, but it is a significant deviation from expected wallet structure.

  • For the second point, somewhat similar I guess: a specific time interval (one month) and a specific number of months feels like it's too inflexible. To whatever extent use-cases exist for timelocked fidelity bonds outside joinmarket, it might be the case that this specific schedule is not at all workable.

These two points seem like a perfect scenario for wallet descriptors, right? We have here a custom scriptPubKey format, a custom derivation, and even more, as per the above, we might realistically need to parameterise that customisation. But unfortunately I don't know if it is even possible to use wallet descriptors for this - as of this discussion, I believe the answer was no, and .. is it still no?

I guess this could be "BIP46 timelocked outputs" which wallets could support, and then another standard exists that is more flexible and supports a wider variety of use cases than just Joinmarket, later?

AdamISZ avatar May 28 '24 14:05 AdamISZ

Hi @theborakompanioni , thanks for this effort.

It also defines how to sign fidelity bond certificates, which are needed when using fidelity bonds that are stored offline.

I don't think this should be in the document. See:

You've convinced me that specifying the exact form of the fidelity bond certificate is a bad idea. I'll leave it more general, saying just that wallets should be able to do SignMessage using the timelocked privkey. And I'll leave the example signature in the test vectors.

(etc.) from here.

Now, to be clear, I think this has already been addressed in an edit to the BIP language. Notice it has a section on Signing Messages which only refers to general principles, not to a specific format for a certificate. So my point here is only that the phrase "it defines how to sign fidelity bond certificates" should be removed. Instead perhaps "It also gives advice to wallet developers on how to use fidelity bonds to sign over messages, such as certificates".

Agreed, changed! Thanks :pray:

It would be useful to be able to keep the private keys of fidelity bonds in cold storage. This would allow the sybil resistance of a system to increase without hot wallet risk.

(minor): Language/stylistic nit: I don't believe these standard defining documents should have "would" type statements; that's about proposals, not definitions. So here for example you'd write "It is very useful to be able ...", "This allows ..".

Agreed, changed! :+1:

\x18Bitcoin Signed Message:\n

(very minor): is there another more unambiguous way to write the starting byte?

Do you think 0x18 || "Bitcoin Signed Message:\n" is less ambiguous?

I have a more general comment which I think might be very important in practice, but I'm not sure how to address:

the document defines two things specifically, as it has to: first, the BIP32 derivation and second, the time intervals.

  • For the first point, would it be more natural to choose a specific account? (See BIP44), than a third branch (2) at the lowest level, deviating from the BIP32 standard structure of external/internal (deposit/change)? My question here is somewhat academic, this has been in place in Joinmarket for a few years. Most wallet developers wouldn't struggle with it, but it is a significant deviation from expected wallet structure.

A specific account does not feel more natural to me. What do others think about this?

  • For the second point, somewhat similar I guess: a specific time interval (one month) and a specific number of months feels like it's too inflexible. To whatever extent use-cases exist for timelocked fidelity bonds outside joinmarket, it might be the case that this specific schedule is not at all workable.

That is true. However, as mentioned in the document, this enables users to not backup timelock values.

Importantly the user does not need to backup any timelock values.

So I suppose this is a justified compromise between functionality and practicability. Is it?

These two points seem like a perfect scenario for wallet descriptors, right? We have here a custom scriptPubKey format, a custom derivation, and even more, as per the above, we might realistically need to parameterise that customisation. But unfortunately I don't know if it is even possible to use wallet descriptors for this - as of this discussion, I believe the answer was no, and .. is it still no?

As mentioned in your link and here, JoinMarket would need to be adapted. I do not know how feasible such a change would be (also, it requires supporting the other script as well for some time, probably forever). I think this is the one (only?) remaining question to really focus on. I am agnostic to the outcome.

I guess this could be "BIP46 timelocked outputs" which wallets could support, and then another standard exists that is more flexible and supports a wider variety of use cases than just Joinmarket, later?

:dart: :100:

theborakompanioni avatar Jun 06 '24 12:06 theborakompanioni

Do you think 0x18 || "Bitcoin Signed Message:\n" is less ambiguous?

Yeah let's go with that. Again, very minor, but why not.

As mentioned in your link and here, JoinMarket would need to be adapted.

I'm really not sure.

I don't think I can pursue further with making any suggestion here until I've understood one point, as per the above stackexchange conversation; specifically, the quote

"Strictly speaking, with just descriptors as specified in the documentation and the BIPs, it is currently not possible to create a descriptor with a timelock."

from @achow101 . Sure, miniscript produces a slightly different script than the one Joinmarket implemented, but why can't there be the facility within a wallet descriptor to just specify the custom scriptPubKey? If it will be possible, then maybe we need to wait for that, if it won't be possible, then, I guess ignore this whole line of thinking in this document and just keep everything as a rigidly defined set of times and BIP32 derivations. After all, it is correct for the isolated case of Joinmarket (but it seems a shame to write a BIP that only refers to one piece of software!).

AdamISZ avatar Jun 06 '24 16:06 AdamISZ

Do you think 0x18 || "Bitcoin Signed Message:\n" is less ambiguous?

Yeah let's go with that. Again, very minor, but why not.

Changed 👍

As mentioned in your link and here, JoinMarket would need to be adapted.

I'm really not sure.

I don't think I can pursue further with making any suggestion here until I've understood one point, as per the above stackexchange conversation; specifically, the quote

"Strictly speaking, with just descriptors as specified in the documentation and the BIPs, it is currently not possible to create a descriptor with a timelock."

from @achow101 .

Yep, @achow101's input and opinion on that would be helpful and highly appreciated :pray:

Sure, miniscript produces a slightly different script than the one Joinmarket implemented, but why can't there be the facility within a wallet descriptor to just specify the custom scriptPubKey? If it will be possible, then maybe we need to wait for that, if it won't be possible, then, I guess ignore this whole line of thinking in this document and just keep everything as a rigidly defined set of times and BIP32 derivations.

Agree :100:%

After all, it is correct for the isolated case of Joinmarket (but it seems a shame to write a BIP that only refers to one piece of software!).

I always appreciate your writing and your way of thinking. With this, I just want to add one point, which is, that it is already extremely valuable for users to have a way to lock up coins for a specific amount of time, without the requirement of backing up any additional information. Having this widely implemented in wallets (when users urge the devs to implement it–or even switch wallets if their favorite software does not support it) is great, even if users subsequently do not make use of the additional benefits or functionality they'll get by "proof of timelocked coins" (e.g. as Fidelity Bonds).

theborakompanioni avatar Jun 07 '24 10:06 theborakompanioni

This PR had a merge conflict with the master branch due to BIP 47 getting updated to final. I merged master into this PR to resolve the merge conflict.

It sounds like you may still be collecting feedback from @AdamISZ and possibly @achow101. Please let us know when you would like an editor to do another review.

murchandamus avatar Jun 07 '24 15:06 murchandamus

This PR had a merge conflict with the master branch due to BIP 47 getting updated to final. I merged master into this PR to resolve the merge conflict.

Nice. Thanks :pray:

It sounds like you may still be collecting feedback from @AdamISZ and possibly @achow101. Please let us know when you would like an editor to do another review.

Yes, I will request another review when the last remaining details have been clarified. Thank you @murchandamus

theborakompanioni avatar Jun 07 '24 18:06 theborakompanioni

Hey @murchandamus. Though not all options and eventualities have been fully investigated, they have been discussed well enough, therefore: Formally requesting a review. :pray:

theborakompanioni avatar Jun 13 '24 08:06 theborakompanioni

Thank you for hanging in there @theborakompanioni, and for having @chris-belcher’s back.

murchandamus avatar Jul 12 '24 12:07 murchandamus

This goes against the spec in BIP-43, BIP-44 and BIP-84.

Should have picked its own purpose, i.e. m/46’ and not modify already existing ones.

prusnak avatar Jul 12 '24 13:07 prusnak

This goes against the spec in BIP-43, BIP-44 and BIP-84.

Should have picked its own purpose, i.e. m/46’ and not modify already existing ones.

Very good point to raise, surprised it was never mentioned before.

Obviously the original author's intention was to "attach" these fidelity bonds to a specific wallet (and notably: though we implemented several address types in JM, we never implemented multiple address types in a single wallet), hence a new account within an existing BIP44 structure was chosen. But I do agree that alternative purpose makes a lot more sense.

Given that as discussed at length above, this document only really currently makes sense as a standardization of Joinmarket wallets, I suggest we "half"-resolve it by adding something like:

To derive a public key from the root account, this BIP uses a similar account-structure as defined in BIP [44](https://github.com/theborakompanioni/bips/blob/4f788d69f501d26f6c88db7fe0cf19696b608059/bip-0084.mediawiki) but with change set to 2.

m / [0/84/49]' / 0' / 0' / 2 / index

The purpose field should match the wallet into which the fidelity bond is being placed (e.g. 49' for p2sh-p2wpkh, etc.). A key derived with this derivation path pattern will be referred to as derived_key further in this document...

(side note: I guess you need to replace the BIP44 link there)

This isn't too relevant today as non-84 wallets are all but deprecated but any future version of Joinmarket may include taproot, so, probably not irrelevant in that sense.

AdamISZ avatar Jul 12 '24 15:07 AdamISZ

It is quite difficult to find a balance between these things and each approach has its pros and cons. There are reasons and good arguments for both of these. I'm agnostic about the outcome, so it would be nice to gather workable suggestions so that a wallet can call itself BIP46-compatible and the "Status" of the document can move from "Draft" to "Proposed" or "Finalized" eventually.

Should have picked its own purpose, i.e. m/46’ and not modify already existing ones.

Besides the adaption of the purpose field of derivation path to m/46’ an additional change has been discussed to consider using script <derived_key> OP_CHECKSIGVERIFY <timelock> OP_CHECKLOCKTIMEVERIFY–effectively saving the OP_DROP statement.

If it were to be changed, @AdamISZ it would be great for JM to support both, the (then) "old" approach, and the new agreed upon approach in this BIP. Even if it means that

the original author's intention was to "attach" these fidelity bonds to a specific wallet

does not hold true any more (in the sense the statement is meant).

theborakompanioni avatar Jul 15 '24 14:07 theborakompanioni