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

[consensus] Add opt-in transaction avoidance #134

Closed
wants to merge 4 commits into
base: segwit2x-dev
from

Conversation

Projects
None yet
@jgarzik
Copy link

jgarzik commented Oct 12, 2017

Re-branched version of PR #117. See #117 for background and discussion.

jgarzik added some commits Oct 9, 2017

@JaredR26

This comment has been minimized.

Copy link

JaredR26 commented Oct 12, 2017

Per discussions earlier, utACK

Summary of earlier discussions with several contributors: Given the constraints, this will be a usable replay protection approach that has fewer potential exploit vectors than the blacklist address(both social and L2 vectors, however unlikely). While #131 was considered, this was favored by more people because it incurs less technical debt and is easier for wallets to add support for in the short timeline.

Technical counterpoints for or against this or other methods of opt-in replay protection are welcome here; The earlier discussions on slack and elsewhere weren't meant to exclude other opinions.

@shesek

This comment has been minimized.

Copy link

shesek commented Oct 12, 2017

While #131 was considered, this was favored by more people because it incurs less technical debt and is easier for wallets to add support for in the short timeline.

Where did this discussion happen? #131 appears to be void of these considerations. Is the NACK/ACK list publicly available somewhere?

@jheathco

This comment has been minimized.

Copy link

jheathco commented Oct 12, 2017

See #117. Further discussion has been in slack as well.

@matsjj

This comment has been minimized.

Copy link

matsjj commented Oct 12, 2017

@jheathco I looked through #117, and through all open Slack channels and was unable to find any discussion. Can you point me in the right direction?

I am glad this kind of replay protection is considered again. Be aware though, that this mainly favours people who want to transact on the 1X chain. Splitting outputs with this method alone requires to broadcast a protected transaction to 1X and wait for it to achieve at least one confirmation.

This will make supporting 2X more difficult, especially if 2X gathers most of the hashing power. (since everyone still needs to get a transaction mined post fork on 1X)

In this way, this and #131 are achieving completely different goals, and only with both of them implemented can users freely choose which chain to transact on, without the need of explicitly splitting outputs. #131 does favour 2X users though (where as this favours 1X users).

I am aware that #131 incurs more technical debt. Without it, safely transacting on 2X chain requires a significant amount of engineering resources though. Resources, that some companies don't want to (or simply can't) afford.

@Sjors

This comment has been minimized.

Copy link

Sjors commented Oct 12, 2017

I suggest renaming this to "Add opt-in replay protection for 1x chain (using OP_RETURN)", as distinguished from #131 which should be called something like "Add opt-in replay protection for 2x chain (using SIGHASH)"

@JaredR26

This comment has been minimized.

Copy link

JaredR26 commented Oct 12, 2017

#131 appears to be void of these considerations. Is the NACK/ACK list publicly available somewhere?

nacks/acks would go here, though a lot of discussion went into this so a nack should be backed up with good reasons and alternative proposals. An alternative proposal is going to need a pull request pretty quick like @jcansdale's or #131

@JaredR26

This comment has been minimized.

Copy link

JaredR26 commented Oct 12, 2017

@jheathco I looked through #117, and through all open Slack channels and was unable to find any discussion.

Happened in various places and sometimes over 1v1 chat, it wasn't organized like a git pull would be, so the final discussion should be here. Unfortunately this tends to get trolled heavily. #tech is the best place to bring it up if you wanted a more interactive chat.

In this way, this and #131 are achieving completely different goals, and only with both of them implemented can users freely choose which chain to transact on

Correct, which was a strong point in favor of it. Unfortunately it also adds more complication to wallets support than this approach does, and adds code that can never be removed from the repo. Even for disabling it, we felt it would require a much longer sunset period, and that might complicate layer2 code as things like lightning begin to be used more in the next 12 months.

Another point against it was that the same thing as #131 can be accomplished without any code changes by using locktime height on 2x. This gives a user ample time to get a transaction mined on 2x with a lower fee, and then use RBF to split coins on 1x. Not quite as easy as #131, but with no downsides in the codebase and no vulnerability risks that we're aware of except the below.

Everyone generally agreed that the biggest downside to using locktime - if the legacy chain overtakes 2x - is a nonissue because if the legacy chain overtakes 2x, 2x will probably be dead.

There is also the option of using a mega-transaction that is > 999920 bytes and < 1000000 bytes as a source of 'tainted' outputs for splitting. This too has no technical debt and does not require changes on the wallet side.

