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

BIP 326: Anti-fee-sniping protection with nSequence in taproot transactions to improve privacy for off-chain protocols #1269

Merged

Conversation

chris-belcher
Copy link
Contributor

This adds a BIP for improve could the privacy of certain off-chain protocols like Lightning, CoinSwap or DLCs.

It has already been implemented by Sparrow wallet.

@kallewoof
Copy link
Member

kallewoof commented Jan 9, 2022

ML discussion dated June 10 2021. No objections that I can see.

Ping @luke-jr.

Edit: June, not July.

Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

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

left some nits/questions

bip-nsequence-anti-fee-snipe.mediawiki Outdated Show resolved Hide resolved
bip-nsequence-anti-fee-snipe.mediawiki Outdated Show resolved Hide resolved
bip-nsequence-anti-fee-snipe.mediawiki Outdated Show resolved Hide resolved
bip-nsequence-anti-fee-snipe.mediawiki Outdated Show resolved Hide resolved
bip-nsequence-anti-fee-snipe.mediawiki Outdated Show resolved Hide resolved
@maflcko
Copy link
Member

maflcko commented Jan 9, 2022

Is there an intuition how much this will help in practice?

I expect smart contract implementations to use "magic numbers" for nsequence, such as 144. Or at the least use an nsequence that is biased toward such magic numbers. This will make it easier to tell them apart from "normal" transactions. I don't have any data, but I think a tx output is (still) most likely to be spent in the block that created it (or in the few next ones that follow it). Such inputs would get marked with a small nsequence, for example 0 or 1. Those values don't really make sense for smart contracts, so it will be easy to tell them apart.

Maybe for small nsequence, it could make sense to also fall back to nlocktime?

@chris-belcher
Copy link
Contributor Author

Thanks so much for the reviews. I can't believe I got the date of the mailing list post wrong.

If fixed locktimes are used then a coinswap peer in a routed coinswap can tell their own position in the route from the locktime. Randomizing locktimes can be used to avoid this, and randomized locktimes will also blend into on-chain traffic a lot better. So this BIP will certainly help the privacy of CoinSwap/Teleport, which intends to eventually use a slightly randomized locktime.

The same privacy leak applies to Lightning, and the Lightning community might also decide to implement randomized locktimes in some form. The discussion is more complicated admittedly since public channel transactions are broadcasted to everyone in the LN.

NSequence values of 1 could be used as an alternative to OP_CSV 1 which is today being discussed as a way to stop transaction pinning.

Because off-chain tech is so much more efficient in block space than on-chain tech, we can expect that the vast majority of block space will be used by on-chain tech for a long time, so it's not a bad thing that this BIP marks on-chain transactions. The purpose of this BIP is to provide cover traffic for the rare occasion when an off-chain contract is closed with the timelock branch. This BIP aims to make that situation not immediately stand out. So I don't think it's worth changing anything for small nsequence values.

@maflcko
Copy link
Member

maflcko commented Jan 9, 2022

Thanks for the context and providing an example of why an nsequence of 1 makes sense. I guess an nsequence value of 0 doesn't really make sense in practice? Moreover, in a (theoretical) edge case where a wallet spends untrusted unconfirmed coins, it might reduce the anti-fee-snipe guarantee. Assuming the incoming tx from another wallet didn't have any anti-fee-snipe mechanism set, then a miner could re-mine the current block while including both the incoming tx and the one just created by the (theoretical) wallet?

Also, I am wondering if the locktime and nsequence should remain unspecified in this BIP if they don't need to be set to achieve anti-fee-snipe? Given that smart contracts in practise will usually use either locktime or nsequence, shouldn't this BIP recommend to set them to their "default" value if not used? (see chris-belcher#2) Otherwise it will again be easy to tell "normal" and "smart" txs apart.

@chris-belcher
Copy link
Contributor Author

Yes you're right on both counts. I've edited the pseudocode to also only use nlocktime if the transaction has any unconfirmed inputs. And I've merged your pull request.

Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

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

ACK

Copy link
Member

@luke-jr luke-jr left a comment

Choose a reason for hiding this comment

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

Let's use BIP number 326 for this

bip-nsequence-anti-fee-snipe.mediawiki Outdated Show resolved Hide resolved
bip-nsequence-anti-fee-snipe.mediawiki Outdated Show resolved Hide resolved
@luke-jr luke-jr changed the title Add BIP nsequence anti-fee-snipe BIP 326: Anti-fee-sniping protection with nSequence in taproot transactions to improve privacy for off-chain protocols Jan 15, 2022
@chris-belcher chris-belcher force-pushed the bip-nsequence-anti-fee-snipe branch 4 times, most recently from 06665d5 to 388bca5 Compare January 16, 2022 15:46
@chris-belcher
Copy link
Contributor Author

Thanks Luke!

I have dealt with your comments, edited the file name and other places to include the bip number, added an entry to the README, and squished all the commits into one.

bip-0326.mediawiki Outdated Show resolved Hide resolved
@maflcko
Copy link
Member

maflcko commented Jan 22, 2022

Created a draft implementation for Bitcoin Core: bitcoin/bitcoin#24128

bip-0326.mediawiki Outdated Show resolved Hide resolved
bip-0326.mediawiki Outdated Show resolved Hide resolved
bip-0326.mediawiki Outdated Show resolved Hide resolved
bip-0326.mediawiki Outdated Show resolved Hide resolved
bip-0326.mediawiki Outdated Show resolved Hide resolved
# always set nlocktime if any of the transaction inputs have more
# confirmations than 65535 or are not taproot inputs, or have
# unconfirmed inputs
# otherwise choose either nlocktime or nsequence with 50% probability
Copy link

Choose a reason for hiding this comment

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

Could you elaborate further why these conditions imply choosing 100% nlocktime or 50% nlocktime and 50% nsequence?

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 don't understand your question, please elaborate

@chris-belcher
Copy link
Contributor Author

I added a small paragraph explaining how this BIP relates to transaction pinning and nSequence=1 values. It's a brief retelling of the discussion between @MarcoFalke and me, which I think is worth saying in the document too.

bip-0326.mediawiki Outdated Show resolved Hide resolved
@bitcoin bitcoin deleted a comment from grayl0921 Feb 2, 2022
Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

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

ACK. RFM?

bip-0326.mediawiki Outdated Show resolved Hide resolved
@maflcko
Copy link
Member

maflcko commented Mar 7, 2022

@kallewoof @luke-jr

@kallewoof kallewoof merged commit 34d211a into bitcoin:master Mar 8, 2022
@chris-belcher chris-belcher deleted the bip-nsequence-anti-fee-snipe branch March 8, 2022 13:14
@katesalazar
Copy link
Contributor

34d211a#r72852428

transaction.inputs[input_index].nsequence = transaction.inputs\
[input_index].confirmations()
if randint(10) == 0:
transaction.inputs[input_index].nsequence = max(1,
Copy link
Member

Choose a reason for hiding this comment

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

Side-note: Actually there may be a use case of 0 here in smart contracts to force RBF-opt-in, absent full-RBF. See https://twitter.com/theinstagibbs/status/1531746713853038593

Though, I guess it doesn't make sense to change this BIP to accommodate this, as 0 in key-path spends doesn't add a cover for script-path spend txs.

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

Successfully merging this pull request may close these issues.

9 participants