OPDATA-7578: Refactor Solana exchange-rate shared utilities#5128
OPDATA-7578: Refactor Solana exchange-rate shared utilities#5128magiodev-cll wants to merge 13 commits into
Conversation
🦋 Changeset detectedLatest commit: fb3738a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Thank you @magiodev-cll for the refactor! A few gaps to close before #5095/#5097 stack on this, mostly in the on-chain access 1. Extract Both endpoint PRs hand-roll PDA derivation (#5097 has 5 near-identical 2. Extract a shared Clock-sysvar reader. The vesting math ( 3. Fold in the remaining duplicated helpers.
4. Manual byte readers
5. Token-2022 + the exact-span Adding **6. the hand-rolled |
Addressed in For point 5: removed Token-2022 from the exact-span For point 6: removed the hand-rolled Also added the shared access-layer pieces from the earlier points: |
|
Code Review: PR #5128 — "refactor: expand Solana account helpers" Verdict: APPROVE — Focused, clean refactoring. Stronger typing from SDK derivation, address validation added to the RPC path, clock sysvar decoding centralised. Two minor points worth noting before merge. What Changed (actual + lines)
Findings buffer-layout-accounts.ts:80-84: [LOW] yagni: assertTokenProgramOwner wraps assertOwnerProgram with two args baked in — one caller in this PR. Inline at the call site; export when a second caller exists. solana-account-utils.ts:7-8: [LOW] correctness: CLOCK_ACCOUNT_LENGTH = 40 and CLOCK_UNIX_TIMESTAMP_OFFSET = 32 are correct but unattributed to the Solana clock sysvar layout. A comment or link makes decodeClockUnixTimestamp self-auditable without knowing the spec. |
There was a problem hiding this comment.
Pull request overview
This PR introduces shared Solana exchange-rate and account utility helpers intended to be reused by upcoming Solana exchange-rate endpoints (e.g., stSLX and strcUSX), along with focused unit tests to validate the new shared behavior.
Changes:
- Added shared exchange-rate helpers (rate bounds parsing/clamping, 18-decimal normalization, vesting/unvested asset math).
- Added shared Solana account helpers (address validation, PDA derivation, base64 account buffer decoding, common account assertions, clock sysvar timestamp decode, batched multi-account reads).
- Extended buffer-layout account utilities with SPL mint + token-account decode helpers and token program owner validation, plus unit coverage.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/sources/solana-functions/src/shared/solana-account-utils.ts | New shared Solana account validation/decoding/assertion helpers and multi-account fetch. |
| packages/sources/solana-functions/src/shared/exchange-rate-utils.ts | New shared bigint-based exchange-rate helpers (bounds, normalization, vesting math). |
| packages/sources/solana-functions/src/shared/buffer-layout-accounts.ts | Adds SPL mint/token-account decode helpers and token-program owner validation alongside existing buffer-layout fetching. |
| packages/sources/solana-functions/src/shared/account-reader.ts | Refactors PDA derivation to reuse the new shared PDA helper and tightens seed typing. |
| packages/sources/solana-functions/test/unit/solana-account-utils.test.ts | Unit coverage for the new shared Solana account helpers. |
| packages/sources/solana-functions/test/unit/exchange-rate-utils.test.ts | Unit coverage for the new exchange-rate helpers. |
| packages/sources/solana-functions/test/unit/buffer-layout-accounts.test.ts | Extends unit coverage for new SPL decode + token program owner helpers. |
| packages/sources/solana-functions/test/unit/account-reader.test.ts | Updates mocks/expectations to align with refactored PDA derivation behavior. |
| .changeset/quiet-ducks-compare.md | Patch changeset for the solana-functions adapter to ship the shared utilities. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
magiodev-cll
changed the title
| throw new AdapterInputError({ | ||
| message: `${name} must be a positive base-10 integer string`, | ||
| statusCode: 400, | ||
| }) |
There was a problem hiding this comment.
Please consider validation using customInputValidation, example
There was a problem hiding this comment.
@magiodev-cll I think this one was missed, please let me know if it's unclear.
| export const derivePda = async (programAddress: string, seeds: PdaSeed[]) => { | ||
| const [pda] = await getProgramDerivedAddress({ | ||
| programAddress: parseSolanaAddress(programAddress, 'programAddress'), | ||
| seeds, | ||
| }) | ||
|
|
||
| return pda | ||
| } |
There was a problem hiding this comment.
Now that derivePda lives here (and PdaSeed is exported just above), account-reader.ts should consume it rather than keeping its own derivation path. Today SolanaAccountReader.fetchAccountInformationByAddressAndSeeds still does:
async fetchAccountInformationByAddressAndSeeds<T>(
rpc, programAddress: Address, seeds: any[], accountName, idl,
): Promise<T> {
const [pda] = await getProgramDerivedAddress({ programAddress, seeds })
}which is the same getProgramDerivedAddress call derivePda wraps. So after this PR there are two derivation paths in the EA: the new shared one (used by the strcUSX/stSLX transports) and this inline one (used by eusx-price/anchor-data via the reader).
Two asks:
- Drop the any. The seeds: any[] has a stale comment ("typed as any due to type not being exported by @solana/addresses"), but that's now obsolete since this PR exports PdaSeed. So seeds: any[] -> seeds: PdaSeed[] removes an any with zero behavioral change.
- Route the reader through
derivePdaso there's a single derivation function(const pda = await derivePda(programAddress, seeds)). One impedance note:derivePdatakes a string and re-validates viaparseSolanaAddress, while the reader already has a typed Address, passing the Address through works (it's a branded string, re-validation is a no-op), or derivePda could accept Address | string.
| export const assertNameMatches = ( | ||
| actualName: string, | ||
| expectedName: string, | ||
| description: string, | ||
| ) => { | ||
| if (actualName !== expectedName) { | ||
| throw new AdapterInputError({ | ||
| message: `Expected ${description} name to be '${expectedName}', found '${actualName}'`, | ||
| statusCode: 500, | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| export const assertAddressMatches = (actual: string, expected: string, description: string) => { | ||
| if (actual !== expected) { | ||
| throw new AdapterInputError({ | ||
| message: `Expected ${description} to be '${expected}', found '${actual}'`, | ||
| statusCode: 500, | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Remove dead exports.** assertNameMatches and assertAddressMatches are no longer used by any endpoint (strcUSX dropped the redundant equality checks). Remove them and their unit tests.
| providerIndicatedTimeUnixMs: undefined, | ||
| }, | ||
| ) | ||
|
|
There was a problem hiding this comment.
Relocate providerError. It currently lives in exchange-rate-utils.ts but it's an error/account concern, not rate math. Move it into solana-account-utils.ts (or a small errors.ts) so exchange-rate-utils.ts is purely rate math + bounds.
| export const getAccountDataBuffer = ( | ||
| accountInfo: AccountInfo | null | undefined, | ||
| description: string, | ||
| ) => { | ||
| const encodedData = accountInfo?.data?.[0] | ||
| if (typeof encodedData !== 'string' || encodedData.length === 0) { | ||
| throw new AdapterInputError({ | ||
| message: `No account data found for ${description}`, | ||
| statusCode: 500, | ||
| }) | ||
| } | ||
|
|
||
| return Buffer.from(encodedData, 'base64') | ||
| } | ||
|
|
||
| export const assertOwnerProgram = ( | ||
| accountInfo: AccountInfo | null | undefined, | ||
| description: string, | ||
| expectedOwners: string[], | ||
| ownerDescription: string, | ||
| ) => { | ||
| const owner = accountInfo?.owner?.toString() | ||
| if (!owner || !expectedOwners.includes(owner)) { | ||
| throw new AdapterInputError({ | ||
| message: `Expected ${description} to be owned by ${ownerDescription} [${expectedOwners.join( | ||
| ', ', | ||
| )}], found '${owner}'`, | ||
| statusCode: 500, | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| export const assertNameMatches = ( | ||
| actualName: string, | ||
| expectedName: string, | ||
| description: string, | ||
| ) => { | ||
| if (actualName !== expectedName) { | ||
| throw new AdapterInputError({ | ||
| message: `Expected ${description} name to be '${expectedName}', found '${actualName}'`, | ||
| statusCode: 500, | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| export const assertAddressMatches = (actual: string, expected: string, description: string) => { | ||
| if (actual !== expected) { | ||
| throw new AdapterInputError({ | ||
| message: `Expected ${description} to be '${expected}', found '${actual}'`, | ||
| statusCode: 500, | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix the error taxonomy in solana-account-utils.ts. The account/decode helpers throw AdapterInputError with statusCode: 500, but these are upstream/data failures, not input errors. They should throw AdapterDataProviderError (502) via the providerError factory:
getAccountDataBufferassertOwnerProgramassertDataLengthassertDiscriminatorfetchMultipleAccounts
KeepparseSolanaAddressonAdapterInputError400 since that's genuine input validation. This fix lets the stSLX PR delete itsasProviderErrorwrapper and makes both endpoints emit consistent 502s for the same failures (today strcUSX emits 500 for these).
Summary
Context
This is the shared-components base PR requested in review for #5095 and #5097. It intentionally adds no endpoint; the stSLX and strcUSX endpoint PRs will be stacked on top and refactored to reuse these helpers.
Testing