From 27803ee8c1a327e902e5efedb3bd0c4cdb7b2bee Mon Sep 17 00:00:00 2001
From: matejcik <ja@matejcik.cz>
Date: Thu, 12 Mar 2020 15:30:21 +0100
Subject: [PATCH] all: drop overwintered field from transaction

---
 common/protob/messages-bitcoin.proto          | 12 ++++-----
 core/src/apps/wallet/sign_tx/helpers.py       |  2 --
 core/src/apps/wallet/sign_tx/segwit_bip143.py |  2 +-
 core/src/apps/wallet/sign_tx/signing.py       | 14 +++++------
 core/src/apps/wallet/sign_tx/zcash.py         |  4 +--
 core/src/trezor/messages/SignTx.py            |  3 ---
 core/src/trezor/messages/TransactionType.py   |  3 ---
 core/tests/test_apps.wallet.zcash.zip143.py   |  1 -
 core/tests/test_apps.wallet.zcash.zip243.py   |  1 -
 legacy/firmware/signing.c                     | 25 +++++++++----------
 python/src/trezorlib/messages/SignTx.py       |  3 ---
 .../src/trezorlib/messages/TransactionType.py |  3 ---
 tests/device_tests/test_msg_signtx_komodo.py  |  2 --
 tests/device_tests/test_msg_signtx_zcash.py   | 10 ++------
 14 files changed, 30 insertions(+), 55 deletions(-)

diff --git a/common/protob/messages-bitcoin.proto b/common/protob/messages-bitcoin.proto
index eb119a17a7..55ac743bde 100644
--- a/common/protob/messages-bitcoin.proto
+++ b/common/protob/messages-bitcoin.proto
@@ -130,10 +130,10 @@ message SignTx {
     optional uint32 version = 4 [default=1];            // transaction version
     optional uint32 lock_time = 5 [default=0];          // transaction lock_time
     optional uint32 expiry = 6;                         // only for Decred and Zcash
-    optional bool overwintered = 7;                     // only for Zcash
-    optional uint32 version_group_id = 8;               // only for Zcash, nVersionGroupId when overwintered is set
+    // optional bool overwintered = 7;                  // deprecated - only for Zcash
+    optional uint32 version_group_id = 8;               // only for Zcash, nVersionGroupId
     optional uint32 timestamp = 9;                      // only for Peercoin, Capricoin
-    optional uint32 branch_id = 10;                     // only for Zcash, BRANCH_ID when overwintered is set
+    optional uint32 branch_id = 10;                     // only for Zcash, BRANCH_ID
 }
 
 /**
@@ -196,10 +196,10 @@ message TxAck {
         optional bytes extra_data = 8;          // only for Dash, Zcash
         optional uint32 extra_data_len = 9;     // only for Dash, Zcash
         optional uint32 expiry = 10;            // only for Decred and Zcash
-        optional bool overwintered = 11;        // only for Zcash
-        optional uint32 version_group_id = 12;  // only for Zcash, nVersionGroupId when overwintered is set
+        // optional bool overwintered = 11;     // deprecated - only for Zcash
+        optional uint32 version_group_id = 12;  // only for Zcash, nVersionGroupId
         optional uint32 timestamp = 13;         // only for Peercoin, Capricoin
-        optional uint32 branch_id = 14;         // only for Zcash, BRANCH_ID when overwintered is set
+        optional uint32 branch_id = 14;         // only for Zcash, BRANCH_ID
         /**
         * Structure representing transaction input
         */
diff --git a/core/src/apps/wallet/sign_tx/helpers.py b/core/src/apps/wallet/sign_tx/helpers.py
index 8ce189521b..d01f35b163 100644
--- a/core/src/apps/wallet/sign_tx/helpers.py
+++ b/core/src/apps/wallet/sign_tx/helpers.py
@@ -172,7 +172,6 @@ def sanitize_sign_tx(tx: SignTx, coin: CoinInfo) -> SignTx:
     tx.outputs_count = tx.outputs_count if tx.outputs_count is not None else 0
     tx.coin_name = tx.coin_name if tx.coin_name is not None else "Bitcoin"
     tx.expiry = tx.expiry if tx.expiry is not None else 0
