fix: sign legacy second signature over the signed transaction hash

crypto/transactions/deserializer.py

@@ -47,8 +47,11 @@ def deserialize(self) -> AbstractTransaction:
 
         if len(decoded_rlp) >= 9:
             data['v'] = Deserializer.__parse_number(decoded_rlp[6]) - (Network.get_network().chain_id() * 2 + 35)
-            data['r'] = Deserializer.__parse_hex(decoded_rlp[7])
-            data['s'] = Deserializer.__parse_hex(decoded_rlp[8])
+            data['r'] = Deserializer.__parse_hex(decoded_rlp[7]).zfill(64)
+            data['s'] = Deserializer.__parse_hex(decoded_rlp[8]).zfill(64)
+
+        if len(decoded_rlp) == 10:
+            data['legacySecondSignature'] = Deserializer.__parse_hex(decoded_rlp[9])
 
         transaction = self.__guess_transaction_from_data(data)
 

crypto/transactions/types/abstract_transaction.py

@@ -8,6 +8,9 @@
 from crypto.identity.public_key import PublicKey
 from crypto.utils.transaction_utils import TransactionUtils
 
+# 0x8f ‖ "MAINSAIL_LSS_V1"
+LEGACY_SECOND_SIGNATURE_DOMAIN = '8f4d41494e5341494c5f4c53535f5631'
+
 class AbstractTransaction:
     def __init__(self, data: dict):
         self.data = data
@@ -36,9 +39,10 @@ def sign(self, private_key: PrivateKey):
         return self
 
     def legacy_second_sign(self, second_private_key: PrivateKey):
-        transaction_hash = TransactionUtils.to_buffer(self.data, skip_signature=True).decode()
+        if self.data.get('v') is None or not self.data.get('r') or not self.data.get('s'):
+            raise ValueError('Transaction must be signed before adding a legacy second signature')
 
-        message = bytes.fromhex(transaction_hash)
+        message = bytes.fromhex(LEGACY_SECOND_SIGNATURE_DOMAIN + self.get_id())
 
         self.data['legacySecondSignature'] = second_private_key.sign_to_ecdsa(message).hex()
 

crypto/utils/rlp_encoder.py

@@ -61,10 +61,13 @@ def __arrayify_integer(cls, value: int) -> list:
     @classmethod
     def __get_bytes(cls, value: Union[str, bytes, int, list]) -> list:
         if isinstance(value, str) or isinstance(value, bytes):
+            # Only strings carry a `0x` hex prefix, raw bytes can start with 0x3078 too
+            is_hex_string = isinstance(value, str) and value.startswith('0x')
+
             if isinstance(value, str):
                 value = value.encode()
 
-            if value.startswith(b'0x'):
+            if is_hex_string:
                 hex_str = value[2:]
                 if hex_str == '':
                     return []

crypto/utils/transaction_utils.py

@@ -7,7 +7,12 @@
 
 class TransactionUtils:
     @classmethod
-    def to_buffer(cls, transaction: dict, skip_signature: bool = False) -> bytes:
+    def to_buffer(
+        cls,
+        transaction: dict,
+        skip_signature: bool = False,
+        skip_legacy_second_signature: bool = False,
+    ) -> bytes:
         hex_address = cls.parse_hex_from_str(transaction.get('to', ''))
 
         # Pad with leading zero if necessary
@@ -28,10 +33,10 @@ def to_buffer(cls, transaction: dict, skip_signature: bool = False) -> bytes:
 
         if not skip_signature and 'v' in transaction and transaction['v'] is not None and 'r' in transaction and 's' in transaction:
             fields.append(cls.to_be_array(int(transaction['v']) + (Network.get_network().chain_id() * 2 + 35)))
-            fields.append(bytes.fromhex(transaction['r']))
-            fields.append(bytes.fromhex(transaction['s']))
+            fields.append(bytes.fromhex(transaction['r']).lstrip(b'\x00'))
+            fields.append(bytes.fromhex(transaction['s']).lstrip(b'\x00'))
 
-            if 'legacySecondSignature' in transaction and transaction['legacySecondSignature']:
+            if not skip_legacy_second_signature and transaction.get('legacySecondSignature'):
                 fields.append(bytes.fromhex(transaction['legacySecondSignature']))
         else:
             # Push chainId + 0s for r and s
@@ -45,7 +50,10 @@ def to_buffer(cls, transaction: dict, skip_signature: bool = False) -> bytes:
 
     @classmethod
     def to_hash(cls, transaction: dict, skip_signature: bool = False) -> str:
-        return keccak.new(data=unhexlify(cls.to_buffer(transaction, skip_signature)), digest_bits=256).hexdigest()
+        # The transaction hash never covers the legacy second signature
+        encoded = cls.to_buffer(transaction, skip_signature, skip_legacy_second_signature=True)
+
+        return keccak.new(data=unhexlify(encoded), digest_bits=256).hexdigest()
 
     @classmethod
     def get_id(cls, transaction: dict) -> str:

tests/fixtures/transactions/transfer-legacy-second-signature-vector.json

@@ -0,0 +1,17 @@
+{
+	"chainId": 10000,
+	"privateKey": "314f59dbc54460b18fc036d67dac36ad6e1fa1c3c0e0b3427a84e43faabfcecb",
+	"secondPrivateKey": "4427335cab240f6cc651b1df5ce5ff489c5a0bcb952507b542afe8f6c90e98e7",
+	"data": {
+		"nonce": "0",
+		"gasPrice": "5000000000",
+		"gasLimit": "21000",
+		"to": "0x0DFeDD974c2e58Dc8E292ec159d1DC9AdeaFb1C0",
+		"value": "1000000000000000000",
+		"data": "",
+		"legacySecondSignature": "ea77adb968255ec1570c0e48cede8cceec3a9aff29e9dac1c53c2a6de4f2a18740df289edfcdfe97a189975cba8945334e5f230d66232751e468c5500dbe7dec01",
+		"hash": "27af7a389f34f32569c90815ced4e23e8770eb2d77ace6f29dabec1a10bef205"
+	},
+	"signed": "f86e8085012a05f200825208940dfedd974c2e58dc8e292ec159d1dc9adeafb1c0880de0b6b3a764000080824e44a00f4f4ac7f1effa669ea1a8970ed37f367e4aa0b549636619feead2d4a2408ffca04492af59379f74385bd951d229456b3b23ead33ce1d10106535c6830a688733d",
+	"serialized": "f8b18085012a05f200825208940dfedd974c2e58dc8e292ec159d1dc9adeafb1c0880de0b6b3a764000080824e44a00f4f4ac7f1effa669ea1a8970ed37f367e4aa0b549636619feead2d4a2408ffca04492af59379f74385bd951d229456b3b23ead33ce1d10106535c6830a688733db841ea77adb968255ec1570c0e48cede8cceec3a9aff29e9dac1c53c2a6de4f2a18740df289edfcdfe97a189975cba8945334e5f230d66232751e468c5500dbe7dec01"
+}

tests/fixtures/transactions/transfer-legacy-second-signature.json

@@ -10,10 +10,10 @@
 		"v": 0,
 		"r": "a1f79cb40a4bb409d6cebd874002ceda3ec0ccb614c1d8155f5c2f7f798135f9",
 		"s": "2d2ef517aaf6feed747385e260c206f46b2ce9d6b2a585427a111685a097bd79",
-		"legacySecondSignature": "094b33b2d10c4d48cff9a9b10f79990c9a902a762b00d2f2a4d2ddad7823b59332b2da193265068c67cefea9ca8bc84e47acec406f3300a0a10b77a56ea8c18801",
+		"legacySecondSignature": "a7576f15a3eb2b4cb54b83e97d25b70c88efb63c783120b81ab0eb4a958631d444347954726642779f019de3db05f568e78915b7f9cf319f7deb093353fa245b01",
 		"senderPublicKey": "0243333347c8cbf4e3cbc7a96964181d02a2b0c854faa2fef86b4b8d92afcf473d",
 		"from": "0x1E6747BEAa5B4076a6A98D735DF8c35a70D18Bdd",
 		"hash": "a39435ec5de418e77479856d06a653efc171afe43e091472af22ee359eeb83be"
 	},
-	"serialized": "f8ad0185012a05f200825208946f0182a0cc707b055322ccf6d4cb6a5aff1aeb228405f5e10080825c6ba0a1f79cb40a4bb409d6cebd874002ceda3ec0ccb614c1d8155f5c2f7f798135f9a02d2ef517aaf6feed747385e260c206f46b2ce9d6b2a585427a111685a097bd79b841094b33b2d10c4d48cff9a9b10f79990c9a902a762b00d2f2a4d2ddad7823b59332b2da193265068c67cefea9ca8bc84e47acec406f3300a0a10b77a56ea8c18801"
+	"serialized": "f8ad0185012a05f200825208946f0182a0cc707b055322ccf6d4cb6a5aff1aeb228405f5e10080825c6ba0a1f79cb40a4bb409d6cebd874002ceda3ec0ccb614c1d8155f5c2f7f798135f9a02d2ef517aaf6feed747385e260c206f46b2ce9d6b2a585427a111685a097bd79b841a7576f15a3eb2b4cb54b83e97d25b70c88efb63c783120b81ab0eb4a958631d444347954726642779f019de3db05f568e78915b7f9cf319f7deb093353fa245b01"
 }

tests/transactions/test_legacy_second_signature.py

@@ -0,0 +1,78 @@
+import pytest
+
+from crypto.configuration.network import Network
+from crypto.identity.private_key import PrivateKey
+from crypto.networks.testnet import Testnet
+from crypto.transactions.deserializer import Deserializer
+from crypto.transactions.types.transfer import Transfer
+
+
+@pytest.fixture
+def fixture(load_transaction_fixture):
+    """Test vector from the Mainsail LSS v1 spec, built for chain 10_000"""
+
+    return load_transaction_fixture('transactions/transfer-legacy-second-signature-vector')
+
+
+@pytest.fixture(autouse=True)
+def vector_network(fixture):
+    class VectorNetwork(Testnet):
+        def chain_id(self):
+            return fixture['chainId']
+
+    Network.set_network(VectorNetwork())
+
+    yield
+
+    Network.set_network(Testnet())
+
+
+@pytest.fixture
+def unsigned_data(fixture):
+    return {
+        key: value
+        for key, value in fixture['data'].items()
+        if key not in ('legacySecondSignature', 'hash')
+    }
+
+
+def test_it_should_sign_the_signed_transaction_hash_with_the_second_key(fixture, unsigned_data):
+    transaction = Transfer(unsigned_data)
+
+    transaction.sign(PrivateKey.from_hex(fixture['privateKey']))
+
+    assert transaction.serialize().hex() == fixture['signed']
+    assert transaction.get_id() == fixture['data']['hash']
+
+    transaction.legacy_second_sign(PrivateKey.from_hex(fixture['secondPrivateKey']))
+
+    assert transaction.data['legacySecondSignature'] == fixture['data']['legacySecondSignature']
+    assert transaction.serialize().hex() == fixture['serialized']
+    assert transaction.get_id() == fixture['data']['hash']
+
+
+def test_it_should_use_a_0_or_1_recovery_id(fixture, unsigned_data):
+    transaction = Transfer(unsigned_data)
+
+    transaction.sign(PrivateKey.from_hex(fixture['privateKey']))
+    transaction.legacy_second_sign(PrivateKey.from_hex(fixture['secondPrivateKey']))
+
+    assert transaction.data['legacySecondSignature'][-2:] in ('00', '01')
+
+
+def test_it_should_require_the_primary_signature_first(fixture, unsigned_data):
+    transaction = Transfer(unsigned_data)
+
+    error = 'Transaction must be signed before adding a legacy second signature'
+
+    with pytest.raises(ValueError, match=error):
+        transaction.legacy_second_sign(PrivateKey.from_hex(fixture['secondPrivateKey']))
+
+
+def test_it_should_deserialize_and_hash_only_the_first_9_fields(fixture):
+    transaction = Deserializer.new(fixture['serialized']).deserialize()
+
+    assert transaction.data['legacySecondSignature'] == fixture['data']['legacySecondSignature']
+    assert transaction.data['hash'] == fixture['data']['hash']
+    assert transaction.serialize().hex() == fixture['serialized']
+    assert transaction.verify()

tests/utils/test_rpl_encoder.py

@@ -29,3 +29,13 @@ def test_encoding_int():
     encoded = RlpEncoder.encode('123456')
 
     assert encoded == '86313233343536'
+
+def test_encoding_bytes_starting_with_0x():
+    encoded = RlpEncoder.encode(bytes.fromhex('3078ab'))
+
+    assert encoded == '833078ab'
+
+def test_encoding_hex_str():
+    encoded = RlpEncoder.encode('0x3078ab')
+
+    assert encoded == '833078ab'