-
Notifications
You must be signed in to change notification settings - Fork 102
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
feat: remove hardcoded base token dependencies from the app #8160
base: develop
Are you sure you want to change the base?
Conversation
packages/shared/lib/core/network/constants/default-network-metadata.constant.ts
Outdated
Show resolved
Hide resolved
delete oldNetwork.baseToken | ||
|
||
const newNetwork = oldNetwork as unknown as IPersistedNetwork | ||
const defaultBech32Hrp = DEFAULT_NETWORK_METADATA[existingProfile.network.id]?.bech32Hrp || '' |
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.
In what cases would DEFAULT_NETWORK_METADATA[existingProfile.network.id]?.bech32Hrp
not exist? I just because it seems weird to me to leave ''
as fallback value, it will likely break if that ever happens.
const _coinType = coinType ?? COIN_TYPE[networkId] ?? 1 | ||
return { | ||
id: networkId, | ||
name: name ?? 'Unknown Network', | ||
coinType: _coinType, | ||
protocol: nodeInfoResponse?.nodeInfo?.protocol, | ||
baseToken: { standard: TokenStandard.BaseToken, ...nodeInfoResponse?.nodeInfo?.baseToken }, | ||
bech32Hrp: bech32Hrp ?? 'Unknown Network', |
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 did you add 'Unknown Network'
as fallback value here now? This will likely break it as well and it doesn't make sense to use a human-readable phrase for this either
|
||
export const nodeInfoBaseToken = derived(nodeInfo, ($nodeInfo) => $nodeInfo?.baseToken) | ||
|
||
export const nodeInfoProtocol = derived(nodeInfo, ($nodeInfo) => $nodeInfo?.protocol) |
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 is not used
export const nodeInfoProtocol = derived(nodeInfo, ($nodeInfo) => $nodeInfo?.protocol) |
|
||
export const nodeInfo = writable<INodeInfo | undefined>(undefined) | ||
|
||
export function setNodeInfo(newNodeInfo: INodeInfo | undefined): void { | ||
return nodeInfo.set(newNodeInfo) | ||
} | ||
|
||
export const nodeInfoBaseToken = derived(nodeInfo, ($nodeInfo) => $nodeInfo?.baseToken) |
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.
What's the point of this store? You don't seem to be reading it from any svelte file so all the reactivity features it comes with are effectively not used at all, thus it seems more appropiate to me to just read the nodeInfo
store
@@ -161,7 +161,7 @@ export function setTotalUnclaimedShimmerRewards(_totalUnclaimedShimmerRewards: n | |||
|
|||
function showRewardsFoundNotification(updatedTotalUnclaimedShimmerRewards: number): void { | |||
const foundRewardsAmount = updatedTotalUnclaimedShimmerRewards - totalUnclaimedShimmerRewards | |||
const foundRewardsAmountFormatted = formatTokenAmountBestMatch(foundRewardsAmount, getOnboardingBaseToken()) | |||
const foundRewardsAmountFormatted = formatTokenAmountBestMatch(foundRewardsAmount, getBaseToken()) |
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 will break because there is no Node Info loaded in the onboarding, it's only loaded once you log in.
…evelop' of github.com:iotaledger/firefly into feat/remove-hardcoded-base-token-dependencies
Closes #8132
Summary
Get the node's base token information instead of getting the hardcoded default information
Changelog
Testing
Platforms
Checklist