Skip to content

feat(pricefeed) Migrate Price Feed IDs to documentation #493

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.

Sign up for GitHub

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

Closed
wants to merge 6 commits into from
Closed

feat(pricefeed) Migrate Price Feed IDs to documentation #493

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f2aa8e1
initial commit
aditya520 Nov 6, 2024
a037765
added copy button
aditya520 Nov 6, 2024
1a298ee
improvements
aditya520 Nov 7, 2024
17f9b75
resolved comments
aditya520 Nov 12, 2024
f899e02
update
aditya520 Nov 13, 2024
4c8af3c
update
aditya520 Nov 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
183 changes: 183 additions & 0 deletions components/PriceFeedTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
import { useState, useEffect } from "react";
import { TextField, Select, MenuItem } from "@mui/material";
import { HermesClient, PriceFeedMetadata } from "@pythnetwork/hermes-client";
import { Table, Td, Th, Tr, CopyToClipboard } from "nextra/components";
import base58 from "bs58";
import { getPriceFeedAccountForProgram } from "@pythnetwork/pyth-solana-receiver";

const fetchPriceFeeds = async (stable: boolean) => {
const priceFeeds = await new HermesClient(
stable ? "https://hermes.pyth.network" : "https://hermes-beta.pyth.network"
).getPriceFeeds();
const assetTypes = Array.from(
new Set(priceFeeds.map((feed) => feed.attributes.asset_type ?? ""))
);
console.log(priceFeeds);
return { priceFeeds, assetTypes };
};

const fetchSolanaPriceFeedAccounts = async () => {
const priceFeeds = await fetchPriceFeeds(true);
const priceFeedIds = priceFeeds.priceFeeds.map((feed) =>
getPriceFeedAccountForProgram(0, base58.decode(feed.id)).toBase58()
);
return priceFeedIds;
};

type AssetTypesSelectorProps = {
priceFeedsState: PriceFeedsState;
selectedAssetType: string;
setSelectedAssetType: (assetType: string) => void;
};

enum PriceFeedsStateType {
NotLoaded,
Loading,
Loaded,
Error,
}

const PriceFeedsState = {
NotLoaded: () => ({ type: PriceFeedsStateType.NotLoaded as const }),
Loading: () => ({ type: PriceFeedsStateType.Loading as const }),
Loaded: (priceFeeds: PriceFeedMetadata[], assetTypes: string[]) => ({
type: PriceFeedsStateType.Loaded as const,
priceFeeds,
assetTypes,
}),
Error: (error: unknown) => ({
type: PriceFeedsStateType.Error as const,
error,
}),
};

type PriceFeedsState = ReturnType<
typeof PriceFeedsState[keyof typeof PriceFeedsState]
>;

