-
Notifications
You must be signed in to change notification settings - Fork 35
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
Adaptor DLC Transactions Specification #57
Adaptor DLC Transactions Specification #57
Conversation
…ature based DLCs and to match the BOLT style
@ariard I thought we had decided that anchor outputs for parties that win nothing for a given outcome are not needed? |
|
||
All fees are paid for in the [Funding Transaction](#funding-transaction) so that the funding output's value is inflated so that the outputs of CETs and the refund transaction are the exact amounts specified in the offer message's contract information (or total collateral specified in the offer and accept messages for the refund transaction) with no fees subtracted. | ||
|
||
All fees are currently paid evenly between the two parties, though this will change in a future version. |
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.
One party can make the cost of aggregating its utxos spread unevenly by the other one. Alice declares 10 inputs, Bob only 2, assuming spends are equal, Bob will pays for 4 of Alice spends ?
A partial counter-measure, left to implementations, is to reject DLC offer if number of inputs are blatantly unbalanced.
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.
So as I've kind of alluded to here, I think the plan for v0.1 is to not have users pay fees evenly but rather to have each user pay for their own inputs and change output (and the rest evenly), but I didn't want to clutter this PR with that discussion so I'm hoping to open a PR once this gets merged with that part of the specification, I'll open an issue for this so I don't forget.
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.
Here it is including some explanation of what change will be needed :) #58
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.
All those fee calculations are really a rabbit hole of petty details that can go wrong.
In the payjoin proposal, I had a similar issue, and the fee calculation was going to take like 60% of the complexity of the protocol, and responsible for 90% of the bikeshedding during review.
I ended up managing to remove the problem entirely by letting the flexibility of choosing what fee to pay for all parties. All parties decide what they want to pay.
In the case nobody wants to pay any fee, and that would make the transaction unbroadcastable, then there is just an error.
In most case, there is no need to maintain the same feerate across funding, CET and refund. Why forcing it? This make the protocol more complicated.
On top of it, it can actually be dangerous! You want the CET to have a higher fee rate than the refund. If the mempool is full and they both have the same feerate and the CET can't propagate on time, the refund would cancel it.
@nkohen Of course I agree you don't care about CET if you don't have funds at stake, but if you do you want to bump feerate to avoid CET stucking in mempools in case of congestion, and thus unilaterally. I can write down the anchor output part later, to avoid slow down this PR |
Doesn't the output which contains the payout act as an anchor output that can be spent using CPFP? Or am I missing something |
|
||
# Contract Execution Transaction | ||
|
||
Also known as a CET. |
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.
Maybe link to the glossary?
Transactions.md
Outdated
Also known as a CET. | ||
|
||
* version: 2 | ||
* locktime: `contract_maturity` |
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.
What is the reasoning for having this again? It seems to me, if we are relying on an oracle, the contract "matures" whenever the Oracle publishes. I think things like this are better handled at the application layer to provide an "estimate" of when that will be rather than in the protocol
As far as I can tell, this doesn't provide any security guarantees, but does add unnecessary complexity when thinking about the protocol? For instance (I'm not 100% on this) this may effect relay policies of CETs on the network depending, it can also effect fee bumping logic (cc @ariard)
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.
So perhaps this field needs to be renamed but I strongly believe it must be included.
I agree that the decision of what this value should be should happen at the application layer but it definitely still needs to be specified here. This value can be set to 0 for example. It can be set to the current block height, it can be set to the time at which you are expected to be back online. Regardless, it definitely must be specified to avoid ambiguity.
It doesn't provide any guarantees about the oracle but it certainly gives parties (should they set this) time to cooperate to change oracles if an oracle becomes compromised.
I don't believe that this time-lock is associated with fee concerns.
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.
This value can be set to 0 for example. It can be set to the current block height, it can be set to the time at which you are expected to be back online. Regardless, it definitely must be specified to avoid ambiguity.
Ok, I agree with you on the need for it to be specified, I think it just offers faux security guarantees if we start setting it to be value in the future. It makes it harder for us as protocol developers to test, and doesn't add any security guarantees to the end user because once the oracle publishes signatures related to the event your leverage is gone.
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'm with @Christewart on this one as I tried to express in #59 (comment) It's a real pain for testing, can affect user experience (in many cases you might not be able to broadcast your TX even after the oracle signature is vailable_ and I agree it doesn't provide anything valuable:
It doesn't provide any guarantees about the oracle but it certainly gives parties (should they set this) time to cooperate to change oracles if an oracle becomes compromised.
I don't see that as a valid argument, if both parties are cooperative and/or honest, they can change oracle. If one of the party thinks the compromise of the oracle is favorable to them, they'll just take advantage of it whether there is a time lock or not, the time lock really doesn't change anything.
If the point is to set it in the past or to current block height I'm ok with it, but then I feel we should specify how that should work in the protocol (e.g. which values a client should accept or reject).
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.
A non-mature transaction isn't going to propagate (where maturity is defined as relative/absolute timelock not expiring at next block). If your transaction isn't in network mempools you can't CPFP it. So it may complexify a bit your broadcast logic as you might learn a valid witness (oracle scalar) to authorize a spend but have to cache broadcast until reaching unix/block height.
I don't think it provides any security for the reason mentioned by Tibo, unless you can get with a proof of oracle compromise and thus active immediately the refund path. Security with regards to oracle compromise is interesting to think of but beyond current convervsation IMO.
Wheres it does provide setting this value to non-far-from-current-blockheight is both as a anti-fee snipping mechanism and avoiding a fingerprint. Bitcoin Core wallet implements fingeprint but set back from few blocks ~10% of transactions to avoid fingerprinting pre-signed Coinjoins to account for slow multi-party signing rounds. It would be great if we can mask DLC in this set. We don't care about the anti-fee snipping as our fee model is likely to be a just-in-time CPFP and won't be interesting for a miner to fee snip a poor-feerate parent.
Transactions.md
Outdated
- OP_DATA: 1 byte (witness_script_SHA256 length) | ||
- witness_script_SHA256: 32 bytes | ||
|
||
change_output: 9 + change_spk_script length bytes |
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 it's reasonable to restrict this to
- p2sh script
- p2wsh script
- p2wpkh script
I only say p2sh because the funding transaction accepts p2sh(segwit) scripts
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 see no reason to restrict the change output #53
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.
Let's continue discussion on the issue: #53 (comment)
- hash: 32 bytes | ||
- index: 4 bytes | ||
- var_int: 1 byte (script_sig length) | ||
- script_sig: 0 bytes |
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.
Since this is allowed to be p2sh, scriptSig is not necessarily 0 bytes because for p2sh(segwit) you need to play the segwit scriptPubKey in the scriptSig IIRC.
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.
- flag: 1 byte | ||
- marker: 1 byte | ||
|
||
witness: 108 bytes |
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.
so if I understand this correctly, we are limiting the funding of our transaction to be a p2sh(p2wpkh) or p2wpkh tx? It seems that are weight calculations are implying this any way.
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 right, I did review with the old model where CETs where relatively timelocked to enable punishment in case of fraudulent broadcast. |
* The funding output script is a P2WSH to: | ||
|
||
``` | ||
2 <offer_funding_pubkey> <accept_funding_pubkey> 2 OP_CHECKMULTISIG |
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.
Maybe we can sort these lexicographically, it should be an easy compute for each party + doesn't reveal which key is being used by each party. IIRC there was some bitcoin proposal to have all OP_CHECKMULTISIG
keys sorted this way before, might have just been something wallets were doing for easier backups.
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.
Looks like this is what LN does, would definitely be good to do this then so we can share anon set with them better.
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 sorting need to be done as well.
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.
Turns out this is a BIP
https://github.com/bitcoin/bips/blob/master/bip-0067.mediawiki
|
||
## Change Outputs | ||
|
||
The funding transaction's change outputs should pay to the address specified in the relevant offer/accept message. A change output's value should equal the total funding amount of that party subtracted by their total collateral, their fees for this transaction, and their fees for the largest possible closing transaction. If this value is below the dust limit of `1000 satoshis`, then that party must include additional funding in order to ensure they have a valid anchor output. |
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 see you already wrote the requirement. Should we copy-paste anchor output scripts and requirements from BOLT or referring them, especially with regards to utxo set pollution prevention ?
At least you should mention to not set the witnessScript with some third-party spendable path to avoid this one interfering with your funding confirmation.
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.
See #54 (comment)
I don't think we need to actually specify anything for anchor outputs
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.
@benthecarman You mean we consider DLC parties to always spend their change outputs even when it's a low-economic value of 1000 satoshis ?
It just a blockchain usage gentleman's rule to avoid utxo set pollution in case of multi-party protocol, we don't have to hold to it.
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 that they always spend them but they have the option to for CPFP
An excellent post from @ariard on tx propogation policy development in bitcoin core, linking here so others can find and make sure we are all on the same page with things we need to consider: bitcoin/bitcoin#19820 |
|
||
## Funding Inputs | ||
|
||
All funding inputs must be Segwit or nested P2SH(Segwit) in order to protect against malleability attacks. |
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.
Even guaranteeing a party always has an anchor output to prevent witness-inflation pinning, I think the malicious party can just double-spend its announced outpoint with a high-fee, low-feerate transaction, stucking in network mempools and thus preventing confirmation of the funding.
If absolute fee of this transaction is higher than funding, it would be rejected by network mempools thus preventing any CPFP bump.
AFAICT, it's a pinning variant of the one LN is currently exposed to, low-feerate package based on concurrent states.
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.
What exactly is the downside of this? It seems that if this happens on the funding tx, the counter party can just double spend their outpoint the same to exit the transaction?
If your counterparty is being malicious during the funding process, you probably don't want to be in a contract with them?
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 have the same thought as @Christewart I don't really see the big issue here. You shouldn't consider the contract established until the fund transaction is confirmed.
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.
@Christewart, this happens post-funding process, when all signatures have been exchanged.
It seems that if this happens on the funding tx, the counter party can just double spend their outpoint the same to exit the transaction?
In theory yes. In practice:
- Bob finds Alice full-node
- Bob mempools-split Alice's full-node from the rest of the network, feeding his high-fee, low-feerate double-spend to Alice, the funding_tx to others
- Bob's double spend doesn't propagate to the rest of network (just provoke a conflict on one of the input)
- Alice broadcast the funding tx, it's rejected by her mempool as Bob's double spend doesn't signal RBF
- After few blocks, Alice try to double-spend her collateral input, her transaction is accepted locally, rejected by the rest of the network
- If DLC outcome isn't favorable to Bob, Bob wait until refund tx maturation and confirm funding_tx with a bump at that time
This assume full-node mapping and rapid feerate change to stuck the funding so it's assuming a lot on attacks resources. But AFAICT it formally works.
A good mitigation is to relative-timelock the refund tx to give opportunity to a honest party to commit CETs in case of malicious funding tx mempool-pinning. Also we should ensure that funding_tx feerate is competitive to hinder pinning.
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.
@ariard Thanks for the explanation, it's really interesting, but I still don't understand everything:
- How does having anchor output solves the problem here? If Alice's mempool rejected the fund transaction, how could she use her anchor output to bump the fees? Could you describe what she should do assuming that she notices the issue?
- How come is Alice's double spend on the fund transaction rejected by the rest of the network, while a potential double spend from Bob is not (I assume that's what bob would do if outcome is unfavorable?).
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.
From our last meeting, my understanding of @ariard explanation is that the main reason to enforce change output was to enable fee bumping a fund tx with CPFP without requiring to resign a tx which would have higher fee. Assuming that my understanding is correct, I would propose not in including this requirement in the first version of these specification since we have already so much to implement.
* nLockTime is `0` | ||
### <a name="FundingInputs">Inputs</a> | ||
* Local Funding Inputs | ||
* All should have nSequence of `0xffffffff` |
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 it still true after your changes?
|
||
## Funding Output | ||
|
||
* The funding output script is a P2WSH to: |
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.
what about calculation of the value?
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 this is ready to merge and be iterated on
* `txin[0]` outpoint: `txid` of funding transaction and `output_index` 0 | ||
* `txin[0]` sequence: 0xFFFFFFFE | ||
* `txin[0]` script bytes: 0 | ||
* `txin[0]` witness: `0 <signature_for_offer_pubkey> <signature_for_accept_pubkey>` |
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.
When doing the pubkey ordering, make sure to change here as well
### Offerer Output | ||
|
||
This output sends funds won by the offerer corresponding to this CET's outcome to the offerer's final address specified in the offer message. | ||
|
||
### Accepter Output | ||
|
||
This output sends funds won by the accepter corresponding to this CET's outcome to the accepter's final address specified in the accept message. |
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.
Should probably make some reference to the contract_info
here for where to get the amounts
Re-wrote the Transactions specification to match the new adaptor signature based DLCs and to match the BOLT style.
Weights are tested in the two transactions associated with the following address: https://blockstream.info/testnet/address/tb1q6zgwt0ahs9jwxu27kl9whlym93vgrn3q6anymsl2lyr9g75wa4pqjwd867
Related to #12
Related to #46
TODO:
Replaces #51 and #56