Fix: failed transactions are not displayed with the correct state in …

…Activities tab and sort order is enforced correctly for activities/transactions within the same block

Specifically:

* transaction ID should be included EventActivity primary key
* transactions that failed were displayed like they are successful
* sorting of activities (i.e events) and transactions within the same block was incorrectly

AlphaWallet/Activities/Coordinators/ActivitiesCoordinator.swift

@@ -219,7 +219,7 @@ class ActivitiesCoordinator: Coordinator {
                 }
                 tokensAndTokenHolders[contract] = (tokenObject: tokenObject, tokenHolders: tokenHolders)
             }
-            return (activity: .init(id: Int.random(in: 0..<Int.max), tokenObject: tokenObject, server: eachEvent.server, name: card.name, eventName: eachEvent.eventName, blockNumber: eachEvent.blockNumber, transactionId: eachEvent.transactionId, date: eachEvent.date, values: (token: tokenAttributes, card: cardAttributes), view: card.view, itemView: card.itemView, isBaseCard: card.isBase, state: .completed), tokenObject: tokenObject, tokenHolder: tokenHolders[0])
+            return (activity: .init(id: Int.random(in: 0..<Int.max), tokenObject: tokenObject, server: eachEvent.server, name: card.name, eventName: eachEvent.eventName, blockNumber: eachEvent.blockNumber, transactionId: eachEvent.transactionId, transactionIndex: eachEvent.transactionIndex, logIndex: eachEvent.logIndex, date: eachEvent.date, values: (token: tokenAttributes, card: cardAttributes), view: card.view, itemView: card.itemView, isBaseCard: card.isBase, state: .completed), tokenObject: tokenObject, tokenHolder: tokenHolders[0])
         }
 
         //TODO fix for activities: special fix to filter out the event we don't want - need to doc this and have to handle with TokenScript design
