-
Notifications
You must be signed in to change notification settings - Fork 9
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
introduction: replace out-dated revault.pdf with introduction.md #113
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few things :) I guess there are many more assumptions like underlying libs being OK, Bitcoin behaving, network connectivity, etc.
introduction.md
Outdated
|
||
The Emergency TX is the first to be signed. It would spend a deposit and pay to the `emergency output descriptor` which is a bitcoin script that is harder to unlock than `deposit output descriptor`. Keeping the `emergency output descriptor` private among stakeholders strengthens the emergency deterrent feature as attackers don't know how they would compromise the emergency vault. | ||
|
||
Then, stakeholders exchange signatures for the Cancel and Unvault Emergency TXs: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any reason why this is a separate step?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's how the practical protocol currently is specified. But it's going to change and in any case this document shouldn't be influenced by the details of the protocol.
So, yeah, it should be merged together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in any case this document shouldn't be influenced by the details of the protocol.
I find this confusing. What is "the protocol" for you? (I think you mean messages?)
In my opinion the intro should be consistent with the rest of practical-revault, so if messages are passed in a certain order (as in messages.md) that should be the same in the intro. Happy to merge these given the upcoming change tho
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the intro should be a conceptual overview of Revault. What i meant there is that we "shouldn't bother" the reader of the intro with low-level communication details.
If there is a fundamental reason why we are doing things a certain way (for instance, separating the communication of the revocation sigs and the unvault sig) it should be noted. But in this case it was a protocol hack to not require a new message for "hey WT do you have enough of these feebumping utxos for a new vault?". So it shouldn't make the intro more complicated, which could confuse the reader (what just happened with Kevin).
introduction.md
Outdated
|
||
#### Bitcoin | ||
|
||
- assume users can propagate revocation transactions to miners. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we did, we wouldn't have the fee discussions all the time :)
introduction.md
Outdated
|
||
- assume users can propagate revocation transactions to miners. | ||
- assume miners will mine the transaction paying the most feerate, regardless of any other (meta)data. | ||
- assume a significant hashrate to prevent reorgs of depth higher than the unvault time-lock length. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Altho it's more complex than that. We don't want "natural reorgs" to be long, but they still statistically can happen. And then there is the game theory as in: are the miners going to mine the next block instead of rewriting the chain?
It's not only a simple hashrate discussion.
introduction.md
Outdated
#### Infrastructure | ||
|
||
- assume a proper ceremony (FIXME: link) as the root of trust for cryptographic protocols used. | ||
- assume a secure signing device allowing users to check the validity of the data being signed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
meh, more like enforcing descriptors, securing keys, and being tamper proof/evident.
introduction.md
Outdated
|
||
- assume a proper ceremony (FIXME: link) as the root of trust for cryptographic protocols used. | ||
- assume a secure signing device allowing users to check the validity of the data being signed. | ||
- assume at least one honest cosigning server AND at least one honest online watchtower per stakeholder. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really, 1 honest WT and 1 honest cosig in total is enough.
Also, does not apply to unvault policies, in which no cosig is necessary.
introduction.md
Outdated
- assume a secure signing device allowing users to check the validity of the data being signed. | ||
- assume at least one honest cosigning server AND at least one honest online watchtower per stakeholder. | ||
- assume the N stakeholders’ keys to not all be compromised. | ||
- assume sufficient fees for Revault TXs such that operations aren't stalled. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this an assumption or part of the protocol?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! Thanks for tackling this, i think it'll make it much simpler to introduce technical people to Revault.
introduction.md
Outdated
|
||
The Unvault TXs have an additional output that allows dynamically allocating fees through child-pays-for-parent (CPFP). | ||
|
||
The Cancel, Emergency and Unvault Emergency TXs aren't compatible with CPFP-carve out (FIXME: Why?), nor with ANYONECANPAY|ALL signatures (because this enables pinning attacks). The safest solution is to prepare multiple variants of the same transactions with a broad distribution of feerates, from typical to extreme and with many in-between. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FIXME: Why?
As discussed since the beginning (Almost 2y ago now), and especially as detailed in the work you and i authored, the CPFP carve-out is specific to 2-parties contract. See this stackexchange answer in the context of Lightning which applies to any transaction that has more than 2-reasily spendable outputs.
nor with ANYONECANPAY|ALL signatures (because this enables pinning attacks).
Since the current state of the repo is to use fee-bumping, i think you should just say we do supplement fees by pre-signing with ANYONECANPAY|ALL. I'll modify that in my PR removing all the fee-bumping stuff from this repo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can even just put a note that it's not detailed as it's going to be removed soon due to pinning concerns.
You did well 👍. But that's not a TODO :) Editing your post to make it an actual TODO list and removing this line from this list. I think the first element is also mostly address by one of my review comments. Besides, you should remove |
Thanks for all the feedback! Updated with a fixup commit so you can check the diff. Will squash after final edits. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly nits, the fixup commit looks mostly good to me
introduction.md
Outdated
|
||
With these in place, stakeholders are able to delegate bitcoin to fund managers and manually broadcast Cancel, Emergency and Unvault-Emergency TXs. Fund managers will also be able to manually broadcast Cancel TXs to recover from mistakes. | ||
|
||
Next, users who want additional features will require additional infrastructure components. Each stakeholder may control one or more watchtowers and/ or a co-signing server. | ||
|
||
- **Watchtower**: An always-online server that monitors the bitcoin network and chain, as well as polls the coordinator for Spend TXs to perform its responsibilities (automated unvault policy enforcement, automated spend policy enforcement, emergency deterrent). | ||
- **Watchtower**: An always-online server that monitors the Bitcoin network and chain, and depending on its responsibilities (which could include; automated unvault policy enforcement, automated spend policy enforcement, emergency deterrent) it will respond to relevant events. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of ;
in the english language i've always found interesting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah I actually used it wrong. It should be :
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK f6b793b
Awesome, very excited to finally have a better introduction. Just a couple final nits:
- Can you move the image to a
img/
subfolder? - Can you link to this document in the README somehow? Like "For a high-level description of Revault, see the introduction"
Removed: The 'Keys' section was removed in favour of a more detailed spec coming with revault#107 The 'Transactions' section was removed as it duplicates transactions.md 'Acknowledgements' section removed. Should it be somewhere else in the repo? New content: Updated transaction diagram using descriptors abstraction. Discussed options for revault deployments and the infrastructure requriements. Renamed "Threat model" section to "Risk" and updated. The 'fee bumping' section was mostly removed as we are approaching it differently. 'Further Discussion' removed as this information is mostly out-dated or duplicated.
95a7061
to
6b35b37
Compare
Applied my two comments myself. Let's move forward. ACK 6b35b37 |
In addition to many language clarifications and minor updates, major differences with revault.pdf are given below.
Removed:
The 'Keys' section was removed in favour of a more detailed spec coming with #107
The 'Transactions' section was removed as it duplicates transactions.md
'Acknowledgements' section removed. Should it be somewhere else in the repo?
'Further Discussion' removed as this information is mostly out-dated or duplicated.
New content:
Updated transaction diagram using descriptors abstraction.
Discussed options for revault deployments and the infrastructure requriements.
Still to do: