feat: Balance Sheet #424
feat: Balance Sheet #424
Conversation
WalkthroughWalkthroughThe recent updates to the Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant FederationAdmin
participant BalanceCard
participant BalanceTable
User->>FederationAdmin: Access Federation Admin page
FederationAdmin->>BalanceCard: Render BalanceCard
BalanceCard->>BalanceTable: Pass balance data and selected unit
BalanceTable-->>BalanceCard: Display formatted balance information
BalanceCard-->>FederationAdmin: Display updated BalanceCard
FederationAdmin-->>User: Show updated Federation Admin page
sequenceDiagram
participant Developer
participant TypeModule
participant FormatUtil
Developer->>TypeModule: Add `Unknown` to `ModuleKind` enum
TypeModule-->>Developer: Updated enum and interface
Developer->>FormatUtil: Add `formatValue` function
FormatUtil-->>Developer: Updated utility functions
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (7)
Files skipped from review as they are similar to previous changes (1)
Additional Context UsedBiome (13)
Additional comments not posted (2)
TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Kodylow
force-pushed
the
assets-liabilities
branch
from
3ccbd07
to
a36dbe9
Compare
|
Added a filter to now put the "unknown" module in the balance sheet, that's just there to test if it'll break clients and it doesn't have any assets/liabilities |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
packages/types/src/modules.ts
Outdated
| [ModuleKind.Meta]: { | ||
| meta_consensus: { | ||
| meta_consensus_version: number; | ||
| }; | ||
| }; | ||
| [ModuleKind.Unknown]: { | ||
| unknown_consensus: { | ||
| unknown_consensus_version: number; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Tip
Codebase Verification
Ensure proper integration of ModuleKind.Meta throughout the application.
ModuleKind.Metais defined inpackages/types/src/modules.tsbut not used elsewhere in the codebase.- Unlike
ModuleKind.Unknown, which is handled inBalanceTable.tsx,ModuleKind.Metalacks integration.
Analysis chain
The addition of configurations for Meta and Unknown module kinds is approved. Ensure these new module kinds are integrated properly throughout the application.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the integration of new module kinds `Meta` and `Unknown`.
# Test: Search for the usage of `ModuleKind.Meta` and `ModuleKind.Unknown`. Expect: Proper handling in the application logic.
rg --type typescript $'ModuleKind.Meta|ModuleKind.Unknown'
Length of output: 94
Script:
#!/bin/bash
# Description: Verify the integration of new module kinds `Meta` and `Unknown`.
# Test: Search for the usage of `ModuleKind.Meta` and `ModuleKind.Unknown`. Expect: Proper handling in the application logic.
rg --type ts $'ModuleKind.Meta|ModuleKind.Unknown'
Length of output: 264
Script:
#!/bin/bash
# Description: Extract relevant lines from `BalanceTable.tsx` to verify the handling of `ModuleKind.Meta` and `ModuleKind.Unknown`.
# Extract lines containing `ModuleKind.Meta` and `ModuleKind.Unknown` and their surrounding context.
rg --type ts --context 5 $'ModuleKind.Meta|ModuleKind.Unknown' apps/guardian-ui/src/components/BalanceTable.tsx
Length of output: 512
Script:
#!/bin/bash
# Description: Verify the integration of `ModuleKind.Meta` across the entire codebase.
# Search for the usage of `ModuleKind.Meta` in the entire codebase.
rg --type ts $'ModuleKind.Meta'
Length of output: 83
|
Missed your last comment. |
| const moduleSummaries = Object.entries(auditSummary.module_summaries).filter( | ||
| ([, module]) => module.kind !== ModuleKind.Unknown | ||
| ); |
There was a problem hiding this comment.
Should we filter the unknown module? It's confusing, but only there in devimint afaik.
| const totalLiabilities = moduleSummaries.reduce((sum, [, module]) => { | ||
| return sum + (module.net_assets < 0 ? module.net_assets : 0); | ||
| }, 0) as MSats; | ||
| const isBalanced = totalAssets + totalLiabilities === 0; |
There was a problem hiding this comment.
This will be false for any federation charging fees. I think as long as assets > liabilities we should say it's balanced.
There was a problem hiding this comment.
We should probably have a way of specifically separating out the fees either into an "equity" column or a way to get the specific amounts collected in fees from the different modules so that we can still display that the outstanding liabilities match the outstanding reserves exactly.
There was a problem hiding this comment.
Modules don't collect fees themselves and we don't record fees anywhere. All that happens is that the federation slowly becomes overfunded. We discussed different approaches, but this didn't seem like a hug problem so far. If you want you can just display any overfunding as a fees entry on the liabilities side.
|
Okay added an Equity (Guardian Fees) column and adjusted the check to "Is Full Reserve" checking that assets > liabilities and equity is > 0 
|
There was a problem hiding this comment.
Actionable comments posted: 6
Outside diff range and nitpick comments (1)
apps/guardian-ui/src/admin/FederationAdmin.tsx (1)
Line range hint
31-31: Replace global parseInt with Number.parseInt for better clarity and consistency in the codebase.- const peerIds = Object.keys(status.federation.status_by_peer).map((id) => - parseInt(id, 10) - ); - const configPeerIds = Object.keys(config.api_endpoints).map((id) => - parseInt(id, 10) - ); + const peerIds = Object.keys(status.federation.status_by_peer).map((id) => Number.parseInt(id, 10)); + const configPeerIds = Object.keys(config.api_endpoints).map((id) => Number.parseInt(id, 10));Also applies to: 34-34
| <Card w='60%'> | ||
| <CardHeader> | ||
| <Text size='lg' fontWeight='600'> | ||
| {t('federation-dashboard.balance.label')} | ||
| </Text> | ||
| <Flex justifyContent='space-between' alignItems='center'> | ||
| <Text size='lg' fontWeight='600'> | ||
| {t('federation-dashboard.balance.label')} | ||
| </Text> | ||
| <ButtonGroup size='xs' borderRadius='md'> | ||
| <Button | ||
| onClick={() => setUnit('msats')} | ||
| variant={unit === 'msats' ? 'solid' : 'ghost'} | ||
| > | ||
| msats | ||
| </Button> | ||
| <Button | ||
| onClick={() => setUnit('sats')} | ||
| variant={unit === 'sats' ? 'solid' : 'ghost'} | ||
| > | ||
| sats | ||
| </Button> | ||
| <Button | ||
| onClick={() => setUnit('btc')} | ||
| variant={unit === 'btc' ? 'solid' : 'ghost'} | ||
| > | ||
| BTC | ||
| </Button> | ||
| </ButtonGroup> | ||
| </Flex> | ||
| </CardHeader> | ||
| <CardBody> | ||
| <KeyValues direction='row' keyValues={keyValues} /> | ||
| {auditSummary ? ( | ||
| <BalanceTable auditSummary={auditSummary} unit={unit} /> | ||
| ) : ( | ||
| <Text>Loading...</Text> | ||
| )} |
There was a problem hiding this comment.
The layout and unit switching functionality are well-implemented. However, consider adding a loading indicator or skeleton screen for a better user experience during data fetch.
- <Text>Loading...</Text>
+ <Skeleton height='20px' width='100%' />Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| <Card w='60%'> | |
| <CardHeader> | |
| <Text size='lg' fontWeight='600'> | |
| {t('federation-dashboard.balance.label')} | |
| </Text> | |
| <Flex justifyContent='space-between' alignItems='center'> | |
| <Text size='lg' fontWeight='600'> | |
| {t('federation-dashboard.balance.label')} | |
| </Text> | |
| <ButtonGroup size='xs' borderRadius='md'> | |
| <Button | |
| onClick={() => setUnit('msats')} | |
| variant={unit === 'msats' ? 'solid' : 'ghost'} | |
| > | |
| msats | |
| </Button> | |
| <Button | |
| onClick={() => setUnit('sats')} | |
| variant={unit === 'sats' ? 'solid' : 'ghost'} | |
| > | |
| sats | |
| </Button> | |
| <Button | |
| onClick={() => setUnit('btc')} | |
| variant={unit === 'btc' ? 'solid' : 'ghost'} | |
| > | |
| BTC | |
| </Button> | |
| </ButtonGroup> | |
| </Flex> | |
| </CardHeader> | |
| <CardBody> | |
| <KeyValues direction='row' keyValues={keyValues} /> | |
| {auditSummary ? ( | |
| <BalanceTable auditSummary={auditSummary} unit={unit} /> | |
| ) : ( | |
| <Text>Loading...</Text> | |
| )} | |
| <Card w='60%'> | |
| <CardHeader> | |
| <Flex justifyContent='space-between' alignItems='center'> | |
| <Text size='lg' fontWeight='600'> | |
| {t('federation-dashboard.balance.label')} | |
| </Text> | |
| <ButtonGroup size='xs' borderRadius='md'> | |
| <Button | |
| onClick={() => setUnit('msats')} | |
| variant={unit === 'msats' ? 'solid' : 'ghost'} | |
| > | |
| msats | |
| </Button> | |
| <Button | |
| onClick={() => setUnit('sats')} | |
| variant={unit === 'sats' ? 'solid' : 'ghost'} | |
| > | |
| sats | |
| </Button> | |
| <Button | |
| onClick={() => setUnit('btc')} | |
| variant={unit === 'btc' ? 'solid' : 'ghost'} | |
| > | |
| BTC | |
| </Button> | |
| </ButtonGroup> | |
| </Flex> | |
| </CardHeader> | |
| <CardBody> | |
| {auditSummary ? ( | |
| <BalanceTable auditSummary={auditSummary} unit={unit} /> | |
| ) : ( | |
| <Skeleton height='20px' width='100%' /> | |
| )} |
| @@ -0,0 +1,90 @@ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
Consider using type-only imports for React to clarify that it's only used for type annotations.
- import React from 'react';
+ import type { FC } from 'react';
+ import React from 'react';Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| import React from 'react'; | |
| import type { FC } from 'react'; | |
| import React from 'react'; |
| {isBalanced ? ( | ||
| <Icon as={CheckIcon} color='green.500' /> | ||
| ) : ( | ||
| <Icon as={CloseIcon} color='red.500' /> | ||
| )} |
There was a problem hiding this comment.
Tip
Codebase Verification
The logic for determining the balance status (isBalanced) in BalanceTable.tsx is not currently covered by any tests. To ensure the reliability and correctness of this logic, it is recommended to add thorough tests for the balance status determination.
- Location of logic:
apps/guardian-ui/src/components/BalanceTable.tsx - Logic definition:
const isBalanced = totalAssets - totalLiabilities + equity === 0 && equity >= 0;
Analysis chain
The visual indicators for balance status are a good UX addition. Ensure that the logic for determining balance status is thoroughly tested.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the logic for determining balance status.
# Test: Search for balance status logic. Expect: Thorough testing.
rg --type typescript $'isBalanced'
Length of output: 68
Script:
#!/bin/bash
# Description: Verify the logic for determining balance status.
# Test: Search for balance status logic. Expect: Thorough testing.
rg --type ts 'isBalanced'
Length of output: 174
Script:
#!/bin/bash
# Description: Extract the definition of `isBalanced` and search for related tests.
# Extract the definition of `isBalanced`
rg --type ts --context 5 'const isBalanced =' apps/guardian-ui/src/components/BalanceTable.tsx
# Search for related tests
rg --type ts 'isBalanced' tests/
Length of output: 578
Script:
#!/bin/bash
# Description: Search for test files related to `BalanceTable.tsx` and check for tests involving `isBalanced`.
# Find test files related to `BalanceTable.tsx`
rg --files | grep -i 'balance.*test'
# Check for tests involving `isBalanced`
rg --type ts 'isBalanced' $(rg --files | grep -i 'balance.*test')
Length of output: 254
| import React from 'react'; | ||
| import { Table, Tbody, Td, Th, Thead, Tr, Icon, Flex } from '@chakra-ui/react'; | ||
| import { ReactComponent as CheckIcon } from '../assets/svgs/check-circle.svg'; | ||
| import { ReactComponent as CloseIcon } from '../assets/svgs/x-circle.svg'; | ||
| import { AuditSummary, ModuleKind, MSats } from '@fedimint/types'; | ||
| import { formatMsatsToBtc } from '@fedimint/utils'; | ||
|
|
||
| interface BalanceTableProps { | ||
| auditSummary: AuditSummary; | ||
| unit: 'msats' | 'sats' | 'btc'; | ||
| } | ||
|
|
||
| export const BalanceTable: React.FC<BalanceTableProps> = ({ | ||
| auditSummary, | ||
| unit, | ||
| }) => { | ||
| const moduleSummaries = Object.entries(auditSummary.module_summaries).filter( | ||
| ([, module]) => module.kind !== ModuleKind.Unknown | ||
| ); | ||
|
|
||
| const totalAssets = moduleSummaries.reduce((sum, [, module]) => { | ||
| return sum + (module.net_assets > 0 ? module.net_assets : 0); | ||
| }, 0) as MSats; | ||
| const totalLiabilities = moduleSummaries.reduce((sum, [, module]) => { | ||
| return sum + Math.abs(module.net_assets < 0 ? module.net_assets : 0); | ||
| }, 0) as MSats; | ||
| const equity = totalAssets - totalLiabilities; | ||
| const isBalanced = | ||
| totalAssets - totalLiabilities + equity === 0 && equity >= 0; | ||
|
|
||
| const formatValue = (value: MSats) => { | ||
| switch (unit) { | ||
| case 'msats': | ||
| return value.toString(); | ||
| case 'sats': | ||
| return (value / 1000).toFixed(0); | ||
| case 'btc': | ||
| return formatMsatsToBtc(value); | ||
| default: | ||
| return value.toString(); | ||
| } | ||
| }; | ||
|
|
||
| return ( | ||
| <Table variant='simple'> | ||
| <Thead> | ||
| <Tr> | ||
| <Th>Module</Th> | ||
| <Th isNumeric>Assets</Th> | ||
| <Th isNumeric>Liabilities</Th> | ||
| <Th isNumeric>Equity (Guardian Fees)</Th> | ||
| <Th>Is Full Reserve</Th> {/* Updated column header */} | ||
| </Tr> | ||
| </Thead> | ||
| <Tbody> | ||
| {moduleSummaries.map(([key, module]) => ( | ||
| <Tr key={key}> | ||
| <Td>{module.kind}</Td> | ||
| <Td isNumeric> | ||
| {module.net_assets > 0 ? formatValue(module.net_assets) : '0'} | ||
| </Td> | ||
| <Td isNumeric> | ||
| {module.net_assets < 0 | ||
| ? formatValue(Math.abs(module.net_assets) as MSats) | ||
| : '0'} | ||
| </Td> | ||
| <Td isNumeric></Td> {/* Set equity to 0 for individual rows */} | ||
| <Td /> {/* Empty cell for alignment */} | ||
| </Tr> | ||
| ))} | ||
| <Tr fontWeight='bold'> | ||
| <Td>Total</Td> | ||
| <Td isNumeric>{formatValue(totalAssets)}</Td> | ||
| <Td isNumeric>{formatValue(Math.abs(totalLiabilities) as MSats)}</Td> | ||
| <Td isNumeric>{formatValue(equity as MSats)}</Td>{' '} | ||
| {/* Display total equity */} | ||
| <Td> | ||
| <Flex justifyContent='center' alignItems='center'> | ||
| {isBalanced ? ( | ||
| <Icon as={CheckIcon} color='green.500' /> | ||
| ) : ( | ||
| <Icon as={CloseIcon} color='red.500' /> | ||
| )} | ||
| </Flex> | ||
| </Td> | ||
| </Tr> | ||
| </Tbody> | ||
| </Table> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
The BalanceTable component is well-implemented with clear handling of assets, liabilities, and equity calculations. It correctly filters out the Unknown module and supports multiple units for value display.
Minor suggestion: Use self-closing tags for empty JSX elements to improve readability.
- <Td></Td> {/* Empty cell for alignment */}
+ <Td /> {/* Empty cell for alignment */}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| import React from 'react'; | |
| import { Table, Tbody, Td, Th, Thead, Tr, Icon, Flex } from '@chakra-ui/react'; | |
| import { ReactComponent as CheckIcon } from '../assets/svgs/check-circle.svg'; | |
| import { ReactComponent as CloseIcon } from '../assets/svgs/x-circle.svg'; | |
| import { AuditSummary, ModuleKind, MSats } from '@fedimint/types'; | |
| import { formatMsatsToBtc } from '@fedimint/utils'; | |
| interface BalanceTableProps { | |
| auditSummary: AuditSummary; | |
| unit: 'msats' | 'sats' | 'btc'; | |
| } | |
| export const BalanceTable: React.FC<BalanceTableProps> = ({ | |
| auditSummary, | |
| unit, | |
| }) => { | |
| const moduleSummaries = Object.entries(auditSummary.module_summaries).filter( | |
| ([, module]) => module.kind !== ModuleKind.Unknown | |
| ); | |
| const totalAssets = moduleSummaries.reduce((sum, [, module]) => { | |
| return sum + (module.net_assets > 0 ? module.net_assets : 0); | |
| }, 0) as MSats; | |
| const totalLiabilities = moduleSummaries.reduce((sum, [, module]) => { | |
| return sum + Math.abs(module.net_assets < 0 ? module.net_assets : 0); | |
| }, 0) as MSats; | |
| const equity = totalAssets - totalLiabilities; | |
| const isBalanced = | |
| totalAssets - totalLiabilities + equity === 0 && equity >= 0; | |
| const formatValue = (value: MSats) => { | |
| switch (unit) { | |
| case 'msats': | |
| return value.toString(); | |
| case 'sats': | |
| return (value / 1000).toFixed(0); | |
| case 'btc': | |
| return formatMsatsToBtc(value); | |
| default: | |
| return value.toString(); | |
| } | |
| }; | |
| return ( | |
| <Table variant='simple'> | |
| <Thead> | |
| <Tr> | |
| <Th>Module</Th> | |
| <Th isNumeric>Assets</Th> | |
| <Th isNumeric>Liabilities</Th> | |
| <Th isNumeric>Equity (Guardian Fees)</Th> | |
| <Th>Is Full Reserve</Th> {/* Updated column header */} | |
| </Tr> | |
| </Thead> | |
| <Tbody> | |
| {moduleSummaries.map(([key, module]) => ( | |
| <Tr key={key}> | |
| <Td>{module.kind}</Td> | |
| <Td isNumeric> | |
| {module.net_assets > 0 ? formatValue(module.net_assets) : '0'} | |
| </Td> | |
| <Td isNumeric> | |
| {module.net_assets < 0 | |
| ? formatValue(Math.abs(module.net_assets) as MSats) | |
| : '0'} | |
| </Td> | |
| <Td isNumeric></Td> {/* Set equity to 0 for individual rows */} | |
| <Td /> {/* Empty cell for alignment */} | |
| </Tr> | |
| ))} | |
| <Tr fontWeight='bold'> | |
| <Td>Total</Td> | |
| <Td isNumeric>{formatValue(totalAssets)}</Td> | |
| <Td isNumeric>{formatValue(Math.abs(totalLiabilities) as MSats)}</Td> | |
| <Td isNumeric>{formatValue(equity as MSats)}</Td>{' '} | |
| {/* Display total equity */} | |
| <Td> | |
| <Flex justifyContent='center' alignItems='center'> | |
| {isBalanced ? ( | |
| <Icon as={CheckIcon} color='green.500' /> | |
| ) : ( | |
| <Icon as={CloseIcon} color='red.500' /> | |
| )} | |
| </Flex> | |
| </Td> | |
| </Tr> | |
| </Tbody> | |
| </Table> | |
| ); | |
| }; | |
| {moduleSummaries.map(([key, module]) => ( | |
| <Tr key={key}> | |
| <Td>{module.kind}</Td> | |
| <Td isNumeric> | |
| {module.net_assets > 0 ? formatValue(module.net_assets) : '0'} | |
| </Td> | |
| <Td isNumeric> | |
| {module.net_assets < 0 | |
| ? formatValue(Math.abs(module.net_assets) as MSats) | |
| : '0'} | |
| </Td> | |
| <Td isNumeric></Td> {/* Set equity to 0 for individual rows */} | |
| <Td /> {/* Empty cell for alignment */} | |
| </Tr> | |
| ))} |
Kodylow
force-pushed
the
assets-liabilities
branch
from
ed051b9
to
4104d0a
Compare
Kodylow
force-pushed
the
assets-liabilities
branch
from
4104d0a
to
e49342e
Compare
|
Actually that took up a lot of real estate, moved into separate cards for Full reserve and fees calculations, if there's no fees it just doesn't show that card: 
|
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range and nitpick comments (1)
apps/guardian-ui/src/admin/FederationAdmin.tsx (1)
Line range hint
31-31: UseNumber.parseIntfor parsing integers.For better clarity and consistency, use
Number.parseIntinstead of the globalparseInt.- const peerIds = Object.keys(status.federation.status_by_peer).map((id) => parseInt(id, 10)); + const peerIds = Object.keys(status.federation.status_by_peer).map((id) => Number.parseInt(id, 10)); - const configPeerIds = Object.keys(config.api_endpoints).map((id) => parseInt(id, 10)); + const configPeerIds = Object.keys(config.api_endpoints).map((id) => Number.parseInt(id, 10));Also applies to: 34-34
| @@ -5,6 +5,7 @@ export enum ModuleKind { | |||
| Mint = 'mint', | |||
| Wallet = 'wallet', | |||
| Meta = 'meta', | |||
| Unknown = 'unknown', | |||
There was a problem hiding this comment.
I worry a bit about this enum, does this mean we will encounter errors when encountering other modules than the ones listed here?
There was a problem hiding this comment.
Going to land this and open as a separate issue for unknown module handling
Adds a balance sheet view of all modules.
Summary by CodeRabbit
New Features
BalanceCardcomponent to switch betweenmsats,sats, andbtc.BalanceTablecomponent to display balance information based on the selected unit.Enhancements
FederationAdminandFederationInfoCardcomponents for improved visual presentation.Bug Fixes
BalanceTableto ensure accurate representation of total equity.Documentation
formatValuefunction to handle unit conversions.Refactor
BalanceCardcomponent for better performance and readability.