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-7719 Add FilecoinNetworkProvider #816

Merged

Conversation

amuraveinik
Copy link
Contributor

@amuraveinik amuraveinik commented Aug 27, 2024

let hash: String

enum CodingKeys: String, CodingKey {
case hash = "/"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Не опечатка, действительно такой ключ

params: [
FilecoinDTOMapper.convertTransactionBody(from: transactionInfo),
nil,
nil
Copy link
Contributor Author

@amuraveinik amuraveinik Aug 27, 2024

Choose a reason for hiding this comment

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

Как бы странно это не выглядело, но апишка файлкоина принимает в качестве параметров не джсон структуру, а массив, в котором мы позиционно передаем наши данные

image

let hash: String

enum CodingKeys: String, CodingKey {
case hash = "/"
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
Contributor Author

Choose a reason for hiding this comment

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

https://docs.filecoin.io/reference/json-rpc/mpool#mpoolpush
В ответе на отправку транзакции прилетает вот такое

{
  "/": "bafy2bzacea3wsdh6y3a36tb3skempjoxqpuyompjbmfeyf34fi3uy6uue42v4"
}

Ну и чтобы поле не называть / я сделал по аналогии с андроидом поле hash, и добавил CodingKeys чтобы слеш распарсился в поле hash

@amuraveinik amuraveinik merged commit bc1677c into blockchains/filecoin Aug 30, 2024
1 check passed
@amuraveinik amuraveinik deleted the feature/IOS-7718_filecoin_network_provider branch August 30, 2024 12:02
import Foundation

struct FilecoinAccountInfo {
let balance: Decimal
Copy link
Contributor

Choose a reason for hiding this comment

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

А какой тип у этого поля в апишке?
Если float - то получается потеря точности
Если string - то корректнее маппить в decimal в маппере/конвертере


import Foundation

struct FilecoinSignedTransactionBody: Encodable {
Copy link
Contributor

Choose a reason for hiding this comment

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

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


import Foundation

struct FilecoinTransactionBody: Encodable {
Copy link
Contributor

Choose a reason for hiding this comment

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

И тут

provider
.getGasUnitPrice(transactionInfo: transactionInfo)
.tryMap { response in
guard let price = UInt64(response) else {
Copy link
Contributor

Choose a reason for hiding this comment

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

А тут хватает uint64?

В чем номинируется цена газа, я у них на сайте нашел упоминание про attoFIL, не в них случайно?

Если да, то 1 attoFIL всего в 18 раз меньше uint64.max

switch type {
case .getActorInfo(let address):
.requestJSONRPC(
id: Constants.jsonRPCMethodId,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: лучше на каждый запрос инкрементить этот id (и хранить его в static var)
В этом его смысл, он позволяет сопоставить request и response в логах на клиенте и сервере

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