Feat/Sui coin module #7
Conversation
ishaba
force-pushed
the
feat/sui-coin-module
branch
from
b781bb6 to
2d53bc7
Compare
ishaba
force-pushed
the
feat/sui-coin-module
branch
from
2d53bc7 to
96a36cb
Compare
| @@ -105,6 +105,7 @@ setSupportedCurrencies([ | |||
| "zksync_sepolia", | |||
| "mantra", | |||
| "xion", | |||
| "sui", | |||
There was a problem hiding this comment.
Should we add sui_testnet here as well?
There was a problem hiding this comment.
Good question actually i didn't figured out why some coins needs this _testnet version. Possible because some coins haas different configs for mainnet and testnet tokens. I didn't spot any difference for now for sui so it looks like we don't need it and is better to remove sui_testnet from everewhere?
There was a problem hiding this comment.
I don't know, to be honest. Let's add testnet everywhere for consistency.
| @@ -182,6 +182,9 @@ const modes: Readonly<Record<DerivationMode, ModeSpec>> = Object.freeze({ | |||
| ton: { | |||
| overridesDerivation: "44'/607'/0'/0'/<account>'/0'", | |||
| }, | |||
| sui: { | |||
| overridesDerivation: "44'/784'/0'/0'/0'", | |||
There was a problem hiding this comment.
I think we should put <account> placeholder somewhere. Probably at the last position.
There was a problem hiding this comment.
Possible. According to documentation https://docs.sui.io/concepts/cryptography/transaction-auth/keys-addresses it should be 44'/784'/<account>'/0'/0'
There was a problem hiding this comment.
Confirmed. It's the proper way and Sui Wallet uses it
| import { getEnv } from "@ledgerhq/live-env"; | ||
| import { ChainAPI } from "./chain"; | ||
|
|
||
| export function traced(api: ChainAPI): ChainAPI { |
There was a problem hiding this comment.
How do we use this? Is it required for anything?
There was a problem hiding this comment.
Now we don't use it. It will be required when we will implement some request to indexer or any other sui related backend services.
There was a problem hiding this comment.
Is anything else use that approach besides Solana?
|
|
||
| function formatMsg({ reqId, msg, duration }: { reqId: number; msg?: string; duration?: number }) { | ||
| const parts = [ | ||
| `solana req id: ${reqId}`, |
| default: "OUT", | ||
| }; | ||
|
|
||
| const MODE_TO_PALLET_METHOD: Record<SuiOperationMode | "sendMax", PalletMethod> = { |
There was a problem hiding this comment.
I assume those are not relevant. I understand that it was inherited from polkadot. Let's revisit it later once more functions are added and refactor.
There was a problem hiding this comment.
Good question for some reason solana and polkadot for example distinguishes regular transfer and transfer of all tokens from account. For now i can't answer why we need this i just followed common pattern.
There was a problem hiding this comment.
I think the reason for it is that in case of "transfer all" for native tokens we need to consider fees that are paid in native coins. On solana, there's also a special case that we should keep some SOL as an account rent fee.
* fix: history icon and label (sent/received) * cleanup
Fix/acc sync
feature: added paginated history for sui
✅ Checklist
npx changesetwas attached.📝 Description
Replace this text by a clear and concise description of what this pull request is about and why it is needed. Be sure to explain the problem you're addressing and the solution you're proposing.
For libraries, you can add a code sample of how to use it.
For bug fixes, you can explain the previous behaviour and how it was fixed.
In case of visual features, please attach screenshots or video recordings to demonstrate the changes.
❓ Context
🧐 Checklist for the PR Reviewers