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-7967 Enenrgy WEB EVM/X #854

Merged
merged 14 commits into from
Oct 9, 2024
Merged

Conversation

amuraveinik
Copy link
Contributor

@amuraveinik amuraveinik commented Sep 30, 2024

@@ -34,7 +34,7 @@ class PolkadotTransactionBuilder {
*/
private var balanceTransferCallIndex: Data {
switch network {
case .polkadot, .azero, .joystream, .bittensor:
case .polkadot, .azero, .joystream, .bittensor, .energyWebX: // TODO: [Energy Web X] Questionable
return Data(hexString: "0x0500")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

?

Copy link
Contributor

Choose a reason for hiding this comment

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

для joystream и bittensor просто брал польковскую, экспериментальным путем проверялось, если транзакции ходят - все ок

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
Collaborator

Choose a reason for hiding this comment

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

нужно попробовать отправить транзакцию обязательно

@@ -126,7 +126,7 @@ class PolkadotTransactionBuilder {
return specVersion < 28
case .kusama:
return specVersion < 2028
case .westend, .azero, .bittensor:
case .westend, .azero, .bittensor, .energyWebX: // TODO: [Energy Web X] Questionable
return false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

?

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
Collaborator

Choose a reason for hiding this comment

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

допиши плиз в код комменты про экспериментальный подбор тут и в других аналогичных местах

@@ -26,7 +26,8 @@ struct SubstrateRuntimeVersionProvider {
// 198 is from the first user report
return meta.specVersion >= 198 ? .v15 : .v14
case .azero,
.joystream:
.joystream,
.energyWebX: // TODO: [Energy Web X] Questionable
return .v14
Copy link
Contributor Author

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

Choose a reason for hiding this comment

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

ставим тут v15?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Оставил 14, транзакция ушла

BlockchainSdk/Common/Blockchain.swift Show resolved Hide resolved
BlockchainSdk/Common/Blockchain/Blockchain+AllCases.swift Outdated Show resolved Hide resolved
@@ -97,6 +98,13 @@ extension PublicKeyType {
default:
throw NSError.makeUnsupportedCurveError(for: blockchain)
}
case .energyWebX(let curve, _):
switch curve {
case .ed25519, .ed25519_slip0010:
Copy link
Contributor Author

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.

его надо переместить под список блокчейнов на ed25519.
Тут возможны варианты

  • секп
  • секп extended (разжатый ключ)
  • ed25519
  • особняком, блокчейны, которые могут работать на разных кривых (secp и еd).

Если говорить про воллет 2, то ed25519_slip0010, это просто ed25519 в воллет кор, а ed25519 у нас это ed25519Cardano в воллет кор.

@amuraveinik amuraveinik marked this pull request as ready for review September 30, 2024 09:57
@amuraveinik
Copy link
Contributor Author

amuraveinik commented Sep 30, 2024

В общем, тут есть спорные моменты по Energy Web X, в которых я не уверен.
Во всех таких местах оставил TODO.
Смогу поправить когда проверю что все работает или не работает, но уже после того как дадут денег на тесты (Леша на дейлике сказал что его добавили в чатик разрабов).

Места помеченные вопросиком - это как раз такие места неопределенности.
Комменты по коду рядом с такими местами прочитал, но картина не прояснилась

BlockchainSdk/Common/API/TestnetAPINodeInfoProvider.swift Outdated Show resolved Hide resolved
@@ -101,6 +101,9 @@ struct EstimationFeeAddressFactory {
return "f1wxdu6d25dc4hmebdfgriswooum22plhmmpxibzq"
case .sei:
return "sei1lhjvds604fvac32j4eygpr820lyc82dlfv0ea4"
case .energyWebX:
// TODO: [Energy Web X] Need to generate
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://tangem.slack.com/archives/GMXC6PP71/p1727690207961099

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.

В целом все ответы по константам можно получить, распарсив метадату
Метадату можно получить дернув ручка metadata в тулзе https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Fpublic-rpc.mainnet.energywebx.com%2F#/runtime

А вот с парсингом сложнее - я нашел только на расте, но сам его не тестил https://docs.rs/substrate_parser/latest/substrate_parser/

@@ -78,6 +82,8 @@ extension PolkadotNetwork {
return Amount(with: .joystream(curve: curve), value: Decimal(stringValue: "0.026666656")!)
case .bittensor(let curve):
return Amount(with: .bittensor(curve: curve), value: Decimal(stringValue: "0.0000005")!)
case .energyWebX(let curve):
return .zeroCoin(for: .energyWebX(curve: curve, testnet: false))
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
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

@amuraveinik amuraveinik Oct 4, 2024

Choose a reason for hiding this comment

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

Обсудили тут, делаем копейку

@@ -34,7 +34,7 @@ class PolkadotTransactionBuilder {
*/
private var balanceTransferCallIndex: Data {
switch network {
case .polkadot, .azero, .joystream, .bittensor:
case .polkadot, .azero, .joystream, .bittensor, .energyWebX: // TODO: [Energy Web X] Questionable
return Data(hexString: "0x0500")
Copy link
Contributor

Choose a reason for hiding this comment

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

По идее у всех парачейнов польки будет это значение

Comment on lines 445 to 446
case .energyWebChain, .energyWebX:
return isTestnet ? "VT" : "EWT"
Copy link
Contributor

Choose a reason for hiding this comment

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

А разве ассет в тестнете парачейна будет VT?
У польки например это все тот же DOT

VT наверное это ассет только в EVM тестнете?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

В итоге выпилил тестнет для Energy Web X, т.к. по запросу все заглохло

Comment on lines 924 to 925
case .energyWebChain: return "energyWebChain"
case .energyWebX: return "energyWebX"
Copy link
Contributor

Choose a reason for hiding this comment

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

Предложил бы прям над каждым кейсом написать коммент что это - парачейн польки или же EVM
Очень сбивают кейсы, из нейминга сетей это понять невозможно

Copy link
Collaborator

Choose a reason for hiding this comment

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

может кейсы так назовем energyWebEVM/energyWebX? Такая же проблема будет с Кавой. Мы добавили kavaEVM как kava. А вот для другой кавы придется придумывать теперь.

Copy link
Contributor

Choose a reason for hiding this comment

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

Можно сделать и названия energyWebEVM/energyWebX и комменты над кейсами оставить, чтобы точно ничего не перепуталось

Мы добавили kavaEVM как kava. А вот для другой кавы придется придумывать теперь.

Как вариант kavaNative, вроде норм)

@@ -135,6 +136,8 @@ public struct DerivationConfigV1: DerivationConfig {
return "m/44'/223'/0'/0/0"
case .filecoin:
return "m/44'/461'/0'/0/0"
case .energyWebX:
return "m/44'/246'/0'/0'/0'"
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
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.

В slip-0044 есть Energy Web, по идее это к EVM относится.
Уточню еще про energy web x

Copy link
Contributor Author

@amuraveinik amuraveinik Oct 4, 2024

Choose a reason for hiding this comment

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

Но вообще для X это указано в ресерче на ноушене

Путь деривации - "m/44'/246'/0'/0’/0’”
Данные взяты из документации, совпадают со SLIP-0044
В документации последние ноды не hardened, что не актуально для ed25519

@@ -780,6 +790,7 @@ extension Blockchain {
case .base: return true
case .cyber: return false
case .blast: return false
case .energyWebChain: return false
Copy link
Contributor

Choose a reason for hiding this comment

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

А это проверялось? Насчет EIP1559

Copy link
Collaborator

Choose a reason for hiding this comment

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

по эксплореру ощущение, что не поддерживается https://explorer.energyweb.org/tx/0x9d284e5addd41933aa9d8325c53c0a117894285427af18dfadf614b81b6a65f5, а что возвращает фи Хистори?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🟠 12:21:29:443: Response: https://rpc.energyweb.org/
🟠 12:21:29:444: body:

{
  "jsonrpc" : "2.0",
  "result" : {
    "reward" : [
      [
        "0x0",
        "0x0",
        "0x0"
      ],
      [
        "0x0",
        "0x0",
        "0x0"
      ],
      [
        "0x0",
        "0x0",
        "0x0"
      ],
      [
        "0x0",
        "0x0",
        "0x0"
      ],
      [
        "0x0",
        "0x0",
        "0x0"
      ]
    ],
    "baseFeePerGas" : [
      "0x7",
      "0x7",
      "0x7",
      "0x7",
      "0x7",
      "0x7"
    ],
    "gasUsedRatio" : [
      0,
      0,
      0,
      0,
      0
    ],
    "oldestBlock" : "0x1ead70d"
  },
  "id" : 21
}

Что маппится в объект

EthereumFeeHistory(
    baseFee: 0, 
    lowBaseFee: 0, 
    marketBaseFee: 0, 
    fastBaseFee: 0, 
    lowPriorityFee: 8, 
    marketPriorityFee: 9, 
    fastPriorityFee: 12
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Со включенным EIP1559 транзакции не отправляются

Copy link
Collaborator

Choose a reason for hiding this comment

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

ну все, значит не поддерживается

BlockchainSdk/Common/Blockchain/Blockchain+AllCases.swift Outdated Show resolved Hide resolved
}

extension EnergyWebXExternalLinkProvider: ExternalLinkProvider {
var testnetFaucetURL: URL? {
Copy link
Collaborator

Choose a reason for hiding this comment

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

в целом у нас в приле это сейчас не поддерживается, запланируем выпил

@@ -78,6 +82,8 @@ extension PolkadotNetwork {
return Amount(with: .joystream(curve: curve), value: Decimal(stringValue: "0.026666656")!)
case .bittensor(let curve):
return Amount(with: .bittensor(curve: curve), value: Decimal(stringValue: "0.0000005")!)
case .energyWebX(let curve):
return .zeroCoin(for: .energyWebX(curve: curve, testnet: false))
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

@@ -34,7 +34,7 @@ class PolkadotTransactionBuilder {
*/
private var balanceTransferCallIndex: Data {
switch network {
case .polkadot, .azero, .joystream, .bittensor:
case .polkadot, .azero, .joystream, .bittensor, .energyWebX: // TODO: [Energy Web X] Questionable
return Data(hexString: "0x0500")
Copy link
Collaborator

Choose a reason for hiding this comment

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

нужно попробовать отправить транзакцию обязательно

@@ -126,7 +126,7 @@ class PolkadotTransactionBuilder {
return specVersion < 28
case .kusama:
return specVersion < 2028
case .westend, .azero, .bittensor:
case .westend, .azero, .bittensor, .energyWebX: // TODO: [Energy Web X] Questionable
return false
Copy link
Collaborator

Choose a reason for hiding this comment

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

допиши плиз в код комменты про экспериментальный подбор тут и в других аналогичных местах

@@ -26,7 +26,8 @@ struct SubstrateRuntimeVersionProvider {
// 198 is from the first user report
return meta.specVersion >= 198 ? .v15 : .v14
case .azero,
.joystream:
.joystream,
.energyWebX: // TODO: [Energy Web X] Questionable
return .v14
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Comment on lines 924 to 925
case .energyWebChain: return "energyWebChain"
case .energyWebX: return "energyWebX"
Copy link
Collaborator

Choose a reason for hiding this comment

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

может кейсы так назовем energyWebEVM/energyWebX? Такая же проблема будет с Кавой. Мы добавили kavaEVM как kava. А вот для другой кавы придется придумывать теперь.

BlockchainSdk/Common/Blockchain/Blockchain+AllCases.swift Outdated Show resolved Hide resolved
@@ -135,6 +136,8 @@ public struct DerivationConfigV1: DerivationConfig {
return "m/44'/223'/0'/0/0"
case .filecoin:
return "m/44'/461'/0'/0/0"
case .energyWebX:
return "m/44'/246'/0'/0'/0'"
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

Choose a reason for hiding this comment

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

подтяни плиз сразу на самый актуальный

@@ -97,6 +98,13 @@ extension PublicKeyType {
default:
throw NSError.makeUnsupportedCurveError(for: blockchain)
}
case .energyWebX(let curve, _):
switch curve {
case .ed25519, .ed25519_slip0010:
Copy link
Collaborator

Choose a reason for hiding this comment

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

его надо переместить под список блокчейнов на ed25519.
Тут возможны варианты

  • секп
  • секп extended (разжатый ключ)
  • ed25519
  • особняком, блокчейны, которые могут работать на разных кривых (secp и еd).

Если говорить про воллет 2, то ed25519_slip0010, это просто ed25519 в воллет кор, а ed25519 у нас это ed25519Cardano в воллет кор.

@@ -79,7 +79,7 @@ private extension EthereumTarget {
return AnyEncodable([AnyEncodable(params), AnyEncodable("latest")])
case .feeHistory:
// Get fee history for 5 blocks (around a minute) with 25,50,75 percentiles (selected empirically)
return AnyEncodable([AnyEncodable(5), AnyEncodable("latest"), AnyEncodable([25,50,75])])
return AnyEncodable([AnyEncodable("0x5"), AnyEncodable("latest"), AnyEncodable([25,50,75])])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Пришлось тут добавить так, потому что у меня запрос фи хистори для EWC падал

🟠 12:17:45:563: Response: https://energy-web-chain.rpc.thirdweb.com/
🟠 12:17:45:563: body:

{
  "jsonrpc": "2.0",
  "error": {
    "message": "Invalid params: invalid type: integer 5, expected a 0x-prefixed hex string with length between (0; 64].",
    "code": -32602
  },
  "id": 25
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Откатил. По видимому разные сети по разному обрабатывают.

@tureck1y Вроде нам же этот запрос не нужен здесь, т.к. можно пренебречь что он будет падать для EWC?

Copy link
Collaborator

Choose a reason for hiding this comment

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

да, допиши плиз этот коммент напротив тоггла eip1559 у этой сети. Что в целом присылает нули и вот этот момент с энкодингом

@amuraveinik
Copy link
Contributor Author

Жду денег на проверку транзакций

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.

В целом только остался адрес для естимейта и конфликты

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.

А вывод скрипта как выглядит? Он норм парсит и польку и парачейны?

Comment on lines +1274 to +1280
case .network: return "energy-web-chain"
case .coin: return "energy-web-token"
}
case .energyWebX:
switch type {
case .network: return "energy-web-x"
case .coin: return "energy-web-token"
Copy link
Contributor

Choose a reason for hiding this comment

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

Тут одинаковые coin ids - это будет вообще работать?
В плане показа цены, синхры на бэке и т.д.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Я вот тоже засомневался, но у бинанся сейчас так, завел задачу провести ревизию использования айдишников

@tureck1y
Copy link
Collaborator

tureck1y commented Oct 9, 2024

Для EVM надо стандартную EVM деривацию

так это же в1, там мы поддерживаем разные, если они есть в слипе

@amuraveinik amuraveinik merged commit 3a9de7f into develop Oct 9, 2024
1 check passed
@amuraveinik amuraveinik deleted the feature/IOS-7967_energy_web branch October 9, 2024 21:15
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