Update transaction account keys size display#1085
Conversation
|
Someone is attempting to deploy a commit to the Solana Foundation Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThis PR fixes the transaction Accounts footer to display the static size of account keys included directly in the transaction (non-LUT keys × 32 bytes), replacing the previous behaviour that summed live on-chain data sizes fetched asynchronously.
Confidence Score: 5/5Safe to merge — the change is a straightforward, well-tested replacement of async on-chain data summation with a deterministic static calculation. The new calculation is pure and synchronous, the !loading guard removal is correct since the value no longer depends on async RPC data, and all meaningful input variants are covered by the new tests. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[AccountsCard renders] --> B{transactionWithMeta?}
B -- no --> C[return null]
B -- yes --> D{meta && message?}
D -- no --> E[ErrorCard: metadata missing]
D -- yes --> F[getTransactionAccountKeysSizeBytes\nmessage.accountKeys]
F --> G[filter: source !== 'lookupTable']
G --> H[count × 32 bytes = totalAccountKeysSize]
H --> I{totalAccountKeysSize > 0?}
I -- yes --> J[Render footer: Total Account Keys Size]
I -- no --> K[Footer hidden]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[AccountsCard renders] --> B{transactionWithMeta?}
B -- no --> C[return null]
B -- yes --> D{meta && message?}
D -- no --> E[ErrorCard: metadata missing]
D -- yes --> F[getTransactionAccountKeysSizeBytes\nmessage.accountKeys]
F --> G[filter: source !== 'lookupTable']
G --> H[count × 32 bytes = totalAccountKeysSize]
H --> I{totalAccountKeysSize > 0?}
I -- yes --> J[Render footer: Total Account Keys Size]
I -- no --> K[Footer hidden]
Reviews (3): Last reviewed commit: "Merge remote-tracking branch 'upstream/m..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| {!loading && totalAccountKeysSize > 0 && ( | ||
| <div className="e-ml-7 e-flex e-items-baseline e-gap-2 e-px-3 e-py-2 e-text-sm e-text-outer-space-300 md:e-px-4 lg:e-ml-10"> | ||
| <div className="e-flex e-flex-col"> | ||
| <span className="e-text-sm e-uppercase e-leading-none">Total Account Size:</span> | ||
| <span className="e-text-[10px] e-leading-none">reflects current state</span> | ||
| <span className="e-text-sm e-uppercase e-leading-none">Total Account Keys Size:</span> | ||
| <span className="e-text-[10px] e-leading-none">excludes address lookup tables</span> | ||
| </div> | ||
| <span className="e-text-white">{totalAccountSize.toLocaleString('en-US')} bytes</span> | ||
| <span className="e-text-white">{totalAccountKeysSize.toLocaleString('en-US')} bytes</span> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
!loading guard is now stale
totalAccountKeysSize is derived entirely from message.accountKeys, which is already in memory when the component renders. It no longer depends on the async useAccountsInfo call. Keeping the !loading condition means the footer is hidden until all on-chain account data finishes loading — even though the size value is ready immediately. Any transaction with slow RPC responses (or a long account list) will show a blank footer longer than necessary. Dropping the !loading check here, or replacing it with !!message, would make the footer appear as soon as the transaction message is available.
| import { describe, expect, test } from 'vitest'; | ||
|
|
||
| import { getTransactionAccountKeysSizeBytes } from '../AccountsCard'; | ||
|
|
||
| describe('getTransactionAccountKeysSizeBytes', () => { | ||
| test('should count transaction account keys as 32 bytes each', () => { | ||
| expect( | ||
| getTransactionAccountKeysSizeBytes([ | ||
| { source: 'transaction' }, | ||
| { source: 'transaction' }, | ||
| { source: 'transaction' }, | ||
| ]), | ||
| ).toBe(96); | ||
| }); | ||
|
|
||
| test('should exclude account keys loaded from address lookup tables', () => { | ||
| expect( | ||
| getTransactionAccountKeysSizeBytes([ | ||
| { source: 'transaction' }, | ||
| { source: 'lookupTable' }, | ||
| { source: 'transaction' }, | ||
| { source: 'lookupTable' }, | ||
| ]), | ||
| ).toBe(64); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
source: undefined
The function signature accepts Array<{ source?: string }>, meaning source can be undefined. The current tests only cover 'transaction' and 'lookupTable' values. An account with source: undefined is treated as non-LUT (correctly counted) by the current implementation, but this behavior isn't covered by any test. Adding a case like [{ source: 'transaction' }, {}, { source: 'lookupTable' }] would pin the intended behaviour and guard against regressions if the filter condition is ever tightened.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
@Samisha68 can you add the before and after screenshots showing how the changes you've made would render? |
Before: shows the total current account data size. 
After: shows the total transaction account keys size, excluding address lookup table accounts. 
|
…total-account-size-in-transaction-accounts-preview # Conflicts: # app/features/transaction/ui/AccountsCard.tsx
|
Hey why do you want this change? What is the usecase for having transaction data without the ALTs? |
| <span className="text-white">{totalAccountSize.toLocaleString('en-US')} bytes</span> | ||
| <span className="text-white">{totalAccountKeysSize.toLocaleString('en-US')} bytes</span> | ||
| </div> | ||
| )} |
The goal is to show the static account-key footprint in the transaction message, rather than the current on-chain data size of those accounts. |
…total-account-size-in-transaction-accounts-preview # Conflicts: # app/features/transaction/ui/AccountsCard.tsx
5 participants
Description
Updates the transaction Accounts preview to show the total size of account keys included directly in the transaction.
Previously, the footer summed the current on-chain data sizes of fetched accounts, which is not useful for estimating transaction account key size. The new calculation counts non-lookup-table account keys and multiplies by 32 bytes.
Type of change
Testing
pnpm lintpnpm vitest:test related --run app/features/transaction/ui/__tests__/AccountsCard.test.tspnpm pretty:format app/features/transaction/ui/AccountsCard.tsx app/features/transaction/ui/__tests__/AccountsCard.test.tsNote:
pnpm testcurrently fails locally in existingtest-setup.tswith a recursiveBuffer.isBufferstack overflow.pnpm typecheckdid not complete because itsnext buildpre-step hung locally.Related Issues
Fixes #882
Checklist
pnpm test,pnpm lint,pnpm typecheck)build:infoscript to update build informationAdditional Notes
This change is limited to transaction account size display logic and does not introduce new dependencies.