-    tx.overwintered = tx.overwintered if tx.overwintered is not None else False
     tx.timestamp = tx.timestamp if tx.timestamp is not None else 0
     return tx
 
@@ -184,7 +183,6 @@ def sanitize_tx_meta(tx: TransactionType, coin: CoinInfo) -> TransactionType:
     tx.outputs_cnt = tx.outputs_cnt if tx.outputs_cnt is not None else 0
     tx.extra_data_len = tx.extra_data_len if tx.extra_data_len is not None else 0
     tx.expiry = tx.expiry if tx.expiry is not None else 0
-    tx.overwintered = tx.overwintered if tx.overwintered is not None else False
     tx.timestamp = tx.timestamp if tx.timestamp is not None else 0
     return tx
 
diff --git a/core/src/apps/wallet/sign_tx/segwit_bip143.py b/core/src/apps/wallet/sign_tx/segwit_bip143.py
index e25a8ff331..a92d4a847d 100644
--- a/core/src/apps/wallet/sign_tx/segwit_bip143.py
+++ b/core/src/apps/wallet/sign_tx/segwit_bip143.py
@@ -58,7 +58,7 @@ class Bip143:
     ) -> bytes:
         h_preimage = HashWriter(sha256())
 
-        ensure(not tx.overwintered)
+        ensure(not coin.overwintered)
 
         write_uint32(h_preimage, tx.version)  # nVersion
         write_bytes(h_preimage, self.get_prevouts_hash(coin))  # hashPrevouts
