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 8249 bsdk krc20 #4144

Open
wants to merge 20 commits into
base: develop
Choose a base branch
from
Open

IOS 8249 bsdk krc20 #4144

wants to merge 20 commits into from

Conversation

siblockchaina
Copy link
Contributor

No description provided.

BlockchainSdk/Blockchains/Kaspa/KaspaFeeMapper.swift Outdated Show resolved Hide resolved
private let provider: NetworkProvider<KaspaTargetKRC20>

init(testnet: Bool, networkConfiguration: NetworkProviderConfiguration) {
url = testnet ? URL("https://tn10api.kasplex.org/v1")! : URL("https://api.kasplex.org/v1/")!
Copy link

Choose a reason for hiding this comment

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

Вот это надо было бы подсветить, кстати, что у нас всего одна апиха для получения баланса токенов. Надо может попробовать в чатике Касплексовцев спросить про резервную

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

@tureck1y tureck1y left a comment

Choose a reason for hiding this comment

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

Я верхнеуровнево комменты накидал, в логику не вдавался

BlockchainSdk/Common/Blockchain.swift Show resolved Hide resolved
BlockchainSdk/Blockchains/Kaspa/KaspaWalletManager.swift Outdated Show resolved Hide resolved
BlockchainSdk/Blockchains/Kaspa/KaspaWalletManager.swift Outdated Show resolved Hide resolved
private let provider: NetworkProvider<KaspaTargetKRC20>

init(testnet: Bool, networkConfiguration: NetworkProviderConfiguration) {
url = testnet ? URL("https://tn10api.kasplex.org/v1")! : URL("https://api.kasplex.org/v1/")!
Copy link
Collaborator

Choose a reason for hiding this comment

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

только урл давай в ассембли передавать, пусть и хардкодом

BlockchainSdk/Blockchains/Kaspa/KaspaFeeMapper.swift Outdated Show resolved Hide resolved
@@ -1,5 +1,5 @@
{
"originHash" : "f4db68609e7f14bf89c4092c7c9275ff72bf1143baf6863092f268fafb155d01",
Copy link
Contributor

@m3g0byt3 m3g0byt3 Nov 20, 2024

Choose a reason for hiding this comment

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

В деве не обновлен SPM lock, обновил его

@@ -1,5 +1,5 @@
{
"originHash" : "6b035953f736066530f0d187d9ba246ef7feaaf3f9e8bf5106f9f10b0ba6d560",
Copy link
Contributor

Choose a reason for hiding this comment

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

И тут

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.

  1. С учетом того, что апишка для токенов одна и она бета - надо обязательно сделать фоллбек, чтобы зафейленная загрузка токенов не фейлила каспу. Примерно как сделано тут IOS-7610 Don't fail balance loading on token error #4253

  2. В ключах, по которым мы храним incomplete txs в сторейдже - надо учитывать адрес воллета, а не только контракт адрес токена. Сторейдж - это единый инстанс для всего BSDK и сейчас получается, что incomplete txs от одного воллета показываются в другом воллете, если на нем тоже добавлен этот токен.

tureck1y
tureck1y previously approved these changes Nov 22, 2024
@m3g0byt3
Copy link
Contributor

  1. С учетом того, что апишка для токенов одна и она бета - надо обязательно сделать фоллбек, чтобы зафейленная загрузка токенов не фейлила каспу. Примерно как сделано тут IOS-7610 Don't fail balance loading on token error #4253
  2. В ключах, по которым мы храним incomplete txs в сторейдже - надо учитывать адрес воллета, а не только контракт адрес токена. Сторейдж - это единый инстанс для всего BSDK и сейчас получается, что incomplete txs от одного воллета показываются в другом воллете, если на нем тоже добавлен этот токен.
  1. Посмотрел внимательнее - фоллбек уже реализован в KaspaNetworkServiceKRC20
  2. Добавил адрес воллета в ключ

func mapTokenFee(mass: Decimal, feeEstimate: KaspaFeeEstimateResponse) -> [Fee] {
let buckets = buckets(from: feeEstimate)

let mass = mass
Copy link
Contributor

Choose a reason for hiding this comment

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

как будто лишнее

let providers: [KaspaNetworkProviderKRC20]
var currentProviderIndex: Int = 0

private let blockchain: Blockchain
Copy link
Contributor

Choose a reason for hiding this comment

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

Как будто не используется, да и не нужен целый enum Blockchain здесь

Comment on lines +26 to +35
.setFailureType(to: Error.self)
.withWeakCaptureOf(self)
.flatMap { networkService, token in
networkService.providerPublisher(for: { provider in
provider.balance(address: address, token: token.contractAddress)
.retry(2)
.eraseToAnyPublisher()
})
.mapToResult()
.setFailureType(to: Error.self)
Copy link
Contributor

Choose a reason for hiding this comment

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

как будто .setFailureType(to: Error.self) лишний, раз mapToResult юзаем
и возвращать можно Never вместо Error

@@ -41,6 +43,14 @@ class KaspaTransactionBuilder {
}

func buildForSign(_ transaction: Transaction) throws -> (KaspaTransaction, [Data]) {
switch transaction.amount.type {
case .token(let token):
let commitTx = try buildCommitTransactionKRC20(transaction: transaction, token: token)

Choose a reason for hiding this comment

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

Странно, что только одну транзакцию возвращаем здесь

Comment on lines +115 to +116
let commitSignatures = Array(signatures[..<txgroup.hashesCommit.count])
let revealSignatures = Array(signatures[txgroup.hashesCommit.count...])

Choose a reason for hiding this comment

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

Непонятный синтаксис какой-то. Но может это я в свифте плохо разбираюсь.

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.

6 participants