Bitcoin-ABC / bitcoin-abc

Bitcoin ABC develops node software and infrastructure for the eCash project. This a mirror of the official Bitcoin-ABC repository. Please see README.md
https://reviews.bitcoinabc.org
MIT License
1.24k stars 779 forks source link

Implement two-way replay protection #24

Closed bendelo closed 7 years ago

bendelo commented 7 years ago

The FAQ on https://www.bitcoincash.org used to say:

Bitcoin Cash transactions use a new flag SIGHASH_FORKID, which are invalid on the legacy blockchain. This prevents Bitcoin Cash transactions from being replayed on the Bitcoin blockchain. Legacy transactions will be non standard on the new chain, but hard replay protection will be opt-in: you must specify with the OP_RETURN flag that BTC transactions be ignored on the Bitcoin Cash chain. This means if you are using a non-upgraded Bitcoin wallet, you will be potentially making a Bitcoin Cash transaction each time you make a Bitcoin transaction, unless you specifically set the flag not to. Since the transactions are non-standard, that means normally this should not happen, but a miner could choose to include your Bitcoin transactions in a Bitcoin Cash block if replay protection is not specified.

However now it says:

Bitcoin Cash transactions use a new flag SIGHASH_FORKID, which is non standard to the legacy blockchain. This prevents Bitcoin Cash transactions from being replayed on the Bitcoin blockchain and vice versa.

How are you now preventing Bitcoin transactions from being replayed onto Bitcoin Cash. Is SIGHASH_FORKID now mandatory?

The spec still contains that opt-in requirement and it needs to be updated: https://github.com/Bitcoin-UAHF/spec/blob/master/uahf-technical-spec.md#req-6-1-disallow-special-op_return-marked-transactions-with-sunset-clause

prusnak commented 7 years ago

I strongly recommend to avoid using OP_RETURN for BTC->BCC replay protection. Lots of wallets (ok, almost all of them) do not support OP_RETURN and users will be constantly leaking BTC transactions to BCC, which will hurt you.

deadalnix commented 7 years ago

Hi @prusnak . This is indeed a concern. The spec was made as a contingency plan for UASF, but the situation has changed quite dramatically since then. We have something in the making to improve the situation.

jonny1000 commented 7 years ago

The spec was made as a contingency plan for UASF, but the situation has changed quite dramatically since then.

What difference does it make if this is a contingency plan for UASF or not? Isn't replay protection important either way?

We have something in the making to improve the situation.

The coin is due to launch in just 7 days, yet the transaction format appears not to be finalized yet. It might be a good idea to delay the launch until these issues are sorted out and tested.

NiKiZe commented 7 years ago

@jonny1000 fork must happen before segwit blocks goes live (but I agree a "few" days later should be ok)

bendelo commented 7 years ago

@deadalnix can you confirm if there will be BTC->BCC replay protection enabled by default (no opt-in required)?

NiKiZe commented 7 years ago

Old clients, such as SPV (or others without cripling blocksize limitations) must be able to send on any chain without being updated - as such replay protection can only be opt-in

prusnak commented 7 years ago

I don't agree. SPV wallet needs to know on which chain it is operating in order to function properly. Also try to keep the discussion civil, nobody is interested in your snarky remarks.

prusnak commented 7 years ago

FTR there is a discussion going on here how to implement replay protection properly by making SIGHASH_FORKID mandatory: https://github.com/Bitcoin-UAHF/spec/pull/17

deadalnix commented 7 years ago

We are not here to debate activation date nor what it changes to be a contingency plan or whatever.

@bendelo I can't promise you that everybody will run that code. I can promise that this'll be in the next ABC release. For better or worse, this system is decentralized and the only piece I can control is ABC itself.

murchandamus commented 7 years ago

We at BitGo would highly appreciate two-way replay protection. It's the right thing for users and for long-term viability of the new coin. It would certainly improve the schedule on which BitGo could offer direct support for the new chain.

bithernet commented 7 years ago

We (Bither Team) strongly recommend Bitcoin Cash to implement two-way replay protection. Protecting normal users' assets is the most important thing if you care about them.

paullinator commented 7 years ago

I would like to add my support for the recommendation of two-way replay protection. This would be very much appreciated by the team at Airbitz and would accelerate our support of Bitcoin Cash as an additional currency.

On a separate note, is there a testnet deployed that is functioning as if the fork had already happened? ie. support for big blocks and proper rejection of transactions that have the OP_RETURN field set? Given the short time frame, it would be good to have a network to test our code against that mirrors the post-fork conditions.

NiKiZe commented 7 years ago

Most support for mandatory replay protection seems to be based on the premise that it is "additional currency" and i think the big question here then becomes, is Bitcoin Cash an altcoin based on Bitcoin, or is it a Bitcoin upgrade?

paullinator commented 7 years ago

@NiKiZe Regardless of how you view it, the protection prevents the funds from being stolen due to a transaction on the 'other' chain.

NiKiZe commented 7 years ago

Might be better to only discuss this in Bitcoin-UAHF/spec#17 instead. As said there there is cases where forced relay protection can also prevent you from using 'this' chain, and it also prevents 'this' chain from becoming the only chain. Opt-in replay protection does indeed allow for loss prevention as long as you aren't careless

philsong commented 7 years ago

I strongly recommend Bitcoin Cash to implement two-way replay protection. Protecting normal users' assets is the most important thing if you care about them.

bendelo commented 7 years ago

The pull request to update the UAHF spec got merged: https://github.com/Bitcoin-UAHF/spec/blob/master/uahf-technical-spec.md#req-6-2-mandatory-signature-shift-via-hash-type

Does this commit https://github.com/Bitcoin-ABC/bitcoin-abc/commit/e49826c1fcc36e5ae26de0ad4d06e2063a759e73 now ensure there is no way a BTC transaction can be replayed onto BCC?

ftrader commented 7 years ago

@bendelo : correct - once this is released (should be in 0.14.5), it will ensure BTC transactions cannot be replayed onto BCC .

bendelo commented 7 years ago

Will version 0.14.5 be released and used by the miners before the hard fork?

deadalnix commented 7 years ago

@bendelo I can't voutch for the miner, but we are int he process of preparing the release so it should be available soon.

ftrader commented 7 years ago

@bendelo : if you are satisfied with the 2-way protection implementation, please close this issue.

deadalnix commented 7 years ago

Closing this one as it is done.

deadalnix commented 6 years ago

ALL | SIGHASH_FORKID

Le 2 mars 2018 08:49, "via1982" notifications@github.com a écrit :

Hi All,

I'm using bitcoincash ABC wallet and bitcoincashd which is running under testnet on my local machine. From my java code I'm tring to sing transaction using RPC call to local bitcoincashd and getting "Signature must use SIGHASH_FORKID (code -8)"

I tried to sign via wallet console and got the same:

signrawtransaction 0200000001b2c2d84d22eae13a4cd0 e58a21a836f334f7e3fa413edc7ae6ba2ae9f6b0a6fa0000000000ffffff ff0153707e41000000001976a914607b1f9527a764190f035da0e8aa3ef3aa01f81f88ac00000000, null, null, ALL

Signature must use SIGHASH_FORKID (code -8)

What I do wrong here. I understand that I have to use SIGHASH_FORKID (0x40) but where? 

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/Bitcoin-ABC/bitcoin-abc/issues/24#issuecomment-369847587, or mute the thread https://github.com/notifications/unsubscribe-auth/AA0Iab8cEAaJCvGcc8QbrEy2wHMZlSBxks5taPlvgaJpZM4OgYib .

via1982 commented 6 years ago

It works thank you! signrawtransaction 0200000001b2c2d84d22eae13a4cd0e58a21a836f334f7e3fa413edc7ae6ba2ae9f6b0a6fa0000000000ffffffff0153707e41000000001976a914607b1f9527a764190f035da0e8aa3ef3aa01f81f88ac00000000, null, null, ALL|FORKID

dalancon commented 6 years ago

@via1982 how fix this problem. what is the FORKID ?