diff --git a/core/src/apps/wallet/sign_tx/signing.py b/core/src/apps/wallet/sign_tx/signing.py
index 71dafe5606..dee13b1b76 100644
--- a/core/src/apps/wallet/sign_tx/signing.py
+++ b/core/src/apps/wallet/sign_tx/signing.py
@@ -64,7 +64,7 @@ async def check_tx_fee(tx: SignTx, keychain: seed.Keychain, coin: coininfo.CoinI
     if not utils.BITCOIN_ONLY and coin.decred:
         hash143 = decred.DecredPrefixHasher(tx)  # pseudo BIP-0143 prefix hashing
         tx_ser = TxRequestSerializedType()
-    elif not utils.BITCOIN_ONLY and tx.overwintered:
+    elif not utils.BITCOIN_ONLY and coin.overwintered:
         if tx.version == 3:
             branch_id = tx.branch_id or 0x5BA81B19  # Overwinter
             hash143 = zcash.Zip143(branch_id)  # ZIP-0143 transaction hashing
@@ -130,7 +130,7 @@ async def check_tx_fee(tx: SignTx, keychain: seed.Keychain, coin: coininfo.CoinI
             InputScriptType.SPENDADDRESS,
             InputScriptType.SPENDMULTISIG,
         ):
-            if coin.force_bip143 or (not utils.BITCOIN_ONLY and tx.overwintered):
+            if coin.force_bip143 or (not utils.BITCOIN_ONLY and coin.overwintered):
                 if not txi.amount:
                     raise SigningError(
                         FailureType.DataError, "Expected input with amount"
@@ -288,7 +288,7 @@ async def sign_tx(tx: SignTx, keychain: seed.Keychain):
             tx_ser.signature = None
             tx_req.serialized = tx_ser
 
-        elif coin.force_bip143 or (not utils.BITCOIN_ONLY and tx.overwintered):
+        elif coin.force_bip143 or (not utils.BITCOIN_ONLY and coin.overwintered):
             # STAGE_REQUEST_SEGWIT_INPUT
             txi_sign = await helpers.request_tx_input(tx_req, i_sign, coin)
             input_check_wallet_path(txi_sign, wallet_path)
@@ -569,7 +569,7 @@ async def sign_tx(tx: SignTx, keychain: seed.Keychain):
 
     writers.write_uint32(tx_ser.serialized_tx, tx.lock_time)
 
-    if not utils.BITCOIN_ONLY and tx.overwintered:
+    if not utils.BITCOIN_ONLY and coin.overwintered:
         if tx.version == 3:
             writers.write_uint32(tx_ser.serialized_tx, tx.expiry)  # expiryHeight
             writers.write_varint(tx_ser.serialized_tx, 0)  # nJoinSplit
@@ -606,7 +606,7 @@ async def get_prevtx_output_value(
     else:
         txh = utils.HashWriter(sha256())
 
-    if not utils.BITCOIN_ONLY and tx.overwintered:
+    if not utils.BITCOIN_ONLY and coin.overwintered:
         writers.write_uint32(
             txh, tx.version | zcash.OVERWINTERED
         )  # nVersion | fOverwintered
@@ -649,7 +649,7 @@ async def get_prevtx_output_value(
 
     writers.write_uint32(txh, tx.lock_time)
 
-    if not utils.BITCOIN_ONLY and (tx.overwintered or coin.decred):
+    if not utils.BITCOIN_ONLY and (coin.overwintered or coin.decred):
         writers.write_uint32(txh, tx.expiry)
 
     ofs = 0
@@ -683,7 +683,7 @@ def get_hash_type(coin: coininfo.CoinInfo) -> int:
 
 def get_tx_header(coin: coininfo.CoinInfo, tx: SignTx, segwit: bool):
     w_txi = bytearray()
-    if not utils.BITCOIN_ONLY and tx.overwintered:
+    if not utils.BITCOIN_ONLY and coin.overwintered:
         writers.write_uint32(
             w_txi, tx.version | zcash.OVERWINTERED
         )  # nVersion | fOverwintered
diff --git a/core/src/apps/wallet/sign_tx/zcash.py b/core/src/apps/wallet/sign_tx/zcash.py
index a192922a2e..d0776fa3fe 100644
--- a/core/src/apps/wallet/sign_tx/zcash.py
+++ b/core/src/apps/wallet/sign_tx/zcash.py
@@ -85,7 +85,7 @@ class Zip143:
             )
         )
 
-        ensure(tx.overwintered)
+        ensure(coin.overwintered)
         ensure(tx.version == 3)
 
         write_uint32(
@@ -132,7 +132,7 @@ class Zip243(Zip143):
             )
         )
 
-        ensure(tx.overwintered)
+        ensure(coin.overwintered)
         ensure(tx.version == 4)
 
         write_uint32(
diff --git a/core/src/trezor/messages/SignTx.py b/core/src/trezor/messages/SignTx.py
index 67e5b3ebcd..2977808ea0 100644
--- a/core/src/trezor/messages/SignTx.py
+++ b/core/src/trezor/messages/SignTx.py
@@ -21,7 +21,6 @@ class SignTx(p.MessageType):
         version: int = None,
         lock_time: int = None,
         expiry: int = None,
-        overwintered: bool = None,
         version_group_id: int = None,
         timestamp: int = None,
         branch_id: int = None,
@@ -32,7 +31,6 @@ class SignTx(p.MessageType):
         self.version = version
         self.lock_time = lock_time
         self.expiry = expiry
-        self.overwintered = overwintered
         self.version_group_id = version_group_id
         self.timestamp = timestamp
         self.branch_id = branch_id
@@ -46,7 +44,6 @@ class SignTx(p.MessageType):
             4: ('version', p.UVarintType, 0),  # default=1
             5: ('lock_time', p.UVarintType, 0),  # default=0
             6: ('expiry', p.UVarintType, 0),
-            7: ('overwintered', p.BoolType, 0),
             8: ('version_group_id', p.UVarintType, 0),
             9: ('timestamp', p.UVarintType, 0),
             10: ('branch_id', p.UVarintType, 0),
diff --git a/core/src/trezor/messages/TransactionType.py b/core/src/trezor/messages/TransactionType.py
index 0d239f6bd5..b2aaaf1ec4 100644
--- a/core/src/trezor/messages/TransactionType.py
+++ b/core/src/trezor/messages/TransactionType.py
@@ -28,7 +28,6 @@ class TransactionType(p.MessageType):
         extra_data: bytes = None,
         extra_data_len: int = None,
         expiry: int = None,
-        overwintered: bool = None,
         version_group_id: int = None,
         timestamp: int = None,
         branch_id: int = None,
@@ -43,7 +42,6 @@ class TransactionType(p.MessageType):
         self.extra_data = extra_data
         self.extra_data_len = extra_data_len
         self.expiry = expiry
-        self.overwintered = overwintered
         self.version_group_id = version_group_id
         self.timestamp = timestamp
         self.branch_id = branch_id
@@ -61,7 +59,6 @@ class TransactionType(p.MessageType):
             8: ('extra_data', p.BytesType, 0),
             9: ('extra_data_len', p.UVarintType, 0),
             10: ('expiry', p.UVarintType, 0),
-            11: ('overwintered', p.BoolType, 0),
             12: ('version_group_id', p.UVarintType, 0),
             13: ('timestamp', p.UVarintType, 0),
             14: ('branch_id', p.UVarintType, 0),
diff --git a/core/tests/test_apps.wallet.zcash.zip143.py b/core/tests/test_apps.wallet.zcash.zip143.py
index 4ee999479d..8c7d3eaba2 100644
--- a/core/tests/test_apps.wallet.zcash.zip143.py
+++ b/core/tests/test_apps.wallet.zcash.zip143.py
@@ -151,7 +151,6 @@ class TestZcashZip143(unittest.TestCase):
                 version=v["version"],
                 lock_time=v["lock_time"],
                 expiry=v["expiry"],
-                overwintered=(v["version"] >= 3),
                 version_group_id=v["version_group_id"],
             )
             zip143 = Zip143(0x5ba81b19)  # Overwinter
diff --git a/core/tests/test_apps.wallet.zcash.zip243.py b/core/tests/test_apps.wallet.zcash.zip243.py
index 776179b85f..b4ed1e0792 100644
--- a/core/tests/test_apps.wallet.zcash.zip243.py
+++ b/core/tests/test_apps.wallet.zcash.zip243.py
@@ -185,7 +185,6 @@ class TestZcashZip243(unittest.TestCase):
                 version=v["version"],
                 lock_time=v["lock_time"],
                 expiry=v["expiry"],
-                overwintered=(v["version"] >= 3),
                 version_group_id=v["version_group_id"],
             )
             zip243 = Zip243(0x76b809bb)  # Sapling
diff --git a/legacy/firmware/signing.c b/legacy/firmware/signing.c
index b8d0b242df..4cc9944f63 100644
--- a/legacy/firmware/signing.c
+++ b/legacy/firmware/signing.c
@@ -70,7 +70,6 @@ static uint64_t to_spend, authorized_amount, spending, change_spend;
 static uint32_t version = 1;
 static uint32_t lock_time = 0;
 static uint32_t expiry = 0;
-static bool overwintered = false;
 static uint32_t version_group_id = 0;
 static uint32_t timestamp = 0;
 #if !BITCOIN_ONLY
@@ -486,12 +485,11 @@ void signing_init(const SignTx *msg, const CoinInfo *_coin,
   lock_time = msg->lock_time;
   expiry = msg->expiry;
 #if !BITCOIN_ONLY
-  overwintered = msg->has_overwintered && msg->overwintered;
   version_group_id = msg->version_group_id;
   timestamp = msg->timestamp;
   branch_id = msg->branch_id;
   // set default values for Zcash if branch_id is unset
-  if (overwintered && (branch_id == 0)) {
+  if (coin->overwintered && (branch_id == 0)) {
     switch (version) {
       case 3:
         branch_id = 0x5BA81B19;  // Overwinter
@@ -536,7 +534,8 @@ void signing_init(const SignTx *msg, const CoinInfo *_coin,
   next_nonsegwit_input = 0xffffffff;
 
   tx_init(&to, inputs_count, outputs_count, version, lock_time, expiry, 0,
-          coin->curve->hasher_sign, overwintered, version_group_id, timestamp);
+          coin->curve->hasher_sign, coin->overwintered, version_group_id,
+          timestamp);
 
 #if !BITCOIN_ONLY
   if (coin->decred) {
@@ -544,7 +543,7 @@ void signing_init(const SignTx *msg, const CoinInfo *_coin,
     to.is_decred = true;
 
     tx_init(&ti, inputs_count, outputs_count, version, lock_time, expiry, 0,
-            coin->curve->hasher_sign, overwintered, version_group_id,
+            coin->curve->hasher_sign, coin->overwintered, version_group_id,
             timestamp);
     ti.version |= (DECRED_SERIALIZE_NO_WITNESS << 16);
     ti.is_decred = true;
@@ -553,7 +552,7 @@ void signing_init(const SignTx *msg, const CoinInfo *_coin,
 
   // segwit hashes for hashPrevouts and hashSequence
 #if !BITCOIN_ONLY
-  if (overwintered) {
+  if (coin->overwintered) {
     hasher_InitParam(&hasher_prevouts, HASHER_BLAKE2B_PERSONAL,
                      "ZcashPrevoutHash", 16);
     hasher_InitParam(&hasher_sequence, HASHER_BLAKE2B_PERSONAL,
@@ -1139,7 +1138,7 @@ void signing_txack(TransactionType *tx) {
         }
 #endif
 
-        if (coin->force_bip143 || overwintered) {
+        if (coin->force_bip143 || coin->overwintered) {
           if (!tx->inputs[0].has_amount) {
             fsm_sendFailure(FailureType_Failure_DataError,
                             _("Expected input with amount"));
@@ -1229,7 +1228,7 @@ void signing_txack(TransactionType *tx) {
       }
       tx_init(&tp, tx->inputs_cnt, tx->outputs_cnt, tx->version, tx->lock_time,
               tx->expiry, tx->extra_data_len, coin->curve->hasher_sign,
-              tx->overwintered, tx->version_group_id, tx->timestamp);
+              coin->overwintered, tx->version_group_id, tx->timestamp);
 #if !BITCOIN_ONLY
       if (coin->decred) {
         tp.version |= (DECRED_SERIALIZE_NO_WITNESS << 16);
@@ -1331,7 +1330,7 @@ void signing_txack(TransactionType *tx) {
                  PROGRESS_PRECISION);
       if (idx2 == 0) {
         tx_init(&ti, inputs_count, outputs_count, version, lock_time, expiry, 0,
-                coin->curve->hasher_sign, overwintered, version_group_id,
+                coin->curve->hasher_sign, coin->overwintered, version_group_id,
                 timestamp);
         hasher_Reset(&hasher_check);
       }
@@ -1427,7 +1426,7 @@ void signing_txack(TransactionType *tx) {
       resp.serialized.has_serialized_tx = true;
       if (tx->inputs[0].script_type == InputScriptType_SPENDMULTISIG ||
           tx->inputs[0].script_type == InputScriptType_SPENDADDRESS) {
-        if (!(coin->force_bip143 || overwintered)) {
+        if (!(coin->force_bip143 || coin->overwintered)) {
           fsm_sendFailure(FailureType_Failure_DataError,
                           _("Transaction has changed during signing"));
           signing_abort();
@@ -1449,7 +1448,7 @@ void signing_txack(TransactionType *tx) {
 
         uint8_t hash[32] = {0};
 #if !BITCOIN_ONLY
-        if (overwintered) {
+        if (coin->overwintered) {
           switch (version) {
             case 3:
               signing_hash_zip143(&tx->inputs[0], hash);
@@ -1575,14 +1574,14 @@ void signing_txack(TransactionType *tx) {
       if (idx1 == 0) {
         // witness
         tx_init(&to, inputs_count, outputs_count, version, lock_time, expiry, 0,
-                coin->curve->hasher_sign, overwintered, version_group_id,
+                coin->curve->hasher_sign, coin->overwintered, version_group_id,
                 timestamp);
         to.is_decred = true;
       }
 
       // witness hash
       tx_init(&ti, inputs_count, outputs_count, version, lock_time, expiry, 0,
-              coin->curve->hasher_sign, overwintered, version_group_id,
+              coin->curve->hasher_sign, coin->overwintered, version_group_id,
               timestamp);
       ti.version |= (DECRED_SERIALIZE_WITNESS_SIGNING << 16);
       ti.is_decred = true;
diff --git a/python/src/trezorlib/messages/SignTx.py b/python/src/trezorlib/messages/SignTx.py
index c34c04bb12..905f019b02 100644
--- a/python/src/trezorlib/messages/SignTx.py
+++ b/python/src/trezorlib/messages/SignTx.py
@@ -21,7 +21,6 @@ class SignTx(p.MessageType):
         version: int = None,
         lock_time: int = None,
         expiry: int = None,
-        overwintered: bool = None,
         version_group_id: int = None,
         timestamp: int = None,
         branch_id: int = None,
@@ -32,7 +31,6 @@ class SignTx(p.MessageType):
         self.version = version
         self.lock_time = lock_time
         self.expiry = expiry
-        self.overwintered = overwintered
         self.version_group_id = version_group_id
         self.timestamp = timestamp
         self.branch_id = branch_id
@@ -46,7 +44,6 @@ class SignTx(p.MessageType):
             4: ('version', p.UVarintType, 0),  # default=1
             5: ('lock_time', p.UVarintType, 0),  # default=0
             6: ('expiry', p.UVarintType, 0),
-            7: ('overwintered', p.BoolType, 0),
             8: ('version_group_id', p.UVarintType, 0),
             9: ('timestamp', p.UVarintType, 0),
             10: ('branch_id', p.UVarintType, 0),
diff --git a/python/src/trezorlib/messages/TransactionType.py b/python/src/trezorlib/messages/TransactionType.py
index c3eb7698d6..c3949337b3 100644
--- a/python/src/trezorlib/messages/TransactionType.py
+++ b/python/src/trezorlib/messages/TransactionType.py
@@ -28,7 +28,6 @@ class TransactionType(p.MessageType):
         extra_data: bytes = None,
         extra_data_len: int = None,
         expiry: int = None,
-        overwintered: bool = None,
         version_group_id: int = None,
         timestamp: int = None,
         branch_id: int = None,
@@ -43,7 +42,6 @@ class TransactionType(p.MessageType):
         self.extra_data = extra_data
         self.extra_data_len = extra_data_len
         self.expiry = expiry
-        self.overwintered = overwintered
         self.version_group_id = version_group_id
         self.timestamp = timestamp
         self.branch_id = branch_id
@@ -61,7 +59,6 @@ class TransactionType(p.MessageType):
             8: ('extra_data', p.BytesType, 0),
             9: ('extra_data_len', p.UVarintType, 0),
             10: ('expiry', p.UVarintType, 0),
-            11: ('overwintered', p.BoolType, 0),
             12: ('version_group_id', p.UVarintType, 0),
             13: ('timestamp', p.UVarintType, 0),
             14: ('branch_id', p.UVarintType, 0),
diff --git a/tests/device_tests/test_msg_signtx_komodo.py b/tests/device_tests/test_msg_signtx_komodo.py
index c7594ee739..46e76252cc 100644
--- a/tests/device_tests/test_msg_signtx_komodo.py
+++ b/tests/device_tests/test_msg_signtx_komodo.py
@@ -85,7 +85,6 @@ class TestMsgSigntxKomodo:
 
             details = proto.SignTx(
                 version=4,
-                overwintered=True,
                 version_group_id=0x892F2085,
                 branch_id=0x76B809BB,
                 lock_time=0x5D2A30B8,
@@ -165,7 +164,6 @@ class TestMsgSigntxKomodo:
 
             details = proto.SignTx(
                 version=4,
-                overwintered=True,
                 version_group_id=0x892F2085,
                 branch_id=0x76B809BB,
                 lock_time=0x5D2AF1F2,
diff --git a/tests/device_tests/test_msg_signtx_zcash.py b/tests/device_tests/test_msg_signtx_zcash.py
index e063c3e938..85ea5939d1 100644
--- a/tests/device_tests/test_msg_signtx_zcash.py
+++ b/tests/device_tests/test_msg_signtx_zcash.py
@@ -79,10 +79,7 @@ class TestMsgSigntxZcash:
             )
 
             details = proto.SignTx(
-                version=3,
-                overwintered=True,
-                version_group_id=0x03C48270,
-                branch_id=0x5BA81B19,
+                version=3, version_group_id=0x03C48270, branch_id=0x5BA81B19,
             )
             _, serialized_tx = btc.sign_tx(
                 client,
@@ -144,10 +141,7 @@ class TestMsgSigntxZcash:
             )
 
             details = proto.SignTx(
-                version=4,
-                overwintered=True,
-                version_group_id=0x892F2085,
-                branch_id=0x76B809BB,
+                version=4, version_group_id=0x892F2085, branch_id=0x76B809BB,
             )
             _, serialized_tx = btc.sign_tx(
                 client,