From 555986233d7d5d2ef1a2498a8b3de1368aa53f5f Mon Sep 17 00:00:00 2001 From: "Michael Bradley, Jr" Date: Mon, 4 Oct 2021 23:55:06 -0500 Subject: [PATCH] refactor: return object instead of string from stickers backend Related to https://github.com/status-im/status-desktop/issues/3725. Introduce `type PendingTransaction` in `status/types/transaction.nim`. Refactor `proc buyPack` in `status/stickers.nim` to return an instance of that type instead of `string`. Eliminate unnecessary threading of `var success` argument through successive calls in favor of tracking `success` in a field on `PendingTransaction`. --- NOTES Several files in this project have "sticker" in their names or make reference to "sticker". All those were reviewed as candidates for changes logically-related to the purpose of this PR: `status/accounts.nim` `status/chat.nim` `status/chat/stickers.nim` `status/eth/contracts.nim` `status/eth/stickers.nim` `status/status.nim` `status/statusgo_backend/chat.nim` `status/statusgo_backend/edn_helpers.nim` `status/stickers.nim` `status/types/message.nim` `status/types/pending_transaction_type.nim` `status/types/setting.nim` `status/types/sticker.nim` `status/utils.nim` `status/wallet/collectibles.nim` `sendStickerMessage` in `status/statusgo_backend/chat.nim` returns `string` but refactoring it seems out of scope for this set of changes. A comment has been left in `status/stickers.nim` re: an additional refactor that can be made to simplify `trackPendingTransaction` as called by `buyPack`. The refactor will involve changes in other modules as well. `type Message` in `status/types/message.nim` has a `sticker*: string` field that doesn't seem to be made use of by the Nim side of status-desktop, but it it is made use of by desktop's QML side (see: `status-desktop/ui/app/AppLayouts/Chat/ChatColumn/ChatMessages.qml`). In the future it may be preferable to use a different type, or a type alias for `string`. `decodeContentHash` in `status/utils.nim` could return an instance of `type Cid` without converting it to a string, leaving it up to call sites in status-lib and status-desktop to convert to `string`. `getStickers` in `status/wallet/collectibles.nim` returns `string`, used by `{.slot.}` procs in status-desktop (see Nim sources in `status-desktop/src/app`) that expose the JSON data (as `string`) to QML. Refactoring the return value seems out of scope for this PR, given ongoing refactors in status-desktop's front-end architecture. --- status/chat/stickers.nim | 9 ++------- status/stickers.nim | 24 ++++++++++++++++++++---- status/types/transaction.nim | 11 ++++++++--- 3 files changed, 30 insertions(+), 14 deletions(-) diff --git a/status/chat/stickers.nim b/status/chat/stickers.nim index 30e6fe6..125e4f6 100644 --- a/status/chat/stickers.nim +++ b/status/chat/stickers.nim @@ -1,9 +1,4 @@ import chronicles -import ../stickers as status_stickers +import ../stickers -logScope: - topics = "sticker-decoding" - -# TODO: this is for testing purposes, the correct function should decode the hash -proc decodeContentHash*(value: string): string = - status_stickers.decodeContentHash(value) +export decodeContentHash diff --git a/status/stickers.nim b/status/stickers.nim index 6a8f183..74fbae8 100644 --- a/status/stickers.nim +++ b/status/stickers.nim @@ -76,7 +76,10 @@ proc estimateGas*(packId: int, address: string, price: string, success: var bool if success: result = fromHex[int](response) -proc buyPack*(self: StickersModel, packId: int, address, price, gas, gasPrice: string, isEIP1559Enabled: bool, maxPriorityFeePerGas: string, maxFeePerGas: string, password: string, success: var bool): string = +proc buyPack*(self: StickersModel, packId: int, address, price, gas, + gasPrice: string, isEIP1559Enabled: bool, maxPriorityFeePerGas: string, + maxFeePerGas: string, password: string): PendingTransaction = + var sntContract: Erc20Contract approveAndCall: ApproveAndCall[100] @@ -93,9 +96,22 @@ proc buyPack*(self: StickersModel, packId: int, address, price, gas, gasPrice: s maxFeePerGas ) - result = sntContract.methods["approveAndCall"].send(tx, approveAndCall, password, success) + var success: bool + let hash = sntContract.methods["approveAndCall"].send(tx, approveAndCall, + password, success) + + result = PendingTransaction(hash: hash, success: success, + txType: PendingTransactionType.BuyStickerPack) + if success: - trackPendingTransaction(result, address, $sntContract.address, PendingTransactionType.BuyStickerPack, $packId) + # `proc trackPendingTransaction*` in `statusgo_backend/wallet.nim` can be + # refactored to accept an instance of `type PendingTransaction` as its + # first argument, allowing its signature to be simplified. However, that + # will also require changes re: usage of `trackPendingTransaction` in + # `./wallet.nim` and `./ens.nim`, and complementary changes in + # status-desktop + trackPendingTransaction(result.hash, address, $sntContract.address, + result.txType, $packId) proc getStickerMarketAddress*(self: StickersModel): Address = let network = status_settings.getCurrentNetwork().toNetwork() @@ -147,7 +163,7 @@ proc uninstallStickerPack*(self: StickersModel, packId: int) = eth_stickers.saveInstalledStickerPacks(self.installedStickerPacks) proc decodeContentHash*(value: string): string = - result = status_utils.decodeContentHash(value) + status_utils.decodeContentHash(value) proc getPackIdFromTokenId*(tokenId: Stuint[256]): int = let network = status_settings.getCurrentNetwork().toNetwork() diff --git a/status/types/transaction.nim b/status/types/transaction.nim index b2cb98c..def12f8 100644 --- a/status/types/transaction.nim +++ b/status/types/transaction.nim @@ -5,6 +5,11 @@ import web3/ethtypes, options, stint include pending_transaction_type type + PendingTransaction* = ref object + hash*: string + success*: bool + txType*: PendingTransactionType + Transaction* = ref object id*: string typeValue*: string @@ -21,8 +26,8 @@ type value*: string fromAddress*: string to*: string - -type + +type TransactionData* = object source*: Address # the address the transaction is send from. to*: Option[Address] # (optional when creating new contract) the address the transaction is directed to. @@ -40,4 +45,4 @@ proc cmpTransactions*(x, y: Transaction): int = # Compares first by block number, then by nonce result = cmp(x.blockNumber.parseHexInt, y.blockNumber.parseHexInt) if result == 0: - result = cmp(x.nonce, y.nonce) \ No newline at end of file + result = cmp(x.nonce, y.nonce)