OP_VAULT draft #26857
OP_VAULT draft #26857
Conversation
82ba876 to
8f81d72
Compare
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
89220ae to
fbf3449
Compare
There was a problem hiding this comment.
Congrats here -- Always cool to see new implemented ideas to extend the Script interpreter flexibility!
From a primitive viewpoint, I think I've few open questions, like if the recovery path should be committed with a signature rather than protected by a simple scriptpubkey preimage. Beyond locking up the inputs/outputs fields for mempool issues, I think there is the accountability upsides of using new private and accountable Schnorr-compatible schemes (e.g TAPS). The current OP_VAULT implementation is using OP_NOP repurposing but this doesn't seem compatible with Taproot-only extensions (e.g ANYPREVOUT) and maybe a OP_SUCCESS could be used. There is a conceptual wonder, if a CTV and template malleability approach wouldn't better suit the vault use-case and allow other ones, as such better re-usability of primitives.
From a procedural viewpoint, recently there has been proposed the bitcoin-inquisition
fork of Core as an experimental platform to test, iterate and play with Script interpreter extensions, notably also aiming to support package relay and ephemeral outputs to observe the fee-bumping primitives trade-offs. I believe the idea is to have a stable Core release where one can have a protocol proof-of-concept running with hopefully minimal rebase maintenance cost (yeah we all hate the rebase hell...). Looking forward to review things there!
|
Great to see this, I hope this functionality comes into Bitcoin one way or another, and this seems like a decent attempt. Your listing of the features makes sense, and I currently don't think a more optimal way to achieve them. By no means blockers, but rather some thoughts I wanted to share on the design:
|
|
I plan to comment about the design on the ML, but just a note about @naumenkogs' comment above. On the contrary i think the batching feature is very compelling. The impossibility to batch Unvaults in Revault is a major drawback: it significantly increases the cost of any realistic operation (you need one whole additional transaction per input, and each have likely more than one output). It also potentially increases the cost on the network (you'd likely want some sort of anchor output on each Unvault tx, that you might not spend, so that's |
7490e5d to
170a1e6
Compare
03fc5a5 to
a8323b9
Compare
|
I second most (if not all) of the comments thus far. Especially this from @ariard.
Thanks also for keeping this in draft for now @jamesob. For it to be moved out of draft I'd like to see a convergence on this being the best proposal to enable this functionality and proofs of concept on bitcoin-inquisition seem like a good step towards that (assuming the bitcoin-inquisition maintainer(s) think this is ready to be merged there). |
src/script/interpreter.cpp
Outdated
| return false; | ||
| } | ||
| const CScript& spk = out.scriptPubKey; | ||
| if (!(spk.size() == 1 && spk[0] == OP_2)) { |
| { | ||
| // We can't use precomputed transaction data here because, since the input lacks a | ||
| // witness, the precomputation routines don't run. I.e. `txdata.hashOutputs` is blank. | ||
| return SHA256Uint256(GetOutputsSHA256(*this->txTo)) == target_outputs_hash; |
There was a problem hiding this comment.
In fbf3449 "wip: draft implementation"
Note that this is a different serialization from the one described in the paper. The paper says to serialize the scriptPubKey first, then nValue. However GetOutputsSHA256 serializes each output as nValue then scriptPubKey. I'm going to assume this is an error in the paper as I think it is better the way it is implemented rather than described.
a8323b9 to
fdfd5e9
Compare
|
Any plans for a |
|
I think the overall approach is sound for vault-specific functionality. I understand the implementation is a draft at this point, so you may already be aware of these issues, but I thought I'd flag them just in case. My apologies if these are premature! I believe the recursion here ought to be explicitly limited. As it stands, an attacker could crash a node by crafting a transaction spending from a vault that has, as its
If you supply A potential attacker could also perform almost any arbitrary computation using a similar method. The recursion also doesn't count any potential additional signature operations toward the Finally, I don't think implementing these opcodes as I think it's generally unsafe for a reimplemented Diff for @@ -21,0 +22 @@ from test_framework.util import assert_equal, assert_raises_rpc_error
+from test_framework.script_util import script_to_p2wsh_script
@@ -64,0 +66,3 @@ class VaultsTest(BitcoinTestFramework):
+ title("testing recursive attack sweep")
+ self.single_vault_test(node, wallet, sweep_from_vault=True)
+
@@ -447 +451,3 @@ DEFAULT_UNVAULT_SECRET = 3
-
+def get_recursive_witness_script() -> CScript:
+ return CScript([script.OP_3, script.OP_PICK, script.OP_3, script.OP_PICK, script.OP_3,
+ script.OP_PICK, script.OP_3, script.OP_PICK, script.OP_VAULT])
@@ -465 +471,3 @@ class VaultSpec:
- self.recovery_spk = CScript([script.OP_TRUE])
+ recursive_script = get_recursive_witness_script()
+ recursive_program = script_to_p2wsh_script(recursive_script)
+ self.recovery_spk = recursive_program
@@ -672,0 +681,5 @@ def get_sweep_to_recovery_tx(
+ get_recursive_witness_script(),
+ vault.recovery_params,
+ CScript([vault.spend_delay]),
+ CScript(b'\x00' * 32),
+ get_recursive_witness_script(),
|
That'd be great, though if no one else wants to do that work I'll have spend some time learning more about descriptors.
@john-moffett this is all great! I'd planned to move OP_VAULT/OP_UNVAULT to OP_SUCCESSx as of late last week, and was vaguely aware that the recursion probably facilitates Bad Things, but your detailed explanations and test cases (!) are very valuable. Never too early to point out a crash bug! Thank you for your help. I'll remedy these things in the coming days. |
f8d55bb to
6ab76f5
Compare
1af14ed to
b32ed43
Compare
There was a problem hiding this comment.
Concept: Specific covenant proposal instead of generalized covenant that achieves multiple things over years
Concept NACK
Overall I agree with covenants being implemented however only supporting one use case doesn't make sense. I could link to several other discussions including some in which reviewers of this PR were not satisfied with OP_CTV because it seems to be redundant and general covenants approach works better, however below links sums up everything without any drama.
Note: Its possible to emulate CTV inefficiently if this is implemented.
Related links:
https://twitter.com/tierotiero/status/1618133549294718976
https://twitter.com/tierotiero/status/1618136358001967104
I appreciate the efforts by PR author to write the paper, mailing list discussion etc. however this has no consensus apart from a few vault enthusiasts in this repository. Author had also requested people to participate in mailing list discussion to share other use cases, I think it has been done already and resulted in being called 'attack on bitcoin', 'personal attacks' etc. Also its not possible to discuss anything on mailing list related to coinjoin. Maybe we need a mailing list on nostr.
11a3836 to
9d5a8bb
Compare
b7620a9 to
803774e
Compare
Implements a framework for accumulated "deferred checks" during vin script execution, and then executing the checks after all inputs have been validated individually. This facilitates new forms of validation which involve gathering information during vin script execution and then using that information to do "aggregate" checks on the transaction as a whole. For vaults in particular, this allows us to conjoin operations across vaults with incompatible parameters into the same transaction, as well as include unrelated inputs and outputs, which facilitates more flexible fee management. There are also applications re: batch validation and cross-input signature aggregation.
OP_VAULT adds a native way of doing vaults - that is, encumbering coins
in such a way that they must pass through a delay period before being
successfully spent, aside from a predetermined path (the recovery
path) which they can be swept to at any time during the life of the
vault.
Vaulting
========
To create a vault, coins must be encumbered under a scriptPubKey that
looks like
<recovery-params> <spend-delay> <trigger-sPK-hash> OP_VAULT
Of course, this can be within a P2TR taptree.
<recovery-params>
-----------------
The `recovery-params` paramater is of the form
<32-byte-hash>[<arbitrary scriptPubKey>]
where the first component is the hash of the recovery path scriptPubKey:
i.e. the scriptPubKey which the full value of recovered vaults must be
spent to in order to successfully sweep the coins from a vault.
The optional bytes following the recovery path hash dictate the optional
recovery authorization. When this is specified, the script given must be
satisfied when sweeping the vaulted inputs to recovery. Using this
parameter has benefits and drawbacks. The benefits are
- unrelated inputs and outputs can be attached to a recovery, making
"exogenous" fee management like ephemeral anchor use unneccessary.
- vaults with incompatible recovery parameters can be recovered in the
same transaction, which allows fee managed UTXOs to be shared.
however the drawbacks are
- increased risk/complexity around the ability to perform recovery
sweeps.
- if an attacker obtains the recovery authorization key and all
information necesary to attempt a recovery (the "location" of the
vaulted coins) as well as the trigger authorization key, they may
attempt to both trigger an unvault and pin a recovery. This is
mitigated by the necessary presence of an ephemeral anchor output
in every recovery transaction.
In either case, any vault outputs which share <recovery-params> can be
recovered together in the same transaction.
<spend-delay>
-------------
This is a relative timelock a la BIP-68 that specifies how many blocks
(or how much time) must elapse before an OP_UNVAULT output becomes
spendable to the declared unvault target (more on this below).
Vaults with compatible <recovery-params> and <spend-delay> can be
unvaulted together.
<trigger-sPK-hash>
------------------
This is the hash of the scriptPubKey which needs to be satisfied in
order to trigger the unvault process.
OP_UNVAULT
==========
To trigger the start of the unvault process, vaulted outputs must be
spent into a transaction which has a corresponding OP_UNVAULT output of
the form
<recovery-params> <spend-delay> <target-hash> OP_UNVAULT
where the first two parameters are identical to the input vault and the
last parameter, <target-hash>, is a content hash of the proposed unvault
destination. It is a hash of each target output's scriptPubKey and
amount.
Because triggers are always signed with the trigger authorization key,
these transaction can be always be accompanied by unrelated inputs and
outputs, e.g. fee management coins or multiple incompatible vaults.
Triggers support a special "revault" output, which allows depositing
some balance of the vault back into the OP_VAULT scriptPubKey that is
being withdrawn from.
When spending an OP_VAULT into an OP_UNVAULT trigger output, the witness
stack must look like
[trigger witness stack ...] <trigger-sPK> <target-hash> <trigger-vout_idx>
The first two parameters are evaluated recursively as script. The
<trigger-vout-idx> parameter indicates which vout is the related
OP_UNVAULT, which allows us to handle script upgradeability for future
witness versions.
Final withdrawal
================
Once an OP_UNVAULT output has been confirmed and the related
<spend-delay> has elapsed, the output can be spent into a transaction
matching the <target-hash> given in the trigger transaction. This final
withdrawal transaction will be responsible for providing its own fees in
whatever manner the withdrawer prefers.
Recovery
========
At any point before final withdrawal, OP_VAULT or OP_UNVAULT outputs can
be swept to the recovery path.
If no optional recovery authorization is used, all inputs to the
recovery transaction must be from recovery-compatible vault outputs.
There can only be two outputs: one transferring the full value of the
vault inputs to the recovery sPK, and one ephemeral anchor output.
If an optional recovery authorization has bene specified, the recovery
transaction can be more freeform. The only requirements are that the
full value of any vault inputs are transferred to a compatible recovery
sPK output, and there must be at least one ephemeral anchor output. But
unrelated inputs/outputs can "ride along" or provide fees, and otherwise
recovery-incompatible vaults can be recovered together.
[NOTE TODO: this has yet to be implemented, but is planned]
Other notes
===========
Use of OP_VAULT is restricted to witness v1 and up (Taproot).
Credit
======
Thanks to Greg Sanders, AJ Towns, and John Moffett for material
suggestions and review that wound up improving this proposal.
- AJ: make recovery authorization optional, revault trigger outputs
- Greg: put trigger-hash onto witness stack to avoid bare OP_UNVAULT
- John Moffett: explicating recursive eval attack and providing a test
case.
eace7e1 to
6f86e39
Compare
6f86e39 to
8eacc25
Compare
|
🐙 This pull request conflicts with the target branch and needs rebase. |
|
Closing this until the BIP is finalized and this has been tested on inquisition (bitcoin-inquisition#21). |
This is a draft for a consensus change enabling on-chain vaults, as detailed in the BIP: bitcoin/bips#1421
In short, it introduces two opcodes,
OP_VAULTandOP_UNVAULT, that facilitate constructing vaults whichFor the last item, the proposal has a hard dependency on package relay and ephemeral anchors.
The code as-written here lacks specific activation mechanism, and there's probably some policy/wallet stuff missing,
but in substance it is implemented and comes with some interesting functional tests.