Skip to content
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.

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

Trading Volume 24h #247

Merged
merged 7 commits into from
Jul 20, 2023
Merged

Trading Volume 24h #247

merged 7 commits into from
Jul 20, 2023

Conversation

jackburrus
Copy link
Contributor

@jackburrus jackburrus commented Jul 19, 2023

This PR implements the 24h trading volume using the events on openLong and openShort.
image

@vercel
Copy link

vercel bot commented Jul 19, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
hyperdrive-fixed-borrow ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 20, 2023 8:53am
hyperdrive-monorepo-hyperdrive-trading ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 20, 2023 8:53am

Comment on lines 34 to 38
export async function getTradingVolume(
hyperdriveAddress: Address,
publicClient: PublicClient<Transport, Chain>,
currentBlockNumber: bigint,
): Promise<string> {
Copy link
Member

Choose a reason for hiding this comment

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

Let's make this an object argument with an interface to be consistent with the other hyperdrive/core functions.

Comment on lines 65 to 78
export function getTradingVolumeQuery({
hyperdriveAddress,
publicClient,
currentBlockNumber,
}: {
hyperdriveAddress: Address;
publicClient: PublicClient<Transport, Chain>;
currentBlockNumber: bigint;
}): QueryObserverOptions<Awaited<ReturnType<typeof getTradingVolume>>> {
return {
queryKey: ["tradingVolume", { hyperdriveAddress, publicClient }],
queryFn: () =>
getTradingVolume(hyperdriveAddress, publicClient, currentBlockNumber),
enabled: !!hyperdriveAddress && !!publicClient && !!currentBlockNumber,
Copy link
Member

Choose a reason for hiding this comment

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

Currently, enabled here isn't necessary because it's checking the truthiness of values that are required to be truthy. But, once you add the interface for the getTradingVolume function, you can replace the inline options type for the argument with Partial<GetTradingVolumeOptions> which will make all options optional.

const totalVolume =
calculateTotalAmount(longEvents) + calculateTotalAmount(shortEvents);

return totalVolume.toString();
Copy link
Member

Choose a reason for hiding this comment

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

Let's return both the bigint and formatted value to be consistent with the other hyperdrive/core functions (https://github.com/delvtech/hyperdrive-monorepo/blob/main/packages/hyperdrive/src/amm/longs/getLongPrice.ts#L55-L58)

Comment on lines 84 to 87
): number {
return events
.map((event) => parseInt(formatUnits(event.eventData.bondAmount, 18)))
.reduce((a, b) => a + b, 0);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
): number {
return events
.map((event) => parseInt(formatUnits(event.eventData.bondAmount, 18)))
.reduce((a, b) => a + b, 0);
): bigint {
return events
.reduce((a, b) => a.eventData.bondAmount + b.eventData.bondAmount, 0n);

iconUrl: string;
}) {
return (
<span className="flex flex-row items-center justify-start font-semibold">
<img className="mr-1 h-4" src={iconUrl} />
{parseInt(liquidity).toLocaleString()}
{parseInt(value).toLocaleString()}
Copy link
Member

Choose a reason for hiding this comment

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

We have a helper function for this called formatBalance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants