From 42b8403a6870ed1a07b791602fdbd9a8bd2d9331 Mon Sep 17 00:00:00 2001 From: Hwee-Boon Yar Date: Fri, 9 Oct 2020 13:06:32 +0800 Subject: [PATCH] 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 --- .../Coordinators/ActivitiesCoordinator.swift | 4 +-- .../ActivitiesViewController.swift | 3 ++ .../ViewModels/ActivitiesViewModel.swift | 29 ++++++++++++++++++- .../Initializers/MigrationInitializer.swift | 8 ++++- .../TrustClient/Models/RawTransaction.swift | 7 ++++- .../EventSourceCoordinatorForActivities.swift | 3 +- .../TokenScriptClient/Models/Activity.swift | 2 ++ .../Models/ActivityOrTransaction.swift | 18 ++++++++++++ .../GetContractInteractions.swift | 2 ++ AlphaWallet/Tokens/Types/EventActivity.swift | 10 ++++--- .../Transactions/Storage/Transaction.swift | 3 ++ .../Transfer/Types/SentTransaction.swift | 1 + 12 files changed, 80 insertions(+), 10 deletions(-) diff --git a/AlphaWallet/Activities/Coordinators/ActivitiesCoordinator.swift b/AlphaWallet/Activities/Coordinators/ActivitiesCoordinator.swift index c98e33b80b..1d42088f68 100644 --- a/AlphaWallet/Activities/Coordinators/ActivitiesCoordinator.swift +++ b/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.. $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)! diff --git a/AlphaWallet/Core/Initializers/MigrationInitializer.swift b/AlphaWallet/Core/Initializers/MigrationInitializer.swift index 583053adf9..ff4c6f25b8 100644 --- a/AlphaWallet/Core/Initializers/MigrationInitializer.swift +++ b/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(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()) + } } } } diff --git a/AlphaWallet/EtherClient/TrustClient/Models/RawTransaction.swift b/AlphaWallet/EtherClient/TrustClient/Models/RawTransaction.swift index f77ba23a84..ecf7693206 100644 --- a/AlphaWallet/EtherClient/TrustClient/Models/RawTransaction.swift +++ b/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, diff --git a/AlphaWallet/TokenScriptClient/Coordinators/EventSourceCoordinatorForActivities.swift b/AlphaWallet/TokenScriptClient/Coordinators/EventSourceCoordinatorForActivities.swift index 4750f7f763..e974d324ce 100644 --- a/AlphaWallet/TokenScriptClient/Coordinators/EventSourceCoordinatorForActivities.swift +++ b/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] { diff --git a/AlphaWallet/TokenScriptClient/Models/Activity.swift b/AlphaWallet/TokenScriptClient/Models/Activity.swift index 94734645b2..ec3bede286 100644 --- a/AlphaWallet/TokenScriptClient/Models/Activity.swift +++ b/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) diff --git a/AlphaWallet/TokenScriptClient/Models/ActivityOrTransaction.swift b/AlphaWallet/TokenScriptClient/Models/ActivityOrTransaction.swift index f35dca04d3..0033c2b29c 100644 --- a/AlphaWallet/TokenScriptClient/Models/ActivityOrTransaction.swift +++ b/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 + } + } } diff --git a/AlphaWallet/Tokens/Coordinators/GetContractInteractions.swift b/AlphaWallet/Tokens/Coordinators/GetContractInteractions.swift index 3c77a34065..8588fab726 100644 --- a/AlphaWallet/Tokens/Coordinators/GetContractInteractions.swift +++ b/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 ) diff --git a/AlphaWallet/Tokens/Types/EventActivity.swift b/AlphaWallet/Tokens/Types/EventActivity.swift index 80e33a7e40..a2d63cf738 100644 --- a/AlphaWallet/Tokens/Types/EventActivity.swift +++ b/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,9 +46,9 @@ 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 @@ -55,6 +56,7 @@ class EventActivity: Object { self.eventName = eventName self.blockNumber = blockNumber self.transactionId = transactionId + self.transactionIndex = transactionIndex self.logIndex = logIndex self.filter = filter self.json = json diff --git a/AlphaWallet/Transactions/Storage/Transaction.swift b/AlphaWallet/Transactions/Storage/Transaction.swift index 0f619f36b9..f326b5b361 100644 --- a/AlphaWallet/Transactions/Storage/Transaction.swift +++ b/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 diff --git a/AlphaWallet/Transfer/Types/SentTransaction.swift b/AlphaWallet/Transfer/Types/SentTransaction.swift index 982cc41b43..478d94a786 100644 --- a/AlphaWallet/Transfer/Types/SentTransaction.swift +++ b/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,