const usePriceFeeds = (): PriceFeedsState => {
const [priceFeeds, setPriceFeeds] = useState<PriceFeedsState>(
PriceFeedsState.NotLoaded()
);

useEffect(() => {
setPriceFeeds(PriceFeedsState.Loading());
// console.log(fetchSolanaPriceFeedAccounts())
fetchPriceFeeds(true)
.then(({ priceFeeds, assetTypes }) => {
setPriceFeeds(PriceFeedsState.Loaded(priceFeeds, assetTypes));
})
.catch((error) => {
setPriceFeeds(PriceFeedsState.Error(error));
});
}, []);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a few issues with this effect and the state it creates:

  1. It's not accurate to initialize priceFeeds to an empty array on line 24, you should properly handle loading states and represent that state differently from a loaded array.
  2. You aren't actually using priceFeeds anywhere except to get the list of asset types. You should extract the asset types and dedupe them when you load the data and store that in the state, rather than deduping in the render path on line 76 below.
  3. The transformation on lines 33 - 37 is IMO not useful; you're basically just changing snake case to camel case. IMO there isn't any serious benefit to doing that, and there are costs (added cognitive overhead, makes the code harder to search, etc). I know that camel case is the convention, but sticking to convention is not worth added complexity and making the code harder to search.
  4. You should always use a catch any time you call a promise without using await, e.g. on line 41. Omitting the catch means uncaught exceptions could potentially put the app into an unstable state; additionally here you should make the UI indicate errors properly.
  5. I always strongly recommend extracting functions like fetchPriceFeeds outside the closure scope. For one, it makes the code simpler and easier to follow; for two it's easy to introduce performance issues by creating closures in a hot path by accident.
  6. You should always prefer ?? over || (on lines 35 and 36), see https://typescript-eslint.io/rules/prefer-nullish-coalescing/.
  7. Minor nit but in es6 you can do [...new Set(...)] instead of Array.from(new Set(...)) to dedupe, the former is considered more idiomatic.
  8. You can use the "fake sum types" pattern I shared to encapsulate the loading/error state management here if you like.

Putting all this together you can change the component to something like this:

export const PriceFeedTable = () => {
  const [selectedAssetType, setSelectedAssetType] = useState("All");

  // ...
  <AssetTypesSelect
    selectedAssetType={selectedAssetType}
    setSelectedAssetType={setSelectedAssetType}
  />
}

type AssetTypesSelectProps = {
  selectedAssetType: string;
  setSelectedAssetType: (value: string) => void;
}

const AssetTypesSelect = ({ selectedAssetType, setSelectedAssetType }: AssetTypesSelectProps) => {
  const assetTypes = useAssetTypes();

  switch (state.type) {
    case AssetTypesStateType.NotLoaded:
    case AssetTypesStateType.Loading: {
      // use any spinner we're already using in the app anywhere else...
      return <Spinner /> 
    }
    case AssetTypesStateType.Error: {
      return <p>Error loading asset types!</p>;
    }
    case AssetTypesStateType.Loaded: {
      return (
        <Select
          value={selectedAssetType}
          onChange={(e) => setSelectedAssetType(e.target.value)}
          size="small"
          sx={{ width: 200 }}
        >
          <MenuItem value="All">All Asset Types</MenuItem>
          {state.assetTypes.map(assetType => (
            <MenuItem key={assetType} value={assetType}>
              {assetType ?? "Uncategorized"}
            </MenuItem>
          ))}
        </Select>
      );
    }
  }
};

const useAssetTypes = (): AssetTypesState => {
  const [assetTypes, setAssetTypes] = useState<AssetTypesState>(AssetTypesState.NotLoaded());

  useEffect(() => {
    setAssetTypes(AssetTypesState.Loading());
    fetchAssetTypes()
      .then(assetTypes => setAssetTypes(AssetTypesState.Loaded(assetTypes)))
      .catch(error => setAssetTypes(AssetTypesState.Error(error)));
  }, []);

  return assetTypes;
}

const fetchAssetTypes = async () => {
  const hermesClient = new HermesClient("https://hermes.pyth.network");
  const feeds = await hermesClient.getPriceFeeds();
  return [...new Set(feeds.map(feed => feed.attributes.asset_type ?? ""))];
};

enum AssetTypesStateType {
  NotLoaded,
  Loading,
  Loaded,
  Error
}

const AssetTypesState = {
  NotLoaded: () => ({ type: AssetTypesStateType.NotLoaded as const }),
  Loading: () => ({ type: AssetTypesStateType.Loading as const }),
  Loaded: (assetTypes: string[]) => ({ type: AssetTypesStateType.Loaded as const, assetTypes }),
  Error: (error: unknown) => ({ type: AssetTypesStateType.Error as const, error }),
}

type AssetTypesState = typeof AssetTypesState[keyof typeof AssetTypesState];

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replying to point 3.
I am using PriceFeeds variable to filter here

    const matchesSearch =
      feed.id.toLowerCase().includes(searchTerm.toLowerCase())

and then to display the data. What should be the best way around?

Copy link
Contributor

@cprussin cprussin Nov 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, apologies, I was so focused on the asset types that I didn't think about the other usage of the feeds. In my suggested code, just replace fetchAssetTypes with:

const fetchPriceFeeds = async () => {
  const priceFeeds = await new HermesClient("https://hermes.pyth.network").getPriceFeeds();
  const assetTypes = [...new Set(priceFeeds.map(feed => feed.attributes.asset_type ?? ""))];
  return { priceFeeds, assetTypes };
};

Then update variable names etc as appropriate to reflect that the state is storing the price feeds and not just asset types (e.g. rename useAssetTypes to usePriceFeeds). Then update the component to e.g.:

export const PriceFeedTable = () => {
  const [selectedAssetType, setSelectedAssetType] = useState("All");
  const priceFeedsState = usePriceFeeds();

  // ...
  <AssetTypesSelect
    priceFeedsState={priceFeedsState}
    selectedAssetType={selectedAssetType}
    setSelectedAssetType={setSelectedAssetType}
  />
}

type AssetTypesSelectProps = {
  priceFeedsState: PriceFeedsState;
  selectedAssetType: string;
  setSelectedAssetType: (value: string) => void;
}

const AssetTypesSelect = ({ priceFeedsState, selectedAssetType, setSelectedAssetType }: AssetTypesSelectProps) => {
  switch (priceFeedsState.type) {
    // ...
  }
}

Let me know if this makes sense.


return priceFeeds;
};

const AssetTypesSelect = ({
priceFeedsState,
selectedAssetType,
setSelectedAssetType,
}: AssetTypesSelectorProps) => {
const priceFeeds = usePriceFeeds();

switch (priceFeeds.type) {
case PriceFeedsStateType.NotLoaded:
case PriceFeedsStateType.Loading:
case PriceFeedsStateType.Error:
return <p>Error loading asset types</p>;
case PriceFeedsStateType.Loaded:
return (
<Select
value={selectedAssetType}
onChange={(e) => setSelectedAssetType(e.target.value)}
size="small"
sx={{ width: 200 }}
>
<MenuItem value="All">All Asset Types</MenuItem>
{priceFeeds.assetTypes.map((type) => (
<MenuItem key={type} value={type}>
{type ?? "Uncategorized"}
</MenuItem>
))}
</Select>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's a great reason to use MUI for text fields or selects; we already have other components in the codebase for both cases, see e.g. https://docs.pyth.network/price-feeds/api-reference/cosmwasm/query-price-feed and the components on that page (Selector.tsx for the select and Input.tsx for the text input)

);
}
};

export function PriceFeedTable() {
const [searchTerm, setSearchTerm] = useState("");
const [selectedAssetType, setSelectedAssetType] = useState("All");
const priceFeedsState = usePriceFeeds();

<AssetTypesSelect
priceFeedsState={priceFeedsState}
selectedAssetType={selectedAssetType}
setSelectedAssetType={setSelectedAssetType}
/>;

let filteredData: PriceFeedMetadata[] = [];
if (priceFeedsState.type === PriceFeedsStateType.Loaded) {
filteredData = priceFeedsState.priceFeeds.filter((feed) => {
const matchesSearch =
feed.id.toLowerCase().includes(searchTerm.toLowerCase()) ||
feed.attributes.display_symbol
.toLowerCase()
.includes(searchTerm.toLowerCase());
const matchesAssetType =
selectedAssetType === "All" ||
feed.attributes.asset_type === selectedAssetType;
return matchesSearch && matchesAssetType;
});
}

return (
<>
<TextField
label="Search price feeds"
variant="outlined"
size="small"
value={searchTerm}
onChange={(e) => setSearchTerm(e.target.value)}
sx={{ width: 300 }}
/>
<Select
value={selectedAssetType}
onChange={(e) => setSelectedAssetType(e.target.value)}
size="small"
sx={{ width: 200 }}
>
<MenuItem value="All">All Asset Types</MenuItem>
{priceFeedsState.type === PriceFeedsStateType.Loaded &&
priceFeedsState.assetTypes.map((type) => (
<MenuItem key={type} value={type}>
{type || "Uncategorized"}
</MenuItem>
))}
</Select>

<Table>
<thead>
<Tr>
<Th>Ticker</Th>
<Th>Asset Type</Th>
<Th>Feed ID</Th>
</Tr>
</thead>
<tbody>
{filteredData.map((feed) => (
<Tr key={feed.id}>
<Td>{feed.attributes.display_symbol}</Td>
<Td>{feed.attributes.asset_type}</Td>
<Td className="font-mono whitespace-nowrap">
{feed.id}
<CopyToClipboard className="ml-2" getValue={() => feed.id} />
</Td>
</Tr>
))}
</tbody>
</Table>
</>
);
}
Loading
Loading