-
Notifications
You must be signed in to change notification settings - Fork 302
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
[Coin Modularization] Solana #6721
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 5 Ignored Deployments
|
30e67d3
to
0f75dee
Compare
c3c09eb
to
a82fe74
Compare
7ab805a
to
f7ee709
Compare
@@ -16,6 +16,8 @@ import { getEnv } from "@ledgerhq/live-env"; | |||
import { Awaited } from "../../logic"; | |||
import { NetworkError } from "@ledgerhq/errors"; | |||
|
|||
export const LATEST_BLOCKHASH_MOCK = "EEbZs6DmDyDjucyYbo3LwVJU7pQYuVopYcYTSEZXskW3"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is it here ? Is it a random hash ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know its purpose.
I move it for convient reason, as it is used within api calls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file looks like a "fourre tout". I think we should clearer names and put functions in more specific files.
Maybe helpers/address
helpers/token
helpers/staking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right about staking.
I am adding this file.
But I will not going beyond as we need a better separation about what needs to be imported by the UI what doesn't.
memo?: string; | ||
}; | ||
|
||
export type TokenCreateATACommand = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe types.ts
files are also too much like a "fourre tout". I think it could be nice to have things organized by purpose/role.
Like a token.ts with the type, the helper functions, etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't want to refacto this coin-module for the time being.
The only purpose is to separate the code from LLC, as we have done for Algorand, Near and Bitcoin.
01a145a
to
998f100
Compare
998f100
to
4ef7359
Compare
4ef7359
to
b31063e
Compare
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
@@ -2,7 +2,7 @@ import { Account, AccountLike } from "@ledgerhq/types-live"; | |||
import invariant from "invariant"; | |||
import flatMap from "lodash/flatMap"; | |||
import { getAccountCurrency } from "../../account"; | |||
import { TokenAccount } from "../solana/api/chain/account/token"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cardano depends on Solana?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It surprised me too.
The fix will be part of Cardano modularization...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm for hub scope.
one possible (historical?) mistake on cardano->solana dep
[Bot] Testing with 'Nitrogen' ($0.00) ⏲ 10min 2s
1 critical spec errorsSpec Solana failed!
Details of the 0 mutationsSpec Solana (failed)
Details of the 8 uncovered mutationsSpec Solana (8)
Portfolio ($0.00) – Details of the 1 currencies
Performance ⏲ 10min 2sTime spent for each spec: (total across mutations)
|
Signed-off-by: Stéphane Prohaszka <[email protected]>
Signed-off-by: Stéphane Prohaszka <[email protected]>
Signed-off-by: Stéphane Prohaszka <[email protected]>
Signed-off-by: Stéphane Prohaszka <[email protected]>
Signed-off-by: Stéphane Prohaszka <[email protected]>
Signed-off-by: Stéphane Prohaszka <[email protected]>
Signed-off-by: Stéphane Prohaszka <[email protected]>
Signed-off-by: Stéphane Prohaszka <[email protected]>
Signed-off-by: Stéphane Prohaszka <[email protected]>
Signed-off-by: Stéphane Prohaszka <[email protected]>
Signed-off-by: Stéphane Prohaszka <[email protected]>
Signed-off-by: Stéphane Prohaszka <[email protected]>
Signed-off-by: Stéphane Prohaszka <[email protected]>
0a9f26f
676a18f
to
0a9f26f
Compare
✅ Checklist
npx changeset
was attached.📝 Description
Extract Solana as a module
❓ Context
🧐 Checklist for the PR Reviewers