-
Notifications
You must be signed in to change notification settings - Fork 146
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
feat: runes balances #5224
feat: runes balances #5224
Conversation
1f2bdba
to
3bd4453
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a really tidy first implementation Fara, great job.
RE how we structure Runes, I imagine we might have a general Rune
/RuneEtching
model in @leather/models
—something very similar to, but not necessarily exactly the same as, the response from the API.
As this is super new though, I don't think it's important to get the modelling right immediately. Doesn't hurt to just use API types for now, and see how things develop. We'll better know how to model it later.
Agree BitcoinClient is a mess. I'm not huge on the class-based approach here, which I think came from copying the auto-generated blockchain api early on. A flat suite of functions taking a simple network config would be sufficient.
src/app/features/asset-list/components/bitcoin-fungible-tokens-asset-list.tsx
Outdated
Show resolved
Hide resolved
I am going to remove all the hardcoded testnet code here and get this merged into dev so it is ready. |
845fa88
to
3a173b8
Compare
3a173b8
to
cf3f7dd
Compare
This PR adds Runes balances to our assets list. This implementation only shows Runes for the first taproot index. We can expand it later (or now) if needed.
@kyranjamie hoping for your feedback here bc I'm trying to consider api response data vs what we need for our types later. I followed a similar pattern that was used for BRC-20 tokens where we select the response and add data to it, like the balance conversion to our Money type seen here, but I do think we need a better way to organize these models/types ...and I don't think we did a great job setting up Bitcoin/Stacks fungible tokens. 🤔 If we want to continue with that pattern, we should really be creating Bitcoin fungible token types in our models, which we haven't been doing. This
bitcoin-client
file is getting kinda messy imo. I'm not sure about doing a big refactor with Runes bc we need to get this out, but I do think this needs to be revisited so we can scale it better?