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

Initial Templates Proposal #2

Closed
wants to merge 1 commit into from
Closed

Initial Templates Proposal #2

wants to merge 1 commit into from

Conversation

Rob1Ham
Copy link
Contributor

@Rob1Ham Rob1Ham commented Sep 15, 2023

I've created 3 initial template documents:

  • mint-000 - An initial proposal on best practices with using time locks with miniscript. This is an attempt to start off on the right foot with established best practices in how to leverage time locks within miniscript templates. It also includes some brief documentation on how to leverage relative time locks utilizing epoch timestamps, which I've found no documentation for previously.

  • mint-001 - What has been referred to as a "degrading multisig" or "decaying multisig" previously, this is a 2 of 3 multisig which after a time lock passes becomes a 1 of 3 multisig. This is composed as a threshold of 4 conditions, 3 keys and one timelock value, where 2 conditions must be satisfied to spend bitcoin in the descriptor.

  • mint-002 - Similar to template 001, this is a 3 of 5 multisig which has two time lock thresholds that can enable a descriptor to eventually become a 1 of 5 multisig. This is achieved by leveraging a threshold of 7 conditions (5 keys, and 2 timelocks), where 3 are required to spend the bitcoin.

As these are the first proposed templates, I'm very open to feedback and edits on details I might have missed, or suggestions on how to improve them.

[EDIT on 11/19/23] - To reflect new links to the files with the new naming convention of MinT

==Absolute Epochtime based Timelocks==

*Since the goal of an epoch timestamp timelock is to align events with wall time instead of block time, epoch timestamps used should be of signifigance. Since there is a much larger searchspace with epoch timestamp timelocks, I'm proposing Epoch Timestamp based Timelocks should be set to either Noon, or Midnight GMT.
*In practice, this means that absolute epoch timelocks should be divisible by either `43,200` (Noon GMT) or `86,400` (Midnight GMT).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In 4b75129:

You could go further and suggest midnight on Thursday (multiple of 604800) as an even-more preferable multiple. Though what's written is fine.


* An Epoch Timestamp is not valid until the network's MTP (Median Time Past) of the past 11 blocks is greater than the epoch timestamp, MTP is defined in: [https://github.com/bitcoin/bips/blob/master/bip-0113.mediawiki BIP113].

* Since the 512 second incrimentor can not align exactly to a 24 hour day, for consistent recovery sake, relative epoch timestamps should be divisible by 100.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In 4b75129:

incriment should be increment (in two places).

Also, FWIW, 512 is a divisor of four days. You could maybe suggest using multiples of 675? Or "675 or 100"? Or 2700 (16 days) which is the LCM :P. Just suggestions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reflecting on this, I think if someone is going to use an epoch time relative timelock, it should be grounded in something is relatable in epoch time. So 675 to represent which neatly folds into 4 day relative timelock is better. It can be more cleanly represented in software and hardware wallet interfaces, and is a unit of time that people can reason about much easier than 100 units of 14.2 hours.

Additionally, the values for an epoch time relative timelock inolder()does not start on 0, so it is going to look unintuive at face value for the output descriptor no matter what.


The 2 of 3 Multisig has become a standard in Multisignature solutions. It offers a well-balanced mix of redundancy, security, flexibility, and practicality for securing Bitcoin. Losing one key is not disasterous, and keys can be distributed for better security.

With Miniscript, the addition of time as a security feature enables further disaster recovery such that if two keys were to be lost, a timelock can be employed to allow the wallet to become a 1 of 3 multisig. This can be employed by using a miniscript `tresh()` with four conditions:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In 4b75129:

type tresh should be thresh

@apoelstra
Copy link
Member