@@ -275,7 +275,7 @@ class ActivitiesCoordinator: Coordinator {
         }
         if let (index, oldActivity) = activitiesIndexLookup[activity.id] {
             let updatedValues = (token: oldActivity.values.token.merging(resolvedAttributeNameValues) { _, new in new }, card: oldActivity.values.card)
-            let updatedActivity: Activity = .init(id: oldActivity.id, tokenObject: tokenObject, server: oldActivity.server, name: oldActivity.name, eventName: oldActivity.eventName, blockNumber: oldActivity.blockNumber, transactionId: oldActivity.transactionId, date: oldActivity.date, values: updatedValues, view: oldActivity.view, itemView: oldActivity.itemView, isBaseCard: oldActivity.isBaseCard, state: oldActivity.state)
+            let updatedActivity: Activity = .init(id: oldActivity.id, tokenObject: tokenObject, server: oldActivity.server, name: oldActivity.name, eventName: oldActivity.eventName, blockNumber: oldActivity.blockNumber, transactionId: oldActivity.transactionId, transactionIndex: oldActivity.transactionIndex, logIndex: oldActivity.logIndex, date: oldActivity.date, values: updatedValues, view: oldActivity.view, itemView: oldActivity.itemView, isBaseCard: oldActivity.isBaseCard, state: oldActivity.state)
             activities[index] = updatedActivity
             reloadViewController()
             if let activityViewController = activityViewController, activityViewController.isForActivity(updatedActivity) {

AlphaWallet/Activities/ViewControllers/ActivitiesViewController.swift

@@ -121,6 +121,9 @@ class ActivitiesViewController: UIViewController {
                 eventName: activityName,
                 blockNumber: transaction.blockNumber,
                 transactionId: transaction.id,
+                transactionIndex: transaction.transactionIndex,
+                //We don't use this for transactions, so it's ok
+                logIndex: 0,
                 date: transaction.date,
                 values: (token: .init(), card: cardAttributes),
                 view: (html: "", style: ""),

AlphaWallet/Activities/ViewModels/ActivitiesViewModel.swift

@@ -19,7 +19,34 @@ struct ActivitiesViewModel {
             newItems[date] = currentItems
         }
         let tuple = newItems.map { each in
-            (date: each.key, items: (each.value as! [ActivityOrTransaction]).sorted { $0.date > $1.date })
+            (date: each.key, items: (each.value as! [ActivityOrTransaction]).sorted {
+                if $0.blockNumber > $1.blockNumber {
+                    return true
+                } else if $0.blockNumber < $1.blockNumber {
+                    return false
+                } else {
+                    if $0.transactionIndex > $1.transactionIndex {
+                        return true
+                    } else if $0.transactionIndex < $1.transactionIndex {
+                        return false
+                    } else {
+                        switch ($0, $1) {
+                        case let (.activity(a0), .activity(a1)):
+                            return a0.logIndex > a1.logIndex
+                        case let (.transaction, .activity):
+                            return false
+                        case let (.activity, .transaction):
+                            return true
+                        case let (.transaction(t0), .transaction(t1)):
+                            if let n0 = Int(t0.nonce), let n1 = Int(t1.nonce) {
+                                return n0 > n1
+                            } else {
+                                return false
+                            }
+                        }
+                    }
+                }
+            })
         }
         items = tuple.sorted { (object1, object2) -> Bool in
             formatter.date(from: object1.date)! > formatter.date(from: object2.date)!

AlphaWallet/Core/Initializers/MigrationInitializer.swift

@@ -15,7 +15,7 @@ class MigrationInitializer: Initializer {
     }
 
     func perform() {
-        config.schemaVersion = 6
+        config.schemaVersion = 7
         config.migrationBlock = { migration, oldSchemaVersion in
             if oldSchemaVersion < 2 {
                 //Fix bug created during multi-chain implementation. Where TokenObject instances are created from transfer Transaction instances, with the primaryKey as a empty string; so instead of updating an existing TokenObject, a duplicate TokenObject instead was created but with primaryKey empty
@@ -62,6 +62,12 @@ class MigrationInitializer: Initializer {
                     newObject["sortIndex"] = RealmOptional<Int>(nil)
                 }
             }
+            if oldSchemaVersion < 7 {
+                //Fix bug where we marked all transactions as completed successfully without checking `isError` from Etherscan
+                migration.deleteData(forType: Transaction.className())
+                RPCServer.allCases.map { Config.setLastFetchedErc20InteractionBlockNumber(0, server: $0, wallet: self.account.address) }
+                migration.deleteData(forType: EventActivity.className())
+            }
         }
     }
 }

AlphaWallet/EtherClient/TrustClient/Models/RawTransaction.swift

@@ -7,6 +7,7 @@ import PromiseKit
 struct RawTransaction: Decodable {
     let hash: String
     let blockNumber: String
+    let transactionIndex: String
     let timeStamp: String
     let nonce: String
     let from: String
@@ -17,6 +18,7 @@ struct RawTransaction: Decodable {
     let input: String
     let gasUsed: String
     let error: String?
+    let isError: String?
 
     ///
     ///It is possible for the etherscan.io API to return an empty `to` even if the transaction actually has a `to`. It doesn't seem to be linked to `"isError" = "1"`, because other transactions that fail (with isError="1") has a non-empty `to`.
@@ -32,6 +34,7 @@ struct RawTransaction: Decodable {
     enum CodingKeys: String, CodingKey {
         case hash = "hash"
         case blockNumber
+        case transactionIndex
         case timeStamp
         case nonce
         case from
@@ -43,6 +46,7 @@ struct RawTransaction: Decodable {
         case gasUsed
         case operationsLocalized = "operations"
         case error = "error"
+        case isError = "isError"
     }
 
     let operationsLocalized: [LocalizedOperation]?
@@ -54,7 +58,7 @@ extension Transaction {
             return Promise.value(nil)
         }
         let state: TransactionState = {
-            if transaction.error?.isEmpty == false {
+            if transaction.error?.isEmpty == false || transaction.isError == "1" {
                 return .error
             }
             return .completed
@@ -67,6 +71,7 @@ extension Transaction {
                     id: transaction.hash,
                     server: tokensStorage.server,
                     blockNumber: Int(transaction.blockNumber)!,
+                    transactionIndex: Int(transaction.transactionIndex)!,
                     from: from.description,
                     to: to,
                     value: transaction.value,

AlphaWallet/TokenScriptClient/Coordinators/EventSourceCoordinatorForActivities.swift

@@ -104,13 +104,14 @@ class EventSourceCoordinatorForActivities {
         guard let blockNumber = event.eventLog?.blockNumber else { return nil }
         guard let logIndex = event.eventLog?.logIndex else { return nil }
         guard let transactionHash = event.eventLog?.transactionHash else { return nil }
+        guard let transactionIndex = event.eventLog?.transactionIndex else { return nil }
         let transactionId = transactionHash.hexEncoded
         let decodedResult = self.convertToJsonCompatible(dictionary: event.decodedResult)
         guard let json = decodedResult.jsonString else { return nil }
         //TODO when TokenScript schema allows it, support more than 1 filter
         let filterTextEquivalent = filterParam.compactMap({ $0?.textEquivalent }).first
         let filterText = filterTextEquivalent ?? "\(eventOrigin.eventFilter.name)=\(eventOrigin.eventFilter.value)"
-        return EventActivity(contract: eventOrigin.contract, tokenContract: token.contractAddress, server: server, date: date, eventName: eventOrigin.eventName, blockNumber: Int(blockNumber), transactionId: transactionId, logIndex: Int(logIndex), filter: filterText, json: json)
+        return EventActivity(contract: eventOrigin.contract, tokenContract: token.contractAddress, server: server, date: date, eventName: eventOrigin.eventName, blockNumber: Int(blockNumber), transactionId: transactionId, transactionIndex: Int(transactionIndex), logIndex: Int(logIndex), filter: filterText, json: json)
     }
 
     private func convertToJsonCompatible(dictionary: [String: Any]) -> [String: Any] {

AlphaWallet/TokenScriptClient/Models/Activity.swift

@@ -32,6 +32,8 @@ struct Activity {
     let eventName: String
     let blockNumber: Int
     let transactionId: String
+    let transactionIndex: Int
+    let logIndex: Int
     let date: Date
     let values: (token: [AttributeId: AssetInternalValue], card: [AttributeId: AssetInternalValue])
     let view: (html: String, style: String)

AlphaWallet/TokenScriptClient/Models/ActivityOrTransaction.swift

@@ -14,4 +14,22 @@ enum ActivityOrTransaction {
             return transaction.date
         }
     }
+
+    var blockNumber: Int {
+        switch self {
+        case .activity(let activity):
+            return activity.blockNumber
+        case .transaction(let transaction):
+            return transaction.blockNumber
+        }
+    }
+
+    var transactionIndex: Int {
+        switch self {
+        case .activity(let activity):
+            return activity.transactionIndex
+        case .transaction(let transaction):
+            return transaction.transactionIndex
+        }
+    }
 }

AlphaWallet/Tokens/Coordinators/GetContractInteractions.swift

@@ -44,6 +44,7 @@ class GetContractInteractions {
                                 id: transactionJson["hash"].description,
                                 server: server,
                                 blockNumber: transactionJson["blockNumber"].intValue,
+                                transactionIndex: transactionJson["transactionIndex"].intValue,
                                 from: transactionJson["from"].description,
                                 to: transactionJson["to"].description,
                                 value: transactionJson["value"].description,
@@ -53,6 +54,7 @@ class GetContractInteractions {
                                 nonce: transactionJson["nonce"].description,
                                 date: Date(timeIntervalSince1970: Double(string: transactionJson["timeStamp"].description) ?? Double(0)),
                                 localizedOperations: [localizedTokenObj],
+                                //The API only returns successful transactions
                                 state: .completed,
                                 isErc20Interaction: true
                         )

AlphaWallet/Tokens/Types/EventActivity.swift

@@ -5,8 +5,8 @@ import BigInt
 import RealmSwift
 
 class EventActivity: Object {
-    static func generatePrimaryKey(fromContract contract: AlphaWallet.Address, tokenContract: AlphaWallet.Address, server: RPCServer, eventName: String, blockNumber: Int, logIndex: Int, filter: String) -> String {
-        "\(contract.eip55String)-\(tokenContract.eip55String)-\(server.chainID)-\(eventName)-\(blockNumber)-\(logIndex)-\(filter)"
+    static func generatePrimaryKey(fromContract contract: AlphaWallet.Address, tokenContract: AlphaWallet.Address, server: RPCServer, eventName: String, blockNumber: Int, transactionId: String, logIndex: Int, filter: String) -> String {
+        "\(contract.eip55String)-\(tokenContract.eip55String)-\(server.chainID)-\(eventName)-\(blockNumber)-\(transactionId)-\(logIndex)-\(filter)"
     }
 
     @objc dynamic var primaryKey: String = ""
@@ -17,6 +17,7 @@ class EventActivity: Object {
     @objc dynamic var eventName: String = ""
     @objc dynamic var blockNumber: Int = 0
     @objc dynamic var transactionId: String = ""
+    @objc dynamic var transactionIndex: Int = 0
     @objc dynamic var logIndex: Int = 0
     @objc dynamic var filter: String = ""
     @objc dynamic var json: String = "{}" {
@@ -45,16 +46,17 @@ class EventActivity: Object {
         .init(chainID: chainId)
     }
 
-    convenience init(contract: AlphaWallet.Address, tokenContract: AlphaWallet.Address, server: RPCServer, date: Date, eventName: String, blockNumber: Int, transactionId: String, logIndex: Int, filter: String, json: String) {
+    convenience init(contract: AlphaWallet.Address, tokenContract: AlphaWallet.Address, server: RPCServer, date: Date, eventName: String, blockNumber: Int, transactionId: String, transactionIndex: Int, logIndex: Int, filter: String, json: String) {
         self.init()
-        self.primaryKey = EventActivity.generatePrimaryKey(fromContract: contract, tokenContract: tokenContract, server: server, eventName: eventName, blockNumber: blockNumber, logIndex: logIndex, filter: filter)
+        self.primaryKey = EventActivity.generatePrimaryKey(fromContract: contract, tokenContract: tokenContract, server: server, eventName: eventName, blockNumber: blockNumber, transactionId: transactionId, logIndex: logIndex, filter: filter)
         self.contract = contract.eip55String
         self.tokenContract = tokenContract.eip55String
         self.chainId = server.chainID
         self.date = date
         self.eventName = eventName
         self.blockNumber = blockNumber
         self.transactionId = transactionId
+        self.transactionIndex = transactionIndex
         self.logIndex = logIndex
         self.filter = filter
         self.json = json

AlphaWallet/Transactions/Storage/Transaction.swift

@@ -8,6 +8,7 @@ class Transaction: Object {
     @objc dynamic var chainId: Int = 0
     @objc dynamic var id: String = ""
     @objc dynamic var blockNumber: Int = 0
+    @objc dynamic var transactionIndex: Int = 0
     @objc dynamic var from = ""
     @objc dynamic var to = ""
     @objc dynamic var value = ""
@@ -24,6 +25,7 @@ class Transaction: Object {
         id: String,
         server: RPCServer,
         blockNumber: Int,
+        transactionIndex: Int,
         from: String,
         to: String,
         value: String,
@@ -42,6 +44,7 @@ class Transaction: Object {
         self.id = id
         self.chainId = server.chainID
         self.blockNumber = blockNumber
+        self.transactionIndex = transactionIndex
         self.from = from
         self.to = to
         self.value = value

AlphaWallet/Transfer/Types/SentTransaction.swift

@@ -13,6 +13,7 @@ extension SentTransaction {
             id: transaction.id,
             server: transaction.original.server,
             blockNumber: 0,
+            transactionIndex: 0,
             from: from.eip55String,
             to: transaction.original.to?.eip55String ?? "",
             value: transaction.original.value.description,