Tron signing #63

OlegGordiichuk · 2018-09-28T23:49:04Z

Hello it is my first implementation of the network.

Could you take a look at implementation before I add unit tests ?

Document that I was using during implementation.

https://github.com/tronprotocol/Documentation/blob/master/TRX/Tron-overview.md#103-signature

vikmeup

Can you add tests?

vikmeup · 2018-09-28T23:58:18Z

Sources/Tron/TransferContract.swift

+    private let toAddress: Data
+    private let amount: Int64
+
+    init(ownerAddress: Data, toAddress: Data, amount: Int64) {


can you pass TronAddress?

vikmeup · 2018-09-28T23:58:56Z

Sources/Tron/TronTransaction.swift

+        let rawData = TronTransactionRawDataBuilder()
+            .contract(contract)
+            .timestamp(Date().currentTimeMilliseconds)
+            .expiration(blockHeader.rawData.timestamp + 10 * 60 * 60 * 1000)


can you make this customizable from the app side? so you pass this value

I'm not sure if builder pattern is suitable here, most of the properties are primitive types...

vikmeup · 2018-09-28T23:59:59Z

Sources/Tron/TransferAssetContract.swift

+struct TransferAssetContract: TronTransactionContract {
+
+    private let assetName: Data
+    private let ownerAddress: Data


use TronAddress

vikmeup · 2018-09-29T00:00:08Z

Sources/Tron/TransferAssetContract.swift

+
+struct TransferAssetContract: TronTransactionContract {
+
+    private let assetName: Data


assets name as String?

vikmeup · 2018-09-29T00:01:06Z

Sources/Tron/Extension/ProtocolTransaction.swift

+
+extension Protocol_Transaction {
+    private func hash() -> Data {
+        return Crypto.sha256(rawData.data)


make a property.

vikmeup · 2018-09-29T00:01:18Z

Sources/Tron/Extension/Date.swift

+extension Date {
+    var currentTimeMilliseconds: Int64 {
+        let now = timeIntervalSince1970
+        return Int64(now * 1000)


just Int64(timeIntervalSince1970 * 1000)

personal opinion, currentTimeMilliseconds is a bit long, like good old objc variable naming

vikmeup · 2018-09-29T00:01:41Z

Sources/Crypto.h

@@ -53,6 +53,9 @@
 /// Computes the SHA256 hash of the SHA256 hash of the data.
 + (nonnull NSData *)sha256sha256:(nonnull NSData *)data;

+/// Computes the SHA256 hash of the data.
+ (nonnull NSData *)sha256:(nonnull NSData *)data;


there is also hash() which is the same thing

@vikmeup not exactly the same, hash() is too general, how about rename to keccak256 or something similar

vikmeup · 2018-09-29T00:02:33Z

Sources/Tron/Protobuf/Tron.pb.swift

@@ -0,0 +1,4581 @@
+// DO NOT EDIT.


Can you exclude files from the commit? we should run script to generate them instead from original protobuf

hewigovens

a few comments, how large is the new introduced SwiftProtobuf.framework?

hewigovens · 2018-09-29T01:34:32Z

Sources/Crypto.h

@@ -53,6 +53,9 @@
 /// Computes the SHA256 hash of the SHA256 hash of the data.
 + (nonnull NSData *)sha256sha256:(nonnull NSData *)data;

+/// Computes the SHA256 hash of the data.
+ (nonnull NSData *)sha256:(nonnull NSData *)data;


@vikmeup not exactly the same, hash() is too general, how about rename to keccak256 or something similar

hewigovens · 2018-09-29T01:39:03Z

Sources/Tron/Extension/Date.swift

+extension Date {
+    var currentTimeMilliseconds: Int64 {
+        let now = timeIntervalSince1970
+        return Int64(now * 1000)


personal opinion, currentTimeMilliseconds is a bit long, like good old objc variable naming

hewigovens · 2018-09-29T01:43:52Z

Sources/Tron/Extension/ProtocolTransaction.swift

+
+import UIKit
+
+extension Protocol_Transaction {


Protocol_Transaction is generated?

@hewigovens yes it is generated by protobuf.

hewigovens · 2018-09-29T01:47:03Z

Sources/Tron/TronTransaction.swift

+
+enum TronTransactionError: Error {
+    case blockHeaderHeight
+    case blockHeaderRawData


maybe LocalizedError? block height/raw data seems not like a error

yeah it makes sense

hewigovens · 2018-09-29T01:55:07Z

Sources/Tron/TronTransaction.swift

+        let rawData = TronTransactionRawDataBuilder()
+            .contract(contract)
+            .timestamp(Date().currentTimeMilliseconds)
+            .expiration(blockHeader.rawData.timestamp + 10 * 60 * 60 * 1000)


I'm not sure if builder pattern is suitable here, most of the properties are primitive types...

OlegGordiichuk added 8 commits September 26, 2018 16:03

Add protobuf pod.

e2f5bad

Add tron proto files.

30d216c

Add new method for sha256.

d9560bc

Add protobuf files.

4571132

Add helper methods.

7739fb9

Add protocol to cover transaction contract build.

169507b

Implementation of the required classes.

9a21ad3

Updata transaction configuration.

bd9ad92

OlegGordiichuk requested review from alejandro-isaza and hewigovens September 28, 2018 23:49

OlegGordiichuk added the DO NOT MERGE label Sep 28, 2018

vikmeup reviewed Sep 29, 2018

View reviewed changes

hewigovens reviewed Sep 29, 2018

View reviewed changes

OlegGordiichuk added 13 commits October 2, 2018 19:31

Update Error -> LocalizedError

2d3216d

Code style fixes.

7b51121

Add test for transaction contract sighn.

97c2d08

Remove of the unnecessary class.

cf49a3a

Remove of the pb files.

a9ff0de

Add .pb files to git ignore.

9076f30

Revert pb files.

1edde77

Remove of the build scripts.

32d25dc

Add references about tron pb.

e35c4a1

Add pod version.

db556ae

Update of the pod spec.

948435c

Add public

60cb237

Code refactroing

e8c277f

OlegGordiichuk closed this Oct 10, 2018

OlegGordiichuk deleted the tron_signing branch October 10, 2018 12:29

madcake mentioned this pull request Feb 17, 2019

Crash on android when signing btc #112

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tron signing #63

Tron signing #63

OlegGordiichuk commented Sep 28, 2018

vikmeup left a comment

vikmeup Sep 28, 2018

vikmeup Sep 28, 2018

hewigovens Sep 29, 2018

vikmeup Sep 28, 2018

vikmeup Sep 29, 2018

vikmeup Sep 29, 2018

vikmeup Sep 29, 2018

hewigovens Sep 29, 2018

vikmeup Sep 29, 2018

hewigovens Sep 29, 2018

vikmeup Sep 29, 2018 •

edited

Loading

hewigovens left a comment

hewigovens Sep 29, 2018

hewigovens Sep 29, 2018

hewigovens Sep 29, 2018

OlegGordiichuk Oct 2, 2018

hewigovens Sep 29, 2018

OlegGordiichuk Oct 2, 2018

hewigovens Sep 29, 2018


		struct TransferAssetContract: TronTransactionContract {

		private let assetName: Data


		import UIKit

		extension Protocol_Transaction {

Tron signing #63

Tron signing #63

Conversation

OlegGordiichuk commented Sep 28, 2018

vikmeup left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vikmeup Sep 29, 2018 • edited Loading

Choose a reason for hiding this comment

hewigovens left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vikmeup Sep 29, 2018 •

edited

Loading