feat: spot #1933
feat: spot #1933
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
src/components/Output.tsx
Outdated
| ${({ $polarity }) => | ||
| $polarity === NumberSign.Positive && | ||
| css` | ||
| color: var(--color-positive) !important; | ||
| `} | ||
| ${({ $polarity }) => | ||
| $polarity === NumberSign.Negative && | ||
| css` | ||
| color: var(--color-negative) !important; | ||
| `} |
There was a problem hiding this comment.
nit: I think we can merge these two into a ternary so we only access props once.
Does it need the !important override?
src/pages/spot/SpotTradeForm.tsx
Outdated
|
|
||
| const price = Number(SOL_PRICE_USDC); | ||
| const toSol = buyType === SpotBuyType.USDC && nextBuyType === SpotBuyType.SOL; | ||
| const factor = toSol ? 1 / price : price; |
There was a problem hiding this comment.
may want to ensure price is not 0
src/pages/spot/SpotTradeForm.tsx
Outdated
| onValueChange={(v) => { | ||
| setTradeType(v as SpotFormType); |
There was a problem hiding this comment.
I think theres a way to make this a generic so v doesn't have to be typescase with as
| }: { | ||
| className?: string; | ||
| marketId: string; | ||
| variant?: 'perp' | 'spot'; |
There was a problem hiding this comment.
will this ever be undefined?
There was a problem hiding this comment.
oh I guess its so you can do the default. Not sure if we should just make it explicit on every <FavoriteButton> use
qardpeet
force-pushed
the
pk/feat-spot-ui
branch
from
d3dcfe1 to
dd3fa60
Compare
qardpeet
force-pushed
the
pk/feat-spot-ui
branch
from
dd3fa60 to
4247be3
Compare
src/bonsai/rest/spot.ts
Outdated
| export function setUpTokenMetadataQuery(store: RootStore) { | ||
| return createStoreEffect( | ||
| store, | ||
| (state) => { |
There was a problem hiding this comment.
this needs to be a selector otherwise it's a new return object every time and will restart the lifecycle every time any store update happens.
qardpeet
force-pushed
the
pk/feat-spot-ui
branch
from
3183125 to
c957943
Compare
There was a problem hiding this comment.
remember to localize and add the textKeys to the validation errors
src/bonsai/forms/spot.ts
Outdated
| } | ||
| } | ||
|
|
||
| const minUsdAmount = 0.1; // Minimum $0.10 USD |
There was a problem hiding this comment.
nit: since this is a true const we should rename MIN_SPOT_USD_AMOUNT and put top level of file
src/components/Output.tsx
Outdated
| className?: string; | ||
| withBaseFont?: boolean; | ||
| withSignColor?: boolean; | ||
| withPolarityColor?: boolean; |
There was a problem hiding this comment.
nit: polarity feels like a stretch semantically (i get the connection tho). withSignedValueColor or something might be more accurate
src/hooks/useAccounts.tsx
Outdated
| }; | ||
| setCosmosWallets(); | ||
| }, [hdKey?.mnemonic, getCosmosOfflineSigner]); | ||
| }, [hdKey?.mnemonic, getCosmosOfflineSigner, canDeriveSolanaWallet]); |
There was a problem hiding this comment.
is canDeriveSolanaWallet necessary? It doesn't seem like its referenced in this side effect and if hdKey.mnemonic is not undefined then i think we already know that we are not utilizing keplr.
There was a problem hiding this comment.
yeah, accidentally left it in there good catch!
There was a problem hiding this comment.
with the removal of blocked geo from the derivation flow, how are we preventing users from interacting with perps as a whole?
src/hooks/useComplianceState.tsx
Outdated
| const { checkForGeo } = useEnvFeatures(); | ||
| const location = useLocation(); | ||
|
|
||
| const isSpotPage = location.pathname.startsWith(AppRoute.Spot); |
There was a problem hiding this comment.
I think we can use useMatchinstead of useLocation + startsWith
|
|
||
| type CustomNoticationOptions = { | ||
| id: string; | ||
| id?: string; |
There was a problem hiding this comment.
Why remove? will this not cause key issues later down the line?
There was a problem hiding this comment.
the id already had a default in place:
const id = options?.id ?? Date.now().toString();
src/hooks/useSpotForm.tsx
Outdated
| body: 'Transaction failed. Please try again.', | ||
| }, | ||
| { | ||
| toastDuration: 8000, |
There was a problem hiding this comment.
why not have matching toastDuration between success and failure state?
| resolveSymbol: async (tokenSymbol, onSymbolResolvedCallback) => { | ||
| const symbolInfo = createSpotSymbolInfo(tokenSymbol); | ||
| const tokenPrice = await waitForSelector(store, (s) => BonsaiCore.spot.tokenPrice.data(s)); | ||
| const symbolInfo = createSpotSymbolInfo(tokenSymbol, tokenPrice); |
There was a problem hiding this comment.
tokenSymbol and tokenMint are being used interchangeably right? in the createSpotSymbolInfo, it uses tokenSymbol (mint) everywhere. Do we actually every use the real token symbol?
There was a problem hiding this comment.
no we don't, its named in a bad way it should be just the mint
| ticker: tokenSymbol, | ||
| name: tokenSymbol, |
There was a problem hiding this comment.
im not sure which of these is the display info in the TradingView chart
* adjust spot form error logic * withSignedValueColor in Output * make MIN_SOL_RESERVE 0.002 * useMatch instead of useLocation * normalize toast duration for spot trading
There was a problem hiding this comment.
https://solana.com/developers/cookbook/wallets/restore-from-mnemonic
Can we do a quick audit to make sure that your bip44 derivation comes out with the same as from the Solana cookbook? The only part of your implementation that i am unsure of is needing the seedHex to call derivePath.
Includes most of the "dumb" UI components needed to make spot work:
Will push up a separate pr that moves as much as state & serialization into redux/bonsai as possible.
Currently missing localization, will prioritize that last after API integration and Solana tx signing are done.