@matsjj I hope you don't take it as a slight if the pull request isn't merged. Everyone did greatly appreciate the pull request, it just a matter of finding the best approach(es) with the fewest downsides and the most upsides. And if there's stuff we missed, bring it up, as far as I know this decision isn't finalized yet (though we do need to move forward with something pretty quickly).

@Sjors

This comment has been minimized.

Copy link

Sjors commented Oct 12, 2017

In this way, this and #131 are achieving completely different goals, and only with both of them implemented can users freely choose which chain to transact on

Correct, which was a strong point in favor of it. Unfortunately it also adds more complication to wallets support than this approach does

While it's true that the 1x-only mechanism here is easier to use than than the 2x-only mechanism in #131, this is of little use if a wallet wants to broadcast a 2x-only transaction. The mechanism in #131 is far easier for that purpose.

Without #131, such a wallet would need to monitor both chains, broadcast the 1x-only transaction, wait for it to confirm and not get re-orged, before making any transaction on the 2x chain. If fees on the 1x chain are significantly higher in BTC terms, this could cause additional problems like insufficient balance of different coin selection. This process also needs to be repeated if the wallet only replay protects the specific coins needed for the transaction, rather than all coins. Replay protecting all coins in a send-to-self fastion is safer, but not particularly good for privacy.

Being able to broadcast on, and exclusively monitor, the 2x chain, is far easier for wallets that only care about the 2x chain (but want to give users the ability access the 1x chain at some point or immedidately).

@JaredR26

This comment has been minimized.

Copy link

JaredR26 commented Oct 12, 2017

Being able to broadcast on, and exclusively monitor, the 2x chain, is far easier for wallets that only care about the 2x chain

Correct, but see my edits about using locktime and/or the outputs of a mega transaction to accomplish the same thing without the downsides of requiring more complex wallet changes or the technical debt of an added hardfork consensus rule.

@Sjors

This comment has been minimized.

Copy link

Sjors commented Oct 12, 2017

Remind me to refresh when spending a long time composing a reply :-)

It would seem that a mega transaction requires a fair bit of coordination, since each wallet user would need to give their address to whoever crafts the mega transaction. And it also requires a send-all-to-self transaction, which brings additional privacy issues, needs to be repeated if a user receives funds in the future on an existing address, etc.

n-locktime requires monitoring both chains, causing roughly the same issues as I described above. It would be useful though for someone to describe the exact procedure for this (and carefully consider all failure modes, which a wallet for non rocket scientists might need to deal with in their UI).

@JaredR26

This comment has been minimized.

Copy link

JaredR26 commented Oct 12, 2017

Remind me to refresh when spending a long time composing a reply :-)

:D Partly my fault for editing repeatedly.

It would seem that a mega transaction requires a fair bit of coordination,

This isn't how we would approach it, we would create P2SH outputs and either hand out or publish the outputs (or simply create anyone-can-spend outputs).

There's a big distinction between the needs of exchanges and the needs of users. Exchanges (in my mind, perhaps someone will say I am wrong) need a guaranteed approach that is efficient, but that approach can be technical and/or specific to them. nLocktime is probably not ideal for them, but they could chain outputs from a mega-tx.

Users on the other hand would probably find locktime to be more usable. With the huge discrepancy in hashing power expected between 2x and 1x, this will probably give days of time where a transaction can't be replayed for most users. As I mentioned, if 1x approaches or passes 2x in PoW, it most likely means 2x has failed and we'll be waiting years with high fees for a blocksize increase on Bitcoin.

(and carefully consider all failure modes, which a wallet for non rocket scientists might need to deal with in their UI).

FYI, I don't think that this is an achievable goal within any reasonable bounds of a replay protection. Even with mandatory 2-way replay protection on Bitcoin Cash, the identical address formats is causing issues for Bitcoin Cash and merchants considering accepting it. Splitting coins is always going to be a somewhat technical problem, and even if wallets had a guaranteed technical method, no UI is ever perfectly idiot proof(or fat-finger proof, or drunken bitcoiner proof, whatever you want to call the human element).

The goal is not perfection. The goal is "good enough" and "as good as we can do with all the tradeoffs." For a non-technical user, the safest way to split coins is going to be for them to either use a service/script/open source website to craft splitting transactions in some way, or to withdraw guaranteed split coins from an exchange that does have the technical knowledge, and then flip all coins to a new wallet with the tainted coins. Flipping-to-new-wallet or using a script someone else created was actually the safest way to do it for nontechnical users even with BCH's mandatory replay protection, which implies that the "good enough" bar must be somewhere around there to begin with.

