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

feat: add query endpoints for balances of specific runes #3675

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

nghuyenthevinh2000
Copy link

@nghuyenthevinh2000 nghuyenthevinh2000 commented Apr 24, 2024

fixes: #3667

The logic will fetch balances of specific rune in each outpoint. Not loading the whole balances map.

Logic

  1. add endpoint "/rune/:rune/balances" which allows specifying rune to fetch balances
  2. add rune_specific_balances() to Server
  3. add get_rune_specific_balances() to Index: use OUTPOINT_TO_RUNE_BALANCES, and then filter specific rune balances

Testing

  1. get_rune_specific_balances(): querying balances from specific rune 0, 1, 2

@raphjaph hi, this is my first PR, I hope to explore more into the codebase. Any constructive feedbacks are welcome!

@nghuyenthevinh2000
Copy link
Author

@casey Hi, can you check if this one satisfies your requirement enough?

#3069 (comment)

Copy link
Collaborator

@raphjaph raphjaph left a comment

Choose a reason for hiding this comment

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

This PR has a good form, you have a good description and also added a test. Unfortunately we need another table to make this an efficient call. You can try your hand at this, or, if you want, I could show how to add another table into the database in the next Ordinals Coding Club. Let me know what you prefer :)

src/index.rs Outdated
Comment on lines 1082 to 1087
for entry in self
.database
.begin_read()?
.open_table(OUTPOINT_TO_RUNE_BALANCES)?
.iter()?
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

In order to make this production ready we would need to add a new table to the index: RUNE_TO_OUTPOINTS.

The way it works right now is that for every request it would iterate through all outpoints, making this a quite expensive call.

@raphjaph raphjaph added the stream label May 6, 2024
@nghuyenthevinh2000
Copy link
Author

This PR has a good form, you have a good description and also added a test. Unfortunately we need another table to make this an efficient call. You can try your hand at this, or, if you want, I could show how to add another table into the database in the next Ordinals Coding Club. Let me know what you prefer :)

I can try adding it to table, thanks for your feedback

@nghuyenthevinh2000
Copy link
Author

@raphjaph I have added a new table: RUNE_ID_TO_OUTPOINTS_BALANCE which tracks rune id to bytes array of (outpoints, balance)

Logic

  1. Add table RUNE_ID_TO_OUTPOINTS_BALANCE update in index/rune_updater.rs
  2. Add encode, decode scheme for bytes array of (outpoints, balance)
  3. Update get_rune_specific_balances(): get exactly a map of (outpoints, balance) from rune id

Test

  1. test_encode_decode_rune_to_outpoints_balance()
  2. past test get_rune_specific_balances() works as intended

Copy link
Collaborator

@raphjaph raphjaph left a comment

Choose a reason for hiding this comment

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

Runes can be in multiple outpoints so this the table should be a multi map. Also we already have the table OUTPOINT_TO_RUNE_BALANCES so we don't have to store the balance inside the new table.

src/index.rs Outdated
@@ -63,6 +61,7 @@ define_table! { OUTPOINT_TO_RUNE_BALANCES, &OutPointValue, &[u8] }
define_table! { OUTPOINT_TO_SAT_RANGES, &OutPointValue, &[u8] }
define_table! { OUTPOINT_TO_VALUE, &OutPointValue, u64}
define_table! { RUNE_ID_TO_RUNE_ENTRY, RuneIdValue, RuneEntryValue }
define_table! { RUNE_ID_TO_OUTPOINTS_BALANCE, RuneIdValue, &[u8] }
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
define_table! { RUNE_ID_TO_OUTPOINTS_BALANCE, RuneIdValue, &[u8] }
define_multimap_table! { RUNE_ID_TO_OUTPOINTS, RuneIdValue, &OutPointValue }

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, let's me see multi map

@nghuyenthevinh2000
Copy link
Author

Runes can be in multiple outpoints so this the table should be a multi map. Also we already have the table OUTPOINT_TO_RUNE_BALANCES so we don't have to store the balance inside the new table.

The query needs a mapping from a rune id to a byte array of multiple n outpoints.

OUTPOINT_TO_RUNE_BALANCES maps an outpoint to a byte array of multiple m (rune, balances) so I would still have to go through the whole byte array to extract the balance of that specific rune, thus O(n*m)

So, I think why not also include balance of such specific rune in each out point (out point, balance of specific rune) as well, one less iteration. Thus, RUNE_ID_TO_OUTPOINTS_BALANCE, the cost of querying is greatly reduced to O(n)

@nghuyenthevinh2000
Copy link
Author

@raphjaph can you check this one again,cargo test are passing on my local mac

Screenshot 2024-05-10 at 13 15 41

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Ready for Review
Development

Successfully merging this pull request may close these issues.

Add /rune/<Rune>/balances
2 participants