feat(xc_admin_frontend): add Lazer integration #2722
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
7 Skipped Deployments
|
cctdaniel
changed the title
| if ( | ||
| !("shardId" in uploadedLazerState) || | ||
| !("feeds" in uploadedLazerState) || | ||
| !("publishers" in uploadedLazerState) | ||
| ) { | ||
| return { | ||
| isValid: false, | ||
| error: "Missing required fields in Lazer configuration", | ||
| }; |
There was a problem hiding this comment.
Is this not guaranteed by the type?
In general I recommend using something like zod to describe and validate schemas in typescript; if this can't be guaranteed by the type system I'd use Zod to parse instead.
| } | ||
|
|
||
| // Calculate changes between existing and uploaded config | ||
| const changes: Record<string, { prev?: any; new?: any }> = {}; |
There was a problem hiding this comment.
Please do not use any -- you can restructure this type slightly by using well known keys to be easier to type (and probably easier to use) with something along these lines:
type ChangeField<T> = { prev: T, new: T };
type Changes = {
shard: ChangeField<Shard>;
feeds: Record<string, ChangeField<Feed>>;
publishers: Record<string, ChangeField<Publisher>>;
}| export type LazerFeedMetadata = { | ||
| priceFeedId: number; | ||
| name: string; | ||
| symbol: string; | ||
| description: string; | ||
| assetType: string; | ||
| exponent: number; | ||
| minPublishers: number; | ||
| minRate: string; | ||
| expiryTime: string; | ||
| isActivated: boolean; | ||
| hermesId?: string; | ||
| cmcId?: number; | ||
| fundingRateInterval?: string; | ||
| quoteCurrency?: string; | ||
| marketSchedule: string; | ||
| }; | ||
|
|
||
| /** | ||
| * Lazer feed type | ||
| */ | ||
| export type LazerFeed = { | ||
| metadata: LazerFeedMetadata; | ||
| pendingActivation?: string; | ||
| }; | ||
|
|
||
| /** | ||
| * Lazer publisher type | ||
| */ | ||
| export type LazerPublisher = { | ||
| publisherId: number; | ||
| name: string; | ||
| publicKeys: string[]; | ||
| isActive: boolean; | ||
| }; | ||
|
|
||
| /** | ||
| * Full Lazer state type | ||
| */ | ||
| export type LazerState = { | ||
| shardId: number; | ||
| lastSequenceNo: string; | ||
| lastTimestamp: string; | ||
| shardName: string; | ||
| minRate: string; | ||
| feeds: LazerFeed[]; | ||
| publishers: LazerPublisher[]; | ||
| }; | ||
|
|
||
| /** | ||
| * Lazer-specific configuration type | ||
| */ | ||
| export type LazerConfig = { | ||
| programType: ProgramType.PYTH_LAZER; | ||
| // The Lazer state data | ||
| state: LazerState; | ||
| }; |
There was a problem hiding this comment.
These types are all specific to lazer so they should live alongside the lazer code
| export type DownloadableConfig = | ||
| | Record<string, DownloadableProduct> | ||
| | LazerState; |
There was a problem hiding this comment.
To me this now looks like two separate types and it's not really clear why the union is ever useful, it doesn't seem like it would ever be usable as a union without the user having to know whether they're working with lazer or core, and if the consumer has to know that, what's the value in having a union or a function that returns the union? If the user knew whether they were working with lazer or core, there's no point in using a function abstracted over both, they should just use the function that's correct for the system they're on.
Even when I look below in this PR where this and the getDownloadableConfig function are being used, they're in PythCore.tsx -- a file explicitly for pyth core -- so then there's no real point in using the abstracted function in program_registry.ts and we should just use the core function from core_functions.ts instead.
Frankly, the more I see stuff like this, the more I question the value of program_registry.ts entirely -- the common interface is useful if the interfaces are common, but it really is starting to look like they aren't common, and the abstraction is not even usable without knowing which program you're working with. If that's the case, it's just added indirection and causes type information to become less precise.
If you can come up with a generic type which Core & Lazer are both concrete implementations of, and define the abstraction in terms of that generic type, and you plan to use them in situations where you know you have a pyth program but you don't know which, I could see it being useful. But if you don't have common arguments AND you don't have common return types, then you should just export the functions from core and lazer separately, and expect the consumers to import the correct one directly.
| export type LazerConfig = { | ||
| programType: ProgramType.PYTH_LAZER; | ||
| // The Lazer state data | ||
| state: LazerState; |
There was a problem hiding this comment.
This type looks redundant, if the type is explicitly a lazer type, why would you also need a programType: ProgramType.PYTH_LAZER ? This is again making me question the program_registry.ts pattern.
| const downloadableConfig = getDownloadableConfig(rawConfig) as Record< | ||
| string, | ||
| DownloadableProduct | ||
| > |
There was a problem hiding this comment.
Why do we need a typecast here? If the return types of getDownloadableConfig are accurate, we shouldn't need the typecast. If they aren't accurate, then we shouldn't be typecasting, we should be fixing the types to be accurate.
| ([key, newValue]) => | ||
| (isNewShard || | ||
| (changes.prev && | ||
| changes.prev[key as keyof typeof changes.prev] !== newValue)) && ( |
There was a problem hiding this comment.
This is kind of awkward, if this is how you plan to use this then I'd rethink the type a bit. The type system can't actually guarantee that the keys are shared between prev and new, which is why you have the typecast. So instead I think it would be better to have the type be something where you have prev and new underneath each key, i.e. something along the lines of (referencing my prior comment):
type Changes = {
shard: Record<keyof Shard, ChangeField<Shard[keyof Shard]>>;
feeds: Record<string, ChangeField<Feed>>;
publishers: Record<string, ChangeField<Publisher>>;
}In general, typecasting should be avoided, and if you have to use it for stuff like this, it generally indicates that you should rethink the types you're working with.
| } | ||
|
|
||
| fetchLazerState() | ||
| }, []) |
There was a problem hiding this comment.
I'm a bit confused why this is async, it looks like everything it does is sync?
Also, you might like this pattern which I always use for client-side data fetching: https://github.com/pyth-network/pyth-crosschain/blob/main/packages/component-library/src/useData/index.ts. It has a few nice things:
- This uses
swrso you can easily configure it to automatically revalidate data if you so desire - It returns a tagged union of states so handling loading / error / before loading states is not only trivial, but typescript will actually enforce that you don't forget to do it
Here's an example of recent code I wrote that uses it: https://github.com/pyth-network/pyth-crosschain/blob/main/apps/entropy-explorer/src/components/Home/results.tsx#L31-L62
|
@cctdaniel I'm thinking that maybe you can actually use protobuf codegen to generate the state types as well. |
…zer, and PythContext components
…, feeds, and publishers
Summary
Updated the PythLazer component to display configuration changes in a detailed, structured format similar to PythCore.
Rationale
The original PythLazer component displayed configuration changes as raw JSON, which was difficult to read and understand. This update aligns the user experience with PythCore by:
The changes also include updates to the underlying validation and type system to properly support the LazerState structure with feeds, publishers, and shard metadata.
How has this been tested?
Manual Testing Steps:
The implementation follows the same patterns as PythCore and integrates seamlessly with the existing validation system and UI components.