@jratcliff63367

This comment has been minimized.

Copy link

jratcliff63367 commented Oct 12, 2017

Why is there an expiration date of only a few months on this? Are we expecting every single bitcoin holder, including Satoshi, to split their coins by then, or are you just assuming one of the two chains will be dead by that date? I'm going to assume there is a reason for the expiration date, but I don't see it in the code comments or in this discussion.

@JaredR26

This comment has been minimized.

Copy link

JaredR26 commented Oct 12, 2017

Are we expecting every single bitcoin holder, including Satoshi, to split their coins by then, or are you just assuming one of the two chains will be dead by that date?

The latter.

If somehow there are two chains, more steps can be taken at that point.

You somehow believe that everyone absolutely cares about having their coins split. Most people won't care.

@jgarzik

This comment has been minimized.

Copy link

jgarzik commented Oct 12, 2017

@jratcliff63367 Statistically, most likely the chain landscape will have been resolved by then.

That said, the current step is review by the wider community. Feedback ("make it date X") is welcome.

@jratcliff63367

This comment has been minimized.

Copy link

jratcliff63367 commented Oct 12, 2017

Ok, I won't get into a debate about it here since this is a code review. It doesn't appear to take into account the scenario that there are two perfectly functioning chains at the same time a few months from now. Our only precedent is BCH and both chains co-exist today and have gone their separate ways.

Obviously I object to this kind of 'soft replay protection' to begin with but, again, that's not code relevant.

You might want to add more comments to the code at least describing the rationale for the hard coded date.

Also, as a general comment, I don't care for hard coded constants like this in code; better to use an enumerated value or at least a #define. But, apparently, that ship has long since sailed on this code base since it appears to be littered with hard coded magic numbers everywhere.

@jgarzik

This comment has been minimized.

Copy link

jgarzik commented Oct 12, 2017

@jratcliff63367 "Magic number" is normal and expected for this area of code. Please look at the same code file in Bitcoin Core, e.g. https://github.com/bitcoin/bitcoin/blob/master/src/chainparams.cpp#L200

chainparams is a file of magic numbers -- that's its purpose.

@jratcliff63367

This comment has been minimized.

Copy link

jratcliff63367 commented Oct 12, 2017

Yeah, I know it's how it's done. It's just a nitpick. I'm pretty big on enums myself; because they are fully type-safe and add additional documentation.

@Sjors

This comment has been minimized.

Copy link

Sjors commented Oct 12, 2017

the identical address formats is causing issues for Bitcoin Cash and merchants considering accepting it.

Creating a special address format for receiving 2x-only transactions would indeed make things easier for the user, but this would be more involved (e.g. also requires block explorers to understand them).

Splitting coins is always going to be a somewhat technical problem

I think BCH is a good bar to stay at or above. Depending on the amount of publicity the fork gets, a technically inferior solution will probably result in even more support load. That's a trade-off only the SegWit2x team can decide on; I'm just pointing out technical issues so they're on record somewhere.

Statistically, most likely the chain landscape will have been resolved by then.

ETH & ETC have been buddies for over a year now and afaik they still have the replay protection code in place. What little statistical evidence we have suggests to me that if a chain doesn't die quickly, it quite possibly stays around a long time, possibly years. BCH, ETH and ETC have different difficulty adjustment mechanisms than BTC; time will tell if that matters.

The sunset code makes a previously invalid transaction valid. @petertodd recently said on the dev list (on a different topic):

This has been discussed many times before; there are severe downsides to
making it possible for transactions to become invalid after the fact.

I think he meant this in a general sense, but I don't understand the precise reason why and under what conditions it might be safe. Probably useful to find out though.

For sunsetting in the other direction (2x only txs), you would be confining the rule set. So perhaps a multi year horizon would be better there, which can be brought forward with a soft fork.

Regarding:

If somehow there are two chains, more steps can be taken at that point.

I would not casually count on another opportunity to hard fork the 2x chain. It's safer to treat this as a now or (more likely?) never situation.

@jcansdale

This comment has been minimized.

Copy link

jcansdale commented Oct 12, 2017

I was specifically asking about the rejection of the valid-on-2x-only transaction format suggested in #131.

This relaxing of rules would be a hard-fork as far as Bitcoin Unlimited (and compatible) nodes were concerned. It would also involve updating software libraries that are currently compatible with both chains. A hard-fork (beyond the 2x parameters) is unrealistic at this late stage.

@JaredR26