Overall 47ace7e looks good! Worth running a spellchecker (I noticed a couple mistakes but wasn't really looking for more) and maybe to remove some of the first-person language like "I propose". You can also squash all the commits together since formatting changes don't really need their own commits.

@Rob1Ham
Copy link
Contributor Author

Rob1Ham commented Sep 16, 2023

Excellent! I'll integrate your suggestions and clean up spelling.

@apoelstra
Copy link
Member

I'm unsure about the rename to "master keys" -- I worry that this term is too general and it might get confused with "master seed" from BIP32 (or the xpub that you get from deriving the root key of a master seed).

I'll hold off on reviewing 09a2cf3 since it's labelled DRAFT. Maybe it should be moved to a different PR?

@Rob1Ham
Copy link
Contributor Author

Rob1Ham commented Sep 19, 2023

Overall 47ace7e looks good! Worth running a spellchecker (I noticed a couple mistakes but wasn't really looking for more) and maybe to remove some of the first-person language like "I propose". You can also squash all the commits together since formatting changes don't really need their own commits.

I squashed the formatting commits, and will take another pass at cleaning up the formatting for the initial templates as well. As far as

I'm unsure about the rename to "master keys" -- I worry that this term is too general and it might get confused with "master seed" from BIP32 (or the xpub that you get from deriving the root key of a master seed).

Understood, happy to revisit this, I'll make an edit and comment here tomorrow.

I'll hold off on reviewing 09a2cf3 since it's labelled DRAFT. Maybe it should be moved to a different PR?

The descriptor as drafted is ready to go, as in its something we've been designing and intended to use within our systems. It is very similar to the proposed template 003, I have it as "draft" because I was trying to think of a better name than something very general like "Multi Institutional Custody". I'll remedy this tomorrow as well as review all of the existing repos and clean up some formatting.

@Rob1Ham
Copy link
Contributor Author

Rob1Ham commented Sep 20, 2023

Cleaned up templates Template 000 , Template 001 and Template 002 to remove 1st person language, a general pass on spelling/grammar.

Additionally I incorporated your suggestion on using relative epochs as multiples of 675 to be 4 days exactly.

I think these all could benefit from visuals as well so will find time to add them as well.

@@ -10,7 +10,7 @@

=Goal to be achieved by template=

Expanding upon the use of the 2 of 3 multisig, a 3 of 5 multisig allows for multiple timelocks to be introduced for additional flexibility.
Expanding upon the use of the concept of a 3 key timed multisig, a 5 key timed multisig allows for multiple timelocks to be introduced for additional flexibility.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In c2cc1af:

I think you meant to write "Expanding upon the concept" rather than "Expanding upon the use of the concept"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

*older(4195000)
*older(4200000)
*older(4259839)
*older(4194979) 4 days, minimum value
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In c2cc1af:

I believe this number should be 675, not 4194979.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My goal was to show how in miniscript policy or output descriptors using miniscript the relative epoch timestamps would be represented. If you were to use older(675), it would be interpreted as a 675 block height timelock.

To enforce a 4 day relative epochtime timelock, you take (1 << 22) to indicate an epoch timestamp relative timelock from BIP68 into decimal form, which is 4194304. From there, you add 675 to do the 4 day relative timelock to get what I arrived to with older(4194979).

I'm very open to presenting thsi information in a different way if you have a suggestion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. I actually had no idea that time-based relative timelocks worked this way (I'm not sure I ever really noticed that you could do time-based relative timelocks at all...)

I think as written this is fine. People will raise their eyebrows at it but they can check the relevant BIP or find this discussion.

Copy link
Contributor Author

@Rob1Ham Rob1Ham Sep 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to do some digging in therust-minisript tests after I spoke with @sanket1729 at Tabconf earlier this month to figure out how to get the miniscript compiler to accurately encode these timelocks. I agree that epoch time relative timelocks are the least likely used/known timelocks, I wanted to provide some reference documentation as a starting place.

@apoelstra
Copy link
Member

c2cc1af looks great other than the two nits that I listed

@Rob1Ham
Copy link
Contributor Author

Rob1Ham commented Sep 20, 2023

I've removed Templates 003 & 004 to focus on these initial templates, I'll make those a separate pull request. Also based on initial feedback I've gathered, I'm currently creating some basic diagrams for templates 001 & 002.

@Rob1Ham
Copy link
Contributor Author

Rob1Ham commented Sep 21, 2023

I've added diagrams for template 001 & template 002, and made a name change after some received feedback for "3 Key Time Layered Multisig" & "5 Key Time Layered Multisig".

I have no more proposed changes updates for the items in this pull request, once we are aligned on the relative timelock note I think we are good here.

@Rob1Ham
Copy link
Contributor Author

Rob1Ham commented Sep 22, 2023

I'm unsure about the rename to "master keys" -- I worry that this term is too general and it might get confused with "master seed" from BIP32 (or the xpub that you get from deriving the root key of a master seed).

Just to document, I renamed these keys from "Master" keys to "Sovereign" keys.

@Rob1Ham
Copy link
Contributor Author

Rob1Ham commented Oct 2, 2023

I collapsed additional commits which were irrelevant for this pull request. I am currently working on the next batch of templates. If there is sufficient detail for these initial templates, I can use these first templates as a baseline for future template creation.

@apoelstra
Copy link
Member

Thanks! But could you re-arrange the commits so that each one simply adds a (complete) file and nothing gets renamed after-the-fact? It's a little hard to follow the flow still. For example 56fce56 has the description "Add Timelock Values Note" but it additionally fixes a typo in Miniscript-Template-000-Timelocks in Templates.mediawiki and adds two entire new files (one of which has copy.mediawiki in the name which suggests that it might be an accidental commit).

Alternately, I think you could just squash all the commits together, and I can use git diff --stat to see the total list of files and view them individually.

@sanket1729
Copy link

code review ACK.

I really want to make it clear that these templates are just suggestions and reviewed examples. The last thing we would want is wallets to reject miniscripts that do not conform to these templates.

For example, importing a miniscript descriptor that does not have timelock ending in 00s, or any other decaying timelock construction.

@RandyMcMillan
Copy link

Concept ACK

@Rob1Ham
Copy link
Contributor Author

Rob1Ham commented Oct 4, 2023

Thanks! But could you re-arrange the commits so that each one simply adds a (complete) file and nothing gets renamed after-the-fact? It's a little hard to follow the flow still. For example 56fce56 has the description "Add Timelock Values Note" but it additionally fixes a typo in Miniscript-Template-000-Timelocks in Templates.mediawiki and adds two entire new files (one of which has copy.mediawiki in the name which suggests that it might be an accidental commit).

Alternately, I think you could just squash all the commits together, and I can use git diff --stat to see the total list of files and view them individually.

Should be cleaned up now as one commit with the final state of each template and associated diagrams.

code review ACK.

I really want to make it clear that these templates are just suggestions and reviewed examples. The last thing we would want is wallets to reject miniscripts that do not conform to these templates.

For example, importing a miniscript descriptor that does not have timelock ending in 00s, or any other decaying timelock construction.

100% Agreed Sanket, these are just suggestions. Speaking with wallet developers and hardware wallet manufacturers, there is a preference to have a suggested way of using Miniscript to help streamline UX.

@RandyMcMillan
Copy link

Rob1Ham#1

I recommend a no space naming convention policy for the repo.

cc: @apoelstra

@apoelstra
Copy link
Member

ACK "no-space naming". I vote to use -s rather than spaces, but I have no strong preferences.

@Rob1Ham
Copy link
Contributor Author

Rob1Ham commented Oct 4, 2023

ACK "no-space naming". I vote to use -s rather than spaces, but I have no strong preferences.

Agreed and already updated main to reflect @RandyMcMillan's suggestion.

@RandyMcMillan
Copy link

Screen Shot 2023-10-04 at 5 43 09 PM

Rob1Ham#2

@@ -0,0 +1,83 @@
<pre>
Miniscript Template: 0

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend the front matter be

MSTemplate: 0

similar to the no space file naming...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, I actually think we can compress it further MST for shorter filenames.

@@ -0,0 +1,63 @@
Miniscript Template: 1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend the front matter be

MSTemplate: 1

similar to the no space file naming...

@Rob1Ham
Copy link
Contributor Author

Rob1Ham commented Oct 6, 2023

Talking with Randy about this a bit more, I think we can collapse the template names down to MST (miniscript templates) to have it be cleaner for formatting/navigating the repo.

I'm going to update the README.md to a .wikimedia file for consistency, as well as throw in a starter table of contents.


1. Name of Template

Proposed Timelock Usage
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In e2167a4:

We can drop "Name of Template" here since the name is now in the header.


Proposed Timelock Usage

2. Goal to be achieved by template
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In e2167a4:

I'd change this to =Motivation= to match the style of other sections.


[https://bitcoin.sipa.be/miniscript/ Miniscript] facilitates easier access to utilizing timelocks in Bitcoin, it opens a new frontier in Bitcoin security concerning the construction of more advanced Scripts. This document aims to standardize timelock application across different wallet solutions, focusing on wallet recovery and standardizing expected timelock usage for streamlined hardware and software wallet interfaces.

In the event an output descriptor has been partially or fully lost, minimizing the overall search space for expected timestamp values can expedite recovery. General Miniscript usage supports any valid timelock value, this proposal seeks to guide implementation to more user-friendly practices.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In e2167a4:

The last clause this proposal seeks... should be its own sentence (replace comma by period, capitalize).


==Absolute block height based Timelocks==

'''Proposal''': Set absolute block height-based timelocks as multiples of 100, always ending in 00.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In e2167a4:

I think we should give each of these proposals numbers. And I think we should call them "Rules" or "Guidelines" rather than "proposals". So here we'd have '''Rule 1''': Set absolute... and when referring to it people can say "MST 0, rule 1".


'''Limitation''': Avoid setting epoch timestamps beyond 2105 (4291704000) to prevent any possible issue with related to its 32 bit unsigned integer used for timestamps to happen in in February of 2106.

Propsoed Examples of valid Epoch Timestamp Absolute Timelocks
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In e2167a4:

Typo Proposed. But also I think you should just drop the word "Proposed" and make this a === subsubsection heading.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same elsewhere; change "Proposed Examples" to just "Examples".

* after(1000)
* after(50700)
* after(82800)
* after(615000)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In e2167a4

All miniscript snippits like this should be in <code> blocks. (This is super annoying to type but I still insist on it :)).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm on it!


3 Key Degrading Multisig

=Goal to be achieved by template=
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In e2167a4:

Similarly here, the "Name of Template" section should be dropped and the "Goal to be Achieved" section should be renamed "Motivation".

@apoelstra
Copy link
Member

Technically this all looks good. My comments are mostly about formatting etc.

Also, if possible can you rebase/cherry-pick Randy's commits rather than merging them? Merge commits require a bunch of extra effort to review, since it is possible to slip backdoors into them and git provides very poor tooling for analyzing them. cherry-pick will preserve authorship (though it will drop GPG signatures unfortunately).

If you want to preserve GPG signatures you can use git merge --ff-only, which would work in this case though won't work in general.

@Rob1Ham
Copy link
Contributor Author

Rob1Ham commented Nov 19, 2023

Hey Andrew, I've been working with @RandyMcMillan over the past few weeks to clean up the initial templates to both incorporate your suggested edits on phrasings, as well as revamping the diagrams to be more robust/dynamic with general open source iconography, and using the MathML notations scheme to have consistent documentation of intended logic.

Randy also had the great idea to Call the Miniscript Templates "mints", Minitapscript Templates as " mintts".

@RandyMcMillan
Copy link

RandyMcMillan commented Nov 20, 2023

I will continue to make myself available to get this launched.

  • I am also happy to help review and test submissions going forward.

@Rob1Ham
Copy link
Contributor Author

Rob1Ham commented Dec 18, 2023

Hi @apoelstra - I hope you're well! @RandyMcMillan and I spent a chunk of time integrating your suggestions as well as cleaning up the general formatting for a good standard to operate on going forward. Let us know your thoughts on if you believe this is ready to be merged!

@apoelstra
Copy link
Member

Are you able to eliminate the merge commits from the PR? It is difficult to review with them present.

Create README.md

mint-000.md

mint-001.md

mint-002.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants