[Breaking] Refactor Ethereum token transfer signing input (trustwalle…

…t#1210) * Factor out Transfer from Eth SigningInput. * Make with oneof, Transfer and ContractGeneric. * Remove amount from generic contract. * Special ERC20 contract type (empty so far) * Various factory methods for Transaction class. * Add new ERC20 test. * Add ERC20 handling, building. * ABI comment * Update iOS tests. * Add ERC20 iOS test * Update android tests * Update typescript test. * Special handling for invalid Address case. * Additional low-level signer test. * Small refactor in transaction building. * Support ERC721 * ERC20 kotlin test * Swift and Kotlin tests for ERC721 * iOS test fix * Rename in proto * Comment in proto, expose ERC20 call building * Add optional payload to plain Transfer. * ContractGeneric rename. * Minor exception leak fix in Solana * Payload -> Transaction rename. * Add ERC20 Approve * ERC20 approve iOS test * Rename, remove transfer_ prefix * Typescript test fix Co-authored-by: Catenocrypt <catenocrypt@users.noreply.github.com>
cornbread78 · Dec 22, 2021 · b4e0b50 · b4e0b50
1 parent 60c5207
commit b4e0b50
Show file tree

Hide file tree

Showing 12 changed files with 575 additions and 62 deletions.
diff --git a/...dTest/java/com/trustwallet/core/app/blockchains/ethereum/TestEthereumTransactionSigner.kt b/...dTest/java/com/trustwallet/core/app/blockchains/ethereum/TestEthereumTransactionSigner.kt
@@ -29,7 +29,11 @@ class TestEthereumTransactionSigner {
             nonce = ByteString.copyFrom("0x9".toHexByteArray())
             gasPrice = ByteString.copyFrom("0x04a817c800".toHexByteArray())
             gasLimit = ByteString.copyFrom("0x5208".toHexByteArray())
-            amount = ByteString.copyFrom("0x0de0b6b3a7640000".toHexByteArray())
+            transaction = Ethereum.Transaction.newBuilder().apply {
+                transfer = Ethereum.Transaction.Transfer.newBuilder().apply {
+                    amount = ByteString.copyFrom("0x0de0b6b3a7640000".toHexByteArray())
+                }.build()
+            }.build()
         }
 
         val output = AnySigner.sign(signingInput.build(), ETHEREUM, SigningOutput.parser())
@@ -42,6 +46,63 @@ class TestEthereumTransactionSigner {
         assertEquals(Numeric.toHexString(encoded), "0xf86c098504a817c800825208943535353535353535353535353535353535353535880de0b6b3a76400008025a028ef61340bd939bc2195fe537567866003e1a15d3c71ff63e1590620aa636276a067cbe9d8997f761aecb703304b3800ccf555c9f3dc64214b297fb1966a3b6d83")
     }
 
+    @Test
+    fun testEthereumERC20Signing() {
+        val signingInput = Ethereum.SigningInput.newBuilder()
+        signingInput.apply {
+            privateKey = ByteString.copyFrom(PrivateKey("0x608dcb1742bb3fb7aec002074e3420e4fab7d00cced79ccdac53ed5b27138151".toHexByteArray()).data())
+            toAddress = "0x6b175474e89094c44da98b954eedeac495271d0f" // DAI
+            chainId = ByteString.copyFrom("0x1".toHexByteArray())
+            nonce = ByteString.copyFrom("0x0".toHexByteArray())
+            gasPrice = ByteString.copyFrom("0x09c7652400".toHexByteArray())
+            gasLimit = ByteString.copyFrom("0x0130B9".toHexByteArray())
+            transaction = Ethereum.Transaction.newBuilder().apply {
+                erc20Transfer = Ethereum.Transaction.ERC20Transfer.newBuilder().apply {
+                    to = "0x5322b34c88ed0691971bf52a7047448f0f4efc84"
+                    amount = ByteString.copyFrom("0x1bc16d674ec80000".toHexByteArray())       
+                }.build()
+            }.build()
+        }
+
+        val output = AnySigner.sign(signingInput.build(), ETHEREUM, SigningOutput.parser())
+        val encoded = AnySigner.encode(signingInput.build(), ETHEREUM)
+
+        assertArrayEquals(output.encoded.toByteArray(), encoded)
+        assertEquals(Numeric.toHexString(output.v.toByteArray()), "0x25")
+        assertEquals(Numeric.toHexString(output.r.toByteArray()), "0x724c62ad4fbf47346b02de06e603e013f26f26b56fdc0be7ba3d6273401d98ce")
+        assertEquals(Numeric.toHexString(output.s.toByteArray()), "0x032131cae15da7ddcda66963e8bef51ca0d9962bfef0547d3f02597a4a58c931")
+        assertEquals(Numeric.toHexString(encoded), "0xf8aa808509c7652400830130b9946b175474e89094c44da98b954eedeac495271d0f80b844a9059cbb0000000000000000000000005322b34c88ed0691971bf52a7047448f0f4efc840000000000000000000000000000000000000000000000001bc16d674ec8000025a0724c62ad4fbf47346b02de06e603e013f26f26b56fdc0be7ba3d6273401d98cea0032131cae15da7ddcda66963e8bef51ca0d9962bfef0547d3f02597a4a58c931")
+    }
+
+    @Test
+    fun testEthereumERC721Signing() {
+        val signingInput = Ethereum.SigningInput.newBuilder()
+        signingInput.apply {
+            privateKey = ByteString.copyFrom(PrivateKey("0x608dcb1742bb3fb7aec002074e3420e4fab7d00cced79ccdac53ed5b27138151".toHexByteArray()).data())
+            toAddress = "0x6b175474e89094c44da98b954eedeac495271d0f" // DAI
+            chainId = ByteString.copyFrom("0x1".toHexByteArray())
+            nonce = ByteString.copyFrom("0x0".toHexByteArray())
+            gasPrice = ByteString.copyFrom("0x09c7652400".toHexByteArray())
+            gasLimit = ByteString.copyFrom("0x0130B9".toHexByteArray())
+            transaction = Ethereum.Transaction.newBuilder().apply {
+                erc721Transfer = Ethereum.Transaction.ERC721Transfer.newBuilder().apply {
+                    from = "0x718046867b5b1782379a14eA4fc0c9b724DA94Fc"
+                    to = "0x5322b34c88ed0691971bf52a7047448f0f4efc84"
+                    tokenId = ByteString.copyFrom("0x23c47ee5".toHexByteArray())
+                }.build()
+            }.build()
+        }
+
+        val output = AnySigner.sign(signingInput.build(), ETHEREUM, SigningOutput.parser())
+        val encoded = AnySigner.encode(signingInput.build(), ETHEREUM)
+
+        assertArrayEquals(output.encoded.toByteArray(), encoded)
+        assertEquals(Numeric.toHexString(output.v.toByteArray()), "0x26")
+        assertEquals(Numeric.toHexString(output.r.toByteArray()), "0x4f35575c8dc6d0c12fd1ae0007a1395f2baa992d5d498f5ee381cdb7d46ed43c")
+        assertEquals(Numeric.toHexString(output.s.toByteArray()), "0x0935b9ceb724ab73806e7f43da6a3079e7404e2dc28fe030fef96cd13779ac04")
+        assertEquals(Numeric.toHexString(encoded), "0xf8b6808509c7652400830130b98080b86423b872dd000000000000000000000000718046867b5b1782379a14ea4fc0c9b724da94fc0000000000000000000000005322b34c88ed0691971bf52a7047448f0f4efc840000000000000000000000000000000000000000000000000000000023c47ee526a04f35575c8dc6d0c12fd1ae0007a1395f2baa992d5d498f5ee381cdb7d46ed43ca00935b9ceb724ab73806e7f43da6a3079e7404e2dc28fe030fef96cd13779ac04")
+    }
+
     @Test
     fun testSignJSON() {
         val json = """
@@ -50,7 +111,11 @@ class TestEthereumTransactionSigner {
                 "gasPrice": "1pOkAA==",
                 "gasLimit": "Ugg=",
                 "toAddress": "0x7d8bf18C7cE84b3E175b339c4Ca93aEd1dD166F1",
-                "amount": "A0i8paFgAA=="
+                "transaction": {
+                    "transfer": {
+                        "amount":"A0i8paFgAA=="
+                    }
+                }                
             }
         """
         val key = "17209af590a86462395d5881e60d11c7fa7d482cfb02b5a01b93c2eeef243543".toHexByteArray()

diff --git a/src/Ethereum/Signer.cpp b/src/Ethereum/Signer.cpp
@@ -12,27 +12,31 @@ using namespace TW;
 using namespace TW::Ethereum;
 
 Proto::SigningOutput Signer::sign(const Proto::SigningInput& input) noexcept {
-    auto signer = Signer(load(input.chain_id()));
-    auto key = PrivateKey(Data(input.private_key().begin(), input.private_key().end()));
-    auto transaction = Signer::build(input);
+    try {
+        auto signer = Signer(load(input.chain_id()));
+        auto key = PrivateKey(Data(input.private_key().begin(), input.private_key().end()));
+        auto transaction = Signer::build(input);
 
-    signer.sign(key, transaction);
+        signer.sign(key, transaction);
 
-    auto output = Proto::SigningOutput();
+        auto output = Proto::SigningOutput();
 
-    auto encoded = RLP::encode(transaction);
-    output.set_encoded(encoded.data(), encoded.size());
+        auto encoded = RLP::encode(transaction);
+        output.set_encoded(encoded.data(), encoded.size());
 
-    auto v = store(transaction.v);
-    output.set_v(v.data(), v.size());
+        auto v = store(transaction.v);
+        output.set_v(v.data(), v.size());
 
-    auto r = store(transaction.r);
-    output.set_r(r.data(), r.size());
+        auto r = store(transaction.r);
+        output.set_r(r.data(), r.size());
 
-    auto s = store(transaction.s);
-    output.set_s(s.data(), s.size());
+        auto s = store(transaction.s);
+        output.set_s(s.data(), s.size());
 
-    return output;
+        return output;
+    } catch (std::exception&) {
+        return Proto::SigningOutput();
+    }
 }
 
 std::string Signer::signJSON(const std::string& json, const Data& key) {
@@ -67,21 +71,86 @@ Signer::sign(const uint256_t &chainID, const PrivateKey &privateKey, const Data&
     return values(chainID, signature);
 }
 
+// May throw
+Data addressStringToData(const std::string& asString) {
+    if (asString.empty()) {
+        return {};
+    }
+    auto address = Address(asString);
+    Data asData;
+    asData.resize(20);
+    std::copy(address.bytes.begin(), address.bytes.end(), asData.data());
+    return asData;
+}
+
 Transaction Signer::build(const Proto::SigningInput &input) {
-    Data toAddress;
-    if (!input.to_address().empty()) {
-        toAddress.resize(20);
-        auto address = Address(input.to_address());
-        std::copy(address.bytes.begin(), address.bytes.end(), toAddress.data());
+    Data toAddress = addressStringToData(input.to_address());
+    switch (input.transaction().transaction_oneof_case()) {
+        case Proto::Transaction::kTransfer:
+            {
+                auto transaction = Transaction::buildTransfer(
+                    /* nonce: */ load(input.nonce()),
+                    /* gasPrice: */ load(input.gas_price()),
+                    /* gasLimit: */ load(input.gas_limit()),
+                    /* to: */ toAddress,
+                    /* amount: */ load(input.transaction().transfer().amount()),
+                    /* optionalTransaction: */ Data(input.transaction().contract_generic().data().begin(), input.transaction().contract_generic().data().end()));
+                return transaction;
+            }
+
+        case Proto::Transaction::kErc20Transfer:
+            {
+                Data tokenToAddress = addressStringToData(input.transaction().erc20_transfer().to());
+                auto transaction = Transaction::buildERC20Transfer(
+                    /* nonce: */ load(input.nonce()),
+                    /* gasPrice: */ load(input.gas_price()),
+                    /* gasLimit: */ load(input.gas_limit()),
+                    /* tokenContract: */ toAddress,
+                    /* toAddress */ tokenToAddress,
+                    /* amount: */ load(input.transaction().erc20_transfer().amount()));
+                return transaction;
+            }
+
+        case Proto::Transaction::kErc20Approve:
+            {
+                Data spenderAddress = addressStringToData(input.transaction().erc20_approve().spender());
+                auto transaction = Transaction::buildERC20Approve(
+                    /* nonce: */ load(input.nonce()),
+                    /* gasPrice: */ load(input.gas_price()),
+                    /* gasLimit: */ load(input.gas_limit()),
+                    /* tokenContract: */ toAddress,
+                    /* toAddress */ spenderAddress,
+                    /* amount: */ load(input.transaction().erc20_transfer().amount()));
+                return transaction;
+            }
+
+        case Proto::Transaction::kErc721Transfer:
+            {
+                Data tokenToAddress = addressStringToData(input.transaction().erc721_transfer().to());
+                Data tokenFromAddress = addressStringToData(input.transaction().erc721_transfer().from());
+                auto transaction = Transaction::buildERC721Transfer(
+                    /* nonce: */ load(input.nonce()),
+                    /* gasPrice: */ load(input.gas_price()),
+                    /* gasLimit: */ load(input.gas_limit()),
+                    /* tokenContract: */ toAddress,
+                    /* fromAddress: */ tokenFromAddress,
+                    /* toAddress */ tokenToAddress,
+                    /* tokenId: */ load(input.transaction().erc721_transfer().token_id()));
+                return transaction;
+            }
+
+        case Proto::Transaction::kContractGeneric:
+        default:
+            {
+                auto transaction = Transaction::buildSmartContract(
+                    /* nonce: */ load(input.nonce()),
+                    /* gasPrice: */ load(input.gas_price()),
+                    /* gasLimit: */ load(input.gas_limit()),
+                    /* to: */ toAddress,
+                    /* transaction: */ Data(input.transaction().contract_generic().data().begin(), input.transaction().contract_generic().data().end()));
+                return transaction;
+            }
     }
-    auto transaction = Transaction(
-        /* nonce: */ load(input.nonce()),
-        /* gasPrice: */ load(input.gas_price()),
-        /* gasLimit: */ load(input.gas_limit()),
-        /* to: */ toAddress,
-        /* amount: */ load(input.amount()),
-        /* payload: */ Data(input.payload().begin(), input.payload().end()));
-    return transaction;
 }
 
 void Signer::sign(const PrivateKey &privateKey, Transaction &transaction) const noexcept {

diff --git a/src/Ethereum/Transaction.cpp b/src/Ethereum/Transaction.cpp
@@ -5,5 +5,56 @@
 // file LICENSE at the root of the source code distribution tree.
 
 #include "Transaction.h"
+#include "ABI/Function.h"
+#include "ABI/ParamBase.h"
+#include "ABI/ParamAddress.h"
 
+using namespace TW::Ethereum::ABI;
+using namespace TW::Ethereum;
 using namespace TW;
+
+Transaction Transaction::buildERC20Transfer(uint256_t nonce, uint256_t gasPrice, uint256_t gasLimit,
+                const Data& tokenContract, const Data& toAddress, uint256_t amount) {
+    return Transaction(nonce, gasPrice, gasLimit, tokenContract, 0, buildERC20TransferCall(toAddress, amount));
+}
+
+Transaction Transaction::buildERC20Approve(uint256_t nonce, uint256_t gasPrice, uint256_t gasLimit,
+                const Data& tokenContract, const Data& spenderAddress, uint256_t amount) {
+    return Transaction(nonce, gasPrice, gasLimit, tokenContract, 0, buildERC20ApproveCall(spenderAddress, amount));
+}
+
+Transaction Transaction::buildERC721Transfer(uint256_t nonce, uint256_t gasPrice, uint256_t gasLimit,
+                const Data& tokenContract, const Data& from, const Data& to, uint256_t tokenId) {
+    return Transaction(nonce, gasPrice, gasLimit, {}, 0, buildERC721TransferFromCall(from, to, tokenId));
+}
+
+Data Transaction::buildERC20TransferCall(const Data& to, uint256_t amount) {
+    auto func = Function("transfer", std::vector<std::shared_ptr<ParamBase>>{
+        std::make_shared<ParamAddress>(to),
+        std::make_shared<ParamUInt256>(amount)
+    });
+    Data payload;
+    func.encode(payload);
+    return payload;
+}
+
+Data Transaction::buildERC20ApproveCall(const Data& spender, uint256_t amount) {
+    auto func = Function("approve", std::vector<std::shared_ptr<ParamBase>>{
+        std::make_shared<ParamAddress>(spender),
+        std::make_shared<ParamUInt256>(amount)
+    });
+    Data payload;
+    func.encode(payload);
+    return payload;
+}
+
+Data Transaction::buildERC721TransferFromCall(const Data& from, const Data& to, uint256_t tokenId) {
+    auto func = Function("transferFrom", std::vector<std::shared_ptr<ParamBase>>{
+        std::make_shared<ParamAddress>(from),
+        std::make_shared<ParamAddress>(to),
+        std::make_shared<ParamUInt256>(tokenId)
+    });
+    Data payload;
+    func.encode(payload);
+    return payload;
+}
diff --git a/src/Ethereum/Transaction.h b/src/Ethereum/Transaction.h
@@ -12,7 +12,7 @@
 namespace TW::Ethereum {
 
 class Transaction {
-  public:
+public:
     uint256_t nonce;
     uint256_t gasPrice;
     uint256_t gasLimit;
@@ -26,8 +26,36 @@ class Transaction {
     uint256_t r = uint256_t();
     uint256_t s = uint256_t();
 
-    Transaction(uint256_t nonce, uint256_t gasPrice, uint256_t gasLimit, const Data& to, uint256_t amount,
-                Data payload)
+    // Factory methods
+    // Create a native coin transfer transaction
+    static Transaction buildTransfer(uint256_t nonce, uint256_t gasPrice, uint256_t gasLimit, const Data& to, uint256_t amount, const Data& optionalData = {}) {
+        return Transaction(nonce, gasPrice, gasLimit, to, amount, optionalData);
+    }
+
+    // Create an ERC20 token transfer transaction
+    static Transaction buildERC20Transfer(uint256_t nonce, uint256_t gasPrice, uint256_t gasLimit,
+        const Data& tokenContract, const Data& toAddress, uint256_t amount);
+
+    // Create an ERC20 approve transaction
+    static Transaction buildERC20Approve(uint256_t nonce, uint256_t gasPrice, uint256_t gasLimit,
+        const Data& tokenContract, const Data& spenderAddress, uint256_t amount);
+
+    // Create an ERC721 NFT transfer transaction
+    static Transaction buildERC721Transfer(uint256_t nonce, uint256_t gasPrice, uint256_t gasLimit,
+        const Data& tokenContract, const Data& from, const Data& to, uint256_t tokenId);
+
+    // Create a generic smart contract transaction
+    static Transaction buildSmartContract(uint256_t nonce, uint256_t gasPrice, uint256_t gasLimit, const Data& to, const Data& data) {
+        return Transaction(nonce, gasPrice, gasLimit, to, 0, data);
+    }
+
+    // Helpers for building contract calls
+    static Data buildERC20TransferCall(const Data& to, uint256_t amount);
+    static Data buildERC20ApproveCall(const Data& spender, uint256_t amount);
+    static Data buildERC721TransferFromCall(const Data& from, const Data& to, uint256_t tokenId);
+
+private:
+    Transaction(uint256_t nonce, uint256_t gasPrice, uint256_t gasLimit, const Data& to, uint256_t amount, const Data& payload)
         : nonce(std::move(nonce))
         , gasPrice(std::move(gasPrice))
         , gasLimit(std::move(gasLimit))