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

[WIP] Add endpoints for Dexscreener support #422

Closed

Conversation

CapCap
Copy link

@CapCap CapCap commented Nov 28, 2024

Description

Delete this sentence and add a description that explains what changes you have
done and why they were necessary.

Testing

Delete this sentence and provide a description of how to test the changes in
this PR.

Checklist

  • Did you update relevant documentation?
  • Did you add tests to cover new code or a fixed issue?
  • Did you update the changelog?
  • Did you check all checkboxes from the linked Linear task?

Copy link

vercel bot commented Nov 28, 2024

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

Name Status Preview Comments Updated (UTC)
emojicoin-dot-fun-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 4, 2024 10:18pm

Copy link

vercel bot commented Nov 28, 2024

@CapCap is attempting to deploy a commit to the Econia Labs Team on Vercel.

A member of the Team first needs to authorize it.

Comment on lines 197 to 209
let reserves;
// Bonding is complete
if (event.state.lpCoinSupply > 0) {
reserves = {
reserves: {
asset0: event.swap.baseVolume.toString(),
asset1: event.swap.quoteVolume.toString(),
}
};
} else {
// Still in bonding
reserves = {};
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
let reserves;
// Bonding is complete
if (event.state.lpCoinSupply > 0) {
reserves = {
reserves: {
asset0: event.swap.baseVolume.toString(),
asset1: event.swap.quoteVolume.toString(),
}
};
} else {
// Still in bonding
reserves = {};
}
const { base, quote } = calculateRealReserves(event.state);
const reserves = {
asset0: base.toString(),
asset1: quote.toString(),
};

I discussed this with @alnoki, it actually does make sense to provide reserves quantities in- and post- bonding curve.

Once #438 is merged, you should be able to just calculate it like above!

Then just return reserves, I think:

  // ...
  return {
    // ...
    reserves,
  }

Copy link
Author

Choose a reason for hiding this comment

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

This is amazing work! I'll rebase on top of your PR then

Copy link
Author

@CapCap CapCap Dec 4, 2024

Choose a reason for hiding this comment

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

@xbtmatt does this same function also apply to the post bonding curve reserves too for liquidity events?

src/typescript/frontend/src/app/dexscreener/pair.ts Outdated Show resolved Hide resolved
Comment on lines +81 to +85
const sybolEmojis = emojiString.split("");

const symbolBytesArray = sybolEmojis.map((e) => SYMBOL_EMOJI_DATA.byEmoji(e)!.bytes);
const symbolBytes = new Uint8Array(symbolBytesArray.flatMap((v) => Array.from(v)));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const sybolEmojis = emojiString.split("");
const symbolBytesArray = sybolEmojis.map((e) => SYMBOL_EMOJI_DATA.byEmoji(e)!.bytes);
const symbolBytes = new Uint8Array(symbolBytesArray.flatMap((v) => Array.from(v)));
const symbolEmojis = getSymbolEmojisInString(emojiString);
const symbolBytes = encodeEmojis(symbolEmojis);

Emojis are parsed really oddly; the zero-width joiners makes for a lot of confusing parsing behavior, because lots of emojis are a base emoji + some modifier to indicate the variant.

An obvious example is a person + a zwj + a skin tone. But there are others like the polar bear emoji, which is basically just 🐻\x200d❄️, see more here if you want, it's kind of interesting lol)

But yeah to reliably parse these use the above or I think some of these will get parsed incorrectly

Copy link
Author

Choose a reason for hiding this comment

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

Yeah this was my fear as well- this is the main place I wanted to write some tests for, too, to ensure stability of the IDs acoss these endpoints- I'll take a read (it is interesting!) and use your helpers 🙏 thank you!

@alnoki
Copy link
Member

alnoki commented Dec 5, 2024

@CapCap it looks like GitHub automatically closed your PR when @xbtmatt force-pushed his branch (which yours used as a base branch in a fork), so I think you might need to update yours to use econia-labs:main as the base branch now that #438 has merged

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.

3 participants