Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IOS-7392 Refactor tron to use raw data #799

Merged
merged 6 commits into from
Aug 13, 2024
Merged

Conversation

tureck1y
Copy link
Collaborator

@tureck1y tureck1y commented Aug 12, 2024

@@ -13,8 +13,6 @@ import TangemSdk
@testable import BlockchainSdk

final class ICPTests: XCTestCase {
private let blockchain = Blockchain.internetComputer
Copy link
Collaborator Author

@tureck1y tureck1y Aug 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

не собирались тесты из-за конфликта с Wallet Core

@@ -29,22 +29,28 @@ class TronTests: XCTestCase {

override func setUp() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Только правка тестов под новый интерфейс


public init() {}

static func toByteForm(_ base58String: String) -> Data? {
Copy link
Collaborator Author

@tureck1y tureck1y Aug 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Перенесено в TronUtils/WalletManager с рефакторингом на throws

import Foundation
import CryptoSwift

enum TronFunction: String {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Собрал с разных файлов. в одном месте

@@ -16,7 +16,7 @@ struct TronTarget: TargetType {
case getAccountResource(address: String)
case getNowBlock
case broadcastHex(data: Data)
case tokenBalance(address: String, contractAddress: String)
case tokenBalance(address: String, contractAddress: String, parameter: String)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Энкодинг с этого уровня перенес в walletManager

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Что-то мне не понятно, зачем "энкодед адрес" тащить через все функции, когда рядом у нас лежит тот же адрес в другой форме? Вроде здесь хорошее место, что бы это конвертить.

Ну и название "параметр" странное, когда речь идёт о конкретном запросе, где мы знаем, что этот параметр - енкодед адрес

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Потому что функция энкодинга может выбрасывать ошибки, а глубого в сетевом слое эту ошибку нормально не обработать, да и не правильно это там делать. Поэтому пришлось поднять наверх.

Параметром я назвал по двум причинам - по аналогии с соседним методом, и потому что в самой модели называется "параметр". В целом переименовать не проблема, но тогда придется еще соседние методы затронуть, а пулл и так стал большой

@tureck1y tureck1y marked this pull request as ready for review August 12, 2024 18:06
@tureck1y tureck1y requested review from dbaturin and a team as code owners August 12, 2024 18:06
let transactionIDs = wallet.pendingTransactions.map { $0.hash }

cancellable = networkService.accountInfo(for: wallet.address, tokens: cardTokens, transactionIDs: transactionIDs)
let encodedAddressPublisher = Result {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: лишний пробел

fedorov-d
fedorov-d previously approved these changes Aug 13, 2024
Copy link
Contributor

@m3g0byt3 m3g0byt3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не нашел в описании задач для чего это нужно
Я правильно понимаю что для стейкинга и для передачи произвольных данных в пейлоаде?

BlockchainSdk/Blockchains/Tron/TronWalletManager.swift Outdated Show resolved Hide resolved
@tureck1y
Copy link
Collaborator Author

Не нашел в описании задач для чего это нужно Я правильно понимаю что для стейкинга и для передачи произвольных данных в пейлоаде?

Возможность передавать raw data в транзакциях, аналогично эфиру. Это понадобится для нового провайдера в экспрессе и, возможно, для стейкинга

@tureck1y tureck1y merged commit 0aa69ee into develop Aug 13, 2024
1 check passed
@tureck1y tureck1y deleted the IOS-7392_tron_refactor_data branch August 13, 2024 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants