-
Notifications
You must be signed in to change notification settings - Fork 15
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
Refactor selections/balances to use DenomMetadata
, and to render a ValueViewComponent when possible
#440
Refactor selections/balances to use DenomMetadata
, and to render a ValueViewComponent when possible
#440
Changes from all commits
abca983
14176b0
4f46a9f
def8f5e
0b2c465
b043c0f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,13 @@ | ||
import { Input, InputProps } from '@penumbra-zone/ui'; | ||
import { cn } from '@penumbra-zone/ui/lib/utils'; | ||
import { fromBaseUnitAmount, joinLoHiAmount } from '@penumbra-zone/types'; | ||
import { joinLoHiAmount } from '@penumbra-zone/types'; | ||
import SelectTokenModal from './select-token-modal'; | ||
import { Validation, validationResult } from './validation-result'; | ||
import { AccountBalance, AssetBalance } from '../../fetchers/balances'; | ||
import { Selection } from '../../state/types'; | ||
import { Fee } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/component/fee/v1alpha1/fee_pb'; | ||
import { ValueViewComponent } from '@penumbra-zone/ui/components/ui/tx/view/value'; | ||
import { ValueView } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/asset/v1alpha1/asset_pb'; | ||
|
||
const PENUMBRA_FEE_DENOMINATOR = 1000; | ||
|
||
|
@@ -14,10 +16,23 @@ const getFeeAsString = (fee: Fee | undefined) => { | |
return `${(Number(joinLoHiAmount(fee.amount)) / PENUMBRA_FEE_DENOMINATOR).toString()} penumbra`; | ||
}; | ||
|
||
const getCurrentBalance = (assetBalance: AssetBalance | undefined) => | ||
assetBalance | ||
? fromBaseUnitAmount(assetBalance.amount, assetBalance.denom.exponent).toFormat() | ||
: '0'; | ||
const getCurrentBalanceValueView = (assetBalance: AssetBalance | undefined): ValueView => { | ||
if (assetBalance?.denomMetadata) | ||
return new ValueView({ | ||
valueView: { | ||
case: 'knownDenom', | ||
value: { amount: assetBalance.amount, denom: assetBalance.denomMetadata }, | ||
}, | ||
}); | ||
else if (assetBalance?.assetId) | ||
return new ValueView({ | ||
valueView: { | ||
case: 'unknownDenom', | ||
value: { amount: assetBalance.amount, assetId: assetBalance.assetId }, | ||
}, | ||
}); | ||
else return new ValueView(); | ||
}; | ||
Comment on lines
+19
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is also something we'll migrate to balances.ts so components won't need to have to do this. |
||
|
||
interface InputTokenProps extends InputProps { | ||
label: string; | ||
|
@@ -47,7 +62,7 @@ export default function InputToken({ | |
}: InputTokenProps) { | ||
const vResult = validationResult(value, validations); | ||
|
||
const currentBalance = getCurrentBalance(selection?.asset); | ||
const currentBalanceValueView = getCurrentBalanceValueView(selection?.asset); | ||
const feeAsString = getFeeAsString(fee); | ||
|
||
return ( | ||
|
@@ -93,7 +108,7 @@ export default function InputToken({ | |
</div> | ||
<div className='flex items-start gap-1'> | ||
<img src='./wallet.svg' alt='Wallet' className='size-5' /> | ||
<p className='font-bold text-muted-foreground'>{currentBalance}</p> | ||
<ValueViewComponent view={currentBalanceValueView} /> | ||
</div> | ||
</div> | ||
</div> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some overlap with: #427. But no worries! I'll definitely make use of your work. I think the end state probably looks like this: export interface AccountBalance {
value: ValueView;
address: AddressView;
usdcValue: number;
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,7 @@ import { | |
} from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/view/v1alpha1/view_pb'; | ||
import { | ||
AssetId, | ||
DenomUnit, | ||
DenomMetadata, | ||
} from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/asset/v1alpha1/asset_pb'; | ||
import { getAddresses, IndexAddrRecord } from './address'; | ||
import { getAllAssets } from './assets'; | ||
|
@@ -16,10 +16,7 @@ import { viewClient } from '../clients/grpc'; | |
import { streamToPromise } from './stream'; | ||
|
||
export interface AssetBalance { | ||
denom: { | ||
display: DenomUnit['denom']; | ||
exponent: DenomUnit['exponent']; | ||
}; | ||
denomMetadata: DenomMetadata; | ||
assetId: AssetId; | ||
amount: Amount; | ||
usdcValue: number; | ||
|
@@ -35,29 +32,19 @@ type NormalizedBalance = AssetBalance & { | |
account: { index: number; address: string }; | ||
}; | ||
|
||
// Given an asset has many denom units, the amount should be formatted using | ||
// the exponent of the display denom (e.g. 1,954,000,000 upenumbra = 1,954 penumbra) | ||
export const displayDenom = (res?: AssetsResponse): { display: string; exponent: number } => { | ||
const display = res?.denomMetadata?.display; | ||
if (!display) return { display: 'unknown', exponent: 0 }; | ||
|
||
const match = res.denomMetadata?.denomUnits.find(d => d.denom === display); | ||
if (!match) return { display, exponent: 0 }; | ||
|
||
return { display, exponent: match.exponent }; | ||
}; | ||
Comment on lines
-40
to
-48
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Eliminated this helper function entirely, as it just takes an Going forward, we want to use the |
||
|
||
const getDenomAmount = (res: BalancesResponse, metadata: AssetsResponse[]) => { | ||
const getDenomAmount = ( | ||
res: BalancesResponse, | ||
metadata: AssetsResponse[], | ||
): { amount: Amount; denomMetadata: DenomMetadata } => { | ||
const assetId = uint8ArrayToBase64(res.balance!.assetId!.inner); | ||
const match = metadata.find(m => { | ||
if (!m.denomMetadata?.penumbraAssetId?.inner) return false; | ||
return assetId === uint8ArrayToBase64(m.denomMetadata.penumbraAssetId.inner); | ||
}); | ||
|
||
const { display, exponent } = displayDenom(match); | ||
const amount = res.balance?.amount ?? new Amount(); | ||
|
||
return { display, exponent, amount }; | ||
return { amount, denomMetadata: match?.denomMetadata ?? new DenomMetadata() }; | ||
}; | ||
|
||
const normalize = | ||
|
@@ -66,10 +53,10 @@ const normalize = | |
const index = res.account?.account ?? 0; | ||
const address = indexAddrRecord?.[index] ?? ''; | ||
|
||
const { display, exponent, amount } = getDenomAmount(res, metadata); | ||
const { denomMetadata, amount } = getDenomAmount(res, metadata); | ||
|
||
return { | ||
denom: { display, exponent }, | ||
denomMetadata, | ||
assetId: res.balance!.assetId!, | ||
amount, | ||
//usdcValue: amount * 0.93245, // TODO: Temporary until pricing implemented | ||
|
@@ -81,7 +68,7 @@ const normalize = | |
const groupByAccount = (balances: AccountBalance[], curr: NormalizedBalance): AccountBalance[] => { | ||
const match = balances.find(b => b.index === curr.account.index); | ||
const newBalance = { | ||
denom: curr.denom, | ||
denomMetadata: curr.denomMetadata, | ||
amount: curr.amount, | ||
usdcValue: curr.usdcValue, | ||
assetId: curr.assetId, | ||
|
@@ -109,7 +96,7 @@ const sortByAmount = (a: AssetBalance, b: AssetBalance): number => { | |
return Number(joinLoHiAmount(b.amount) - joinLoHiAmount(a.amount)); | ||
|
||
// If both are equal, sort by asset name in ascending order | ||
return a.denom.display.localeCompare(b.denom.display); | ||
return a.denomMetadata.display.localeCompare(b.denomMetadata.display); | ||
}; | ||
|
||
// Sort by account (lowest first) | ||
|
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.
These are places we'd want to use
ValueViewComponent
as well. However, I'll pick up this work in: #427. This would require more refactoring that is currently in progress in balances.ts.