This comment has been minimized.

Copy link

JaredR26 commented Oct 12, 2017

Creating a special address format for receiving 2x-only transactions would indeed make things easier for the user, but this would be more involved (e.g. also requires block explorers to understand them).

FYI, another potential problem with that is that it does have the possibility of a social exploit, i.e. the "Send your BTC to <address> and they'll be replay protected!" type instructions, as opposed to the more complicated "Generate and send your coins there to be replay protected" type instructions. This was a nontrivial concern with the blacklisted address approach because people will definitely attempt to exploit non-technical users. :/

Depending on the amount of publicity the fork gets, a technically inferior solution will probably result in even more support load. That's a trade-off only the SegWit2x team can decide on;

Another FYI the lack of a clear best solution has contributed to our own indecisiveness. The opinions to open this pull request instead of moving forward with #131 were simply built out of a desire for the least bad, least intrusive option after balancing the alternatives.

Similarly the decision for opt-in replay protection versus mandatory doesn't come from a place of wanting to steal people's coins or any such nonsense, it is just a matter of picking the least-bad option amidst an array of people pushing a variety of bad options, most of whom care more about the decision affects their goals rather than how the decision affects the rest of the ecosystem. So... Least bad option.

@@ -80,6 +80,7 @@ class CMainParams : public CChainParams {
consensus.BIP65Height = 388381; // 000000000000000004c2b624ed5d7756c508d90fd0da2c7c679febfa6c4735f0
consensus.BIP66Height = 363725; // 00000000000000000379eaa19dce8c9b722d46ae6a57c2f1a988119488b50931
consensus.BIP102HeightDelta = 144 * 90;
consensus.BIP102ReplaySunsetTime = 1518134459; // February 9, 2018

This comment has been minimized.

@afk11

afk11 Oct 12, 2017

Why does this behaviour expire?

This comment has been minimized.

@jratcliff63367

jratcliff63367 Oct 12, 2017

This was answered by Jeff on the main thread. It is his opinion that by the time this date arrives one of the two forks will be the sole survivor (even though precedent of other controversial hard forks does not bear this assumption out). He also stated that if it needs to be revised, it can be later. However this would require yet another hard fork of the network and, as we all know, hard forks are difficult to pull off. Personally, I agree that something so 'temporary' doesn't make sense, especially if it is dependent on yet another hard fork in the future. In my view this should be better thought out. Long term holders may wait many months, if not years, to split their coins.

This comment has been minimized.

@jcansdale

jcansdale Oct 12, 2017

However this would require yet another hard fork of the network and, as we all know, hard forks are difficult to pull off.

If we don't build in a sunset, it would take a hard-fork to remove it later. Knowing how tricky hard-forks are to pull off, this would likely never happen. By building in a sunset, we can revisit later and it's only a soft-fork if we decide to renew.

The hope is that over time exchanges will develop more sophisticated solutions (tracking and comparing unspent outputs on both chains). Because OP_RETURN replay protections takes a little extra tx space, there will be incentive to do this. In a few months time, there will be pools of unspent outputs on both chains that could be used to taint.

This comment has been minimized.

@christophebiocca

christophebiocca Oct 12, 2017

However this would require yet another hard fork of the network and, as we all know, hard forks are difficult to pull off.

That is inaccurate. Extending the behaviour, or even making it permanent, is just a soft-fork. This means it can be enforced by a simple majority (but ideally a super-majority) of hashpower, without relying on everyone to upgrade.

This comment has been minimized.

@Sjors

Sjors Oct 13, 2017

Not be confused with extending #131 protection on the 2x chain, which works the opposite way and would require a hard fork to extend (and a soft-fork to end it sooner) IIUC.

This comment has been minimized.

@afk11

afk11 Oct 13, 2017

Long term holders may wait many months, if not years, to split their coins.

This is precisely my concern! It places un-due urgency on making a transaction within a certain timeframe - because later is categorically less safe.

Just leave it indefinitely, since the extra data isn't even hitting this chain.

@avmn87

This comment has been minimized.

Copy link

avmn87 commented Oct 15, 2017

So as a non-technical user, what am I to make of this pull request? Is this the replay protection that everyone wants to protect intended transactions on both chains?

Can we just settle on the need for replay protection and avoid the potential for mutually assured destruction of both Bitcoin chains and just let the best one win?

@jgarzik

This comment has been minimized.

Copy link

jgarzik commented Nov 8, 2017

Closing - not needed

@jgarzik jgarzik closed this Nov 8, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment